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

[Andrew Morton]

On Wed, 26 Nov 2014 14:16:46 -0800 (PST) David Rientjes <rientjes@xxxxxxxxxx> 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.

--
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/