From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754057AbZECM1S (ORCPT ); Sun, 3 May 2009 08:27:18 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1751325AbZECM1E (ORCPT ); Sun, 3 May 2009 08:27:04 -0400 Received: from courier.cs.helsinki.fi ([128.214.9.1]:56195 "EHLO mail.cs.helsinki.fi" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751147AbZECM1C (ORCPT ); Sun, 3 May 2009 08:27:02 -0400 Subject: Re: [PATCH -tip] x86: uv - prevent NULL dereference in uv_system_init From: Pekka Enberg To: Cyrill Gorcunov Cc: David Rientjes , Ingo Molnar , Jack Steiner , Andrew Morton , "H. Peter Anvin" , Thomas Gleixner , LKML In-Reply-To: <20090503121228.GC4615@lenovo> References: <20090501195638.GC4633@lenovo> <20090501200331.GA2645@elte.hu> <20090501200937.GD4633@lenovo> <20090501202511.GE4633@lenovo> <20090501203123.GA10878@sgi.com> <20090503084847.GA20394@elte.hu> <84144f020905030259i59ea304ftdc9224e6a9b5c285@mail.gmail.com> <20090503121228.GC4615@lenovo> Date: Sun, 03 May 2009 15:27:00 +0300 Message-Id: <1241353621.27683.3.camel@penberg-laptop> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 7bit X-Mailer: Evolution 2.24.3 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 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