Messages in this thread | | | Date | Mon, 8 May 2006 09:28:19 -0700 (PDT) | From | Linus Torvalds <> | Subject | Re: [PATCH] fs: fcntl_setlease defies lease_init assumptions |
| |
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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |