Messages in this thread Patch in this message | | | Date | Wed, 26 Nov 2014 14:24:04 -0800 | From | Andrew Morton <> | Subject | Re: [patch] fs, seq_file: fallback to vmalloc instead of oom kill processes |
| |
On Wed, 26 Nov 2014 14:16:46 -0800 (PST) David Rientjes <rientjes@google.com> wrote:
> Since commit 058504edd026 ("fs/seq_file: fallback to vmalloc allocation"), > seq_buf_alloc() falls back to vmalloc() when the kmalloc() for contiguous > memory fails. This was done to address order-4 slab allocations for > reading /proc/stat on large machines and noticed because > PAGE_ALLOC_COSTLY_ORDER < 4, so there is no infinite loop in the page > allocator when allocating new slab for such high-order allocations. > > Contiguous memory isn't necessary for caller of seq_buf_alloc(), however. > Other GFP_KERNEL high-order allocations that are <= > PAGE_ALLOC_COSTLY_ORDER will simply loop forever in the page allocator > and oom kill processes as a result. > > We don't want to kill processes so that we can allocate contiguous memory > in situations when contiguous memory isn't necessary. > > This patch does the kmalloc() allocation with __GFP_NORETRY for > high-order allocations. This still utilizes memory compaction and direct > reclaim in the allocation path, the only difference is that it will fail > immediately instead of oom kill processes when out of memory. > > --- a/fs/seq_file.c > +++ b/fs/seq_file.c > @@ -36,7 +36,7 @@ static void *seq_buf_alloc(unsigned long size) > { > void *buf; > > - buf = kmalloc(size, GFP_KERNEL | __GFP_NOWARN); > + buf = kmalloc(size, GFP_KERNEL | __GFP_NORETRY | __GFP_NOWARN); > if (!buf && size > PAGE_SIZE) > buf = vmalloc(size); > return buf;
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); Is __GFP_NORETRY our preferred way of preventing oom-killings? If so, it's a bit obscure - wouldn't it be better to create a __GFP_NO_OOM_KILL?
There are eleventy billion places where we do the open coded kmalloc-or-vmalloc thing. Sigh. Perhaps it is time to add a helper function which does this, so that all such callers use __GFP_NO_OOM_KILL.
| |