Re: [PATCH v2] mm/slub: fix missing debugfs entries for caches created before sysfs init

[Vlastimil Babka (SUSE)]

On 7/23/26 13:55, Li Xiasong wrote:
> slab_debugfs_init() creates the slab debugfs root at device initcall
> time, while slab_sysfs_init() moves slab_state to FULL at late initcall
> time. SLAB_STORE_USER caches created in this window miss their debugfs
> entries because do_kmem_cache_create() skips debugfs_slab_add() when
> slab_state <= UP.
> 
> The affected window is:
> 
>   slab_debugfs_init()
>     slab_debugfs_root = debugfs_create_dir(...)
>     list_for_each_entry(s, &slab_caches, list)
>       debugfs_slab_add(s)
> 
>   kmem_cache_create(..., SLAB_STORE_USER, ...)
>     do_kmem_cache_create()
>       if (slab_state <= UP)
>         return without debugfs entries
> 
>   slab_sysfs_init()
>     slab_state = FULL
> 
> Initialize the debugfs root and add debugfs entries while
> slab_sysfs_init() holds slab_mutex and walks slab_caches. This gives the
> sysfs and debugfs initialization an explicit order and prevents caches
> from being created between the debugfs scan and slab_state reaching
> FULL.
> 
> Since the debugfs root is now initialized later, a cache may be released
> before the root exists. Guard debugfs_slab_release() in that case to
> avoid looking up the cache name in the debugfs root directory.
> 
> Fixes: 1a5ad30b89b4 ("mm: slub: make slab_sysfs_init() a late_initcall")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Li Xiasong <lixiasong1@xxxxxxxxxx>

This is better, thanks. But I think now we have a problem that debugfs
enabled and sysfs disabled (which is possible), debugfs will not be initialized.

So we might need a more generically named late_initcall function that's
executed with either sysfs or debugfs enabled, and wrap slab_kset creation
and alias_list processing in functions that have an empty no-sysfs variant.

> ---
> 
> Changes in v2:
> - Move debugfs root and cache entry initialization into
>   slab_sysfs_init() so both use the same mutex-protected cache walk.
> - Guard debugfs_slab_release() against an uninitialized debugfs root.
> - Update the commit message accordingly.
> 
> v1: https://lore.kernel.org/linux-mm/20260720145124.3684976-1-lixiasong1@xxxxxxxxxx/
> 
> ---
>  mm/slub.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/mm/slub.c b/mm/slub.c
> index 9ec774dc7009..70e9aeda6a8f 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -339,8 +339,10 @@ static inline int sysfs_slab_add(struct kmem_cache *s) { return 0; }
>  
>  #if defined(CONFIG_DEBUG_FS) && defined(CONFIG_SLUB_DEBUG)
>  static void debugfs_slab_add(struct kmem_cache *);
> +static void __init slab_debugfs_root_init(void);
>  #else
>  static inline void debugfs_slab_add(struct kmem_cache *s) { }
> +static inline void slab_debugfs_root_init(void) { }
>  #endif
>  
>  enum add_mode {
> @@ -9771,11 +9773,16 @@ static int __init slab_sysfs_init(void)
>  
>  	slab_state = FULL;
>  
> +	slab_debugfs_root_init();
> +
>  	list_for_each_entry(s, &slab_caches, list) {
>  		err = sysfs_slab_add(s);
>  		if (err)
>  			pr_err("SLUB: Unable to add boot slab %s to sysfs\n",
>  			       s->name);
> +
> +		if (s->flags & SLAB_STORE_USER)
> +			debugfs_slab_add(s);
>  	}
>  
>  	while (alias_list) {
> @@ -9985,23 +9992,16 @@ static void debugfs_slab_add(struct kmem_cache *s)
>  
>  void debugfs_slab_release(struct kmem_cache *s)
>  {
> +	if (unlikely(!slab_debugfs_root))
> +		return;
> +
>  	debugfs_lookup_and_remove(s->name, slab_debugfs_root);
>  }
>  
> -static int __init slab_debugfs_init(void)
> +static void __init slab_debugfs_root_init(void)
>  {
> -	struct kmem_cache *s;
> -
>  	slab_debugfs_root = debugfs_create_dir("slab", NULL);
> -
> -	list_for_each_entry(s, &slab_caches, list)
> -		if (s->flags & SLAB_STORE_USER)
> -			debugfs_slab_add(s);
> -
> -	return 0;
> -
>  }
> -__initcall(slab_debugfs_init);
>  #endif
>  /*
>   * The /proc/slabinfo ABI