Re: [RFC PATCH] mm: page_alloc: High-order per-cpu page allocator

[Mel Gorman]

On Wed, Nov 23, 2016 at 04:37:06PM +0100, Vlastimil Babka wrote:
> On 11/21/2016 04:55 PM, Mel Gorman wrote:
> 
> ...
> 
> > hackbench was also tested with both socket and pipes and both processes
> > and threads and the results are interesting in terms of how variability
> > is imapcted
> > 
> > 1-socket machine -- pipes and processes
> >                         4.9.0-rc5             4.9.0-rc5
> >                           vanilla        highmark-v1r12
> > Amean    1      12.9637 (  0.00%)     12.9570 (  0.05%)
> > Amean    3      13.4770 (  0.00%)     13.4447 (  0.24%)
> > Amean    5      18.5333 (  0.00%)     19.0917 ( -3.01%)
> > Amean    7      24.5690 (  0.00%)     26.1010 ( -6.24%)
> > Amean    12     39.7990 (  0.00%)     40.6763 ( -2.20%)
> > Amean    16     56.0520 (  0.00%)     58.2530 ( -3.93%)
> 
> Here, higher values are better or worse?
> 

Higher values are worse.

> > diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> > index 0f088f3a2fed..02eb24d90d70 100644
> > --- a/include/linux/mmzone.h
> > +++ b/include/linux/mmzone.h
> > @@ -255,6 +255,24 @@ enum zone_watermarks {
> >  	NR_WMARK
> >  };
> > 
> > +/*
> > + * One per migratetype for order-0 pages and one per high-order up to
> > + * and including PAGE_ALLOC_COSTLY_ORDER. This may allow unmovable
> > + * allocations to contaminate reclaimable pageblocks if high-order
> > + * pages are heavily used.
> > + */
> > +#define NR_PCP_LISTS (MIGRATE_PCPTYPES + PAGE_ALLOC_COSTLY_ORDER + 1)
> 
> Should it be "- 1" instead of "+ 1"?
> 

Yes.

> > +
> > +static inline unsigned int pindex_to_order(unsigned int pindex)
> > +{
> > +	return pindex < MIGRATE_PCPTYPES ? 0 : pindex - MIGRATE_PCPTYPES + 1;
> > +}
> > +
> > +static inline unsigned int order_to_pindex(int migratetype, unsigned int order)
> > +{
> > +	return (order == 0) ? migratetype : MIGRATE_PCPTYPES - 1 + order;
> 
> Here I think that "MIGRATE_PCPTYPES + order - 1" would be easier to
> understand as the array is for all migratetypes, but the order is shifted?
> 

As in migratetypes * costly_order ? That would be excessively large.

> > @@ -1083,10 +1083,12 @@ static bool bulkfree_pcp_prepare(struct page *page)
> >   * pinned" detection logic.
> >   */
> >  static void free_pcppages_bulk(struct zone *zone, int count,
> > -					struct per_cpu_pages *pcp)
> > +					struct per_cpu_pages *pcp,
> > +					int migratetype)
> >  {
> > -	int migratetype = 0;
> > -	int batch_free = 0;
> > +	unsigned int pindex = 0;
> 
> Should pindex be initialized to migratetype to match the list below?
> 

Functionally it doesn't matter. It affects which list is tried first if
the preferred list is empty. Arguably it would make more sense to init
it to NR_PCP_LISTS - 1 so all order-0 lists are always drained before the
high-order pages but there is not much justification for that.

I'll take your suggestion until there is data supporting that high-order
caches should be preserved.

Thanks.

-- 
Mel Gorman
SUSE Labs