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

On Mon, 23 Jul 2012, Glauber Costa wrote:

quoted

quoted

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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>