Re: [PATCH] mm: gfp_to_alloc_flags()

[Mel Gorman]

On Mon, Feb 23, 2009 at 12:55:03PM +0100, Peter Zijlstra wrote:
> I've always found the below a clean-up, respun it on top of your changes.
> Test box still boots ;-)
> 
> ---
> Subject: mm: gfp_to_alloc_flags()
> From: Peter Zijlstra <a.p.zijlstra@xxxxxxxxx>
> Date: Mon Feb 23 12:46:36 CET 2009
> 
> Clean up the code by factoring out the gfp to alloc_flags mapping.
> 
> [neilb@xxxxxxx says]
> As the test:
> 
> -       if (((p->flags & PF_MEMALLOC) || unlikely(test_thread_flag(TIF_MEMDIE)))
> -                       && !in_interrupt()) {
> -               if (!(gfp_mask & __GFP_NOMEMALLOC)) {
> 

At what point was this code deleted?

If it still exists, then I like the idea of this patch anyway. It takes
more code out of the loop. We end up checking if __GFP_WAIT is set twice,
but no major harm in that.

> has been replaced with a slightly weaker one:
> 
> +       if (alloc_flags & ALLOC_NO_WATERMARKS) {
> 
> we need to ensure we don't recurse when PF_MEMALLOC is set
> 
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@xxxxxxxxx>
> ---
>  mm/page_alloc.c |   90 ++++++++++++++++++++++++++++++++------------------------
>  1 file changed, 52 insertions(+), 38 deletions(-)
> 
> Index: linux-2.6/mm/page_alloc.c
> ===================================================================
> --- linux-2.6.orig/mm/page_alloc.c
> +++ linux-2.6/mm/page_alloc.c
> @@ -1658,16 +1658,6 @@ __alloc_pages_direct_reclaim(gfp_t gfp_m
>  	return page;
>  }
>  
> -static inline int is_allocation_high_priority(struct task_struct *p,
> -							gfp_t gfp_mask)
> -{
> -	if (((p->flags & PF_MEMALLOC) || unlikely(test_thread_flag(TIF_MEMDIE)))
> -			&& !in_interrupt())
> -		if (!(gfp_mask & __GFP_NOMEMALLOC))
> -			return 1;
> -	return 0;
> -}
> -
>  /*
>   * This is called in the allocator slow-path if the allocation request is of
>   * sufficient urgency to ignore watermarks and take other desperate measures
> @@ -1702,6 +1692,44 @@ void wake_all_kswapd(unsigned int order,
>  		wakeup_kswapd(zone, order);
>  }
>  
> +/*
> + * get the deepest reaching allocation flags for the given gfp_mask
> + */
> +static int gfp_to_alloc_flags(gfp_t gfp_mask)
> +{
> +	struct task_struct *p = current;
> +	int alloc_flags = ALLOC_WMARK_MIN | ALLOC_CPUSET;
> +	const gfp_t wait = gfp_mask & __GFP_WAIT;
> +
> +	/*
> +	 * The caller may dip into page reserves a bit more if the caller
> +	 * cannot run direct reclaim, or if the caller has realtime scheduling
> +	 * policy or is asking for __GFP_HIGH memory.  GFP_ATOMIC requests will
> +	 * set both ALLOC_HARDER (!wait) and ALLOC_HIGH (__GFP_HIGH).
> +	 */
> +	if (gfp_mask & __GFP_HIGH)
> +		alloc_flags |= ALLOC_HIGH;
> +
> +	if (!wait) {
> +		alloc_flags |= ALLOC_HARDER;
> +		/*
> +		 * Ignore cpuset if GFP_ATOMIC (!wait) rather than fail alloc.
> +		 * See also cpuset_zone_allowed() comment in kernel/cpuset.c.
> +		 */
> +		alloc_flags &= ~ALLOC_CPUSET;
> +	} else if (unlikely(rt_task(p)) && !in_interrupt())
> +		alloc_flags |= ALLOC_HARDER;
> +
> +	if (likely(!(gfp_mask & __GFP_NOMEMALLOC))) {
> +		if (!in_interrupt() &&
> +		    ((p->flags & PF_MEMALLOC) ||
> +		     unlikely(test_thread_flag(TIF_MEMDIE))))
> +			alloc_flags |= ALLOC_NO_WATERMARKS;
> +	}
> +
> +	return alloc_flags;
> +}
> +
>  static struct page * noinline
>  __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
>  	struct zonelist *zonelist, enum zone_type high_zoneidx,
> @@ -1732,48 +1760,34 @@ __alloc_pages_slowpath(gfp_t gfp_mask, u
>  	 * OK, we're below the kswapd watermark and have kicked background
>  	 * reclaim. Now things get more complex, so set up alloc_flags according
>  	 * to how we want to proceed.
> -	 *
> -	 * The caller may dip into page reserves a bit more if the caller
> -	 * cannot run direct reclaim, or if the caller has realtime scheduling
> -	 * policy or is asking for __GFP_HIGH memory.  GFP_ATOMIC requests will
> -	 * set both ALLOC_HARDER (!wait) and ALLOC_HIGH (__GFP_HIGH).
>  	 */
> -	alloc_flags = ALLOC_WMARK_MIN;
> -	if ((unlikely(rt_task(p)) && !in_interrupt()) || !wait)
> -		alloc_flags |= ALLOC_HARDER;
> -	if (gfp_mask & __GFP_HIGH)
> -		alloc_flags |= ALLOC_HIGH;
> -	if (wait)
> -		alloc_flags |= ALLOC_CPUSET;
> +	alloc_flags = gfp_to_alloc_flags(gfp_mask);
>  
>  restart:
> -	/*
> -	 * Go through the zonelist again. Let __GFP_HIGH and allocations
> -	 * coming from realtime tasks go deeper into reserves.
> -	 *
> -	 * This is the last chance, in general, before the goto nopage.
> -	 * Ignore cpuset if GFP_ATOMIC (!wait) rather than fail alloc.
> -	 * See also cpuset_zone_allowed() comment in kernel/cpuset.c.
> -	 */
> +	/* This is the last chance, in general, before the goto nopage. */
>  	page = get_page_from_freelist(gfp_mask, nodemask, order, zonelist,
> -						high_zoneidx, alloc_flags,
> -						preferred_zone,
> -						migratetype);
> +			high_zoneidx, alloc_flags & ~ALLOC_NO_WATERMARKS,
> +			preferred_zone, migratetype);
>  	if (page)
>  		goto got_pg;
>  
>  	/* Allocate without watermarks if the context allows */
> -	if (is_allocation_high_priority(p, gfp_mask))
> +	if (alloc_flags & ALLOC_NO_WATERMARKS) {
>  		page = __alloc_pages_high_priority(gfp_mask, order,
> -			zonelist, high_zoneidx, nodemask, preferred_zone,
> -			migratetype);
> -	if (page)
> -		goto got_pg;
> +				zonelist, high_zoneidx, nodemask,
> +				preferred_zone, migratetype);
> +		if (page)
> +			goto got_pg;
> +	}
>  
>  	/* Atomic allocations - we can't balance anything */
>  	if (!wait)
>  		goto nopage;
>  
> +	/* Avoid recursion of direct reclaim */
> +	if (p->flags & PF_MEMALLOC)
> +		goto nopage;
> +
>  	/* Try direct reclaim and then allocating */
>  	page = __alloc_pages_direct_reclaim(gfp_mask, order,
>  					zonelist, high_zoneidx,
> 

Looks good eyeballing it here at least. I'll slot it in and see what the
end result looks like but I think it'll be good.

Thanks

-- 
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/