Re: [rfc][patch 3/3] use SLAB_ALIGN_SMP

[Nick Piggin <npiggin@suse.de>]

On Mon, Mar 03, 2008 at 02:00:51PM +0100, Eric Dumazet wrote:
> Nick Piggin a écrit :
> >On Mon, Mar 03, 2008 at 10:53:52AM +0100, Eric Dumazet wrote:
> >  
> >>Nick Piggin a écrit :
> >>    
> >>>Use SLAB_SMP_ALIGN in a few places.
> >>>
> >>> 
> >>>      
> >>I dont understand why you added SLAB_SMP_ALIGN, without removing 
> >>SLAB_HWCACHE_ALIGN on these places.
> >>    
> >
> >Because I thought that in most of the cases, we also want some cacheline
> >alignment on UP systems as well because we care about the layout of the
> >structure WRT the cachelines for the mandatory/capacity miss cases, as
> >well as wanting to avoid false sharing misses on SMP.
> >
> >Actually I didn't think _too_ hard about them, possibly some could be
> >removed. But the problem is that these things do require careful
> >thought so I should not change them unless I have done that ;)
> >
> >I guess there are some basic guidelines -- if size is a problem (ie if
> >there can be lots of these structures), then that is going to be a
> >factor; if the total pool of objects is likely to be fairly densely
> >resident in cache, then it will start to favour dense packing rather
> >than good alignment.
> >
> >  
> Well, if a kmem_cache_create() is used, this is probably because number 
> of objects can be large, so kmalloc() power-of-two granularity could 
> waste lot of ram.

Or because you want explicit control over alignment ;)

 
> But yes, you are right that SLAB_SMP_ALIGN doesnt imply SLAB_HWCACHE_ALIGN
> 
> - SMP_ALIGN is a hint about false sharing (when object contains a refcnt 
> for example), that is a concern only if
> 
> num_possible_cpus() > 1
> 
> While HWCACHE_ALIGN might be a hint saying :
> - The writer carefully designed the structure so that max performance is 
> obtained when all objects starts on a cache line boundary, even on 
> Uniprocessor.

Yes. In which case, we are also happy if the objects are small if they
share cachelines (so long as they are still nicely aligned, which slub
currently does not do)... unless SMP_ALIGN is set, of course.

 
> But I suspect some uses of HWCACHE_ALIGN are not a hint but a strong 
> requirement.
> 
> Maybe we need to use three flags to separate the meanings ?
> 
> 
> SLAB_HINT_SMP_ALIGN
> SLAB_HINT_HWCACHE_ALIGN
> SLAB_HWCACHE_ALIGN /* strong requirement that two objects dont share a 
> cache line */

Possibly, but I'm beginning to prefer that strong requirements should
request the explicit alignment (they can even use cache_line_size() after
Pekka's patch to make it generic). I don't like how the name implies
that you get a guarantee, however I guess in practice people are using it
more as a hint (or because they vaguely hope it makes their code run
faster :))

So I wouldn't be adverse to a rename...