Messages in this thread | | | Date | Sun, 3 May 2009 18:38:24 +0400 | From | Cyrill Gorcunov <> | Subject | Re: [PATCH -tip] x86: uv - prevent NULL dereference in uv_system_init |
| |
[Pekka Enberg - Sun, May 03, 2009 at 03:27:00PM +0300] | Hi Cyrill, | | On Sun, 2009-05-03 at 16:12 +0400, Cyrill Gorcunov wrote: | > [Pekka Enberg - Sun, May 03, 2009 at 12:59:13PM +0300] | > | Hi David, | > | | > | On Sun, May 3, 2009 at 12:09 PM, David Rientjes <rientjes@google.com> wrote: | > | > SLUB stores two new slab allocation orders: the cache's adjustable order | > | > which is calculated at kmem_cache_create(), and the smallest order that | > | > can accommodate at least one object allocation. The latter is used as a | > | > fallback when the former fails in the page allocator. | > | > | > | > So for __GFP_PANIC to work in this case, it could not be implemented in | > | > the page allocator (SLUB also passes __GFP_NORETRY for new slabs) but | > | > rather above it in allocate_slab(). It would then be a no-op for | > | > alloc_pages(). | > | | > | It's probably better to implement __GFP_PANIC in alloc_pages() because | > | of kmalloc_large(). You can easily mask the __GFP_PANIC from the first | > | call to alloc_slab_page() where we use __GFP_NOWARN to suppress | > | out-of-memory warnings. | > | | > | But anyway, enough talk, show me the patch! :-) | > | | > | Pekka | > | | > | > I was thinking about the approach showed below. | > | > Note even if we will agree on this idea a number | > of questions remain opened -- like where is a better | > place to define kmalloc_panic in slub/slab_def.h | > or rather in slab.h. Should we include kernel.h | > to have panic and pr_ properly defined? | > | > I don't dare start/introduce handling of __GFP_PANIC | > flag since it would require more efforts to be done | > correctly and what is more important -- for most | > cases we would just don't need it. | > | > -- Cyrill | > | > --- | > include/linux/slab_def.h | 12 ++++++++++++ | > 1 file changed, 12 insertions(+) | > | > Index: linux-2.6.git/include/linux/slab_def.h | > ===================================================================== | > --- linux-2.6.git.orig/include/linux/slab_def.h | > +++ linux-2.6.git/include/linux/slab_def.h | > @@ -220,4 +220,16 @@ found: | > | > #endif /* CONFIG_NUMA */ | > | > +static inline void *kmalloc_panic(size_t size, gfp_t flags) | > +{ | > + void *p = kmalloc(size, flags); | > + | > + if (size && ZERO_OR_NULL_PTR(p)) { | > + pr_emerg("Failed to allocate: %z bytes\n", size); | > + panic("Out of memory\n"); | > + } | > + | > + return p; | > +} | > + | > #endif /* _LINUX_SLAB_DEF_H */ | | I don't like this approach because you'd need to do a kzalloc_panic() | and so on for it to be truly useful. What's wrong with adding a | __GFP_PANIC check in __alloc_pages_internal() (or whatever it's called | in -mm now) next to __GFP_NOWARN? | | Pekka |
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.
-- Cyrill
| |