Re: [patch 08/11 -mmotm] oom: invoke oom killer for __GFP_NOFAIL

From: Mel Gorman
Date: Tue May 12 2009 - 06:05:33 EST


On Mon, May 11, 2009 at 03:11:30PM -0700, Andrew Morton wrote:
> On Mon, 11 May 2009 14:45:18 -0700 (PDT)
> David Rientjes <rientjes@xxxxxxxxxx> wrote:
>
> > On Mon, 11 May 2009, Andrew Morton wrote:
> >
> > > > The oom killer must be invoked regardless of the order if the allocation
> > > > is __GFP_NOFAIL, otherwise it will loop forever when reclaim fails to
> > > > free some memory.
> > >
> > > We should discourage callers from using __GFP_NOFAIL at all. We should
> > > electrocute callers for using __GFP_NOFAIL on large allocations. How's about
> > >
> > > WARN_ON_ONCE(order > PAGE_ALLOC_COSTLY_ORDER &&
> > > (gfp_mask & __GFP_NOFAIL));
> > > or, preferably:
> > >
> > > WARN_ON_ONCE(order > 0 && (gfp_mask & __GFP_NOFAIL));
> > >
> >
> > Not sure it would help since the oom killer will be now be called for such
> > an allocation and that dumps the stack (and will actually show the order
> > and gfp flags as well).
>
> No, the intent of that warning is to find all call sites which use
> __GFP_NOFAIL on order>0 so we can hunt down and eliminate them.
>
>
> please review...
>
> From: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
>
> __GFP_NOFAIL is a bad fiction. Allocations _can_ fail, and callers should
> detect and suitably handle this (and not by lamely moving the infinite
> loop up to the caller level either).
>
> Attempting to use __GFP_NOFAIL for a higher-order allocation is even
> worse, so add a once-off runtime check for this to slap people around for
> even thinking about trying it.
>
> Cc: David Rientjes <rientjes@xxxxxxxxxx>
> Cc: Mel Gorman <mel@xxxxxxxxx>
> Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>

Acked-by: Mel Gorman <mel@xxxxxxxxx>

> ---
>
> mm/page_alloc.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff -puN mm/page_alloc.c~a mm/page_alloc.c
> --- a/mm/page_alloc.c~a
> +++ a/mm/page_alloc.c
> @@ -1201,8 +1201,19 @@ static int should_fail_alloc_page(gfp_t
> {
> if (order < fail_page_alloc.min_order)
> return 0;
> - if (gfp_mask & __GFP_NOFAIL)
> + if (gfp_mask & __GFP_NOFAIL) {
> + /*
> + * __GFP_NOFAIL is not to be used in new code.
> + *
> + * All __GFP_NOFAIL callers should be fixed so that they
> + * properly detect and handle allocation failures.
> + *
> + * We most definitely don't want callers attempting to allocate
> + * greater than single-page units with __GFP_NOFAIL.
> + */
> + WARN_ON_ONCE(order > 0);
> return 0;
> + }
> if (fail_page_alloc.ignore_gfp_highmem && (gfp_mask & __GFP_HIGHMEM))
> return 0;
> if (fail_page_alloc.ignore_gfp_wait && (gfp_mask & __GFP_WAIT))
> _
>
>

--
Mel Gorman
Part-time Phd Student Linux Technology Center
University of Limerick IBM Dublin Software Lab
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/