RE: [PATCH 02/11] mm: Pass order to __alloc_pages_nodemask in GFP flags

["Weiny, Ira" <ira.weiny@intel.com>]

> 
> On Wed, May 08, 2019 at 06:50:16PM -0700, Ira Weiny wrote:
> > On Mon, May 06, 2019 at 09:06:00PM -0700, Matthew Wilcox wrote:
> > > Save marshalling an extra argument in all the callers at the expense
> > > of using five bits of the GFP flags.  We still have three GFP bits
> > > remaining after doing this (and we can release one more by
> > > reallocating NORETRY, RETRY_MAYFAIL and NOFAIL).
> 
> > > -static void *dsalloc_pages(size_t size, gfp_t flags, int cpu)
> > > +static void *dsalloc_pages(size_t size, gfp_t gfp, int cpu)
> > >  {
> > >  	unsigned int order = get_order(size);
> > >  	int node = cpu_to_node(cpu);
> > >  	struct page *page;
> > >
> > > -	page = __alloc_pages_node(node, flags | __GFP_ZERO, order);
> > > +	page = __alloc_pages_node(node, gfp | __GFP_ZERO |
> > > +__GFP_ORDER(order));
> >
> > Order was derived from size in this function.  Is this truely equal to
> > the old function?
> >
> > At a minimum if I am wrong the get_order call above should be removed,
> no?
> 
> I think you have a misunderstanding, but I'm not sure what it is.
> 
> Before this patch, we pass 'order' (a small integer generally less than 10) in
> the bottom few bits of a parameter called 'order'.  After this patch, we pass
> the order in some of the high bits of the GFP flags.  So we can't remove the
> call to get_order() because that's what calculates 'order' from 'size'.

Ah I see it now.  Sorry was thinking the wrong thing when I saw that line.

Yep you are correct,
Ira


> 
> > > +#define __GFP_ORDER(order) ((__force gfp_t)(order <<
> __GFP_BITS_SHIFT))
> > > +#define __GFP_ORDER_PMD	__GFP_ORDER(PMD_SHIFT -
> PAGE_SHIFT)
> > > +#define __GFP_ORDER_PUD	__GFP_ORDER(PUD_SHIFT -
> PAGE_SHIFT)
> > > +
> > > +/*
> > > + * Extract the order from a GFP bitmask.
> > > + * Must be the top bits to avoid an AND operation.  Don't let
> > > + * __GFP_BITS_SHIFT get over 27, or we won't be able to encode
> > > +orders
> > > + * above 15 (some architectures allow configuring MAX_ORDER up to
> > > +64,
> > > + * but I doubt larger than 31 are ever used).
> > > + */
> > > +#define gfp_order(gfp)	(((__force unsigned int)gfp) >>
> __GFP_BITS_SHIFT)