slub: fix small HWCACHE_ALIGN alignment

Introduce SLAB_SMP_ALIGN, for allocations where false sharing must be minimised on SMP systems.

Quoted 2 lines · Nick Piggin

Introduce SLAB_SMP_ALIGN, for allocations where false sharing must be minimised on SMP systems.

Mandatory alignment are specified as a parameter to kmem_cache_create not as flags.

Ycan specify the alignment as a parameter. i.e. L1_CACHE_BYTES or cache_line_size().

Quoted 2 lines

Mandatory alignment are specified as a parameter to kmem_cache_create not as flags.

Not after this patch.

Quoted 2 lines

Ycan specify the alignment as a parameter. i.e. L1_CACHE_BYTES or cache_line_size().

We don't want either of those, though.

I don't see why this is a problem to do inside slab.

Quoted 4 lines · Nick Piggin

Quoted 2 lines

Mandatory alignment are specified as a parameter to kmem_cache_create not as flags.

Not after this patch.

You want two ways of specifying alignments?

Quoted 1 line

You want two ways of specifying alignments?

I want a way to ask for SMP friendly allocation.

Quoted 3 lines · Nick Piggin

Quoted 1 line

You want two ways of specifying alignments?

I want a way to ask for SMP friendly allocation.

Then align the objects at cacheline boundaries by providing a value for the align parameter to kmem_cache_create().

Quoted 2 lines

Then align the objects at cacheline boundaries by providing a value for the align parameter to kmem_cache_create().

max(num_possible_cpus() > 1 ? cache_line_size() : 0, mandatory_alignment)?

Then suppose we want a CONFIG_TINY option to eliminate it?

Hmm... Can't we just fix SLAB_HWCACHE_ALIGN in SLUB to follow the semantics of SLAB?

64 X X

Quoted 8 lines · Christoph Lameter

Quoted 3 lines · Nick Piggin

Quoted 1 line

You want two ways of specifying alignments?

I want a way to ask for SMP friendly allocation.

Then align the objects at cacheline boundaries by providing a value for the align parameter to kmem_cache_create().

max(num_possible_cpus() > 1 ? cache_line_size() : 0, mandatory_alignment)?

Then suppose we want a CONFIG_TINY option to eliminate it?

And maybe the VSMP guys will want to blow this out to their internode alignment?

max(!CONFIG_TINY && num_possible_cpus() > 1 ? (is_vsmp ? internode_alignemnt : cache_line_size()) : 0, mandatory_alignment)

Quoted 7 lines · Nick Piggin

Quoted 6 lines · Christoph Lameter

Quoted 3 lines · Nick Piggin

Quoted 1 line

You want two ways of specifying alignments?

I want a way to ask for SMP friendly allocation.

Quoted 4 lines

Quoted 2 lines

Then align the objects at cacheline boundaries by providing a value for the align parameter to kmem_cache_create().

max(num_possible_cpus() > 1 ? cache_line_size() : 0, mandatory_alignment)?

The mandatory alignment is applied anyways. You do not need to max() on that.

One could simply specify cache_line_size() with Pekka's patch. cache_line_size() could default to 0 if !SMP.

Quoted 1 line

Then suppose we want a CONFIG_TINY option to eliminate it?

No slab allocator supports that right now.

However, SLOB in the past has
ignored the alignment in order to reduce memory use. So maybe Matt wants to introduce this?

Quoted 4 lines

And maybe the VSMP guys will want to blow this out to their internode alignment?

max(!CONFIG_TINY && num_possible_cpus() > 1 ? (is_vsmp ? internode_alignemnt : cache_line_size()) : 0, mandatory_alignment)

No the slab allocators were optimized for VSMP so that the internode_alignment is not necessary there. That was actually one of the requirements that triggered the slab numa work.

Quoted 4 lines · Nick Piggin

And maybe the VSMP guys will want to blow this out to their internode alignment?

max(!CONFIG_TINY && num_possible_cpus() > 1 ? (is_vsmp ? internode_alignemnt : cache_line_size()) : 0, mandatory_alignment)

No the slab allocators were optimized for VSMP so that the internode_alignment is not necessary there. That was actually one of the requirements that triggered the slab numa work.

Quoted 3 lines

No the slab allocators were optimized for VSMP so that the internode_alignment is not necessary there. That was actually one of the requirements that triggered the slab numa work.

BTW. can you explain this incredible claim that the slab allocators do not need internode_alignment to avoid false sharing of allocated objects on VSMP? I don't understand how that would work at all.

Quoted 4 lines · Nick Piggin

Quoted 3 lines

No the slab allocators were optimized for VSMP so that the internode_alignment is not necessary there. That was actually one of the requirements that triggered the slab numa work.

Quoted 3 lines

BTW. can you explain this incredible claim that the slab allocators do not need internode_alignment to avoid false sharing of allocated objects on VSMP? I don't understand how that would work at all.

The queues are per node which means that pages from which we allocate are distinct per node. As long as processes allocate and free object on a single node all is fine. Problems can arise though if objects are used across node.

Internode alignment is 4k. If you need an object aligned like that then it may be better to go to the page allocator.

Internode alignment is 4k. If you need an object aligned like that then it may be better to go to the page allocator.

Quoted 2 lines · Christoph Lameter

Internode alignment is 4k. If you need an object aligned like that then it may be better to go to the page allocator.

I never got a reply about this. Again: how can the modern slab allocator avoid the need for internode alignment in order to prevent cacheline pingpong on vsmp? Because I'm going to resubmit the SMP_ALIGN patch.

Also, the last I heard about the HWCACHE_ALIGN from you is that it didn't make sense... I see it's merged now, which I appreciate, but I think it is really important that we're on the same page here, so let me know if it still doesn't make sense.

Use SLAB_SMP_ALIGN in a few places.

Signed-off-by Nick Piggin <[email protected]>

---
Index: linux-2.6/fs/block_dev.c
===================================================================

fs/block_dev.c +2 -2

@@ -332,8 +332,8 @@ void __init bdev_cache_init(void)
 {
 	int err;
 	bdev_cachep = kmem_cache_create("bdev_cache", sizeof(struct bdev_inode),
-			0, (SLAB_HWCACHE_ALIGN|SLAB_RECLAIM_ACCOUNT|
-				SLAB_MEM_SPREAD|SLAB_PANIC),
+			0, (SLAB_HWCACHE_ALIGN|SLAB_SMP_ALIGN|
+			SLAB_RECLAIM_ACCOUNT|SLAB_MEM_SPREAD|SLAB_PANIC),
 			init_once);
 	err = register_filesystem(&bd_type);
 	if (err)

Index: linux-2.6/kernel/fork.c
===================================================================

kernel/fork.c +5 -5

@@ -1547,23 +1547,23 @@ void __init proc_caches_init(void)
 {
 	sighand_cachep = kmem_cache_create("sighand_cache",
 			sizeof(struct sighand_struct), 0,
-			SLAB_HWCACHE_ALIGN|SLAB_PANIC|SLAB_DESTROY_BY_RCU,
+			SLAB_HWCACHE_ALIGN|SLAB_SMP_ALIGN|SLAB_PANIC|SLAB_DESTROY_BY_RCU,
 			sighand_ctor);
 	signal_cachep = kmem_cache_create("signal_cache",
 			sizeof(struct signal_struct), 0,
-			SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL);
+			SLAB_HWCACHE_ALIGN|SLAB_SMP_ALIGN|SLAB_PANIC, NULL);
 	files_cachep = kmem_cache_create("files_cache",
 			sizeof(struct files_struct), 0,
-			SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL);
+			SLAB_HWCACHE_ALIGN|SLAB_SMP_ALIGN|SLAB_PANIC, NULL);
 	fs_cachep = kmem_cache_create("fs_cache",
 			sizeof(struct fs_struct), 0,
-			SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL);
+			SLAB_HWCACHE_ALIGN|SLAB_SMP_ALIGN|SLAB_PANIC, NULL);
 	vm_area_cachep = kmem_cache_create("vm_area_struct",
 			sizeof(struct vm_area_struct), 0,
 			SLAB_PANIC, NULL);
 	mm_cachep = kmem_cache_create("mm_struct",
 			sizeof(struct mm_struct), ARCH_MIN_MMSTRUCT_ALIGN,
-			SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL);
+			SLAB_HWCACHE_ALIGN|SLAB_SMP_ALIGN|SLAB_PANIC, NULL);
 }
 
 /*

Index: linux-2.6/kernel/pid.c
===================================================================

kernel/pid.c +1 -1

@@ -526,5 +526,5 @@ void __init pidmap_init(void)
 	atomic_dec(&init_pid_ns.pidmap[0].nr_free);
 
 	init_pid_ns.pid_cachep = KMEM_CACHE(pid,
-			SLAB_HWCACHE_ALIGN | SLAB_PANIC);
+			SLAB_HWCACHE_ALIGN | SLAB_SMP_ALIGN | SLAB_PANIC);
 }

Index: linux-2.6/kernel/user.c
===================================================================

kernel/user.c +1 -2

@@ -503,7 +503,7 @@ static int __init uid_cache_init(void)
 	int n;
 
 	uid_cachep = kmem_cache_create("uid_cache", sizeof(struct user_struct),
-			0, SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL);
+			0, SLAB_HWCACHE_ALIGN|SLAB_SMP_ALIGN|SLAB_PANIC, NULL);
 
 	for(n = 0; n < UIDHASH_SZ; ++n)
 		INIT_HLIST_HEAD(init_user_ns.uidhash_table + n);
--

To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 3 lines · Nick Piggin

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.

Quoted 86 lines

Signed-off-by Nick Piggin <[email protected]>

---
Index: linux-2.6/fs/block_dev.c
===================================================================

fs/block_dev.c +2 -2

@@ -332,8 +332,8 @@ void __init bdev_cache_init(void)
 {
 	int err;
 	bdev_cachep = kmem_cache_create("bdev_cache", sizeof(struct 
 	bdev_inode),
-			0, (SLAB_HWCACHE_ALIGN|SLAB_RECLAIM_ACCOUNT|
-				SLAB_MEM_SPREAD|SLAB_PANIC),
+			0, (SLAB_HWCACHE_ALIGN|SLAB_SMP_ALIGN|
+			SLAB_RECLAIM_ACCOUNT|SLAB_MEM_SPREAD|SLAB_PANIC),
 			init_once);
 	err = register_filesystem(&bd_type);

if (err)
Index: linux-2.6/kernel/fork.c
===================================================================

kernel/fork.c +5 -5

@@ -1547,23 +1547,23 @@ void __init proc_caches_init(void)
 {
 	sighand_cachep = kmem_cache_create("sighand_cache",
 			sizeof(struct sighand_struct), 0,
-			SLAB_HWCACHE_ALIGN|SLAB_PANIC|SLAB_DESTROY_BY_RCU,
+		 
 SLAB_HWCACHE_ALIGN|SLAB_SMP_ALIGN|SLAB_PANIC|SLAB_DESTROY_BY_RCU,
 			sighand_ctor);
 	signal_cachep = kmem_cache_create("signal_cache",
 			sizeof(struct signal_struct), 0,
-			SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL);
+			SLAB_HWCACHE_ALIGN|SLAB_SMP_ALIGN|SLAB_PANIC, NULL);
 	files_cachep = kmem_cache_create("files_cache",
 			sizeof(struct files_struct), 0,
-			SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL);
+			SLAB_HWCACHE_ALIGN|SLAB_SMP_ALIGN|SLAB_PANIC, NULL);
 	fs_cachep = kmem_cache_create("fs_cache",
 			sizeof(struct fs_struct), 0,
-			SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL);
+			SLAB_HWCACHE_ALIGN|SLAB_SMP_ALIGN|SLAB_PANIC, NULL);
 	vm_area_cachep = kmem_cache_create("vm_area_struct",
 			sizeof(struct vm_area_struct), 0,
 			SLAB_PANIC, NULL);
 	mm_cachep = kmem_cache_create("mm_struct",
 			sizeof(struct mm_struct), ARCH_MIN_MMSTRUCT_ALIGN,
-			SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL);
+			SLAB_HWCACHE_ALIGN|SLAB_SMP_ALIGN|SLAB_PANIC, NULL);
 }
 

/*
Index: linux-2.6/kernel/pid.c
===================================================================

kernel/pid.c +1 -1

@@ -526,5 +526,5 @@ void __init pidmap_init(void)
 	atomic_dec(&init_pid_ns.pidmap[0].nr_free);
 
 	init_pid_ns.pid_cachep = KMEM_CACHE(pid,
-			SLAB_HWCACHE_ALIGN | SLAB_PANIC);
+			SLAB_HWCACHE_ALIGN | SLAB_SMP_ALIGN | SLAB_PANIC);
 }

Index: linux-2.6/kernel/user.c
===================================================================

kernel/user.c +1 -1

@@ -503,7 +503,7 @@ static int __init uid_cache_init(void)
 	int n;
 
 	uid_cachep = kmem_cache_create("uid_cache", sizeof(struct 
 	user_struct),
-			0, SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL);
+			0, SLAB_HWCACHE_ALIGN|SLAB_SMP_ALIGN|SLAB_PANIC, 
 NULL);
 

	for(n = 0; n < UIDHASH_SZ; ++n)
		INIT_HLIST_HEAD(init_user_ns.uidhash_table + n);

--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 8 lines · Eric Dumazet

Quoted 4 lines · Nick Piggin

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.

Quoted 21 lines · Nick Piggin

Quoted 9 lines · Eric Dumazet

Quoted 4 lines · Nick Piggin

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 ;)

Quoted 7 lines

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.

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.

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 */

Quoted 6 lines

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 :))

Hi,

Quoted 7 lines · Nick Piggin

Quoted 6 lines

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 */

Quoted 7 lines

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 :))

At least historically SLAB_HWCACHE_ALIGN has been just a hint, although slab tries very hard to satisfy it (see the comments in mm/slab.c). Why do we need stronger guarantees than that, btw?

This reminds me a previous attempt of removing SLAB_HWCACHE_ALIGN

http://kernel.org/pub/linux/kernel/people/christoph/patch-archive/2007/2.6.21-rc6/remove_hwcache_align

At that time Christoph didnt took into account the CONFIG_SMP thing (false sharing avoidance), but also that L1_CACHE_SIZE is a compile constant, that can differs with cache_line_size()

At least historically SLAB_HWCACHE_ALIGN has been just a hint, although slab tries very hard to satisfy it (see the comments in mm/slab.c). Why do we need stronger guarantees than that, btw?

Quoted 3 lines · Pekka Enberg

At least historically SLAB_HWCACHE_ALIGN has been just a hint, although slab tries very hard to satisfy it (see the comments in mm/slab.c). Why do we need stronger guarantees than that, btw?

Its a hint. Alignment is specified as a parameter to kmem_cache_create not as a flag. Maybe we could remove SLAB_HWCACHE_ALIGN because it is causing so much confusion?

Yeah, that's what I thought too, when I got confused by these new SLUB semantics that you made up. Actually if you look at SLAB, it has very precise and rational semantics. SLUB should respect that.

Quoted 3 lines · Nick Piggin

Yeah, that's what I thought too, when I got confused by these new SLUB semantics that you made up. Actually if you look at SLAB, it has very precise and rational semantics. SLUB should respect that.

These crappy semantics are SLAB semantics that SLUB must support.

They are only crappy in SLUB. In SLAB they are actually useful.

SLUB should pack even small objects nicely into cachelines if that is what has been asked for. Use the same algorithm as SLAB for this.

@@ -1896,12 +1896,15 @@ static unsigned long calculate_alignment
* specified alignment though. If that is greater
* then use it.
*/

- if ((flags & SLAB_HWCACHE_ALIGN) &&
- size > cache_line_size() / 2)
- return max_t(unsigned long, align, cache_line_size());
+ if (flags & SLAB_HWCACHE_ALIGN) {
+ unsigned long ralign = cache_line_size();
+ while (size <= ralign / 2)
+ ralign /= 2;
+ align = max(align, ralign);
+ }

Hi Nick,

Quoted 2 lines · Nick Piggin

SLUB should pack even small objects nicely into cachelines if that is what has been asked for. Use the same algorithm as SLAB for this.

The patches look good but this changelog is missing context. Why do we want to do this?

Quoted 4 lines

@@ -1896,12 +1896,15 @@ static unsigned long calculate_alignment
* specified alignment though. If that is greater
* then use it.
*/

You might want to fix the above comment too.

Quoted 10 lines

- if ((flags & SLAB_HWCACHE_ALIGN) &&
- size > cache_line_size() / 2)
- return max_t(unsigned long, align, cache_line_size());
+ if (flags & SLAB_HWCACHE_ALIGN) {
+ unsigned long ralign = cache_line_size();
+ while (size <= ralign / 2)
+ ralign /= 2;
+ align = max(align, ralign);
+ }

SLUB should pack even small objects nicely into cachelines if that is what has been asked for. Use the same algorithm as SLAB for this.

Quoted 2 lines · Nick Piggin

SLUB should pack even small objects nicely into cachelines if that is what has been asked for. Use the same algorithm as SLAB for this.

Why? SLAB does not cacheline align objects smaller than cache_line_size() /2.

If they are already not cache line aligned then we can make them as dense as possible. That is what SLUB does.

Quoted 2 lines

If they are already not cache line aligned then we can make them as dense as possible. That is what SLUB does.

Because when you specify HWCACHE_ALIGN, it means that you want the object not to cross cacheline boundaries for at least cache_line_size() bytes. SLAB does this. SLUB does not without this patch.

Quoted 3 lines · Nick Piggin

Quoted 2 lines

If they are already not cache line aligned then we can make them as dense as possible. That is what SLUB does.

Quoted 3 lines

Because when you specify HWCACHE_ALIGN, it means that you want the object not to cross cacheline boundaries for at least cache_line_size() bytes. SLAB does this. SLUB does not without this patch.

HWCACHE_ALIGN means that you want the object to be aligned at cacheline boundaries for optimization. Why does crossing cacheline boundaries matter in this case?

Quoted 3 lines

HWCACHE_ALIGN means that you want the object to be aligned at cacheline boundaries for optimization. Why does crossing cacheline boundaries matter in this case?

No, HWCACHE_ALIGN means that you want the object not to cross cacheline boundaries for at least cache_line_size() bytes. You invented new

Quoted 6 lines

Quoted 3 lines

HWCACHE_ALIGN means that you want the object to be aligned at cacheline boundaries for optimization. Why does crossing cacheline boundaries matter in this case?

No, HWCACHE_ALIGN means that you want the object not to cross cacheline boundaries for at least cache_line_size() bytes. You invented new

Interesting new definition....

Well, not my definition either but SLAB has guaranteed that for small objects in the past, so I think Nick has a point here. However, with all this back and forth, I've lost track why this matters. I suppose it causes regression on some workload?

Well the guarantee can only be exploited if you would check the cacheline sizes and the object size from the code that creates the slab cache. Basically you would have to guestimate what the slab allocator is doing.

So the guarantee is basically meaningless. If the object is larger than a cacheline then this will never work.

Hi,

Quoted 4 lines · Christoph Lameter

Well the guarantee can only be exploited if you would check the cacheline sizes and the object size from the code that creates the slab cache. Basically you would have to guestimate what the slab allocator is doing.

Quoted 2 lines

So the guarantee is basically meaningless. If the object is larger than a cacheline then this will never work.

Yes, I know that. That's why I am asking why this matters. If there's some sort of regression because SLUB does HWCACHE_ALIGN bit differently, we need to fix that.

Not that it necessarily means we have to change HWCACHE_ALIGN but I am assuming Nick has some reason why he wants to introduce the SMP alignment flag.

It started out as a SLUB regression that was exposing poor code in the percpu allocator due to different SLUB kmalloc alignments. That prompted

The SMP flag was just an RFC. I think some people (like Christoph) were being confused about the HWCACHE_ALIGN flag being for avoiding false sharing on SMP systems. It would actually be also generally useful to have the SMP flag (eg. see the sites I added it to in patch #3).

Quoted 2 lines · Nick Piggin

It started out as a SLUB regression that was exposing poor code in the percpu allocator due to different SLUB kmalloc alignments. That prompted

That was due to SLUB's support for smaller allocation sizes. AFAICT has nothing to do with alignment.

Quoted 4 lines

The SMP flag was just an RFC. I think some people (like Christoph) were being confused about the HWCACHE_ALIGN flag being for avoiding false sharing on SMP systems. It would actually be also generally useful to have the SMP flag (eg. see the sites I added it to in patch #3).

Hmmm. We could define a global constant for that? Determine it on bootup and then pass it as an alignment parameter?

Quoted 2 lines

That was due to SLUB's support for smaller allocation sizes. AFAICT has nothing to do with alignment.

The smaller sizes meant objects were less often aligned on cacheline boundaries.

We could, but I'd rather just use the flag.

Quoted 5 lines · Nick Piggin

Quoted 2 lines

That was due to SLUB's support for smaller allocation sizes. AFAICT has nothing to do with alignment.

The smaller sizes meant objects were less often aligned on cacheline boundaries.

Right since SLAB_HWCACHE_ALIGN does not align for very small objects.

Quoted 1 line

We could, but I'd rather just use the flag.

Do you have a case in mind where that would be useful? We had a

SLAB_HWCACHE_MUST_ALIGN or so at some point but it was rarely to never used.

Note that there is also KMEM_CACHE which picks up the alignment from the compiler.

Quoted 1 line

Do you have a case in mind where that would be useful? We had a

Patch 3/3

Quoted 2 lines

Note that there is also KMEM_CACHE which picks up the alignment from the compiler.

Yeah, that's not quite as good either. My allocation flag is dynamic, so it will not bloat things for no reason on UP machines and SMP kernels. It also aligns to the detected machine cacheline size rather than a compile time constant.

Quoted 3 lines · Nick Piggin

Quoted 1 line

Do you have a case in mind where that would be useful? We had a

Patch 3/3

Those already have SLAB_HWCACHE_ALIGN.

The point is to switch off alignment for UP? Cant we do that with SLAB_HWCACHE_ALIGN too since SLOB seemed to work successfully without it in the past?

Quoted 7 lines

Quoted 2 lines

Note that there is also KMEM_CACHE which picks up the alignment from the compiler.

Yeah, that's not quite as good either. My allocation flag is dynamic, so it will not bloat things for no reason on UP machines and SMP kernels. It also aligns to the detected machine cacheline size rather than a compile time constant.

Is that really a noteworthy effect?

Quoted 1 line

Is that really a noteworthy effect?

Yes.

Quoted 3 lines · Nick Piggin

Quoted 1 line

Is that really a noteworthy effect?

Yes.

Numbers?

Quoted 7 lines · Christoph Lameter

Quoted 3 lines · Nick Piggin

Quoted 1 line

Is that really a noteworthy effect?

Yes.

Numbers?

Uhh... big number > small number

Quoted 5 lines · Nick Piggin

Quoted 4 lines · Christoph Lameter

Quoted 2 lines · Nick Piggin

Quoted 1 line

Is that really a noteworthy effect?

Quoted 5 lines

Quoted 3 lines

Quoted 1 line

Yes.

Numbers?

Uhh... big number > small number

Can you quantify the effect?

Quoted 4 lines · Pekka Enberg

Well, not my definition either but SLAB has guaranteed that for small objects in the past, so I think Nick has a point here. However, with all this back and forth, I've lost track why this matters. I suppose it causes regression on some workload?

Well the guarantee can only be exploited if you would check the cacheline sizes and the object size from the code that creates the slab cache. Basically you would have to guestimate what the slab allocator is doing.

So the guarantee is basically meaningless. If the object is larger than a cacheline then this will never work.

Quoted 6 lines · Nick Piggin

Quoted 3 lines

HWCACHE_ALIGN means that you want the object to be aligned at cacheline boundaries for optimization. Why does crossing cacheline boundaries matter in this case?

No, HWCACHE_ALIGN means that you want the object not to cross cacheline boundaries for at least cache_line_size() bytes. You invented new

Interesting new definition....

Quoted 6 lines · Nick Piggin

Quoted 5 lines

Quoted 4 lines

HWCACHE_ALIGN means that you want the object to be aligned at cacheline boundaries for optimization. Why does crossing cacheline boundaries matter in this case?

Quoted 4 lines

Quoted 2 lines

No, HWCACHE_ALIGN means that you want the object not to cross cacheline boundaries for at least cache_line_size() bytes. You invented new

Interesting new definition....

Huh?? It is not a new definition, it is exactly what SLAB does. And then you go and do something different and claim that you follow what slab does.

Quoted 3 lines

Huh?? It is not a new definition, it is exactly what SLAB does. And then you go and do something different and claim that you follow what slab does.

I completely agree with Nick.

Quoted 5 lines · David Miller

Quoted 3 lines

Huh?? It is not a new definition, it is exactly what SLAB does. And then you go and do something different and claim that you follow what slab does.

I completely agree with Nick.

So you also want subalignment because of cacheline crossing for 24 byte slabs? We then only have 2 objects per cacheline instead of 3 but no crossing anymore.

Well okay if there are multiple requests then lets merge Nick's patch that does this. Still think that this will do much ...
Instead of 170 we will only have 128 objects per slab (64 byte cacheline).

It will affect the following slab caches (mm) reducing the density of objects.

scsi_bidi_sdb numa_policy fasync_cache xfs_bmap_free_item xfs_dabuf fstrm_item dm_target_io

Nothing related to networking....

There looks like definitely some networking slabs that pass the flag and can be non-power-of-2. And don't forget cachelines can be anywhere up to 256 bytes in size. So yeah it definitely makes sense to merge the patch and then examine the callers if you feel strongly about it.

Quoted 4 lines · Nick Piggin

There looks like definitely some networking slabs that pass the flag and can be non-power-of-2. And don't forget cachelines can be anywhere up to 256 bytes in size. So yeah it definitely makes sense to merge the patch and then examine the callers if you feel strongly about it.

Just do not like to add fluff that has basically no effect. I just tried to improve things by not doing anything special if we cannot cacheline align object. Least surprise (at least for me). It is bad enough that we just decide to ignore the request for alignment for small caches.

That's just because you (apparently still) have a misconception about what the flag is supposed to be for. It is not for aligning things to the start of a cacheline boundary. It is not for avoiding false sharing on SMP. It

Quoted 3 lines · Nick Piggin

That's just because you (apparently still) have a misconception about what the flag is supposed to be for. It is not for aligning things to the start of a cacheline boundary. It is not for avoiding false sharing on SMP. It

The alignment of the object to the start of a cacheline is the obvious meaning and that is also reflected in the comment in slab.h.

It doesn't say start of cache line. It says align them *on* cachelines. 2 32 byte objects on a 64 byte cacheline are aligned on the cacheline. 2.67 24 bytes objects on a 64 byte cacheline are not aligned on the cacheline.

Quoted 4 lines

It doesn't say start of cache line. It says align them *on* cachelines. 2 32 byte objects on a 64 byte cacheline are aligned on the cacheline. 2.67 24 bytes objects on a 64 byte cacheline are not aligned on the cacheline.

2 32 byte objects means only one is aligned on a cache line.

Certainly cacheline contention is reduced and performance potentially increased if there are less objects in a cacheline.

The same argument can be made of aligning 8 byte objects on 32 byte boundaries. Instead of 8 objects per cacheline you only have two. Why 8?

Isnt all of this a bit arbitrary and contrary to the intend of avoiding cacheline contention?

No, it *is not about avoiding cacheline contention*. As such, the rest of what you wrote below about smp_align etc is rubbish.

Can you actually read what I posted?

Quoted 4 lines · Nick Piggin

No, it *is not about avoiding cacheline contention*. As such, the rest of what you wrote below about smp_align etc is rubbish.

Can you actually read what I posted?

I am trying but it barely makes sense. You first state that it is *not* about avoiding cacheline contention and then argue that it reduces cacheline contention.

Where do I argue that it reduces cacheline contention? All arguments I made apply to a UP system equally.

Quoted 2 lines · Nick Piggin

Where do I argue that it reduces cacheline contention? All arguments I made apply to a UP system equally.

Huh? I thought you wanted to avoid cacheline alignment on UP to save memory?