linux-kernel - Re: [PATCH 3/5] mm: Use alloc_flags to record if kswapd can wake

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 26 Nov 2018 14:38:07 +0100
From:   Vlastimil Babka <vbabka@...e.cz>
To:     Mel Gorman <mgorman@...hsingularity.net>,
        Andrew Morton <akpm@...ux-foundation.org>
Cc:     David Rientjes <rientjes@...gle.com>,
        Andrea Arcangeli <aarcange@...hat.com>,
        Zi Yan <zi.yan@...rutgers.edu>,
        Michal Hocko <mhocko@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Linux-MM <linux-mm@...ck.org>
Subject: Re: [PATCH 3/5] mm: Use alloc_flags to record if kswapd can wake

On 11/23/18 12:45 PM, Mel Gorman wrote:
> This is a preparation patch that copies the GFP flag __GFP_KSWAPD_RECLAIM
> into alloc_flags. This is a preparation patch only that avoids having to
> pass gfp_mask through a long callchain in a future patch.
> 
> Note that the setting in the fast path happens in alloc_flags_nofragment()
> and it may be claimed that this has nothing to do with ALLOC_NO_FRAGMENT.
> That's true in this patch but is not true later so it's done now for
> easier review to show where the flag needs to be recorded.
> 
> No functional change.
> 
> Signed-off-by: Mel Gorman <mgorman@...hsingularity.net>

Acked-by: Vlastimil Babka <vbabka@...e.cz>

Small bug below:

> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -3278,10 +3278,15 @@ static bool zone_allows_reclaim(struct zone *local_zone, struct zone *zone)
>   * fragmentation between the Normal and DMA32 zones.
>   */
>  static inline unsigned int
> -alloc_flags_nofragment(struct zone *zone)
> +alloc_flags_nofragment(struct zone *zone, gfp_t gfp_mask)
>  {
> +	unsigned int alloc_flags = 0;
> +
> +	if (gfp_mask & __GFP_KSWAPD_RECLAIM)
> +		alloc_flags |= ALLOC_KSWAPD;
> +
>  	if (zone_idx(zone) != ZONE_NORMAL)
> -		return 0;
> +		goto out;
>  
>  	/*
>  	 * If ZONE_DMA32 exists, assume it is the one after ZONE_NORMAL and
> @@ -3290,13 +3295,14 @@ alloc_flags_nofragment(struct zone *zone)
>  	 */
>  	BUILD_BUG_ON(ZONE_NORMAL - ZONE_DMA32 != 1);
>  	if (nr_online_nodes > 1 && !populated_zone(--zone))
> -		return 0;
> +		goto out;
>  
> -	return ALLOC_NOFRAGMENT;
> +out:
> +	return alloc_flags;
>  }
>  #else
>  static inline unsigned int
> -alloc_flags_nofragment(struct zone *zone)
> +alloc_flags_nofragment(struct zone *zone, gfp_t gfp_mask)
>  {
>  	return 0;

The !CONFIG_ZONE_DMA32 version should still set ALLOC_KSWAPD, right?

>  }
> @@ -3939,6 +3945,9 @@ gfp_to_alloc_flags(gfp_t gfp_mask)
>  	} else if (unlikely(rt_task(current)) && !in_interrupt())
>  		alloc_flags |= ALLOC_HARDER;
>  
> +	if (gfp_mask & __GFP_KSWAPD_RECLAIM)
> +		alloc_flags |= ALLOC_KSWAPD;
> +
>  #ifdef CONFIG_CMA
>  	if (gfpflags_to_migratetype(gfp_mask) == MIGRATE_MOVABLE)
>  		alloc_flags |= ALLOC_CMA;