lkml.org 
[lkml]   [2014]   [Nov]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [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.



\
 
 \ /
  Last update: 2014-11-27 00:01    [W:0.057 / U:0.968 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site