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

Matthew Wilcox
2019-05-09T13:58:21Z

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'.

> > +#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)