Re: [GIT PULL v2] Early SLAB fixes for 2.6.31

[Pekka Enberg <penberg@cs.helsinki.fi>]

Hi Ben,

On Thu, 2009-06-18 at 13:24 +1000, Benjamin Herrenschmidt wrote:
> On Thu, 2009-06-18 at 12:00 +1000, Benjamin Herrenschmidt wrote:
> > > So I'm very much ok with the whole "use magic gfp_mask to indicate what 
> > > works at what stage". And yes, I think it makes sense to extend it to the 
> > > page allocator and might_sleep too, because GFP_KERNEL has all the same 
> > > issues regardless of whether it's about page allocation or about slab 
> > > allocators. And any "might_sleep" suppression really does tend to be about 
> > > the exact same thing.
> > 
> > Argh... still broken.
> > 
> > In fact, my initial patch added it to the page allocator, which worked
> > for me. Pekka patch removed that and made it slab-only. So I'm blowing
> > up at boot in lockdep or so because I'm allocating page tables on
> > ppc32 with __get_free_pages() and GFP_KERNEL.
> > 
> > I'll cook up a patch.
> 
> Here it is:
> 
> mm: Extend gfp masking to the page allocator
> 
> The page allocator also needs the masking of gfp flags during boot,
> so this moves it out of slab/slub and uses it with the page allocator
> as well.
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> 
> This will also make it easier to use it for limiting allocations that
> can block during suspend/resume, though doing this really fool-proof
> will require some kind of synchronization in set_gfp_allowed_mask()
> vs. allocations that have already started sleeping waiting for IOs.
> 
> Index: linux-work/include/linux/gfp.h
> ===================================================================
> --- linux-work.orig/include/linux/gfp.h	2009-06-18 12:03:14.000000000 +1000
> +++ linux-work/include/linux/gfp.h	2009-06-18 12:08:21.000000000 +1000
> @@ -99,7 +99,7 @@ struct vm_area_struct;
>  			__GFP_NORETRY|__GFP_NOMEMALLOC)
>  
>  /* Control slab gfp mask during early boot */
> -#define SLAB_GFP_BOOT_MASK __GFP_BITS_MASK & ~(__GFP_WAIT|__GFP_IO|__GFP_FS)
> +#define GFP_BOOT_MASK __GFP_BITS_MASK & ~(__GFP_WAIT|__GFP_IO|__GFP_FS)
>  
>  /* Control allocation constraints */
>  #define GFP_CONSTRAINT_MASK (__GFP_HARDWALL|__GFP_THISNODE)
> @@ -348,4 +348,11 @@ static inline void oom_killer_enable(voi
>  	oom_killer_disabled = false;
>  }
>  
> +extern gfp_t gfp_allowed_mask;
> +
> +static inline void set_gfp_allowed_mask(gfp_t mask)
> +{
> +	gfp_allowed_mask = mask;
> +}

The only thing I don't like about this patch is that the caller gets to
decide which bits should be masked. I really think this should be a
mm_late_init() function that sets the mask _internally_ in the page
allocator.

But anyway, I am about to go completely off-line until Sunday or so
consider the approach:

Acked-by: Pekka Enberg <penberg@cs.helsinki.fi>

			Pekka