Re: [PATCH TRIVIAL] mm: Fix build warning in kmem_cache_create()

From: Glauber Costa <glommer@parallels.com>
To: Christoph Lameter <cl@linux.com>
Cc: Pekka Enberg <penberg@kernel.org>,
	David Rientjes <rientjes@google.com>,
	Shuah Khan <shuah.khan@hp.com>, <js1304@gmail.com>,
	<linux-mm@kvack.org>, <linux-kernel@vger.kernel.org>,
	<shuahkhan@gmail.com>
Subject: Re: [PATCH TRIVIAL] mm: Fix build warning in kmem_cache_create()
Date: Mon, 23 Jul 2012 11:04:34 +0400	[thread overview]
Message-ID: <500CF782.4060407@parallels.com> (raw)
In-Reply-To: <alpine.DEB.2.00.1207171005550.15061@router.home>

On 07/17/2012 07:11 PM, Christoph Lameter wrote:
> On Tue, 17 Jul 2012, Pekka Enberg wrote:
> 
>> Well, even SLUB checks for !name in mainline so that's definitely
>> 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.
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.