Re: [PATCH] fs: fcntl_setlease defies lease_init assumptions

[Linus Torvalds]

On Mon, 8 May 2006, Pekka Enberg wrote:
> 
> I was under the impression that virt_to_page() is expensive, even more
> so on NUMA.

virt_to_page() should be pretty cheap, but you're right, that's probably 
the much higher expense than the test. And reading the "struct page" can 
get a cache-miss.  Although - especially for NUMA - we're actually going 
to do that virt_to_page() _anyway_ just a few lines below (as part of 
"virt_to_slab()".

Similarly, for "kfree()", we will actually have done that same thing 
already (kfree() does "virt_to_cache(objp);").

So we're actually only left with a single case that doesn't currently do 
it (and didn't trigger my trivial two additions): kmem_cache_free() just 
trusts the cachep pointer implicitly. And that's obviously the case that 
the whole fcntl_setlease thing used.

Everybody else would have triggered from my patch which added it at a 
point where it was basically free (because we had looked up the page 
anyway, and we were going to read from the "struct page" info regardless).

So from a performance standpoint, maybe my previous trivial patch is the 
right thing to do, along with an even _stronger_ test for 
kmem_cache_free(), where we could do

	BUG_ON(virt_to_cache(objp) != cachep);

which you can then remove from the slab debug case.

So for a lot of the normal paths, you'd basically have no extra cost (two 
instructions, no data cache pressure), but for kmem_cache_free() we'd have 
a slight overhead (but a lot lower than SLAB_DEBUG, and at least for NUMA 
it's reading a cacheline that we'd be using regardless.

I think it sounds like it's worth it, but I'm not going to really push it.

		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/