break out page allocation warning code

I was tracking down a page allocation failure that ended up in vmalloc(). Since vmalloc() uses 0-order pages, if somebody asks for an insane amount of memory, we'll still get a warning with "order:0" in it. That's not very useful.

During recovery, vmalloc() also nicely frees all of the memory that it got up to the point of the failure. That is wonderful, but it also quickly hides any issues. We have a much different sitation if vmalloc() repeatedly fails 10GB in to:

	vmalloc(100 * 1<<30);

versus repeatedly failing 4096 bytes in to a:

	vmalloc(8192);

This patch will print out messages that look like this:

[   30.040774] bash: vmalloc failure allocating after 0 / 73728 bytes

As a side issue, I also noticed that ctl_ioctl() does vmalloc() based solely on an unverified value passed in from userspace. Granted, it's under CAP_SYS_ADMIN, but it still frightens me a bit.

Signed-off-by Dave Hansen <[email protected]>

---

 linux-2.6.git-dave/mm/vmalloc.c |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff -puN mm/vmalloc.c~vmalloc-warn mm/vmalloc.c

mm/vmalloc.c renamed from mm/vmalloc.c~vmalloc-warn +2 -2

 		if (node < 0)
-			page = alloc_page(gfp_mask);
+			page = alloc_page(tmp_mask);
 		else
-			page = alloc_pages_node(node, gfp_mask, 0);
+			page = alloc_pages_node(node, tmp_mask, order);
 
 		if (unlikely(!page)) {
 			/* Successfully allocated i pages, free them in __vunmap() */

Quoted 4 lines · Dave Hansen

This patch will print out messages that look like this:

[   30.040774] bash: vmalloc failure allocating after 0 / 73728 bytes

Either the changelog or the patch is still wrong because the format of this string is inconsistent.

mm/vmalloc.c static void *__vmalloc_node(unsigned lon · line 1537

Quoted 8 lines

diff -puN mm/vmalloc.c~vmalloc-warn mm/vmalloc.c

mm/vmalloc.c renamed from mm/vmalloc.c~vmalloc-warn +1 -0

@@ -1534,6 +1534,7 @@ static void *__vmalloc_node(unsigned lon
 static void *__vmalloc_area_node(struct vm_struct *area, gfp_t gfp_mask,
 				 pgprot_t prot, int node, void *caller)
 {
+	int order = 0;

Unnecessary, we can continue to hardcode the 0, vmalloc isn't going to use higher order allocs (it's there to avoid such things!).

mm/vmalloc.c static void *__vmalloc_area_node(struct · line 1586

Quoted 3 lines

mm/vmalloc.c renamed from mm/vmalloc.c~vmalloc-warn +3 -0

+	nopage_warning(gfp_mask, 0,  "vmalloc: allocation failure, "
+			"allocated %ld of %ld bytes\n",
+			(area->nr_pages*PAGE_SIZE), area->size);

mm/vmalloc.c static void *__vmalloc_area_node(struct · line 1564

Quoted 8 lines

mm/vmalloc.c renamed from mm/vmalloc.c~vmalloc-warn +1 -0

 	struct page **pages;
 	unsigned int nr_pages, array_size, i;
 	gfp_t nested_gfp = (gfp_mask & GFP_RECLAIM_MASK) | __GFP_ZERO;

@@ -1560,11 +1561,12 @@ static void *__vmalloc_area_node(struct 
 
 	for (i = 0; i < area->nr_pages; i++) {
 		struct page *page;
+		gfp_t tmp_mask = gfp_mask | __GFP_NOWARN;

I think it would be better to just do away with this as well and just hardwire the __GFP_NOWARN directly into the two allocation calls.

This originally started as a simple patch to give vmalloc() some more verbose output on failure on top of the plain page allocator messages. Johannes suggested that it might be nicer to lead with the vmalloc() info _before_ the page allocator messages.

But, I do think there's a lot of value in what __alloc_pages_slowpath() does with its filtering and so forth.

This patch creates a new function which other allocators can call instead of relying on the internal page allocator warnings. It also gives this function private rate-limiting which separates it from other printk_ratelimit() users.

---

 linux-2.6.git-dave/include/linux/mm.h |    2 +
 linux-2.6.git-dave/mm/page_alloc.c    |   65 +++++++++++++++++++++++-----------
 2 files changed, 46 insertions(+), 21 deletions(-)

diff -puN include/linux/mm.h~break-out-alloc-failure-messages include/linux/mm.h

include/linux/mm.h renamed from include/linux/mm.h~break-out-alloc-failure-messages +2 -0

@@ -1365,6 +1365,8 @@ extern void si_meminfo(struct sysinfo * 
 extern void si_meminfo_node(struct sysinfo *val, int nid);
 extern int after_bootmem;
 
+extern void nopage_warning(gfp_t gfp_mask, int order, const char *fmt, ...);
+
 extern void setup_per_cpu_pageset(void);
 
 extern void zone_pcp_update(struct zone *zone);

diff -puN mm/page_alloc.c~break-out-alloc-failure-messages mm/page_alloc.c

mm/page_alloc.c renamed from mm/page_alloc.c~break-out-alloc-failure-messages +1 -0

@@ -54,6 +54,7 @@
 #include <trace/events/kmem.h>
 #include <linux/ftrace_event.h>
 #include <linux/memcontrol.h>
+#include <linux/ratelimit.h>
 
 #include <asm/tlbflush.h>
 #include <asm/div64.h>

mm/page_alloc.c renamed from mm/page_alloc.c~break-out-alloc-failure-messages +21 -0

+{
+	va_list args;
+	int r;
+	unsigned int filter = SHOW_MEM_FILTER_NODES;
+	const gfp_t wait = gfp_mask & __GFP_WAIT;
+
+	if ((gfp_mask & __GFP_NOWARN) || !__ratelimit(&nopage_rs))
+		return;
+
+	/*
+	 * This documents exceptions given to allocations in certain
+	 * contexts that are allowed to allocate outside current's set
+	 * of allowed nodes.
+	 */
+	if (!(gfp_mask & __GFP_NOMEMALLOC))
+		if (test_thread_flag(TIF_MEMDIE) ||
+		    (current->flags & (PF_MEMALLOC | PF_EXITING)))
+			filter &= ~SHOW_MEM_FILTER_NODES;
+	if (in_interrupt() || !wait)
+		filter &= ~SHOW_MEM_FILTER_NODES;
+

mm/page_alloc.c renamed from mm/page_alloc.c~break-out-alloc-failure-messages +6 -15

+
+	dump_stack();
+	if (!should_suppress_show_mem())
+		show_mem(filter);
+}
+
 static inline int
 should_alloc_retry(gfp_t gfp_mask, unsigned int order,
 				unsigned long pages_reclaimed)

@@ -2176,27 +2219,7 @@ rebalance:
 	}
 
 nopage:
-	if (!(gfp_mask & __GFP_NOWARN) && printk_ratelimit()) {
-		unsigned int filter = SHOW_MEM_FILTER_NODES;
-
-		/*
-		 * This documents exceptions given to allocations in certain
-		 * contexts that are allowed to allocate outside current's set
-		 * of allowed nodes.
-		 */
-		if (!(gfp_mask & __GFP_NOMEMALLOC))
-			if (test_thread_flag(TIF_MEMDIE) ||
-			    (current->flags & (PF_MEMALLOC | PF_EXITING)))
-				filter &= ~SHOW_MEM_FILTER_NODES;
-		if (in_interrupt() || !wait)
-			filter &= ~SHOW_MEM_FILTER_NODES;
-

mm/page_alloc.c static inline bool should_suppress_show_ · line 1742

Quoted 5 lines

mm/page_alloc.c renamed from mm/page_alloc.c~break-out-alloc-failure-messages +5 -0

+static DEFINE_RATELIMIT_STATE(nopage_rs,
+		DEFAULT_RATELIMIT_INTERVAL,
+		DEFAULT_RATELIMIT_BURST);
+
+void nopage_warning(gfp_t gfp_mask, int order, const char *fmt, ...)

I suggest a different name for this, something like warn_alloc_failure() or such.

I guess this isn't general enough where it could be used in the oom killer as well?

Quoted 25 lines

mm/page_alloc.c renamed from mm/page_alloc.c~break-out-alloc-failure-messages +25 -0

+{
+	va_list args;
+	int r;
+	unsigned int filter = SHOW_MEM_FILTER_NODES;
+	const gfp_t wait = gfp_mask & __GFP_WAIT;
+
+	if ((gfp_mask & __GFP_NOWARN) || !__ratelimit(&nopage_rs))
+		return;
+
+	/*
+	 * This documents exceptions given to allocations in certain
+	 * contexts that are allowed to allocate outside current's set
+	 * of allowed nodes.
+	 */
+	if (!(gfp_mask & __GFP_NOMEMALLOC))
+		if (test_thread_flag(TIF_MEMDIE) ||
+		    (current->flags & (PF_MEMALLOC | PF_EXITING)))
+			filter &= ~SHOW_MEM_FILTER_NODES;
+	if (in_interrupt() || !wait)
+		filter &= ~SHOW_MEM_FILTER_NODES;
+
+	if (fmt) {
+		printk(KERN_WARNING);
+		va_start(args, fmt);
+		r = vprintk(fmt, args);

mm/page_alloc.c static inline bool should_suppress_show_ · line 1773

Quoted 6 lines

mm/page_alloc.c renamed from mm/page_alloc.c~break-out-alloc-failure-messages +6 -0

+		va_end(args);
+	}
+
+	printk(KERN_WARNING);
+	printk("%s: page allocation failure: order:%d, mode:0x%x\n",
+			current->comm, order, gfp_mask);

This shouldn't be here, it should have been printed already.

mm/page_alloc.c static inline bool should_suppress_show_ · line 1769

Quoted 6 lines · Dave Hansen

mm/page_alloc.c renamed from mm/page_alloc.c~break-out-alloc-failure-messages +6 -0

+       if (fmt) {
+               printk(KERN_WARNING);
+               va_start(args, fmt);
+               r = vprintk(fmt, args);
+               va_end(args);
+       }

Could we make the "printk(KERN_WARNING);" go away and require caller to specify level?

mm/page_alloc.c static inline bool should_suppress_show_ · line 1773

Quoted 3 lines

mm/page_alloc.c renamed from mm/page_alloc.c~break-out-alloc-failure-messages +3 -0

+       printk(KERN_WARNING);
+       printk("%s: page allocation failure: order:%d, mode:0x%x\n",
+                       current->comm, order, gfp_mask);

Even more so here. Why not pr_warning instead of two non-atomic calls to printk?

Quoted 7 lines · Dave Hansen

Quoted 6 lines

mm/page_alloc.c renamed from mm/page_alloc.c~break-out-alloc-failure-messages +6 -0

+       if (fmt) {
+               printk(KERN_WARNING);
+               va_start(args, fmt);
+               r = vprintk(fmt, args);
+               va_end(args);
+       }

Quoted 2 lines · Michał Nazarewicz

Could we make the "printk(KERN_WARNING);" go away and require caller to specify level?

The core problem is this: I want two lines of output: one for the order/mode gunk, and one for the user-specified message.

If we have the user pass in a string for the printk() level, we're stuck doing what I have here. If we have them _prepend_ it to the "fmt" string, then it's harder to figure out below. I guess we could fish in the string for it.