Re: [PATCH] fs: fcntl_setlease defies lease_init assumptions

[Linus Torvalds]

On Mon, 8 May 2006, Pekka Enberg wrote:
> 
> On 5/8/06, Pekka Enberg <penberg@xxxxxxxxxxxxxx> wrote:
> > page_get_cache and page_get_slab are too late. You would need to do
> > the check in __cache_free; otherwise the stack pointer goes to per-CPU
> > caches and can be given back by kmalloc(). Adding PageSlab debugging
> > to __cache_free is probably too much of a performance hit, though.
> 
> Btw, CONFIG_DEBUG_SLAB should catch this case, see kfree_debugcheck()
> for details.

Yeah, but CONFIG_DEBUG_SLAB is _really_ expensive. 

We do have a lot of very basic debug checks (unconditionally) in the 
kernel to verify various "must be true" kinds of things. It might slow 
things down a bit, but in general, I think anything that helps catch 
problems early tends to pay itself back very quickly. So I'm more than 
happy with a simple BUG_ON() in even a hot path, if it just ends up being 
compiled into a "test and branch to unlikely" and doesn't need any costly 
locking etc around it.

Fedora had DEBUG_SLAB enabled in their development kernel, and that 
actually helped a lot. But I suspect they may _not_ have it in their 
non-development ones, and those have a much bigger test-base, so it might 
well be worth it to have a good base-line that catches serious problems, 
and have DEBUG_SLAB enable the expensive tests.

It's not like trying to free a non-kmalloc'ed pointer is a really strange 
event. malloc/free bugs are some of the most common serious problems in 
user space, and I suspect they are _less_ common in the kernel, but 
still..

		Linus
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/