Re: [PATCH 12/12] mm, page_alloc: Only enforce watermarks for order-0 allocations

[Mel Gorman <mgorman@techsingularity.net>]

On Fri, Aug 28, 2015 at 02:10:51PM +0200, Michal Hocko wrote:
> On Mon 24-08-15 13:30:15, Mel Gorman wrote:
> > The primary purpose of watermarks is to ensure that reclaim can always
> > make forward progress in PF_MEMALLOC context (kswapd and direct reclaim).
> > These assume that order-0 allocations are all that is necessary for
> > forward progress.
> > 
> > High-order watermarks serve a different purpose. Kswapd had no high-order
> > awareness before they were introduced (https://lkml.org/lkml/2004/9/5/9).
> 
> lkml.org sucks. Could you plase replace it by something else e.g.
> https://lkml.kernel.org/r/413AA7B2.4000907@yahoo.com.au?
> 

Done.

> > This was particularly important when there were high-order atomic requests.
> > The watermarks both gave kswapd awareness and made a reserve for those
> > atomic requests.
> > 
> > There are two important side-effects of this. The most important is that
> > a non-atomic high-order request can fail even though free pages are available
> > and the order-0 watermarks are ok. The second is that high-order watermark
> > checks are expensive as the free list counts up to the requested order must
> > be examined.
> > 
> > With the introduction of MIGRATE_HIGHATOMIC it is no longer necessary to
> > have high-order watermarks. Kswapd and compaction still need high-order
> > awareness which is handled by checking that at least one suitable high-order
> > page is free.
> > 
> > With the patch applied, there was little difference in the allocation
> > failure rates as the atomic reserves are small relative to the number of
> > allocation attempts. The expected impact is that there will never be an
> > allocation failure report that shows suitable pages on the free lists.
> > 
> > The one potential side-effect of this is that in a vanilla kernel, the
> > watermark checks may have kept a free page for an atomic allocation. Now,
> > we are 100% relying on the HighAtomic reserves and an early allocation to
> > have allocated them.  If the first high-order atomic allocation is after
> > the system is already heavily fragmented then it'll fail.
> > 
> > Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
> 
> Acked-by: Michal Hocko <mhocko@suse.com>
> 

Thanks.

> [...]
> > @@ -2289,7 +2291,7 @@ static bool __zone_watermark_ok(struct zone *z, unsigned int order,
> >  {
> >  	long min = mark;
> >  	int o;
> > -	long free_cma = 0;
> > +	const bool atomic = (alloc_flags & ALLOC_HARDER);
> 
> I just find the naming a bit confusing. ALLOC_HARDER != __GFP_ATOMIC. RT tasks
> might get access to this reserve as well.
> 

I'll just call it alloc_harder then.

> [...]
> > +	/* Check at least one high-order page is free */
> > +	for (o = order; o < MAX_ORDER; o++) {
> > +		struct free_area *area = &z->free_area[o];
> > +		int mt;
> > +
> > +		if (atomic && area->nr_free)
> > +			return true;
> 
> Didn't you want
> 		if (atomic) {
> 			if (area->nr_free)
> 				return true;
> 			continue;
> 		}
> 

That is slightly more efficient so yes, I'll use it. Thanks.

-- 
Mel Gorman
SUSE Labs