Messages in this thread Patch in this message | | | Date | Mon, 4 May 2009 00:45:42 +0400 | From | Cyrill Gorcunov <> | Subject | Re: introducing __GFP_PANIC |
| |
[Pekka Enberg - Sun, May 03, 2009 at 08:38:01PM +0300] | On Sun, May 3, 2009 at 8:23 PM, Cyrill Gorcunov <gorcunov@gmail.com> wrote: | > | > Hi Pekka, | > | > | > | > ufortunatelly __alloc_pages_internal is not the only place where | > | > we do return NULL from kmalloc. As example - failslab facility | > | > (in slab_alloc call). Anyway -- I'll take a closer look. | > | | > | Right. I think failslab needs some fixing _not_ to return NULL if | > | __GFP_PANIC is set. | > | | > | > Ok, as a first raw draft (_not_ covering all the cases) it could | > be something like this. It touches only __alloc_pages_internal | > and we have to bespread as well: | > | > 1) alloc_pages with order >= MAX_ORDER (gfp.h) | > 2) the same for alloc_pages_node (both used by SLOB) | > 3) all __kmalloc should be explored as well. | > 4) ??? | > | > Anyway -- take a look on __alloc_pages_internal part :) | > | > -- Cyrill | > | > --- | > include/linux/gfp.h | 4 +++- | > mm/page_alloc.c | 8 +++++--- | > 2 files changed, 8 insertions(+), 4 deletions(-) | > | > Index: linux-2.6.git/include/linux/gfp.h | > ===================================================================== | > --- linux-2.6.git.orig/include/linux/gfp.h | > +++ linux-2.6.git/include/linux/gfp.h | > @@ -58,7 +58,9 @@ struct vm_area_struct; | > #define __GFP_NOTRACK ((__force gfp_t)0) | > #endif | > | > -#define __GFP_BITS_SHIFT 22 /* Room for 22 __GFP_FOO bits */ | > +#define __GFP_PANIC ((__force gfp_t)0x400000u) /* Panic on page alloction failure */ | > + | > +#define __GFP_BITS_SHIFT 23 /* Room for 23 __GFP_FOO bits */ | > #define __GFP_BITS_MASK ((__force gfp_t)((1 << __GFP_BITS_SHIFT) - 1)) | > | > /* This equals 0, but use constants in case they ever change */ | > Index: linux-2.6.git/mm/page_alloc.c | > ===================================================================== | > --- linux-2.6.git.orig/mm/page_alloc.c | > +++ linux-2.6.git/mm/page_alloc.c | > @@ -1496,7 +1496,7 @@ __alloc_pages_internal(gfp_t gfp_mask, u | > might_sleep_if(wait); | > | > if (should_fail_alloc_page(gfp_mask, order)) | > - return NULL; | > + goto nopage; | | The point of fault injection is to increase coverage out-of-memory | error handling code. So this doesn't make much sense to me. Why would | you want to cause a __GFP_PANIC call-sites to panic()? It doesn't help | testing one bit. | | So I still think you should just fix up should_fail_alloc_page() _not_ | to return true if __GFP_PANIC is set. |
Just before I get some sleep. Here is an another attempt.
Not covered cases I know of:
1) SLAB with builtin constant as size and we don't find appropriate cache for it do return NULL 2) alloc_pages_node and alloc_pages do check for MAX_ORDER which could be exceeded as well and return NULL then.
ok, here is a preliminary patch just to hear some complains :)
-- Cyrill --- include/linux/gfp.h | 4 +++- mm/failslab.c | 3 +++ mm/page_alloc.c | 8 ++++++-- 3 files changed, 12 insertions(+), 3 deletions(-)
Index: linux-2.6.git/include/linux/gfp.h ===================================================================== --- linux-2.6.git.orig/include/linux/gfp.h +++ linux-2.6.git/include/linux/gfp.h @@ -58,7 +58,9 @@ struct vm_area_struct; #define __GFP_NOTRACK ((__force gfp_t)0) #endif -#define __GFP_BITS_SHIFT 22 /* Room for 22 __GFP_FOO bits */ +#define __GFP_PANIC ((__force gfp_t)0x400000u) /* Panic on page alloction failure */ + +#define __GFP_BITS_SHIFT 23 /* Room for 23 __GFP_FOO bits */ #define __GFP_BITS_MASK ((__force gfp_t)((1 << __GFP_BITS_SHIFT) - 1)) /* This equals 0, but use constants in case they ever change */ Index: linux-2.6.git/mm/failslab.c ===================================================================== --- linux-2.6.git.orig/mm/failslab.c +++ linux-2.6.git/mm/failslab.c @@ -17,6 +17,9 @@ bool should_failslab(size_t size, gfp_t if (gfpflags & __GFP_NOFAIL) return false; + if (gfpflags & __GFP_PANIC) + return false; + if (failslab.ignore_gfp_wait && (gfpflags & __GFP_WAIT)) return false; Index: linux-2.6.git/mm/page_alloc.c ===================================================================== --- linux-2.6.git.orig/mm/page_alloc.c +++ linux-2.6.git/mm/page_alloc.c @@ -1185,6 +1185,8 @@ static int should_fail_alloc_page(gfp_t return 0; if (gfp_mask & __GFP_NOFAIL) return 0; + if (gfp_mask & __GFP_PANIC) + return 0; if (fail_page_alloc.ignore_gfp_highmem && (gfp_mask & __GFP_HIGHMEM)) return 0; if (fail_page_alloc.ignore_gfp_wait && (gfp_mask & __GFP_WAIT)) @@ -1506,7 +1508,7 @@ restart: * Happens if we have an empty zonelist as a result of * GFP_THISNODE being used on a memoryless node */ - return NULL; + goto nopage; } page = get_page_from_freelist(gfp_mask|__GFP_HARDWALL, nodemask, order, @@ -1685,7 +1687,9 @@ nopage: dump_stack(); show_mem(); } - return page; + if (unlikely(gfp_mask & __GFP_PANIC)) + panic("Out of memory: panic due to __GFP_PANIC\n"); + return NULL; got_pg: if (kmemcheck_enabled) kmemcheck_pagealloc_alloc(page, order, gfp_mask); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |