Re: [PATCH v3 04/10] mm, slab: introduce slab_debug=N

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mm, slab: missing SLAB_DEBUG_NOOP in create_unique_id() causes sysfs collision
--

commit 72eeb0d7d0de3c870ebf6a85a45b89ab817bcdff
Author: Vlastimil Babka (SUSE) <vbabka@kernel.org>

mm, slab: introduce slab_debug=N

This commit introduces a new slab_debug=N (SLAB_DEBUG_NOOP) option that forces
slab debugging slowpaths and zero sheaf capacity without enabling actual debug
checks, while keeping caches mergeable among caches with the same flag and
avoiding disabling printk pointer hashing.

> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 01c64ef661e7f..6036ce1ac333b 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -47,12 +47,13 @@ struct kmem_cache *kmem_cache;
>   * Any flag that adds per-object metadata should be included,
>   * since slab merging can update s->inuse that affects the metadata layout.
>   */
> -#define SLAB_NEVER_MERGE (SLAB_DEBUG_FLAGS | SLAB_TYPESAFE_BY_RCU | \
> -		SLAB_NOLEAKTRACE | SLAB_FAILSLAB | SLAB_NO_MERGE | \
> -		SLAB_OBJ_EXT_IN_OBJ)
> +#define SLAB_NEVER_MERGE ((SLAB_DEBUG_FLAGS & ~SLAB_DEBUG_NOOP) | \
> +		SLAB_TYPESAFE_BY_RCU | SLAB_NOLEAKTRACE | SLAB_FAILSLAB | \
> +		SLAB_NO_MERGE | SLAB_OBJ_EXT_IN_OBJ)
>  
>  #define SLAB_MERGE_SAME (SLAB_RECLAIM_ACCOUNT | SLAB_CACHE_DMA | \
> -			 SLAB_CACHE_DMA32 | SLAB_ACCOUNT | SLAB_MAY_ACCOUNT)
> +			 SLAB_CACHE_DMA32 | SLAB_ACCOUNT | SLAB_MAY_ACCOUNT | \
> +			 SLAB_DEBUG_NOOP)

[Severity: Medium]
As reported by Sashiko on RFC v2 [1], when SLAB_DEBUG_NOOP is excluded from
SLAB_NEVER_MERGE and added to SLAB_MERGE_SAME, does create_unique_id() in
mm/slub.c also need to encode SLAB_DEBUG_NOOP into the generated sysfs cache
name?

When SLAB_DEBUG_NOOP is enabled on a subset of mergeable caches (for example
via per-cache slab_debug=N,<cache> boot parameters), find_mergeable() keeps
mergeable caches with SLAB_DEBUG_NOOP separate from otherwise identical
mergeable caches of the same size without SLAB_DEBUG_NOOP.

Because slab_unmergeable() returns 0 for both caches, sysfs_slab_add() calls
create_unique_id() to generate their /sys/kernel/slab/ directory names:

mm/slub.c:create_unique_id() {
	...
	if (s->flags & SLAB_CACHE_DMA)
		*p++ = 'd';
	if (s->flags & SLAB_CACHE_DMA32)
		*p++ = 'D';
	if (s->flags & SLAB_RECLAIM_ACCOUNT)
		*p++ = 'a';
	if (s->flags & SLAB_CONSISTENCY_CHECKS)
		*p++ = 'F';
	if (s->flags & SLAB_ACCOUNT)
		*p++ = 'A';
	if (p != name + 1)
		*p++ = '-';
	p += snprintf(p, ID_STR_LENGTH - (p - name), "%07u", s->size);
	...
}

Since create_unique_id() does not append a character for SLAB_DEBUG_NOOP, both
caches receive the exact same sysfs name, causing kobject_init_and_add() in
sysfs_slab_add() to fail with -EEXIST and trigger a duplicate sysfs filename
warning.

[1] https://lore.kernel.org/r/20261024-slub_tiny_rework-v2-2-bf1419ab5ffd@kernel.org

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105-slub_tiny_rework-v3-0-0484ce59eabb@kernel.org?part=4