Messages in this thread | | | Date | Tue, 20 Apr 2004 02:26:02 +0200 | From | Andrea Arcangeli <> | Subject | Re: slab-alignment-rework.patch in -mc |
| |
On Mon, Apr 19, 2004 at 06:42:36PM +0200, Manfred Spraul wrote: > Then pass "4" as the align parameter to kmem_cache_create. That's the > main point of the patch: it's now possible to explicitely specify the > requested alignment. 32 for the 3rd level page tables, the optimal > number for the pte_chains, etc. > > >No best-guess must > >be made automatically by the slab code, rounding it to 16 bytes. > > > If you pass 0 as align to kmem_cache_create, then it's rounded to L2 > size. It's questionable if that's really the best thing - on > uniprocessor, 16-byte might result is better performance - there is no > risk of false sharing.
my point is that 0 as align must not round to l2 size or you break stuff. you likely have broken the vma alignment to make the most obvious example because I don't think it's a l1 cache multiple.
Removing the HW_ALIGN bitflag isn't nearly enough to avoid breaking stuff after your changes to the kmem_cache_create API, to avoid breaking stuff you need to change _all_ other kmem_cache_create and replace 0 with 4 if they didn't have the HW_ALIGN parameter, you didn't do that and things breaks, not just for small objects, but for the ones bigger than the cachesize too.
If you fixup all other kmem_cache_create callers to pass 4 instead of 0 that could be fine with me, but I still think this removal of the HW_ALIGN bitflag is completely worthless, you still have a branch checking !align, so you don't seem to gain anything and you break stuff.
Note: I really appreciate the runtime evaluation of the cachesize, those parts are fine, changing offset to "align" is also more than welcome, only the removal of HW_ALIGN bitflag w/o s/0/4/ where HW_ALIGN was missing is bogus and breaks stuff. - 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/
| |