From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from psmtp.com (na3sys010amx125.postini.com [74.125.245.125]) by kanga.kvack.org (Postfix) with SMTP id 5DE686B004D for ; Wed, 25 Jul 2012 11:28:35 -0400 (EDT) Date: Wed, 25 Jul 2012 10:28:31 -0500 (CDT) From: Christoph Lameter Subject: Re: [PATCH TRIVIAL] mm: Fix build warning in kmem_cache_create() In-Reply-To: <500CF782.4060407@parallels.com> Message-ID: References: <1342221125.17464.8.camel@lorien2> <1342407840.3190.5.camel@lorien2> <500CF782.4060407@parallels.com> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-linux-mm@kvack.org List-ID: To: Glauber Costa Cc: Pekka Enberg , David Rientjes , Shuah Khan , js1304@gmail.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org, shuahkhan@gmail.com On Mon, 23 Jul 2012, Glauber Costa wrote: > >> worth including unconditionally. Furthermore, the size related checks > >> certainly make sense and I don't see any harm in having them as well. > > > > There is a WARN_ON() there and then it returns NULL!!! Crazy. Causes a > > NULL pointer dereference later in the caller? > > > > It obviously depends on the caller. This is a violation of the calling convention to say the least. This means if you have SLAB_PANIC set and accidentally set the name to NULL the function will return despite the error and not panic! > Although most of the calls to kmem_cache_create are made from static > data, we can't assume that. Of course whoever is using static data > should do those very same tests from the outside to be safe, but in case > they do not, this seems to fall in the category of things that make > debugging easier - even if we later on get to a NULL pointer dereference. > > Your mentioned bias towards minimum code size, however, is totally > valid, IMHO. But I doubt those checks would introduce a huge footprint. > I would imagine you being much more concerned about being able to wipe > out entire subsystems like memcg, which will give you a lot more. They are useless checks since any use of the name will also cause a NULL pointer dereference. Same is true for interrupt checks. Checks like that indicate a deterioration of the code base. People are afraid that something goes wrong because they no longer understand the code so they build a embroidery around it instead of relying on the already existing checks at vital places. The embroidery can be useful for debugging thats why I left it in for the CONFIG_DEBUG_VM but certainly should not be included in production kernels. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org