Re: [PATCH v4 04/22] mm/slab: move and refactor __kmem_cache_alias()

["Liam R. Howlett" <Liam.Howlett@oracle.com>]

* Vlastimil Babka <vbabka@suse.cz> [260123 01:53]:
> Move __kmem_cache_alias() to slab_common.c since it's called by
> __kmem_cache_create_args() and calls find_mergeable() that both
> are in this file. We can remove two slab.h declarations and make
> them static. Instead declare sysfs_slab_alias() from slub.c so
> that __kmem_cache_alias() can keep calling it.
> 
> Add args parameter to __kmem_cache_alias() and find_mergeable() instead
> of align and ctor. With that we can also move the checks for usersize
> and sheaf_capacity there from __kmem_cache_create_args() and make the
> result more symmetric with slab_unmergeable().
> 
> No functional changes intended.
> 
> Reviewed-by: Harry Yoo <harry.yoo@oracle.com>
> Reviewed-by: Suren Baghdasaryan <surenb@google.com>
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>

One nit.

Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com>

> ---
>  mm/slab.h        |  8 +++-----
>  mm/slab_common.c | 44 +++++++++++++++++++++++++++++++++++++-------
>  mm/slub.c        | 30 +-----------------------------
>  3 files changed, 41 insertions(+), 41 deletions(-)
> 
> diff --git a/mm/slab.h b/mm/slab.h
> index e767aa7e91b0..cb48ce5014ba 100644
> --- a/mm/slab.h
> +++ b/mm/slab.h
> @@ -281,9 +281,12 @@ struct kmem_cache {
>  #define SLAB_SUPPORTS_SYSFS 1
>  void sysfs_slab_unlink(struct kmem_cache *s);
>  void sysfs_slab_release(struct kmem_cache *s);
> +int sysfs_slab_alias(struct kmem_cache *, const char *);

nit: the names of the variables are missing.  I guess because they were
missing before.  *s and *name,  I guess, although they are *s and *p
in the other declaration.

>  #else
>  static inline void sysfs_slab_unlink(struct kmem_cache *s) { }
>  static inline void sysfs_slab_release(struct kmem_cache *s) { }
> +static inline int sysfs_slab_alias(struct kmem_cache *s, const char *p)
> +							{ return 0; }
>  #endif
>  
>  void *fixup_red_left(struct kmem_cache *s, void *p);
> @@ -400,11 +403,6 @@ extern void create_boot_cache(struct kmem_cache *, const char *name,
>  			unsigned int useroffset, unsigned int usersize);
>  
>  int slab_unmergeable(struct kmem_cache *s);
> -struct kmem_cache *find_mergeable(unsigned size, unsigned align,
> -		slab_flags_t flags, const char *name, void (*ctor)(void *));
> -struct kmem_cache *
> -__kmem_cache_alias(const char *name, unsigned int size, unsigned int align,
> -		   slab_flags_t flags, void (*ctor)(void *));
>  
>  slab_flags_t kmem_cache_flags(slab_flags_t flags, const char *name);
>  
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index e691ede0e6a8..ee245a880603 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -174,15 +174,22 @@ int slab_unmergeable(struct kmem_cache *s)
>  	return 0;
>  }
>  
> -struct kmem_cache *find_mergeable(unsigned int size, unsigned int align,
> -		slab_flags_t flags, const char *name, void (*ctor)(void *))
> +static struct kmem_cache *find_mergeable(unsigned int size, slab_flags_t flags,
> +		const char *name, struct kmem_cache_args *args)
>  {
>  	struct kmem_cache *s;
> +	unsigned int align;
>  
>  	if (slab_nomerge)
>  		return NULL;
>  
> -	if (ctor)
> +	if (args->ctor)
> +		return NULL;
> +
> +	if (IS_ENABLED(CONFIG_HARDENED_USERCOPY) && args->usersize)
> +		return NULL;
> +
> +	if (args->sheaf_capacity)
>  		return NULL;
>  
>  	flags = kmem_cache_flags(flags, name);
> @@ -191,7 +198,7 @@ struct kmem_cache *find_mergeable(unsigned int size, unsigned int align,
>  		return NULL;
>  
>  	size = ALIGN(size, sizeof(void *));
> -	align = calculate_alignment(flags, align, size);
> +	align = calculate_alignment(flags, args->align, size);
>  	size = ALIGN(size, align);
>  
>  	list_for_each_entry_reverse(s, &slab_caches, list) {
> @@ -252,6 +259,31 @@ static struct kmem_cache *create_cache(const char *name,
>  	return ERR_PTR(err);
>  }
>  
> +static struct kmem_cache *
> +__kmem_cache_alias(const char *name, unsigned int size, slab_flags_t flags,
> +		   struct kmem_cache_args *args)
> +{
> +	struct kmem_cache *s;
> +
> +	s = find_mergeable(size, flags, name, args);
> +	if (s) {
> +		if (sysfs_slab_alias(s, name))
> +			pr_err("SLUB: Unable to add cache alias %s to sysfs\n",
> +			       name);
> +
> +		s->refcount++;
> +
> +		/*
> +		 * Adjust the object sizes so that we clear
> +		 * the complete object on kzalloc.
> +		 */
> +		s->object_size = max(s->object_size, size);
> +		s->inuse = max(s->inuse, ALIGN(size, sizeof(void *)));
> +	}
> +
> +	return s;
> +}
> +
>  /**
>   * __kmem_cache_create_args - Create a kmem cache.
>   * @name: A string which is used in /proc/slabinfo to identify this cache.
> @@ -323,9 +355,7 @@ struct kmem_cache *__kmem_cache_create_args(const char *name,
>  		    object_size - args->usersize < args->useroffset))
>  		args->usersize = args->useroffset = 0;
>  
> -	if (!args->usersize && !args->sheaf_capacity)
> -		s = __kmem_cache_alias(name, object_size, args->align, flags,
> -				       args->ctor);
> +	s = __kmem_cache_alias(name, object_size, flags, args);
>  	if (s)
>  		goto out_unlock;
>  
> diff --git a/mm/slub.c b/mm/slub.c
> index 4eb60e99abd7..9d86c0505dcd 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -350,11 +350,8 @@ enum track_item { TRACK_ALLOC, TRACK_FREE };
>  
>  #ifdef SLAB_SUPPORTS_SYSFS
>  static int sysfs_slab_add(struct kmem_cache *);
> -static int sysfs_slab_alias(struct kmem_cache *, const char *);
>  #else
>  static inline int sysfs_slab_add(struct kmem_cache *s) { return 0; }
> -static inline int sysfs_slab_alias(struct kmem_cache *s, const char *p)
> -							{ return 0; }
>  #endif
>  
>  #if defined(CONFIG_DEBUG_FS) && defined(CONFIG_SLUB_DEBUG)
> @@ -8570,31 +8567,6 @@ void __init kmem_cache_init_late(void)
>  	WARN_ON(!flushwq);
>  }
>  
> -struct kmem_cache *
> -__kmem_cache_alias(const char *name, unsigned int size, unsigned int align,
> -		   slab_flags_t flags, void (*ctor)(void *))
> -{
> -	struct kmem_cache *s;
> -
> -	s = find_mergeable(size, align, flags, name, ctor);
> -	if (s) {
> -		if (sysfs_slab_alias(s, name))
> -			pr_err("SLUB: Unable to add cache alias %s to sysfs\n",
> -			       name);
> -
> -		s->refcount++;
> -
> -		/*
> -		 * Adjust the object sizes so that we clear
> -		 * the complete object on kzalloc.
> -		 */
> -		s->object_size = max(s->object_size, size);
> -		s->inuse = max(s->inuse, ALIGN(size, sizeof(void *)));
> -	}
> -
> -	return s;
> -}
> -
>  int do_kmem_cache_create(struct kmem_cache *s, const char *name,
>  			 unsigned int size, struct kmem_cache_args *args,
>  			 slab_flags_t flags)
> @@ -9827,7 +9799,7 @@ struct saved_alias {
>  
>  static struct saved_alias *alias_list;
>  
> -static int sysfs_slab_alias(struct kmem_cache *s, const char *name)
> +int sysfs_slab_alias(struct kmem_cache *s, const char *name)
>  {
>  	struct saved_alias *al;
>  
> 
> -- 
> 2.52.0
>