linux-kernel - Re: [patch] fs, seq_file: fallback to vmalloc instead of oom kill processes

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] [day] [month] [year] [list]

Message-Id: <20141126144846.d7a3ec1a2a0c20d9cb1a7c49@linux-foundation.org>
Date:	Wed, 26 Nov 2014 14:48:46 -0800
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	David Rientjes <rientjes@...gle.com>
Cc:	Heiko Carstens <heiko.carstens@...ibm.com>,
	Christoph Hellwig <hch@...radead.org>,
	Al Viro <viro@...iv.linux.org.uk>, linux-kernel@...r.kernel.org
Subject: Re: [patch] fs, seq_file: fallback to vmalloc instead of oom kill
 processes

On Wed, 26 Nov 2014 14:40:06 -0800 (PST) David Rientjes <rientjes@...gle.com> wrote:

> On Wed, 26 Nov 2014, Andrew Morton wrote:
> 
> > You forgot something.
> > 
> > --- a/fs/seq_file.c~fs-seq_file-fallback-to-vmalloc-instead-of-oom-kill-processes-fix
> > +++ a/fs/seq_file.c
> > @@ -36,6 +36,10 @@ static void *seq_buf_alloc(unsigned long
> >  {
> >  	void *buf;
> >  
> > +	/*
> > +	 * __GFP_NORETRY to avoid oom-killings with high-order allocations -
> > +	 * it's better to fall back to vmalloc() than to kill things.
> > +	 */
> >  	buf = kmalloc(size, GFP_KERNEL | __GFP_NORETRY | __GFP_NOWARN);
> >  	if (!buf && size > PAGE_SIZE)
> >  		buf = vmalloc(size);
> > 
> 
> ...
>
> The slowpath tries to allocate, calls memory compaction if necessary, 
> tries to allocate, calls direct reclaim, tries to allocate, call the oom 
> killer and tries to allocate if we are going to loop, and then loop if 
> allowed.  There's no need to try to allocate if we don't call the oom 
> killer since it won't succeed and there's no need to call the oom killer 
> to free memory if we aren't going to retry.

My concern is that an open-coded __GFP_NORETRY is very obscure.  Even
something like

#define __GFP_NO_OOM_KILL __GFP_NORETRY

would help make things a bit self-documenting.

> Even if __GFP_NO_OOM_KILL existed, it wouldn't be applicable to this 
> patch: the change here is that seqfile will now return -ENOMEM instead of 
> oom killing processes;

Not really?  The change makes seq_buf_alloc() fall back to vmalloc()
rather than killing things.  Doesn't it?

Unless the allocation is < PAGE_SIZE, in which case we do go ENOMEM. 
That's daft - it would be better to vmalloc() a whole page in this
case.  Not that the vmalloc is likely to be successful anyway..

> the slab allocation will no longer loop forever 
> trying to allocate memory.
--
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/