linux-kernel - Re: [patch 08/11 -mmotm] oom: invoke oom killer for __GFP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1242106795.11251.286.camel@twins>
Date:	Tue, 12 May 2009 07:39:55 +0200
From:	Peter Zijlstra <a.p.zijlstra@...llo.nl>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	David Rientjes <rientjes@...gle.com>, gregkh@...e.de,
	npiggin@...e.de, mel@....ul.ie, cl@...ux-foundation.org,
	dave@...ux.vnet.ibm.com, san@...roid.com, arve@...roid.com,
	linux-kernel@...r.kernel.org
Subject: Re: [patch 08/11 -mmotm] oom: invoke oom killer for __GFP_NOFAIL

On Mon, 2009-05-11 at 15:11 -0700, Andrew Morton wrote:
> On Mon, 11 May 2009 14:45:18 -0700 (PDT)
> David Rientjes <rientjes@...gle.com> 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...

Fully agreed, people should use banker's algorithm to guarantee
progress, not create deadlocks with inf loops.

Acked-by: Peter Zijlstra <a.p.zijlstra@...llo.nl>

> From: Andrew Morton <akpm@...ux-foundation.org>
> 
> __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@...gle.com>
> Cc: Mel Gorman <mel@....ul.ie>
> Signed-off-by: Andrew Morton <akpm@...ux-foundation.org>
> ---
> 
>  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))
> _
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/