[PATCH v2] mm/slab: fix warning caused by duplicate kmem_cache creation in kmem_buckets_create

Koichiro Den posted 1 patch 2 weeks, 5 days ago
mm/slab_common.c | 36 +++++++++++++++++++++++-------------
1 file changed, 23 insertions(+), 13 deletions(-)
[PATCH v2] mm/slab: fix warning caused by duplicate kmem_cache creation in kmem_buckets_create
Posted by Koichiro Den 2 weeks, 5 days ago
Commit b035f5a6d852 ("mm: slab: reduce the kmalloc() minimum alignment
if DMA bouncing possible") reduced ARCH_KMALLOC_MINALIGN to 8 on arm64.
However, with KASAN_HW_TAGS enabled, arch_slab_minalign() becomes 16.
This causes kmalloc_caches[*][8] to be aliased to kmalloc_caches[*][16],
resulting in kmem_buckets_create() attempting to create a kmem_cache for
size 16 twice. This duplication triggers warnings on boot:

[    2.325108] ------------[ cut here ]------------
[    2.325135] kmem_cache of name 'memdup_user-16' already exists
[    2.325783] WARNING: CPU: 0 PID: 1 at mm/slab_common.c:107 __kmem_cache_create_args+0xb8/0x3b0
[    2.327957] Modules linked in:
[    2.328550] CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.12.0-rc5mm-unstable-arm64+ #12
[    2.328683] Hardware name: QEMU QEMU Virtual Machine, BIOS 2024.02-2 03/11/2024
[    2.328790] pstate: 61000009 (nZCv daif -PAN -UAO -TCO +DIT -SSBS BTYPE=--)
[    2.328911] pc : __kmem_cache_create_args+0xb8/0x3b0
[    2.328930] lr : __kmem_cache_create_args+0xb8/0x3b0
[    2.328942] sp : ffff800083d6fc50
[    2.328961] x29: ffff800083d6fc50 x28: f2ff0000c1674410 x27: ffff8000820b0598
[    2.329061] x26: 000000007fffffff x25: 0000000000000010 x24: 0000000000002000
[    2.329101] x23: ffff800083d6fce8 x22: ffff8000832222e8 x21: ffff800083222388
[    2.329118] x20: f2ff0000c1674410 x19: f5ff0000c16364c0 x18: ffff800083d80030
[    2.329135] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000
[    2.329152] x14: 0000000000000000 x13: 0a73747369786520 x12: 79646165726c6120
[    2.329169] x11: 656820747563205b x10: 2d2d2d2d2d2d2d2d x9 : 0000000000000000
[    2.329194] x8 : 0000000000000000 x7 : 0000000000000000 x6 : 0000000000000000
[    2.329210] x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000
[    2.329226] x2 : 0000000000000000 x1 : 0000000000000000 x0 : 0000000000000000
[    2.329291] Call trace:
[    2.329407]  __kmem_cache_create_args+0xb8/0x3b0
[    2.329499]  kmem_buckets_create+0xfc/0x320
[    2.329526]  init_user_buckets+0x34/0x78
[    2.329540]  do_one_initcall+0x64/0x3c8
[    2.329550]  kernel_init_freeable+0x26c/0x578
[    2.329562]  kernel_init+0x3c/0x258
[    2.329574]  ret_from_fork+0x10/0x20
[    2.329698] ---[ end trace 0000000000000000 ]---

[    2.403704] ------------[ cut here ]------------
[    2.404716] kmem_cache of name 'msg_msg-16' already exists
[    2.404801] WARNING: CPU: 2 PID: 1 at mm/slab_common.c:107 __kmem_cache_create_args+0xb8/0x3b0
[    2.404842] Modules linked in:
[    2.404971] CPU: 2 UID: 0 PID: 1 Comm: swapper/0 Tainted: G        W          6.12.0-rc5mm-unstable-arm64+ #12
[    2.405026] Tainted: [W]=WARN
[    2.405043] Hardware name: QEMU QEMU Virtual Machine, BIOS 2024.02-2 03/11/2024
[    2.405057] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[    2.405079] pc : __kmem_cache_create_args+0xb8/0x3b0
[    2.405100] lr : __kmem_cache_create_args+0xb8/0x3b0
[    2.405111] sp : ffff800083d6fc50
[    2.405115] x29: ffff800083d6fc50 x28: fbff0000c1674410 x27: ffff8000820b0598
[    2.405135] x26: 000000000000ffd0 x25: 0000000000000010 x24: 0000000000006000
[    2.405153] x23: ffff800083d6fce8 x22: ffff8000832222e8 x21: ffff800083222388
[    2.405169] x20: fbff0000c1674410 x19: fdff0000c163d6c0 x18: ffff800083d80030
[    2.405185] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000
[    2.405201] x14: 0000000000000000 x13: 0a73747369786520 x12: 79646165726c6120
[    2.405217] x11: 656820747563205b x10: 2d2d2d2d2d2d2d2d x9 : 0000000000000000
[    2.405233] x8 : 0000000000000000 x7 : 0000000000000000 x6 : 0000000000000000
[    2.405248] x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000
[    2.405271] x2 : 0000000000000000 x1 : 0000000000000000 x0 : 0000000000000000
[    2.405287] Call trace:
[    2.405293]  __kmem_cache_create_args+0xb8/0x3b0
[    2.405305]  kmem_buckets_create+0xfc/0x320
[    2.405315]  init_msg_buckets+0x34/0x78
[    2.405326]  do_one_initcall+0x64/0x3c8
[    2.405337]  kernel_init_freeable+0x26c/0x578
[    2.405348]  kernel_init+0x3c/0x258
[    2.405360]  ret_from_fork+0x10/0x20
[    2.405370] ---[ end trace 0000000000000000 ]---

To address this, alias kmem_cache for sizes smaller than min alignment
to the aligned sized kmem_cache, as done with the default system kmalloc
bucket.

Cc: <stable@vger.kernel.org> # 6.11.x
Fixes: b32801d1255b ("mm/slab: Introduce kmem_buckets_create() and family")
Signed-off-by: Koichiro Den <koichiro.den@gmail.com>
---
Changes in v2:
- Simplify by just reusing calculated aligned size stored in
  the default kmalloc_caches
---
 mm/slab_common.c | 36 +++++++++++++++++++++++-------------
 1 file changed, 23 insertions(+), 13 deletions(-)

diff --git a/mm/slab_common.c b/mm/slab_common.c
index 3d26c257ed8b..db6ffe53c23e 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -380,8 +380,11 @@ kmem_buckets *kmem_buckets_create(const char *name, slab_flags_t flags,
 				  unsigned int usersize,
 				  void (*ctor)(void *))
 {
+	unsigned long mask = 0;
+	unsigned int idx;
 	kmem_buckets *b;
-	int idx;
+
+	BUILD_BUG_ON(ARRAY_SIZE(kmalloc_caches[KMALLOC_NORMAL]) > BITS_PER_LONG);
 
 	/*
 	 * When the separate buckets API is not built in, just return
@@ -403,7 +406,7 @@ kmem_buckets *kmem_buckets_create(const char *name, slab_flags_t flags,
 	for (idx = 0; idx < ARRAY_SIZE(kmalloc_caches[KMALLOC_NORMAL]); idx++) {
 		char *short_size, *cache_name;
 		unsigned int cache_useroffset, cache_usersize;
-		unsigned int size;
+		unsigned int size, aligned_idx;
 
 		if (!kmalloc_caches[KMALLOC_NORMAL][idx])
 			continue;
@@ -416,10 +419,6 @@ kmem_buckets *kmem_buckets_create(const char *name, slab_flags_t flags,
 		if (WARN_ON(!short_size))
 			goto fail;
 
-		cache_name = kasprintf(GFP_KERNEL, "%s-%s", name, short_size + 1);
-		if (WARN_ON(!cache_name))
-			goto fail;
-
 		if (useroffset >= size) {
 			cache_useroffset = 0;
 			cache_usersize = 0;
@@ -427,18 +426,29 @@ kmem_buckets *kmem_buckets_create(const char *name, slab_flags_t flags,
 			cache_useroffset = useroffset;
 			cache_usersize = min(size - cache_useroffset, usersize);
 		}
-		(*b)[idx] = kmem_cache_create_usercopy(cache_name, size,
-					0, flags, cache_useroffset,
-					cache_usersize, ctor);
-		kfree(cache_name);
-		if (WARN_ON(!(*b)[idx]))
-			goto fail;
+
+		aligned_idx = __kmalloc_index(size, false);
+		if (!(*b)[aligned_idx]) {
+			cache_name = kasprintf(GFP_KERNEL, "%s-%s", name, short_size + 1);
+			if (WARN_ON(!cache_name))
+				goto fail;
+			(*b)[aligned_idx] = kmem_cache_create_usercopy(cache_name, size,
+						0, flags, cache_useroffset,
+						cache_usersize, ctor);
+			if (WARN_ON(!(*b)[aligned_idx])) {
+				kfree(cache_name);
+				goto fail;
+			}
+			set_bit(aligned_idx, &mask);
+		}
+		if (idx != aligned_idx)
+			(*b)[idx] = (*b)[aligned_idx];
 	}
 
 	return b;
 
 fail:
-	for (idx = 0; idx < ARRAY_SIZE(kmalloc_caches[KMALLOC_NORMAL]); idx++)
+	for_each_set_bit(idx, &mask, ARRAY_SIZE(kmalloc_caches[KMALLOC_NORMAL]))
 		kmem_cache_destroy((*b)[idx]);
 	kmem_cache_free(kmem_buckets_cache, b);
 
-- 
2.43.0
Re: [PATCH v2] mm/slab: fix warning caused by duplicate kmem_cache creation in kmem_buckets_create
Posted by Vlastimil Babka 2 weeks, 4 days ago
On 11/5/24 03:27, Koichiro Den wrote:
> Commit b035f5a6d852 ("mm: slab: reduce the kmalloc() minimum alignment
> if DMA bouncing possible") reduced ARCH_KMALLOC_MINALIGN to 8 on arm64.
> However, with KASAN_HW_TAGS enabled, arch_slab_minalign() becomes 16.
> This causes kmalloc_caches[*][8] to be aliased to kmalloc_caches[*][16],
> resulting in kmem_buckets_create() attempting to create a kmem_cache for
> size 16 twice. This duplication triggers warnings on boot:
> 
> [    2.325108] ------------[ cut here ]------------
> [    2.325135] kmem_cache of name 'memdup_user-16' already exists
> [    2.325783] WARNING: CPU: 0 PID: 1 at mm/slab_common.c:107 __kmem_cache_create_args+0xb8/0x3b0
> [    2.327957] Modules linked in:
> [    2.328550] CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.12.0-rc5mm-unstable-arm64+ #12
> [    2.328683] Hardware name: QEMU QEMU Virtual Machine, BIOS 2024.02-2 03/11/2024
> [    2.328790] pstate: 61000009 (nZCv daif -PAN -UAO -TCO +DIT -SSBS BTYPE=--)
> [    2.328911] pc : __kmem_cache_create_args+0xb8/0x3b0
> [    2.328930] lr : __kmem_cache_create_args+0xb8/0x3b0
> [    2.328942] sp : ffff800083d6fc50
> [    2.328961] x29: ffff800083d6fc50 x28: f2ff0000c1674410 x27: ffff8000820b0598
> [    2.329061] x26: 000000007fffffff x25: 0000000000000010 x24: 0000000000002000
> [    2.329101] x23: ffff800083d6fce8 x22: ffff8000832222e8 x21: ffff800083222388
> [    2.329118] x20: f2ff0000c1674410 x19: f5ff0000c16364c0 x18: ffff800083d80030
> [    2.329135] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000
> [    2.329152] x14: 0000000000000000 x13: 0a73747369786520 x12: 79646165726c6120
> [    2.329169] x11: 656820747563205b x10: 2d2d2d2d2d2d2d2d x9 : 0000000000000000
> [    2.329194] x8 : 0000000000000000 x7 : 0000000000000000 x6 : 0000000000000000
> [    2.329210] x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000
> [    2.329226] x2 : 0000000000000000 x1 : 0000000000000000 x0 : 0000000000000000
> [    2.329291] Call trace:
> [    2.329407]  __kmem_cache_create_args+0xb8/0x3b0
> [    2.329499]  kmem_buckets_create+0xfc/0x320
> [    2.329526]  init_user_buckets+0x34/0x78
> [    2.329540]  do_one_initcall+0x64/0x3c8
> [    2.329550]  kernel_init_freeable+0x26c/0x578
> [    2.329562]  kernel_init+0x3c/0x258
> [    2.329574]  ret_from_fork+0x10/0x20
> [    2.329698] ---[ end trace 0000000000000000 ]---
> 
> [    2.403704] ------------[ cut here ]------------
> [    2.404716] kmem_cache of name 'msg_msg-16' already exists
> [    2.404801] WARNING: CPU: 2 PID: 1 at mm/slab_common.c:107 __kmem_cache_create_args+0xb8/0x3b0
> [    2.404842] Modules linked in:
> [    2.404971] CPU: 2 UID: 0 PID: 1 Comm: swapper/0 Tainted: G        W          6.12.0-rc5mm-unstable-arm64+ #12
> [    2.405026] Tainted: [W]=WARN
> [    2.405043] Hardware name: QEMU QEMU Virtual Machine, BIOS 2024.02-2 03/11/2024
> [    2.405057] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [    2.405079] pc : __kmem_cache_create_args+0xb8/0x3b0
> [    2.405100] lr : __kmem_cache_create_args+0xb8/0x3b0
> [    2.405111] sp : ffff800083d6fc50
> [    2.405115] x29: ffff800083d6fc50 x28: fbff0000c1674410 x27: ffff8000820b0598
> [    2.405135] x26: 000000000000ffd0 x25: 0000000000000010 x24: 0000000000006000
> [    2.405153] x23: ffff800083d6fce8 x22: ffff8000832222e8 x21: ffff800083222388
> [    2.405169] x20: fbff0000c1674410 x19: fdff0000c163d6c0 x18: ffff800083d80030
> [    2.405185] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000
> [    2.405201] x14: 0000000000000000 x13: 0a73747369786520 x12: 79646165726c6120
> [    2.405217] x11: 656820747563205b x10: 2d2d2d2d2d2d2d2d x9 : 0000000000000000
> [    2.405233] x8 : 0000000000000000 x7 : 0000000000000000 x6 : 0000000000000000
> [    2.405248] x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000
> [    2.405271] x2 : 0000000000000000 x1 : 0000000000000000 x0 : 0000000000000000
> [    2.405287] Call trace:
> [    2.405293]  __kmem_cache_create_args+0xb8/0x3b0
> [    2.405305]  kmem_buckets_create+0xfc/0x320
> [    2.405315]  init_msg_buckets+0x34/0x78
> [    2.405326]  do_one_initcall+0x64/0x3c8
> [    2.405337]  kernel_init_freeable+0x26c/0x578
> [    2.405348]  kernel_init+0x3c/0x258
> [    2.405360]  ret_from_fork+0x10/0x20
> [    2.405370] ---[ end trace 0000000000000000 ]---
> 
> To address this, alias kmem_cache for sizes smaller than min alignment
> to the aligned sized kmem_cache, as done with the default system kmalloc
> bucket.
> 
> Cc: <stable@vger.kernel.org> # 6.11.x
> Fixes: b32801d1255b ("mm/slab: Introduce kmem_buckets_create() and family")
> Signed-off-by: Koichiro Den <koichiro.den@gmail.com>

Thanks. Given this warning was introduced in 6.12, I'm adding it to the
slab/for-6.12-rc7/fixes branch so we fix it before 6.12 is final.

> @@ -427,18 +426,29 @@ kmem_buckets *kmem_buckets_create(const char *name, slab_flags_t flags,
>  			cache_useroffset = useroffset;
>  			cache_usersize = min(size - cache_useroffset, usersize);
>  		}
> -		(*b)[idx] = kmem_cache_create_usercopy(cache_name, size,
> -					0, flags, cache_useroffset,
> -					cache_usersize, ctor);
> -		kfree(cache_name);
> -		if (WARN_ON(!(*b)[idx]))
> -			goto fail;
> +
> +		aligned_idx = __kmalloc_index(size, false);
> +		if (!(*b)[aligned_idx]) {
> +			cache_name = kasprintf(GFP_KERNEL, "%s-%s", name, short_size + 1);
> +			if (WARN_ON(!cache_name))
> +				goto fail;
> +			(*b)[aligned_idx] = kmem_cache_create_usercopy(cache_name, size,
> +						0, flags, cache_useroffset,
> +						cache_usersize, ctor);
> +			if (WARN_ON(!(*b)[aligned_idx])) {
> +				kfree(cache_name);

Note we need to free cache_name always, because kmem_cache_create() does a
kstrdup_const(), so your change would be creating a memory leak. I've fixed
it up locally.

Vlastimil

> +				goto fail;
> +			}
> +			set_bit(aligned_idx, &mask);
> +		}
> +		if (idx != aligned_idx)
> +			(*b)[idx] = (*b)[aligned_idx];
>  	}
>  
>  	return b;
>  
>  fail:
> -	for (idx = 0; idx < ARRAY_SIZE(kmalloc_caches[KMALLOC_NORMAL]); idx++)
> +	for_each_set_bit(idx, &mask, ARRAY_SIZE(kmalloc_caches[KMALLOC_NORMAL]))
>  		kmem_cache_destroy((*b)[idx]);
>  	kmem_cache_free(kmem_buckets_cache, b);
>
Re: [PATCH v2] mm/slab: fix warning caused by duplicate kmem_cache creation in kmem_buckets_create
Posted by Koichiro Den 2 weeks, 4 days ago
On Tue, Nov 05, 2024 at 05:48:56PM +0100, Vlastimil Babka wrote:
> On 11/5/24 03:27, Koichiro Den wrote:
> > Commit b035f5a6d852 ("mm: slab: reduce the kmalloc() minimum alignment
> > if DMA bouncing possible") reduced ARCH_KMALLOC_MINALIGN to 8 on arm64.
> > However, with KASAN_HW_TAGS enabled, arch_slab_minalign() becomes 16.
> > This causes kmalloc_caches[*][8] to be aliased to kmalloc_caches[*][16],
> > resulting in kmem_buckets_create() attempting to create a kmem_cache for
> > size 16 twice. This duplication triggers warnings on boot:
> > 
> > [    2.325108] ------------[ cut here ]------------
> > [    2.325135] kmem_cache of name 'memdup_user-16' already exists
> > [    2.325783] WARNING: CPU: 0 PID: 1 at mm/slab_common.c:107 __kmem_cache_create_args+0xb8/0x3b0
> > [    2.327957] Modules linked in:
> > [    2.328550] CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.12.0-rc5mm-unstable-arm64+ #12
> > [    2.328683] Hardware name: QEMU QEMU Virtual Machine, BIOS 2024.02-2 03/11/2024
> > [    2.328790] pstate: 61000009 (nZCv daif -PAN -UAO -TCO +DIT -SSBS BTYPE=--)
> > [    2.328911] pc : __kmem_cache_create_args+0xb8/0x3b0
> > [    2.328930] lr : __kmem_cache_create_args+0xb8/0x3b0
> > [    2.328942] sp : ffff800083d6fc50
> > [    2.328961] x29: ffff800083d6fc50 x28: f2ff0000c1674410 x27: ffff8000820b0598
> > [    2.329061] x26: 000000007fffffff x25: 0000000000000010 x24: 0000000000002000
> > [    2.329101] x23: ffff800083d6fce8 x22: ffff8000832222e8 x21: ffff800083222388
> > [    2.329118] x20: f2ff0000c1674410 x19: f5ff0000c16364c0 x18: ffff800083d80030
> > [    2.329135] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000
> > [    2.329152] x14: 0000000000000000 x13: 0a73747369786520 x12: 79646165726c6120
> > [    2.329169] x11: 656820747563205b x10: 2d2d2d2d2d2d2d2d x9 : 0000000000000000
> > [    2.329194] x8 : 0000000000000000 x7 : 0000000000000000 x6 : 0000000000000000
> > [    2.329210] x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000
> > [    2.329226] x2 : 0000000000000000 x1 : 0000000000000000 x0 : 0000000000000000
> > [    2.329291] Call trace:
> > [    2.329407]  __kmem_cache_create_args+0xb8/0x3b0
> > [    2.329499]  kmem_buckets_create+0xfc/0x320
> > [    2.329526]  init_user_buckets+0x34/0x78
> > [    2.329540]  do_one_initcall+0x64/0x3c8
> > [    2.329550]  kernel_init_freeable+0x26c/0x578
> > [    2.329562]  kernel_init+0x3c/0x258
> > [    2.329574]  ret_from_fork+0x10/0x20
> > [    2.329698] ---[ end trace 0000000000000000 ]---
> > 
> > [    2.403704] ------------[ cut here ]------------
> > [    2.404716] kmem_cache of name 'msg_msg-16' already exists
> > [    2.404801] WARNING: CPU: 2 PID: 1 at mm/slab_common.c:107 __kmem_cache_create_args+0xb8/0x3b0
> > [    2.404842] Modules linked in:
> > [    2.404971] CPU: 2 UID: 0 PID: 1 Comm: swapper/0 Tainted: G        W          6.12.0-rc5mm-unstable-arm64+ #12
> > [    2.405026] Tainted: [W]=WARN
> > [    2.405043] Hardware name: QEMU QEMU Virtual Machine, BIOS 2024.02-2 03/11/2024
> > [    2.405057] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> > [    2.405079] pc : __kmem_cache_create_args+0xb8/0x3b0
> > [    2.405100] lr : __kmem_cache_create_args+0xb8/0x3b0
> > [    2.405111] sp : ffff800083d6fc50
> > [    2.405115] x29: ffff800083d6fc50 x28: fbff0000c1674410 x27: ffff8000820b0598
> > [    2.405135] x26: 000000000000ffd0 x25: 0000000000000010 x24: 0000000000006000
> > [    2.405153] x23: ffff800083d6fce8 x22: ffff8000832222e8 x21: ffff800083222388
> > [    2.405169] x20: fbff0000c1674410 x19: fdff0000c163d6c0 x18: ffff800083d80030
> > [    2.405185] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000
> > [    2.405201] x14: 0000000000000000 x13: 0a73747369786520 x12: 79646165726c6120
> > [    2.405217] x11: 656820747563205b x10: 2d2d2d2d2d2d2d2d x9 : 0000000000000000
> > [    2.405233] x8 : 0000000000000000 x7 : 0000000000000000 x6 : 0000000000000000
> > [    2.405248] x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000
> > [    2.405271] x2 : 0000000000000000 x1 : 0000000000000000 x0 : 0000000000000000
> > [    2.405287] Call trace:
> > [    2.405293]  __kmem_cache_create_args+0xb8/0x3b0
> > [    2.405305]  kmem_buckets_create+0xfc/0x320
> > [    2.405315]  init_msg_buckets+0x34/0x78
> > [    2.405326]  do_one_initcall+0x64/0x3c8
> > [    2.405337]  kernel_init_freeable+0x26c/0x578
> > [    2.405348]  kernel_init+0x3c/0x258
> > [    2.405360]  ret_from_fork+0x10/0x20
> > [    2.405370] ---[ end trace 0000000000000000 ]---
> > 
> > To address this, alias kmem_cache for sizes smaller than min alignment
> > to the aligned sized kmem_cache, as done with the default system kmalloc
> > bucket.
> > 
> > Cc: <stable@vger.kernel.org> # 6.11.x
> > Fixes: b32801d1255b ("mm/slab: Introduce kmem_buckets_create() and family")
> > Signed-off-by: Koichiro Den <koichiro.den@gmail.com>
> 
> Thanks. Given this warning was introduced in 6.12, I'm adding it to the
> slab/for-6.12-rc7/fixes branch so we fix it before 6.12 is final.
> 
> > @@ -427,18 +426,29 @@ kmem_buckets *kmem_buckets_create(const char *name, slab_flags_t flags,
> >  			cache_useroffset = useroffset;
> >  			cache_usersize = min(size - cache_useroffset, usersize);
> >  		}
> > -		(*b)[idx] = kmem_cache_create_usercopy(cache_name, size,
> > -					0, flags, cache_useroffset,
> > -					cache_usersize, ctor);
> > -		kfree(cache_name);
> > -		if (WARN_ON(!(*b)[idx]))
> > -			goto fail;
> > +
> > +		aligned_idx = __kmalloc_index(size, false);
> > +		if (!(*b)[aligned_idx]) {
> > +			cache_name = kasprintf(GFP_KERNEL, "%s-%s", name, short_size + 1);
> > +			if (WARN_ON(!cache_name))
> > +				goto fail;
> > +			(*b)[aligned_idx] = kmem_cache_create_usercopy(cache_name, size,
> > +						0, flags, cache_useroffset,
> > +						cache_usersize, ctor);
> > +			if (WARN_ON(!(*b)[aligned_idx])) {
> > +				kfree(cache_name);
> 
> Note we need to free cache_name always, because kmem_cache_create() does a
> kstrdup_const(), so your change would be creating a memory leak. I've fixed
> it up locally.
> 
> Vlastimil

Ugh, sorry about that. Thanks.

> 
> > +				goto fail;
> > +			}
> > +			set_bit(aligned_idx, &mask);
> > +		}
> > +		if (idx != aligned_idx)
> > +			(*b)[idx] = (*b)[aligned_idx];
> >  	}
> >  
> >  	return b;
> >  
> >  fail:
> > -	for (idx = 0; idx < ARRAY_SIZE(kmalloc_caches[KMALLOC_NORMAL]); idx++)
> > +	for_each_set_bit(idx, &mask, ARRAY_SIZE(kmalloc_caches[KMALLOC_NORMAL]))
> >  		kmem_cache_destroy((*b)[idx]);
> >  	kmem_cache_free(kmem_buckets_cache, b);
> >  
>
Re: [PATCH v2] mm/slab: fix warning caused by duplicate kmem_cache creation in kmem_buckets_create
Posted by Catalin Marinas 2 weeks, 5 days ago
On Tue, Nov 05, 2024 at 11:27:47AM +0900, Koichiro Den wrote:
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 3d26c257ed8b..db6ffe53c23e 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -380,8 +380,11 @@ kmem_buckets *kmem_buckets_create(const char *name, slab_flags_t flags,
>  				  unsigned int usersize,
>  				  void (*ctor)(void *))
>  {
> +	unsigned long mask = 0;
> +	unsigned int idx;
>  	kmem_buckets *b;
> -	int idx;
> +
> +	BUILD_BUG_ON(ARRAY_SIZE(kmalloc_caches[KMALLOC_NORMAL]) > BITS_PER_LONG);
>  
>  	/*
>  	 * When the separate buckets API is not built in, just return
> @@ -403,7 +406,7 @@ kmem_buckets *kmem_buckets_create(const char *name, slab_flags_t flags,
>  	for (idx = 0; idx < ARRAY_SIZE(kmalloc_caches[KMALLOC_NORMAL]); idx++) {
>  		char *short_size, *cache_name;
>  		unsigned int cache_useroffset, cache_usersize;
> -		unsigned int size;
> +		unsigned int size, aligned_idx;
>  
>  		if (!kmalloc_caches[KMALLOC_NORMAL][idx])
>  			continue;
> @@ -416,10 +419,6 @@ kmem_buckets *kmem_buckets_create(const char *name, slab_flags_t flags,
>  		if (WARN_ON(!short_size))
>  			goto fail;
>  
> -		cache_name = kasprintf(GFP_KERNEL, "%s-%s", name, short_size + 1);
> -		if (WARN_ON(!cache_name))
> -			goto fail;
> -
>  		if (useroffset >= size) {
>  			cache_useroffset = 0;
>  			cache_usersize = 0;
> @@ -427,18 +426,29 @@ kmem_buckets *kmem_buckets_create(const char *name, slab_flags_t flags,
>  			cache_useroffset = useroffset;
>  			cache_usersize = min(size - cache_useroffset, usersize);
>  		}
> -		(*b)[idx] = kmem_cache_create_usercopy(cache_name, size,
> -					0, flags, cache_useroffset,
> -					cache_usersize, ctor);
> -		kfree(cache_name);
> -		if (WARN_ON(!(*b)[idx]))
> -			goto fail;
> +
> +		aligned_idx = __kmalloc_index(size, false);
> +		if (!(*b)[aligned_idx]) {
> +			cache_name = kasprintf(GFP_KERNEL, "%s-%s", name, short_size + 1);
> +			if (WARN_ON(!cache_name))
> +				goto fail;
> +			(*b)[aligned_idx] = kmem_cache_create_usercopy(cache_name, size,
> +						0, flags, cache_useroffset,
> +						cache_usersize, ctor);
> +			if (WARN_ON(!(*b)[aligned_idx])) {
> +				kfree(cache_name);
> +				goto fail;
> +			}
> +			set_bit(aligned_idx, &mask);
> +		}
> +		if (idx != aligned_idx)
> +			(*b)[idx] = (*b)[aligned_idx];
>  	}

It looks fine. This pretty much matches the logic in new_kmalloc_cache()
(from commit 963e84b0f262).

>  	return b;
>  
>  fail:
> -	for (idx = 0; idx < ARRAY_SIZE(kmalloc_caches[KMALLOC_NORMAL]); idx++)
> +	for_each_set_bit(idx, &mask, ARRAY_SIZE(kmalloc_caches[KMALLOC_NORMAL]))
>  		kmem_cache_destroy((*b)[idx]);
>  	kmem_cache_free(kmem_buckets_cache, b);

I gave this a try with swiotlb=noforce as well (which pushed the minimum
alignment to 64). So:

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Tested-by: Catalin Marinas <catalin.marinas@arm.com>