[PATCH v2 00/15] slab: add struct kmem_cache_args

[PATCH v2 00/15] slab: add struct kmem_cache_args

[Christian Brauner]

Hey,

As discussed last week the various kmem_cache_*() functions should be
replaced by a unified function that is based around a struct, with only
the basic parameters passed separately.

Vlastimil already said that he would like to keep core parameters out
of the struct: name, object size, and flags. I personally don't care
much and would not object to moving everything into the struct but
that's a matter of taste and I yield that decision power to the
maintainer.

In the first version I pointed out that the choice of name is somewhat
forced as kmem_cache_create() is taken and the only way to reuse it
would be to replace all users in one go. Or to do a global
sed/kmem_cache_create()/kmem_cache_create2()/g. And then introduce
kmem_cache_setup(). That doesn't strike me as a viable option.

If we really cared about the *_create() suffix then an alternative might
be to do a sed/kmem_cache_setup()/kmem_cache_create()/g after every user
in the kernel is ported. I honestly don't think that's worth it but I
wanted to at least mention it to highlight the fact that this might lead
to a naming compromise.

However, I came up with an alternative using _Generic() to create a
compatibility layer that will call the correct variant of
kmem_cache_create() depending on whether struct kmem_cache_args is
passed or not. That compatibility layer can stay in place until we
updated all calls to be based on struct kmem_cache_args.

From a cursory grep (and not excluding Documentation mentions) we will
have to replace 44 kmem_cache_create_usercopy() calls and about 463
kmem_cache_create() calls which makes for a bit above 500 calls to port
to kmem_cache_setup(). That'll probably be good work for people getting
into kernel development.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
Changes in v2:
- Remove kmem_cache_setup() and add a compatibility layer built around
  _Generic() so that we can keep the kmem_cache_create() name and type
  switch on the third argument to either call __kmem_cache_create() or
  __kmem_cache_create_args().
- Link to v1: https://lore.kernel.org/r/20240902-work-kmem_cache_args-v1-0-27d05bc05128@kernel.org

---
Christian Brauner (15):
      sl*b: s/__kmem_cache_create/do_kmem_cache_create/g
      slab: add struct kmem_cache_args
      slab: port kmem_cache_create() to struct kmem_cache_args
      slab: port kmem_cache_create_rcu() to struct kmem_cache_args
      slab: port kmem_cache_create_usercopy() to struct kmem_cache_args
      slab: pass struct kmem_cache_args to create_cache()
      slub: pull kmem_cache_open() into do_kmem_cache_create()
      slab: pass struct kmem_cache_args to do_kmem_cache_create()
      sl*b: remove rcu_freeptr_offset from struct kmem_cache
      slab: port KMEM_CACHE() to struct kmem_cache_args
      slab: port KMEM_CACHE_USERCOPY() to struct kmem_cache_args
      slab: create kmem_cache_create() compatibility layer
      file: port to struct kmem_cache_args
      slab: remove kmem_cache_create_rcu()
      io_uring: port to struct kmem_cache_args

 fs/file_table.c      |  11 +++-
 include/linux/slab.h |  60 ++++++++++++++-----
 io_uring/io_uring.c  |  14 +++--
 mm/slab.h            |   6 +-
 mm/slab_common.c     | 150 +++++++++++++++++++----------------------------
 mm/slub.c            | 162 +++++++++++++++++++++++++--------------------------
 6 files changed, 203 insertions(+), 200 deletions(-)
---
base-commit: 6e016babce7c845ed015da25c7a097fa3482d95a
change-id: 20240902-work-kmem_cache_args-e9760972c7d4

[PATCH v2 01/15] sl*b: s/__kmem_cache_create/do_kmem_cache_create/g

[Christian Brauner]

Free up reusing the double-underscore variant for follow-up patches.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 mm/slab.h        | 2 +-
 mm/slab_common.c | 4 ++--
 mm/slub.c        | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/mm/slab.h b/mm/slab.h
index a6051385186e..684bb48c4f39 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -424,7 +424,7 @@ kmalloc_slab(size_t size, kmem_buckets *b, gfp_t flags, unsigned long caller)
 gfp_t kmalloc_fix_flags(gfp_t flags);
 
 /* Functions provided by the slab allocators */
-int __kmem_cache_create(struct kmem_cache *, slab_flags_t flags);
+int do_kmem_cache_create(struct kmem_cache *, slab_flags_t flags);
 
 void __init kmem_cache_init(void);
 extern void create_boot_cache(struct kmem_cache *, const char *name,
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 95db3702f8d6..91e0e36e4379 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -234,7 +234,7 @@ static struct kmem_cache *create_cache(const char *name,
 	s->useroffset = useroffset;
 	s->usersize = usersize;
 #endif
-	err = __kmem_cache_create(s, flags);
+	err = do_kmem_cache_create(s, flags);
 	if (err)
 		goto out_free_cache;
 
@@ -778,7 +778,7 @@ void __init create_boot_cache(struct kmem_cache *s, const char *name,
 	s->usersize = usersize;
 #endif
 
-	err = __kmem_cache_create(s, flags);
+	err = do_kmem_cache_create(s, flags);
 
 	if (err)
 		panic("Creation of kmalloc slab %s size=%u failed. Reason %d\n",
diff --git a/mm/slub.c b/mm/slub.c
index 9aa5da1e8e27..23d9d783ff26 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -5902,7 +5902,7 @@ __kmem_cache_alias(const char *name, unsigned int size, unsigned int align,
 	return s;
 }
 
-int __kmem_cache_create(struct kmem_cache *s, slab_flags_t flags)
+int do_kmem_cache_create(struct kmem_cache *s, slab_flags_t flags)
 {
 	int err;
 

-- 
2.45.2

Re: [PATCH v2 01/15] sl*b: s/__kmem_cache_create/do_kmem_cache_create/g

[Mike Rapoport]

On Tue, Sep 03, 2024 at 04:20:42PM +0200, Christian Brauner wrote:
> Free up reusing the double-underscore variant for follow-up patches.
> 
> Signed-off-by: Christian Brauner <brauner@kernel.org>

Reviewed-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

> ---
>  mm/slab.h        | 2 +-
>  mm/slab_common.c | 4 ++--
>  mm/slub.c        | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/slab.h b/mm/slab.h
> index a6051385186e..684bb48c4f39 100644
> --- a/mm/slab.h
> +++ b/mm/slab.h
> @@ -424,7 +424,7 @@ kmalloc_slab(size_t size, kmem_buckets *b, gfp_t flags, unsigned long caller)
>  gfp_t kmalloc_fix_flags(gfp_t flags);
>  
>  /* Functions provided by the slab allocators */
> -int __kmem_cache_create(struct kmem_cache *, slab_flags_t flags);
> +int do_kmem_cache_create(struct kmem_cache *, slab_flags_t flags);
>  
>  void __init kmem_cache_init(void);
>  extern void create_boot_cache(struct kmem_cache *, const char *name,
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 95db3702f8d6..91e0e36e4379 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -234,7 +234,7 @@ static struct kmem_cache *create_cache(const char *name,
>  	s->useroffset = useroffset;
>  	s->usersize = usersize;
>  #endif
> -	err = __kmem_cache_create(s, flags);
> +	err = do_kmem_cache_create(s, flags);
>  	if (err)
>  		goto out_free_cache;
>  
> @@ -778,7 +778,7 @@ void __init create_boot_cache(struct kmem_cache *s, const char *name,
>  	s->usersize = usersize;
>  #endif
>  
> -	err = __kmem_cache_create(s, flags);
> +	err = do_kmem_cache_create(s, flags);
>  
>  	if (err)
>  		panic("Creation of kmalloc slab %s size=%u failed. Reason %d\n",
> diff --git a/mm/slub.c b/mm/slub.c
> index 9aa5da1e8e27..23d9d783ff26 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -5902,7 +5902,7 @@ __kmem_cache_alias(const char *name, unsigned int size, unsigned int align,
>  	return s;
>  }
>  
> -int __kmem_cache_create(struct kmem_cache *s, slab_flags_t flags)
> +int do_kmem_cache_create(struct kmem_cache *s, slab_flags_t flags)
>  {
>  	int err;
>  
> 
> -- 
> 2.45.2
> 

-- 
Sincerely yours,
Mike.

[PATCH v2 02/15] slab: add struct kmem_cache_args

[Christian Brauner]

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 include/linux/slab.h | 21 ++++++++++++++++
 mm/slab_common.c     | 67 +++++++++++++++++++++++++++++++++++++++-------------
 2 files changed, 72 insertions(+), 16 deletions(-)

diff --git a/include/linux/slab.h b/include/linux/slab.h
index 5b2da2cf31a8..79d8c8bca4a4 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -240,6 +240,27 @@ struct mem_cgroup;
  */
 bool slab_is_available(void);
 
+/**
+ * @align: The required alignment for the objects.
+ * @useroffset: Usercopy region offset
+ * @usersize: Usercopy region size
+ * @freeptr_offset: Custom offset for the free pointer in RCU caches
+ * @use_freeptr_offset: Whether a @freeptr_offset is used
+ * @ctor: A constructor for the objects.
+ */
+struct kmem_cache_args {
+	unsigned int align;
+	unsigned int useroffset;
+	unsigned int usersize;
+	unsigned int freeptr_offset;
+	bool use_freeptr_offset;
+	void (*ctor)(void *);
+};
+
+struct kmem_cache *__kmem_cache_create_args(const char *name,
+					    unsigned int object_size,
+					    struct kmem_cache_args *args,
+					    slab_flags_t flags);
 struct kmem_cache *kmem_cache_create(const char *name, unsigned int size,
 			unsigned int align, slab_flags_t flags,
 			void (*ctor)(void *));
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 91e0e36e4379..0f13c045b8d1 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -248,14 +248,24 @@ static struct kmem_cache *create_cache(const char *name,
 	return ERR_PTR(err);
 }
 
-static struct kmem_cache *
-do_kmem_cache_create_usercopy(const char *name,
-		  unsigned int size, unsigned int freeptr_offset,
-		  unsigned int align, slab_flags_t flags,
-		  unsigned int useroffset, unsigned int usersize,
-		  void (*ctor)(void *))
+/**
+ * __kmem_cache_create_args - Create a kmem cache
+ * @name: A string which is used in /proc/slabinfo to identify this cache.
+ * @object_size: The size of objects to be created in this cache.
+ * @args: Arguments for the cache creation (see struct kmem_cache_args).
+ * @flags: See %SLAB_* flags for an explanation of individual @flags.
+ *
+ * Cannot be called within a interrupt, but can be interrupted.
+ *
+ * Return: a pointer to the cache on success, NULL on failure.
+ */
+struct kmem_cache *__kmem_cache_create_args(const char *name,
+					    unsigned int object_size,
+					    struct kmem_cache_args *args,
+					    slab_flags_t flags)
 {
 	struct kmem_cache *s = NULL;
+	unsigned int freeptr_offset = UINT_MAX;
 	const char *cache_name;
 	int err;
 
@@ -275,7 +285,7 @@ do_kmem_cache_create_usercopy(const char *name,
 
 	mutex_lock(&slab_mutex);
 
-	err = kmem_cache_sanity_check(name, size);
+	err = kmem_cache_sanity_check(name, object_size);
 	if (err) {
 		goto out_unlock;
 	}
@@ -296,12 +306,14 @@ do_kmem_cache_create_usercopy(const char *name,
 
 	/* Fail closed on bad usersize of useroffset values. */
 	if (!IS_ENABLED(CONFIG_HARDENED_USERCOPY) ||
-	    WARN_ON(!usersize && useroffset) ||
-	    WARN_ON(size < usersize || size - usersize < useroffset))
-		usersize = useroffset = 0;
-
-	if (!usersize)
-		s = __kmem_cache_alias(name, size, align, flags, ctor);
+	    WARN_ON(!args->usersize && args->useroffset) ||
+	    WARN_ON(object_size < args->usersize ||
+		    object_size - args->usersize < args->useroffset))
+		args->usersize = args->useroffset = 0;
+
+	if (!args->usersize)
+		s = __kmem_cache_alias(name, object_size, args->align, flags,
+				       args->ctor);
 	if (s)
 		goto out_unlock;
 
@@ -311,9 +323,11 @@ do_kmem_cache_create_usercopy(const char *name,
 		goto out_unlock;
 	}
 
-	s = create_cache(cache_name, size, freeptr_offset,
-			 calculate_alignment(flags, align, size),
-			 flags, useroffset, usersize, ctor);
+	if (args->use_freeptr_offset)
+		freeptr_offset = args->freeptr_offset;
+	s = create_cache(cache_name, object_size, freeptr_offset,
+			 calculate_alignment(flags, args->align, object_size),
+			 flags, args->useroffset, args->usersize, args->ctor);
 	if (IS_ERR(s)) {
 		err = PTR_ERR(s);
 		kfree_const(cache_name);
@@ -335,6 +349,27 @@ do_kmem_cache_create_usercopy(const char *name,
 	}
 	return s;
 }
+EXPORT_SYMBOL(__kmem_cache_create_args);
+
+static struct kmem_cache *
+do_kmem_cache_create_usercopy(const char *name,
+                 unsigned int size, unsigned int freeptr_offset,
+                 unsigned int align, slab_flags_t flags,
+                 unsigned int useroffset, unsigned int usersize,
+                 void (*ctor)(void *))
+{
+	struct kmem_cache_args kmem_args = {
+		.align			= align,
+		.use_freeptr_offset	= freeptr_offset != UINT_MAX,
+		.freeptr_offset		= freeptr_offset,
+		.useroffset		= useroffset,
+		.usersize		= usersize,
+		.ctor			= ctor,
+	};
+
+	return __kmem_cache_create_args(name, size, &kmem_args, flags);
+}
+
 
 /**
  * kmem_cache_create_usercopy - Create a cache with a region suitable

-- 
2.45.2

Re: [PATCH v2 02/15] slab: add struct kmem_cache_args

[Mike Rapoport]

On Tue, Sep 03, 2024 at 04:20:43PM +0200, Christian Brauner wrote:
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
>  include/linux/slab.h | 21 ++++++++++++++++
>  mm/slab_common.c     | 67 +++++++++++++++++++++++++++++++++++++++-------------
>  2 files changed, 72 insertions(+), 16 deletions(-)
> 
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index 5b2da2cf31a8..79d8c8bca4a4 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -240,6 +240,27 @@ struct mem_cgroup;
>   */
>  bool slab_is_available(void);
>  
> +/**
> + * @align: The required alignment for the objects.
> + * @useroffset: Usercopy region offset
> + * @usersize: Usercopy region size
> + * @freeptr_offset: Custom offset for the free pointer in RCU caches
> + * @use_freeptr_offset: Whether a @freeptr_offset is used
> + * @ctor: A constructor for the objects.
> + */
> +struct kmem_cache_args {
> +	unsigned int align;
> +	unsigned int useroffset;
> +	unsigned int usersize;
> +	unsigned int freeptr_offset;
> +	bool use_freeptr_offset;
> +	void (*ctor)(void *);
> +};
> +
> +struct kmem_cache *__kmem_cache_create_args(const char *name,
> +					    unsigned int object_size,
> +					    struct kmem_cache_args *args,
> +					    slab_flags_t flags);

I'd name it __kmem_cache_create() and then
s/kmem_cache_create/_kmem_cache_create/ in patch 12.

Other than that

Reviewed-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

>  struct kmem_cache *kmem_cache_create(const char *name, unsigned int size,
>  			unsigned int align, slab_flags_t flags,
>  			void (*ctor)(void *));

-- 
Sincerely yours,
Mike.

Re: [PATCH v2 02/15] slab: add struct kmem_cache_args

[Vlastimil Babka]

On 9/3/24 16:20, Christian Brauner wrote:

You could describe that it's to hold less common args and there's
__kmem_cache_create_args() that takes it, and
do_kmem_cache_create_usercopy() is converted to it? Otherwise LGTM.

> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
>  include/linux/slab.h | 21 ++++++++++++++++
>  mm/slab_common.c     | 67 +++++++++++++++++++++++++++++++++++++++-------------
>  2 files changed, 72 insertions(+), 16 deletions(-)
> 
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index 5b2da2cf31a8..79d8c8bca4a4 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -240,6 +240,27 @@ struct mem_cgroup;
>   */
>  bool slab_is_available(void);
>  
> +/**
> + * @align: The required alignment for the objects.
> + * @useroffset: Usercopy region offset
> + * @usersize: Usercopy region size
> + * @freeptr_offset: Custom offset for the free pointer in RCU caches
> + * @use_freeptr_offset: Whether a @freeptr_offset is used
> + * @ctor: A constructor for the objects.
> + */
> +struct kmem_cache_args {
> +	unsigned int align;
> +	unsigned int useroffset;
> +	unsigned int usersize;
> +	unsigned int freeptr_offset;
> +	bool use_freeptr_offset;
> +	void (*ctor)(void *);
> +};
> +
> +struct kmem_cache *__kmem_cache_create_args(const char *name,
> +					    unsigned int object_size,
> +					    struct kmem_cache_args *args,
> +					    slab_flags_t flags);
>  struct kmem_cache *kmem_cache_create(const char *name, unsigned int size,
>  			unsigned int align, slab_flags_t flags,
>  			void (*ctor)(void *));
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 91e0e36e4379..0f13c045b8d1 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -248,14 +248,24 @@ static struct kmem_cache *create_cache(const char *name,
>  	return ERR_PTR(err);
>  }
>  
> -static struct kmem_cache *
> -do_kmem_cache_create_usercopy(const char *name,
> -		  unsigned int size, unsigned int freeptr_offset,
> -		  unsigned int align, slab_flags_t flags,
> -		  unsigned int useroffset, unsigned int usersize,
> -		  void (*ctor)(void *))
> +/**
> + * __kmem_cache_create_args - Create a kmem cache
> + * @name: A string which is used in /proc/slabinfo to identify this cache.
> + * @object_size: The size of objects to be created in this cache.
> + * @args: Arguments for the cache creation (see struct kmem_cache_args).
> + * @flags: See %SLAB_* flags for an explanation of individual @flags.
> + *
> + * Cannot be called within a interrupt, but can be interrupted.
> + *
> + * Return: a pointer to the cache on success, NULL on failure.
> + */
> +struct kmem_cache *__kmem_cache_create_args(const char *name,
> +					    unsigned int object_size,
> +					    struct kmem_cache_args *args,
> +					    slab_flags_t flags)
>  {
>  	struct kmem_cache *s = NULL;
> +	unsigned int freeptr_offset = UINT_MAX;
>  	const char *cache_name;
>  	int err;
>  
> @@ -275,7 +285,7 @@ do_kmem_cache_create_usercopy(const char *name,
>  
>  	mutex_lock(&slab_mutex);
>  
> -	err = kmem_cache_sanity_check(name, size);
> +	err = kmem_cache_sanity_check(name, object_size);
>  	if (err) {
>  		goto out_unlock;
>  	}
> @@ -296,12 +306,14 @@ do_kmem_cache_create_usercopy(const char *name,
>  
>  	/* Fail closed on bad usersize of useroffset values. */
>  	if (!IS_ENABLED(CONFIG_HARDENED_USERCOPY) ||
> -	    WARN_ON(!usersize && useroffset) ||
> -	    WARN_ON(size < usersize || size - usersize < useroffset))
> -		usersize = useroffset = 0;
> -
> -	if (!usersize)
> -		s = __kmem_cache_alias(name, size, align, flags, ctor);
> +	    WARN_ON(!args->usersize && args->useroffset) ||
> +	    WARN_ON(object_size < args->usersize ||
> +		    object_size - args->usersize < args->useroffset))
> +		args->usersize = args->useroffset = 0;
> +
> +	if (!args->usersize)
> +		s = __kmem_cache_alias(name, object_size, args->align, flags,
> +				       args->ctor);
>  	if (s)
>  		goto out_unlock;
>  
> @@ -311,9 +323,11 @@ do_kmem_cache_create_usercopy(const char *name,
>  		goto out_unlock;
>  	}
>  
> -	s = create_cache(cache_name, size, freeptr_offset,
> -			 calculate_alignment(flags, align, size),
> -			 flags, useroffset, usersize, ctor);
> +	if (args->use_freeptr_offset)
> +		freeptr_offset = args->freeptr_offset;
> +	s = create_cache(cache_name, object_size, freeptr_offset,
> +			 calculate_alignment(flags, args->align, object_size),
> +			 flags, args->useroffset, args->usersize, args->ctor);
>  	if (IS_ERR(s)) {
>  		err = PTR_ERR(s);
>  		kfree_const(cache_name);
> @@ -335,6 +349,27 @@ do_kmem_cache_create_usercopy(const char *name,
>  	}
>  	return s;
>  }
> +EXPORT_SYMBOL(__kmem_cache_create_args);
> +
> +static struct kmem_cache *
> +do_kmem_cache_create_usercopy(const char *name,
> +                 unsigned int size, unsigned int freeptr_offset,
> +                 unsigned int align, slab_flags_t flags,
> +                 unsigned int useroffset, unsigned int usersize,
> +                 void (*ctor)(void *))
> +{
> +	struct kmem_cache_args kmem_args = {
> +		.align			= align,
> +		.use_freeptr_offset	= freeptr_offset != UINT_MAX,
> +		.freeptr_offset		= freeptr_offset,
> +		.useroffset		= useroffset,
> +		.usersize		= usersize,
> +		.ctor			= ctor,
> +	};
> +
> +	return __kmem_cache_create_args(name, size, &kmem_args, flags);
> +}
> +
>  
>  /**
>   * kmem_cache_create_usercopy - Create a cache with a region suitable
> 

Re: [PATCH v2 02/15] slab: add struct kmem_cache_args

[Christian Brauner]

On Wed, Sep 04, 2024 at 10:13:11AM GMT, Vlastimil Babka wrote:
> On 9/3/24 16:20, Christian Brauner wrote:
> 
> You could describe that it's to hold less common args and there's
> __kmem_cache_create_args() that takes it, and
> do_kmem_cache_create_usercopy() is converted to it? Otherwise LGTM.

Hm, I seem to have dropped the contents of the commit message when I
split it into multiple individual commits. I've now added:

"Currently we have multiple kmem_cache_create*() variants that take up to
seven separate parameters with one of the functions having to grow an
eigth parameter in the future to handle both usercopy and a custom
freelist pointer.

Add a struct kmem_cache_args structure and move less common parameters
into it. Core parameters such as name, object size, and flags continue
to be passed separately.

Add a new function __kmem_cache_create_args() that takes a struct
kmem_cache_args pointer and port do_kmem_cache_create_usercopy() over to
it.

In follow-up patches we will port the other kmem_cache_create*()
variants over to it as well."

to the work.kmem_cache_args branch.

> 
> > Signed-off-by: Christian Brauner <brauner@kernel.org>
> > ---
> >  include/linux/slab.h | 21 ++++++++++++++++
> >  mm/slab_common.c     | 67 +++++++++++++++++++++++++++++++++++++++-------------
> >  2 files changed, 72 insertions(+), 16 deletions(-)
> > 
> > diff --git a/include/linux/slab.h b/include/linux/slab.h
> > index 5b2da2cf31a8..79d8c8bca4a4 100644
> > --- a/include/linux/slab.h
> > +++ b/include/linux/slab.h
> > @@ -240,6 +240,27 @@ struct mem_cgroup;
> >   */
> >  bool slab_is_available(void);
> >  
> > +/**
> > + * @align: The required alignment for the objects.
> > + * @useroffset: Usercopy region offset
> > + * @usersize: Usercopy region size
> > + * @freeptr_offset: Custom offset for the free pointer in RCU caches
> > + * @use_freeptr_offset: Whether a @freeptr_offset is used
> > + * @ctor: A constructor for the objects.
> > + */
> > +struct kmem_cache_args {
> > +	unsigned int align;
> > +	unsigned int useroffset;
> > +	unsigned int usersize;
> > +	unsigned int freeptr_offset;
> > +	bool use_freeptr_offset;
> > +	void (*ctor)(void *);
> > +};
> > +
> > +struct kmem_cache *__kmem_cache_create_args(const char *name,
> > +					    unsigned int object_size,
> > +					    struct kmem_cache_args *args,
> > +					    slab_flags_t flags);
> >  struct kmem_cache *kmem_cache_create(const char *name, unsigned int size,
> >  			unsigned int align, slab_flags_t flags,
> >  			void (*ctor)(void *));
> > diff --git a/mm/slab_common.c b/mm/slab_common.c
> > index 91e0e36e4379..0f13c045b8d1 100644
> > --- a/mm/slab_common.c
> > +++ b/mm/slab_common.c
> > @@ -248,14 +248,24 @@ static struct kmem_cache *create_cache(const char *name,
> >  	return ERR_PTR(err);
> >  }
> >  
> > -static struct kmem_cache *
> > -do_kmem_cache_create_usercopy(const char *name,
> > -		  unsigned int size, unsigned int freeptr_offset,
> > -		  unsigned int align, slab_flags_t flags,
> > -		  unsigned int useroffset, unsigned int usersize,
> > -		  void (*ctor)(void *))
> > +/**
> > + * __kmem_cache_create_args - Create a kmem cache
> > + * @name: A string which is used in /proc/slabinfo to identify this cache.
> > + * @object_size: The size of objects to be created in this cache.
> > + * @args: Arguments for the cache creation (see struct kmem_cache_args).
> > + * @flags: See %SLAB_* flags for an explanation of individual @flags.
> > + *
> > + * Cannot be called within a interrupt, but can be interrupted.
> > + *
> > + * Return: a pointer to the cache on success, NULL on failure.
> > + */
> > +struct kmem_cache *__kmem_cache_create_args(const char *name,
> > +					    unsigned int object_size,
> > +					    struct kmem_cache_args *args,
> > +					    slab_flags_t flags)
> >  {
> >  	struct kmem_cache *s = NULL;
> > +	unsigned int freeptr_offset = UINT_MAX;
> >  	const char *cache_name;
> >  	int err;
> >  
> > @@ -275,7 +285,7 @@ do_kmem_cache_create_usercopy(const char *name,
> >  
> >  	mutex_lock(&slab_mutex);
> >  
> > -	err = kmem_cache_sanity_check(name, size);
> > +	err = kmem_cache_sanity_check(name, object_size);
> >  	if (err) {
> >  		goto out_unlock;
> >  	}
> > @@ -296,12 +306,14 @@ do_kmem_cache_create_usercopy(const char *name,
> >  
> >  	/* Fail closed on bad usersize of useroffset values. */
> >  	if (!IS_ENABLED(CONFIG_HARDENED_USERCOPY) ||
> > -	    WARN_ON(!usersize && useroffset) ||
> > -	    WARN_ON(size < usersize || size - usersize < useroffset))
> > -		usersize = useroffset = 0;
> > -
> > -	if (!usersize)
> > -		s = __kmem_cache_alias(name, size, align, flags, ctor);
> > +	    WARN_ON(!args->usersize && args->useroffset) ||
> > +	    WARN_ON(object_size < args->usersize ||
> > +		    object_size - args->usersize < args->useroffset))
> > +		args->usersize = args->useroffset = 0;
> > +
> > +	if (!args->usersize)
> > +		s = __kmem_cache_alias(name, object_size, args->align, flags,
> > +				       args->ctor);
> >  	if (s)
> >  		goto out_unlock;
> >  
> > @@ -311,9 +323,11 @@ do_kmem_cache_create_usercopy(const char *name,
> >  		goto out_unlock;
> >  	}
> >  
> > -	s = create_cache(cache_name, size, freeptr_offset,
> > -			 calculate_alignment(flags, align, size),
> > -			 flags, useroffset, usersize, ctor);
> > +	if (args->use_freeptr_offset)
> > +		freeptr_offset = args->freeptr_offset;
> > +	s = create_cache(cache_name, object_size, freeptr_offset,
> > +			 calculate_alignment(flags, args->align, object_size),
> > +			 flags, args->useroffset, args->usersize, args->ctor);
> >  	if (IS_ERR(s)) {
> >  		err = PTR_ERR(s);
> >  		kfree_const(cache_name);
> > @@ -335,6 +349,27 @@ do_kmem_cache_create_usercopy(const char *name,
> >  	}
> >  	return s;
> >  }
> > +EXPORT_SYMBOL(__kmem_cache_create_args);
> > +
> > +static struct kmem_cache *
> > +do_kmem_cache_create_usercopy(const char *name,
> > +                 unsigned int size, unsigned int freeptr_offset,
> > +                 unsigned int align, slab_flags_t flags,
> > +                 unsigned int useroffset, unsigned int usersize,
> > +                 void (*ctor)(void *))
> > +{
> > +	struct kmem_cache_args kmem_args = {
> > +		.align			= align,
> > +		.use_freeptr_offset	= freeptr_offset != UINT_MAX,
> > +		.freeptr_offset		= freeptr_offset,
> > +		.useroffset		= useroffset,
> > +		.usersize		= usersize,
> > +		.ctor			= ctor,
> > +	};
> > +
> > +	return __kmem_cache_create_args(name, size, &kmem_args, flags);
> > +}
> > +
> >  
> >  /**
> >   * kmem_cache_create_usercopy - Create a cache with a region suitable
> > 
> 

Re: [PATCH v2 02/15] slab: add struct kmem_cache_args

[Mike Rapoport]

On Tue, Sep 03, 2024 at 04:20:43PM +0200, Christian Brauner wrote:
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
>  include/linux/slab.h | 21 ++++++++++++++++
>  mm/slab_common.c     | 67 +++++++++++++++++++++++++++++++++++++++-------------
>  2 files changed, 72 insertions(+), 16 deletions(-)
> 
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index 5b2da2cf31a8..79d8c8bca4a4 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -240,6 +240,27 @@ struct mem_cgroup;
>   */
>  bool slab_is_available(void);
>  
> +/**
> + * @align: The required alignment for the objects.
> + * @useroffset: Usercopy region offset
> + * @usersize: Usercopy region size
> + * @freeptr_offset: Custom offset for the free pointer in RCU caches
> + * @use_freeptr_offset: Whether a @freeptr_offset is used
> + * @ctor: A constructor for the objects.
> + */
> +struct kmem_cache_args {
> +	unsigned int align;
> +	unsigned int useroffset;
> +	unsigned int usersize;
> +	unsigned int freeptr_offset;
> +	bool use_freeptr_offset;
> +	void (*ctor)(void *);
> +};
> +
> +struct kmem_cache *__kmem_cache_create_args(const char *name,
> +					    unsigned int object_size,
> +					    struct kmem_cache_args *args,
> +					    slab_flags_t flags);
>  struct kmem_cache *kmem_cache_create(const char *name, unsigned int size,
>  			unsigned int align, slab_flags_t flags,
>  			void (*ctor)(void *));
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 91e0e36e4379..0f13c045b8d1 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -248,14 +248,24 @@ static struct kmem_cache *create_cache(const char *name,
>  	return ERR_PTR(err);
>  }
>  
> -static struct kmem_cache *
> -do_kmem_cache_create_usercopy(const char *name,
> -		  unsigned int size, unsigned int freeptr_offset,
> -		  unsigned int align, slab_flags_t flags,
> -		  unsigned int useroffset, unsigned int usersize,
> -		  void (*ctor)(void *))
> +/**
> + * __kmem_cache_create_args - Create a kmem cache
> + * @name: A string which is used in /proc/slabinfo to identify this cache.
> + * @object_size: The size of objects to be created in this cache.
> + * @args: Arguments for the cache creation (see struct kmem_cache_args).
> + * @flags: See %SLAB_* flags for an explanation of individual @flags.
> + *
> + * Cannot be called within a interrupt, but can be interrupted.
> + *
> + * Return: a pointer to the cache on success, NULL on failure.
> + */
> +struct kmem_cache *__kmem_cache_create_args(const char *name,
> +					    unsigned int object_size,
> +					    struct kmem_cache_args *args,
> +					    slab_flags_t flags)
>  {
>  	struct kmem_cache *s = NULL;
> +	unsigned int freeptr_offset = UINT_MAX;
>  	const char *cache_name;
>  	int err;
>  
> @@ -275,7 +285,7 @@ do_kmem_cache_create_usercopy(const char *name,
>  
>  	mutex_lock(&slab_mutex);
>  
> -	err = kmem_cache_sanity_check(name, size);
> +	err = kmem_cache_sanity_check(name, object_size);
>  	if (err) {
>  		goto out_unlock;
>  	}
> @@ -296,12 +306,14 @@ do_kmem_cache_create_usercopy(const char *name,
>  
>  	/* Fail closed on bad usersize of useroffset values. */
>  	if (!IS_ENABLED(CONFIG_HARDENED_USERCOPY) ||
> -	    WARN_ON(!usersize && useroffset) ||
> -	    WARN_ON(size < usersize || size - usersize < useroffset))
> -		usersize = useroffset = 0;
> -
> -	if (!usersize)
> -		s = __kmem_cache_alias(name, size, align, flags, ctor);
> +	    WARN_ON(!args->usersize && args->useroffset) ||
> +	    WARN_ON(object_size < args->usersize ||
> +		    object_size - args->usersize < args->useroffset))
> +		args->usersize = args->useroffset = 0;
> +
> +	if (!args->usersize)
> +		s = __kmem_cache_alias(name, object_size, args->align, flags,
> +				       args->ctor);

Sorry I missed it in the previous review, but nothing guaranties that
nobody will call kmem_cache_create_args with args != NULL.

I think there should be a check for args != NULL and a substitution of args
with defaults if it actually was NULL.

>  	if (s)
>  		goto out_unlock;
>  
> @@ -311,9 +323,11 @@ do_kmem_cache_create_usercopy(const char *name,
>  		goto out_unlock;
>  	}
>  
> -	s = create_cache(cache_name, size, freeptr_offset,
> -			 calculate_alignment(flags, align, size),
> -			 flags, useroffset, usersize, ctor);
> +	if (args->use_freeptr_offset)
> +		freeptr_offset = args->freeptr_offset;
> +	s = create_cache(cache_name, object_size, freeptr_offset,
> +			 calculate_alignment(flags, args->align, object_size),
> +			 flags, args->useroffset, args->usersize, args->ctor);
>  	if (IS_ERR(s)) {
>  		err = PTR_ERR(s);
>  		kfree_const(cache_name);
> @@ -335,6 +349,27 @@ do_kmem_cache_create_usercopy(const char *name,
>  	}
>  	return s;
>  }
> +EXPORT_SYMBOL(__kmem_cache_create_args);
> +
> +static struct kmem_cache *
> +do_kmem_cache_create_usercopy(const char *name,
> +                 unsigned int size, unsigned int freeptr_offset,
> +                 unsigned int align, slab_flags_t flags,
> +                 unsigned int useroffset, unsigned int usersize,
> +                 void (*ctor)(void *))
> +{
> +	struct kmem_cache_args kmem_args = {
> +		.align			= align,
> +		.use_freeptr_offset	= freeptr_offset != UINT_MAX,
> +		.freeptr_offset		= freeptr_offset,
> +		.useroffset		= useroffset,
> +		.usersize		= usersize,
> +		.ctor			= ctor,
> +	};
> +
> +	return __kmem_cache_create_args(name, size, &kmem_args, flags);
> +}
> +
>  
>  /**
>   * kmem_cache_create_usercopy - Create a cache with a region suitable
> 
> -- 
> 2.45.2
> 

-- 
Sincerely yours,
Mike.

Re: [PATCH v2 02/15] slab: add struct kmem_cache_args

[Christian Brauner]

On Wed, Sep 04, 2024 at 06:16:16PM GMT, Mike Rapoport wrote:
> On Tue, Sep 03, 2024 at 04:20:43PM +0200, Christian Brauner wrote:
> > Signed-off-by: Christian Brauner <brauner@kernel.org>
> > ---
> >  include/linux/slab.h | 21 ++++++++++++++++
> >  mm/slab_common.c     | 67 +++++++++++++++++++++++++++++++++++++++-------------
> >  2 files changed, 72 insertions(+), 16 deletions(-)
> > 
> > diff --git a/include/linux/slab.h b/include/linux/slab.h
> > index 5b2da2cf31a8..79d8c8bca4a4 100644
> > --- a/include/linux/slab.h
> > +++ b/include/linux/slab.h
> > @@ -240,6 +240,27 @@ struct mem_cgroup;
> >   */
> >  bool slab_is_available(void);
> >  
> > +/**
> > + * @align: The required alignment for the objects.
> > + * @useroffset: Usercopy region offset
> > + * @usersize: Usercopy region size
> > + * @freeptr_offset: Custom offset for the free pointer in RCU caches
> > + * @use_freeptr_offset: Whether a @freeptr_offset is used
> > + * @ctor: A constructor for the objects.
> > + */
> > +struct kmem_cache_args {
> > +	unsigned int align;
> > +	unsigned int useroffset;
> > +	unsigned int usersize;
> > +	unsigned int freeptr_offset;
> > +	bool use_freeptr_offset;
> > +	void (*ctor)(void *);
> > +};
> > +
> > +struct kmem_cache *__kmem_cache_create_args(const char *name,
> > +					    unsigned int object_size,
> > +					    struct kmem_cache_args *args,
> > +					    slab_flags_t flags);
> >  struct kmem_cache *kmem_cache_create(const char *name, unsigned int size,
> >  			unsigned int align, slab_flags_t flags,
> >  			void (*ctor)(void *));
> > diff --git a/mm/slab_common.c b/mm/slab_common.c
> > index 91e0e36e4379..0f13c045b8d1 100644
> > --- a/mm/slab_common.c
> > +++ b/mm/slab_common.c
> > @@ -248,14 +248,24 @@ static struct kmem_cache *create_cache(const char *name,
> >  	return ERR_PTR(err);
> >  }
> >  
> > -static struct kmem_cache *
> > -do_kmem_cache_create_usercopy(const char *name,
> > -		  unsigned int size, unsigned int freeptr_offset,
> > -		  unsigned int align, slab_flags_t flags,
> > -		  unsigned int useroffset, unsigned int usersize,
> > -		  void (*ctor)(void *))
> > +/**
> > + * __kmem_cache_create_args - Create a kmem cache
> > + * @name: A string which is used in /proc/slabinfo to identify this cache.
> > + * @object_size: The size of objects to be created in this cache.
> > + * @args: Arguments for the cache creation (see struct kmem_cache_args).
> > + * @flags: See %SLAB_* flags for an explanation of individual @flags.
> > + *
> > + * Cannot be called within a interrupt, but can be interrupted.
> > + *
> > + * Return: a pointer to the cache on success, NULL on failure.
> > + */
> > +struct kmem_cache *__kmem_cache_create_args(const char *name,
> > +					    unsigned int object_size,
> > +					    struct kmem_cache_args *args,
> > +					    slab_flags_t flags)
> >  {
> >  	struct kmem_cache *s = NULL;
> > +	unsigned int freeptr_offset = UINT_MAX;
> >  	const char *cache_name;
> >  	int err;
> >  
> > @@ -275,7 +285,7 @@ do_kmem_cache_create_usercopy(const char *name,
> >  
> >  	mutex_lock(&slab_mutex);
> >  
> > -	err = kmem_cache_sanity_check(name, size);
> > +	err = kmem_cache_sanity_check(name, object_size);
> >  	if (err) {
> >  		goto out_unlock;
> >  	}
> > @@ -296,12 +306,14 @@ do_kmem_cache_create_usercopy(const char *name,
> >  
> >  	/* Fail closed on bad usersize of useroffset values. */
> >  	if (!IS_ENABLED(CONFIG_HARDENED_USERCOPY) ||
> > -	    WARN_ON(!usersize && useroffset) ||
> > -	    WARN_ON(size < usersize || size - usersize < useroffset))
> > -		usersize = useroffset = 0;
> > -
> > -	if (!usersize)
> > -		s = __kmem_cache_alias(name, size, align, flags, ctor);
> > +	    WARN_ON(!args->usersize && args->useroffset) ||
> > +	    WARN_ON(object_size < args->usersize ||
> > +		    object_size - args->usersize < args->useroffset))
> > +		args->usersize = args->useroffset = 0;
> > +
> > +	if (!args->usersize)
> > +		s = __kmem_cache_alias(name, object_size, args->align, flags,
> > +				       args->ctor);
> 
> Sorry I missed it in the previous review, but nothing guaranties that
> nobody will call kmem_cache_create_args with args != NULL.
> 
> I think there should be a check for args != NULL and a substitution of args
> with defaults if it actually was NULL.

I think that callers that pass NULL should all be switched to
KMEM_CACHE() and passing NULL should simply not be supported. And the
few callers that need some very special alignment need to pass struct
kmem_cache_args anyway. So there should never be a need to pass NULL.

Re: [PATCH v2 02/15] slab: add struct kmem_cache_args

[Mike Rapoport]

On Wed, Sep 04, 2024 at 05:48:31PM +0200, Christian Brauner wrote:
> On Wed, Sep 04, 2024 at 06:16:16PM GMT, Mike Rapoport wrote:
> > On Tue, Sep 03, 2024 at 04:20:43PM +0200, Christian Brauner wrote:
> > > Signed-off-by: Christian Brauner <brauner@kernel.org>
> > > ---
> > >  include/linux/slab.h | 21 ++++++++++++++++
> > >  mm/slab_common.c     | 67 +++++++++++++++++++++++++++++++++++++++-------------
> > >  2 files changed, 72 insertions(+), 16 deletions(-)
> > > 
> > > diff --git a/include/linux/slab.h b/include/linux/slab.h
> > > index 5b2da2cf31a8..79d8c8bca4a4 100644
> > > --- a/include/linux/slab.h
> > > +++ b/include/linux/slab.h
> > > @@ -240,6 +240,27 @@ struct mem_cgroup;
> > >   */
> > >  bool slab_is_available(void);
> > >  
> > > +/**
> > > + * @align: The required alignment for the objects.
> > > + * @useroffset: Usercopy region offset
> > > + * @usersize: Usercopy region size
> > > + * @freeptr_offset: Custom offset for the free pointer in RCU caches
> > > + * @use_freeptr_offset: Whether a @freeptr_offset is used
> > > + * @ctor: A constructor for the objects.
> > > + */
> > > +struct kmem_cache_args {
> > > +	unsigned int align;
> > > +	unsigned int useroffset;
> > > +	unsigned int usersize;
> > > +	unsigned int freeptr_offset;
> > > +	bool use_freeptr_offset;
> > > +	void (*ctor)(void *);
> > > +};
> > > +
> > > +struct kmem_cache *__kmem_cache_create_args(const char *name,
> > > +					    unsigned int object_size,
> > > +					    struct kmem_cache_args *args,
> > > +					    slab_flags_t flags);
> > >  struct kmem_cache *kmem_cache_create(const char *name, unsigned int size,
> > >  			unsigned int align, slab_flags_t flags,
> > >  			void (*ctor)(void *));
> > > diff --git a/mm/slab_common.c b/mm/slab_common.c
> > > index 91e0e36e4379..0f13c045b8d1 100644
> > > --- a/mm/slab_common.c
> > > +++ b/mm/slab_common.c
> > > @@ -248,14 +248,24 @@ static struct kmem_cache *create_cache(const char *name,
> > >  	return ERR_PTR(err);
> > >  }
> > >  
> > > -static struct kmem_cache *
> > > -do_kmem_cache_create_usercopy(const char *name,
> > > -		  unsigned int size, unsigned int freeptr_offset,
> > > -		  unsigned int align, slab_flags_t flags,
> > > -		  unsigned int useroffset, unsigned int usersize,
> > > -		  void (*ctor)(void *))
> > > +/**
> > > + * __kmem_cache_create_args - Create a kmem cache
> > > + * @name: A string which is used in /proc/slabinfo to identify this cache.
> > > + * @object_size: The size of objects to be created in this cache.
> > > + * @args: Arguments for the cache creation (see struct kmem_cache_args).
> > > + * @flags: See %SLAB_* flags for an explanation of individual @flags.
> > > + *
> > > + * Cannot be called within a interrupt, but can be interrupted.
> > > + *
> > > + * Return: a pointer to the cache on success, NULL on failure.
> > > + */
> > > +struct kmem_cache *__kmem_cache_create_args(const char *name,
> > > +					    unsigned int object_size,
> > > +					    struct kmem_cache_args *args,
> > > +					    slab_flags_t flags)
> > >  {
> > >  	struct kmem_cache *s = NULL;
> > > +	unsigned int freeptr_offset = UINT_MAX;
> > >  	const char *cache_name;
> > >  	int err;
> > >  
> > > @@ -275,7 +285,7 @@ do_kmem_cache_create_usercopy(const char *name,
> > >  
> > >  	mutex_lock(&slab_mutex);
> > >  
> > > -	err = kmem_cache_sanity_check(name, size);
> > > +	err = kmem_cache_sanity_check(name, object_size);
> > >  	if (err) {
> > >  		goto out_unlock;
> > >  	}
> > > @@ -296,12 +306,14 @@ do_kmem_cache_create_usercopy(const char *name,
> > >  
> > >  	/* Fail closed on bad usersize of useroffset values. */
> > >  	if (!IS_ENABLED(CONFIG_HARDENED_USERCOPY) ||
> > > -	    WARN_ON(!usersize && useroffset) ||
> > > -	    WARN_ON(size < usersize || size - usersize < useroffset))
> > > -		usersize = useroffset = 0;
> > > -
> > > -	if (!usersize)
> > > -		s = __kmem_cache_alias(name, size, align, flags, ctor);
> > > +	    WARN_ON(!args->usersize && args->useroffset) ||
> > > +	    WARN_ON(object_size < args->usersize ||
> > > +		    object_size - args->usersize < args->useroffset))
> > > +		args->usersize = args->useroffset = 0;
> > > +
> > > +	if (!args->usersize)
> > > +		s = __kmem_cache_alias(name, object_size, args->align, flags,
> > > +				       args->ctor);
> > 
> > Sorry I missed it in the previous review, but nothing guaranties that
> > nobody will call kmem_cache_create_args with args != NULL.
> > 
> > I think there should be a check for args != NULL and a substitution of args
> > with defaults if it actually was NULL.
> 
> I think that callers that pass NULL should all be switched to
> KMEM_CACHE() and passing NULL should simply not be supported. And the
> few callers that need some very special alignment need to pass struct
> kmem_cache_args anyway. So there should never be a need to pass NULL.

But you can't guarantee that some random driver won't call

	__kmem_cache_create_args("name", size, NULL, flags);

At least we'd need
	
	if (!args)
		return -EINVAL;

-- 
Sincerely yours,
Mike.

Re: [PATCH v2 02/15] slab: add struct kmem_cache_args

[Christian Brauner]

On Wed, Sep 04, 2024 at 07:16:07PM GMT, Mike Rapoport wrote:
> On Wed, Sep 04, 2024 at 05:48:31PM +0200, Christian Brauner wrote:
> > On Wed, Sep 04, 2024 at 06:16:16PM GMT, Mike Rapoport wrote:
> > > On Tue, Sep 03, 2024 at 04:20:43PM +0200, Christian Brauner wrote:
> > > > Signed-off-by: Christian Brauner <brauner@kernel.org>
> > > > ---
> > > >  include/linux/slab.h | 21 ++++++++++++++++
> > > >  mm/slab_common.c     | 67 +++++++++++++++++++++++++++++++++++++++-------------
> > > >  2 files changed, 72 insertions(+), 16 deletions(-)
> > > > 
> > > > diff --git a/include/linux/slab.h b/include/linux/slab.h
> > > > index 5b2da2cf31a8..79d8c8bca4a4 100644
> > > > --- a/include/linux/slab.h
> > > > +++ b/include/linux/slab.h
> > > > @@ -240,6 +240,27 @@ struct mem_cgroup;
> > > >   */
> > > >  bool slab_is_available(void);
> > > >  
> > > > +/**
> > > > + * @align: The required alignment for the objects.
> > > > + * @useroffset: Usercopy region offset
> > > > + * @usersize: Usercopy region size
> > > > + * @freeptr_offset: Custom offset for the free pointer in RCU caches
> > > > + * @use_freeptr_offset: Whether a @freeptr_offset is used
> > > > + * @ctor: A constructor for the objects.
> > > > + */
> > > > +struct kmem_cache_args {
> > > > +	unsigned int align;
> > > > +	unsigned int useroffset;
> > > > +	unsigned int usersize;
> > > > +	unsigned int freeptr_offset;
> > > > +	bool use_freeptr_offset;
> > > > +	void (*ctor)(void *);
> > > > +};
> > > > +
> > > > +struct kmem_cache *__kmem_cache_create_args(const char *name,
> > > > +					    unsigned int object_size,
> > > > +					    struct kmem_cache_args *args,
> > > > +					    slab_flags_t flags);
> > > >  struct kmem_cache *kmem_cache_create(const char *name, unsigned int size,
> > > >  			unsigned int align, slab_flags_t flags,
> > > >  			void (*ctor)(void *));
> > > > diff --git a/mm/slab_common.c b/mm/slab_common.c
> > > > index 91e0e36e4379..0f13c045b8d1 100644
> > > > --- a/mm/slab_common.c
> > > > +++ b/mm/slab_common.c
> > > > @@ -248,14 +248,24 @@ static struct kmem_cache *create_cache(const char *name,
> > > >  	return ERR_PTR(err);
> > > >  }
> > > >  
> > > > -static struct kmem_cache *
> > > > -do_kmem_cache_create_usercopy(const char *name,
> > > > -		  unsigned int size, unsigned int freeptr_offset,
> > > > -		  unsigned int align, slab_flags_t flags,
> > > > -		  unsigned int useroffset, unsigned int usersize,
> > > > -		  void (*ctor)(void *))
> > > > +/**
> > > > + * __kmem_cache_create_args - Create a kmem cache
> > > > + * @name: A string which is used in /proc/slabinfo to identify this cache.
> > > > + * @object_size: The size of objects to be created in this cache.
> > > > + * @args: Arguments for the cache creation (see struct kmem_cache_args).
> > > > + * @flags: See %SLAB_* flags for an explanation of individual @flags.
> > > > + *
> > > > + * Cannot be called within a interrupt, but can be interrupted.
> > > > + *
> > > > + * Return: a pointer to the cache on success, NULL on failure.
> > > > + */
> > > > +struct kmem_cache *__kmem_cache_create_args(const char *name,
> > > > +					    unsigned int object_size,
> > > > +					    struct kmem_cache_args *args,
> > > > +					    slab_flags_t flags)
> > > >  {
> > > >  	struct kmem_cache *s = NULL;
> > > > +	unsigned int freeptr_offset = UINT_MAX;
> > > >  	const char *cache_name;
> > > >  	int err;
> > > >  
> > > > @@ -275,7 +285,7 @@ do_kmem_cache_create_usercopy(const char *name,
> > > >  
> > > >  	mutex_lock(&slab_mutex);
> > > >  
> > > > -	err = kmem_cache_sanity_check(name, size);
> > > > +	err = kmem_cache_sanity_check(name, object_size);
> > > >  	if (err) {
> > > >  		goto out_unlock;
> > > >  	}
> > > > @@ -296,12 +306,14 @@ do_kmem_cache_create_usercopy(const char *name,
> > > >  
> > > >  	/* Fail closed on bad usersize of useroffset values. */
> > > >  	if (!IS_ENABLED(CONFIG_HARDENED_USERCOPY) ||
> > > > -	    WARN_ON(!usersize && useroffset) ||
> > > > -	    WARN_ON(size < usersize || size - usersize < useroffset))
> > > > -		usersize = useroffset = 0;
> > > > -
> > > > -	if (!usersize)
> > > > -		s = __kmem_cache_alias(name, size, align, flags, ctor);
> > > > +	    WARN_ON(!args->usersize && args->useroffset) ||
> > > > +	    WARN_ON(object_size < args->usersize ||
> > > > +		    object_size - args->usersize < args->useroffset))
> > > > +		args->usersize = args->useroffset = 0;
> > > > +
> > > > +	if (!args->usersize)
> > > > +		s = __kmem_cache_alias(name, object_size, args->align, flags,
> > > > +				       args->ctor);
> > > 
> > > Sorry I missed it in the previous review, but nothing guaranties that
> > > nobody will call kmem_cache_create_args with args != NULL.
> > > 
> > > I think there should be a check for args != NULL and a substitution of args
> > > with defaults if it actually was NULL.
> > 
> > I think that callers that pass NULL should all be switched to
> > KMEM_CACHE() and passing NULL should simply not be supported. And the
> > few callers that need some very special alignment need to pass struct
> > kmem_cache_args anyway. So there should never be a need to pass NULL.
> 
> But you can't guarantee that some random driver won't call
> 
> 	__kmem_cache_create_args("name", size, NULL, flags);
> 
> At least we'd need
> 	
> 	if (!args)
> 		return -EINVAL;

Calling __kmem_cache_create_args() directly is a bug. That's why it's __*().
And we don't check for non-NULL @name either. In fact we almost never do
such checks.

Plus, if someone did:

kmem_cache_create("foo", sizeof(foo), NULL, flags);

they'd get a compile time error due to _Generic().

Re: [PATCH v2 02/15] slab: add struct kmem_cache_args

[Vlastimil Babka]

On 9/4/24 17:16, Mike Rapoport wrote:
> On Tue, Sep 03, 2024 at 04:20:43PM +0200, Christian Brauner wrote:
>> @@ -275,7 +285,7 @@ do_kmem_cache_create_usercopy(const char *name,
>>  
>>  	mutex_lock(&slab_mutex);
>>  
>> -	err = kmem_cache_sanity_check(name, size);
>> +	err = kmem_cache_sanity_check(name, object_size);
>>  	if (err) {
>>  		goto out_unlock;
>>  	}
>> @@ -296,12 +306,14 @@ do_kmem_cache_create_usercopy(const char *name,
>>  
>>  	/* Fail closed on bad usersize of useroffset values. */
>>  	if (!IS_ENABLED(CONFIG_HARDENED_USERCOPY) ||
>> -	    WARN_ON(!usersize && useroffset) ||
>> -	    WARN_ON(size < usersize || size - usersize < useroffset))
>> -		usersize = useroffset = 0;
>> -
>> -	if (!usersize)
>> -		s = __kmem_cache_alias(name, size, align, flags, ctor);
>> +	    WARN_ON(!args->usersize && args->useroffset) ||
>> +	    WARN_ON(object_size < args->usersize ||
>> +		    object_size - args->usersize < args->useroffset))
>> +		args->usersize = args->useroffset = 0;
>> +
>> +	if (!args->usersize)
>> +		s = __kmem_cache_alias(name, object_size, args->align, flags,
>> +				       args->ctor);
> 
> Sorry I missed it in the previous review, but nothing guaranties that
> nobody will call kmem_cache_create_args with args != NULL.
> 
> I think there should be a check for args != NULL and a substitution of args
> with defaults if it actually was NULL.

Hm there might be a bigger problem with this? If we wanted to do a
(non-flag-day) conversion to the new kmem_cache_create() for some callers
that need none of the extra args, passing NULL wouldn't work for the
_Generic((__args) looking for "struct kmem_cache_args *" as NULL is not of
that type, right?

I tried and it really errors out.

Re: [PATCH v2 02/15] slab: add struct kmem_cache_args

[Mike Rapoport]

On Wed, Sep 04, 2024 at 05:49:15PM +0200, Vlastimil Babka wrote:
> On 9/4/24 17:16, Mike Rapoport wrote:
> > On Tue, Sep 03, 2024 at 04:20:43PM +0200, Christian Brauner wrote:
> >> @@ -275,7 +285,7 @@ do_kmem_cache_create_usercopy(const char *name,
> >>  
> >>  	mutex_lock(&slab_mutex);
> >>  
> >> -	err = kmem_cache_sanity_check(name, size);
> >> +	err = kmem_cache_sanity_check(name, object_size);
> >>  	if (err) {
> >>  		goto out_unlock;
> >>  	}
> >> @@ -296,12 +306,14 @@ do_kmem_cache_create_usercopy(const char *name,
> >>  
> >>  	/* Fail closed on bad usersize of useroffset values. */
> >>  	if (!IS_ENABLED(CONFIG_HARDENED_USERCOPY) ||
> >> -	    WARN_ON(!usersize && useroffset) ||
> >> -	    WARN_ON(size < usersize || size - usersize < useroffset))
> >> -		usersize = useroffset = 0;
> >> -
> >> -	if (!usersize)
> >> -		s = __kmem_cache_alias(name, size, align, flags, ctor);
> >> +	    WARN_ON(!args->usersize && args->useroffset) ||
> >> +	    WARN_ON(object_size < args->usersize ||
> >> +		    object_size - args->usersize < args->useroffset))
> >> +		args->usersize = args->useroffset = 0;
> >> +
> >> +	if (!args->usersize)
> >> +		s = __kmem_cache_alias(name, object_size, args->align, flags,
> >> +				       args->ctor);
> > 
> > Sorry I missed it in the previous review, but nothing guaranties that
> > nobody will call kmem_cache_create_args with args != NULL.
> > 
> > I think there should be a check for args != NULL and a substitution of args
> > with defaults if it actually was NULL.
> 
> Hm there might be a bigger problem with this? If we wanted to do a
> (non-flag-day) conversion to the new kmem_cache_create() for some callers
> that need none of the extra args, passing NULL wouldn't work for the
> _Generic((__args) looking for "struct kmem_cache_args *" as NULL is not of
> that type, right?
> 
> I tried and it really errors out.

How about

#define kmem_cache_create(__name, __object_size, __args, ...)           \
	_Generic((__args),                                              \
		struct kmem_cache_args *: __kmem_cache_create_args,	\
		void *: __kmem_cache_create_args,			\
		default: __kmem_cache_create)(__name, __object_size, __args, __VA_ARGS__)

 

-- 
Sincerely yours,
Mike.

Re: [PATCH v2 02/15] slab: add struct kmem_cache_args

[Vlastimil Babka]

On 9/4/24 18:16, Mike Rapoport wrote:
> On Wed, Sep 04, 2024 at 05:49:15PM +0200, Vlastimil Babka wrote:
>> On 9/4/24 17:16, Mike Rapoport wrote:
>> > On Tue, Sep 03, 2024 at 04:20:43PM +0200, Christian Brauner wrote:
>> >> @@ -275,7 +285,7 @@ do_kmem_cache_create_usercopy(const char *name,
>> >>  
>> >>  	mutex_lock(&slab_mutex);
>> >>  
>> >> -	err = kmem_cache_sanity_check(name, size);
>> >> +	err = kmem_cache_sanity_check(name, object_size);
>> >>  	if (err) {
>> >>  		goto out_unlock;
>> >>  	}
>> >> @@ -296,12 +306,14 @@ do_kmem_cache_create_usercopy(const char *name,
>> >>  
>> >>  	/* Fail closed on bad usersize of useroffset values. */
>> >>  	if (!IS_ENABLED(CONFIG_HARDENED_USERCOPY) ||
>> >> -	    WARN_ON(!usersize && useroffset) ||
>> >> -	    WARN_ON(size < usersize || size - usersize < useroffset))
>> >> -		usersize = useroffset = 0;
>> >> -
>> >> -	if (!usersize)
>> >> -		s = __kmem_cache_alias(name, size, align, flags, ctor);
>> >> +	    WARN_ON(!args->usersize && args->useroffset) ||
>> >> +	    WARN_ON(object_size < args->usersize ||
>> >> +		    object_size - args->usersize < args->useroffset))
>> >> +		args->usersize = args->useroffset = 0;
>> >> +
>> >> +	if (!args->usersize)
>> >> +		s = __kmem_cache_alias(name, object_size, args->align, flags,
>> >> +				       args->ctor);
>> > 
>> > Sorry I missed it in the previous review, but nothing guaranties that
>> > nobody will call kmem_cache_create_args with args != NULL.
>> > 
>> > I think there should be a check for args != NULL and a substitution of args
>> > with defaults if it actually was NULL.
>> 
>> Hm there might be a bigger problem with this? If we wanted to do a
>> (non-flag-day) conversion to the new kmem_cache_create() for some callers
>> that need none of the extra args, passing NULL wouldn't work for the
>> _Generic((__args) looking for "struct kmem_cache_args *" as NULL is not of
>> that type, right?
>> 
>> I tried and it really errors out.
> 
> How about
> 
> #define kmem_cache_create(__name, __object_size, __args, ...)           \
> 	_Generic((__args),                                              \
> 		struct kmem_cache_args *: __kmem_cache_create_args,	\
> 		void *: __kmem_cache_create_args,			\
> 		default: __kmem_cache_create)(__name, __object_size, __args, __VA_ARGS__)

Seems to work. I'd agree with the "if NULL, use a static default" direction
then. It just seems like a more user-friendly API to me.

Re: [PATCH v2 02/15] slab: add struct kmem_cache_args

[Christian Brauner]

On Wed, Sep 04, 2024 at 06:22:45PM GMT, Vlastimil Babka wrote:
> On 9/4/24 18:16, Mike Rapoport wrote:
> > On Wed, Sep 04, 2024 at 05:49:15PM +0200, Vlastimil Babka wrote:
> >> On 9/4/24 17:16, Mike Rapoport wrote:
> >> > On Tue, Sep 03, 2024 at 04:20:43PM +0200, Christian Brauner wrote:
> >> >> @@ -275,7 +285,7 @@ do_kmem_cache_create_usercopy(const char *name,
> >> >>  
> >> >>  	mutex_lock(&slab_mutex);
> >> >>  
> >> >> -	err = kmem_cache_sanity_check(name, size);
> >> >> +	err = kmem_cache_sanity_check(name, object_size);
> >> >>  	if (err) {
> >> >>  		goto out_unlock;
> >> >>  	}
> >> >> @@ -296,12 +306,14 @@ do_kmem_cache_create_usercopy(const char *name,
> >> >>  
> >> >>  	/* Fail closed on bad usersize of useroffset values. */
> >> >>  	if (!IS_ENABLED(CONFIG_HARDENED_USERCOPY) ||
> >> >> -	    WARN_ON(!usersize && useroffset) ||
> >> >> -	    WARN_ON(size < usersize || size - usersize < useroffset))
> >> >> -		usersize = useroffset = 0;
> >> >> -
> >> >> -	if (!usersize)
> >> >> -		s = __kmem_cache_alias(name, size, align, flags, ctor);
> >> >> +	    WARN_ON(!args->usersize && args->useroffset) ||
> >> >> +	    WARN_ON(object_size < args->usersize ||
> >> >> +		    object_size - args->usersize < args->useroffset))
> >> >> +		args->usersize = args->useroffset = 0;
> >> >> +
> >> >> +	if (!args->usersize)
> >> >> +		s = __kmem_cache_alias(name, object_size, args->align, flags,
> >> >> +				       args->ctor);
> >> > 
> >> > Sorry I missed it in the previous review, but nothing guaranties that
> >> > nobody will call kmem_cache_create_args with args != NULL.
> >> > 
> >> > I think there should be a check for args != NULL and a substitution of args
> >> > with defaults if it actually was NULL.
> >> 
> >> Hm there might be a bigger problem with this? If we wanted to do a
> >> (non-flag-day) conversion to the new kmem_cache_create() for some callers
> >> that need none of the extra args, passing NULL wouldn't work for the
> >> _Generic((__args) looking for "struct kmem_cache_args *" as NULL is not of
> >> that type, right?
> >> 
> >> I tried and it really errors out.
> > 
> > How about
> > 
> > #define kmem_cache_create(__name, __object_size, __args, ...)           \
> > 	_Generic((__args),                                              \
> > 		struct kmem_cache_args *: __kmem_cache_create_args,	\
> > 		void *: __kmem_cache_create_args,			\
> > 		default: __kmem_cache_create)(__name, __object_size, __args, __VA_ARGS__)
> 
> Seems to work. I'd agree with the "if NULL, use a static default" direction
> then. It just seems like a more user-friendly API to me.

Sure. So can you fold your suggestion above and the small diff below
into the translation layer patch?

diff --git a/mm/slab_common.c b/mm/slab_common.c
index 30000dcf0736..3c12d87825e3 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -255,9 +255,14 @@ struct kmem_cache *__kmem_cache_create_args(const char *name,
                                            slab_flags_t flags)
 {
        struct kmem_cache *s = NULL;
+       struct kmem_cache_args kmem_args = {};
        const char *cache_name;
        int err;

+       /* If no custom arguments are requested just assume the default values. */
+       if (!args)
+               args = &kmem_args;
+
 #ifdef CONFIG_SLUB_DEBUG
        /*
         * If no slab_debug was enabled globally, the static key is not yet

Re: [PATCH v2 02/15] slab: add struct kmem_cache_args

[Linus Torvalds]

On Wed, 4 Sept 2024 at 11:21, Christian Brauner <brauner@kernel.org> wrote:
>
> Sure. So can you fold your suggestion above and the small diff below
> into the translation layer patch?

Please don't.

This seems horrible. First you have a _Generic() macro that turns NULL
into the same function that a proper __kmem_cache_create_args() with a
real argument uses, and then you make that function check for NULL and
turn it into something else.

That seems *entirely* pointless.

I think the right model is to either

 (a) not allow a NULL pointer at all (ie not have a _Generic() case
for 'void *') and just error for that behavior

OR

 (b) make a NULL pointer explicitly go to some other function than the
one that gets a proper pointer

but not this "do extra work in the function to make it accept the NULL
we shunted to it".

IOW, something like this:

  #define kmem_cache_create(__name, __object_size, __args, ...)           \
       _Generic((__args),                                              \
               struct kmem_cache_args *: __kmem_cache_create_args,     \
               void *: __kmem_cache_default_args,                       \
              default: __kmem_cache_create)(__name, __object_size,
__args, __VA_ARGS__)

and then we have

 static inline struct kmem_cache *__kmem_cache_default_args(const char *name,
                                           unsigned int object_size,
                                           struct kmem_cache_args *args,
                                           slab_flags_t flags)
  { WARN_ON_ONCE(args); // It had *better* be NULL, not some random 'void *'
     return __kmem_cache_create_args(name, size, &kmem_args, flags); }

which basically just does a "turn NULL into &kmem_args" thing.

Notice how that does *not* add some odd NULL pointer check to the main
path (and the WARN_ON_ONCE() check should be compiled away for any
actual constant NULL argument, which is the only valid reason to have
that 'void *' anyway).

                     Linus

Re: [PATCH v2 02/15] slab: add struct kmem_cache_args

[Christian Brauner]

[-- Attachment #1: Type: text/plain, Size: 2282 bytes --]

On Wed, Sep 04, 2024 at 11:53:05AM GMT, Linus Torvalds wrote:
> On Wed, 4 Sept 2024 at 11:21, Christian Brauner <brauner@kernel.org> wrote:
> >
> > Sure. So can you fold your suggestion above and the small diff below
> > into the translation layer patch?
> 
> Please don't.
> 
> This seems horrible. First you have a _Generic() macro that turns NULL
> into the same function that a proper __kmem_cache_create_args() with a
> real argument uses, and then you make that function check for NULL and
> turn it into something else.
> 
> That seems *entirely* pointless.
> 
> I think the right model is to either
> 
>  (a) not allow a NULL pointer at all (ie not have a _Generic() case
> for 'void *') and just error for that behavior

Fine by me and what this did originally by erroring out with a compile
time error.

> 
> OR
> 
>  (b) make a NULL pointer explicitly go to some other function than the
> one that gets a proper pointer
> 
> but not this "do extra work in the function to make it accept the NULL
> we shunted to it".
> 
> IOW, something like this:
> 
>   #define kmem_cache_create(__name, __object_size, __args, ...)           \
>        _Generic((__args),                                              \
>                struct kmem_cache_args *: __kmem_cache_create_args,     \
>                void *: __kmem_cache_default_args,                       \
>               default: __kmem_cache_create)(__name, __object_size,
> __args, __VA_ARGS__)
> 
> and then we have
> 
>  static inline struct kmem_cache *__kmem_cache_default_args(const char *name,
>                                            unsigned int object_size,
>                                            struct kmem_cache_args *args,
>                                            slab_flags_t flags)
>   { WARN_ON_ONCE(args); // It had *better* be NULL, not some random 'void *'
>      return __kmem_cache_create_args(name, size, &kmem_args, flags); }
> 
> which basically just does a "turn NULL into &kmem_args" thing.
> 
> Notice how that does *not* add some odd NULL pointer check to the main
> path (and the WARN_ON_ONCE() check should be compiled away for any
> actual constant NULL argument, which is the only valid reason to have
> that 'void *' anyway).

Also fine by me. See appended updated patch.

[-- Attachment #2: 0001-slab-create-kmem_cache_create-compatibility-layer.patch --]
[-- Type: text/x-diff, Size: 4001 bytes --]

From ede72f93668827497fb8d1c0f7286cfb4bd4f204 Mon Sep 17 00:00:00 2001
From: Christian Brauner <brauner@kernel.org>
Date: Tue, 3 Sep 2024 14:49:49 +0200
Subject: [PATCH] slab: create kmem_cache_create() compatibility layer

Use _Generic() to create a compatibility layer that type switches on the
third argument to either call __kmem_cache_create() or
__kmem_cache_create_args(). If NULL is passed for the struct
kmem_cache_args argument use default args making porting for callers
that don't care about additional arguments easy.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 include/linux/slab.h | 29 ++++++++++++++++++++++++++---
 mm/slab_common.c     | 10 +++++-----
 2 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/include/linux/slab.h b/include/linux/slab.h
index aced16a08700..d406d00cabbb 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -261,9 +261,10 @@ struct kmem_cache *__kmem_cache_create_args(const char *name,
 					    unsigned int object_size,
 					    struct kmem_cache_args *args,
 					    slab_flags_t flags);
-struct kmem_cache *kmem_cache_create(const char *name, unsigned int size,
-			unsigned int align, slab_flags_t flags,
-			void (*ctor)(void *));
+
+struct kmem_cache *__kmem_cache_create(const char *name, unsigned int size,
+				       unsigned int align, slab_flags_t flags,
+				       void (*ctor)(void *));
 struct kmem_cache *kmem_cache_create_usercopy(const char *name,
 			unsigned int size, unsigned int align,
 			slab_flags_t flags,
@@ -272,6 +273,28 @@ struct kmem_cache *kmem_cache_create_usercopy(const char *name,
 struct kmem_cache *kmem_cache_create_rcu(const char *name, unsigned int size,
 					 unsigned int freeptr_offset,
 					 slab_flags_t flags);
+
+/* If NULL is passed for @args, use this variant with default arguments. */
+static inline struct kmem_cache *
+__kmem_cache_default_args(const char *name, unsigned int size,
+			  const struct kmem_cache_args *args,
+			  slab_flags_t flags)
+{
+	struct kmem_cache_args kmem_default_args = {};
+
+	/* Make sure we don't get passed garbage. */
+	if (WARN_ON_ONCE(args))
+		return NULL;
+
+	return __kmem_cache_create_args(name, size, &kmem_default_args, flags);
+}
+
+#define kmem_cache_create(__name, __object_size, __args, ...)           \
+	_Generic((__args),                                              \
+		struct kmem_cache_args *: __kmem_cache_create_args,	\
+		void *: __kmem_cache_default_args,			\
+		default: __kmem_cache_create)(__name, __object_size, __args, __VA_ARGS__)
+
 void kmem_cache_destroy(struct kmem_cache *s);
 int kmem_cache_shrink(struct kmem_cache *s);
 
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 19ae3dd6e36f..418459927670 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -383,7 +383,7 @@ kmem_cache_create_usercopy(const char *name, unsigned int size,
 EXPORT_SYMBOL(kmem_cache_create_usercopy);
 
 /**
- * kmem_cache_create - Create a cache.
+ * __kmem_cache_create - Create a cache.
  * @name: A string which is used in /proc/slabinfo to identify this cache.
  * @size: The size of objects to be created in this cache.
  * @align: The required alignment for the objects.
@@ -407,9 +407,9 @@ EXPORT_SYMBOL(kmem_cache_create_usercopy);
  *
  * Return: a pointer to the cache on success, NULL on failure.
  */
-struct kmem_cache *
-kmem_cache_create(const char *name, unsigned int size, unsigned int align,
-		slab_flags_t flags, void (*ctor)(void *))
+struct kmem_cache *__kmem_cache_create(const char *name, unsigned int size,
+				       unsigned int align, slab_flags_t flags,
+				       void (*ctor)(void *))
 {
 	struct kmem_cache_args kmem_args = {
 		.align	= align,
@@ -418,7 +418,7 @@ kmem_cache_create(const char *name, unsigned int size, unsigned int align,
 
 	return __kmem_cache_create_args(name, size, &kmem_args, flags);
 }
-EXPORT_SYMBOL(kmem_cache_create);
+EXPORT_SYMBOL(__kmem_cache_create);
 
 /**
  * kmem_cache_create_rcu - Create a SLAB_TYPESAFE_BY_RCU cache.
-- 
2.45.2

[PATCH v2 03/15] slab: port kmem_cache_create() to struct kmem_cache_args

[Christian Brauner]

Port kmem_cache_create() to struct kmem_cache_args.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 mm/slab_common.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/mm/slab_common.c b/mm/slab_common.c
index 0f13c045b8d1..ac0832dac01e 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -439,8 +439,12 @@ struct kmem_cache *
 kmem_cache_create(const char *name, unsigned int size, unsigned int align,
 		slab_flags_t flags, void (*ctor)(void *))
 {
-	return do_kmem_cache_create_usercopy(name, size, UINT_MAX, align, flags,
-					     0, 0, ctor);
+	struct kmem_cache_args kmem_args = {
+		.align	= align,
+		.ctor	= ctor,
+	};
+
+	return __kmem_cache_create_args(name, size, &kmem_args, flags);
 }
 EXPORT_SYMBOL(kmem_cache_create);
 

-- 
2.45.2

Re: [PATCH v2 03/15] slab: port kmem_cache_create() to struct kmem_cache_args

[Mike Rapoport]

On Tue, Sep 03, 2024 at 04:20:44PM +0200, Christian Brauner wrote:
> Port kmem_cache_create() to struct kmem_cache_args.
> 
> Signed-off-by: Christian Brauner <brauner@kernel.org>

Reviewed-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

> ---
>  mm/slab_common.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 0f13c045b8d1..ac0832dac01e 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -439,8 +439,12 @@ struct kmem_cache *
>  kmem_cache_create(const char *name, unsigned int size, unsigned int align,
>  		slab_flags_t flags, void (*ctor)(void *))
>  {
> -	return do_kmem_cache_create_usercopy(name, size, UINT_MAX, align, flags,
> -					     0, 0, ctor);
> +	struct kmem_cache_args kmem_args = {
> +		.align	= align,
> +		.ctor	= ctor,
> +	};
> +
> +	return __kmem_cache_create_args(name, size, &kmem_args, flags);
>  }
>  EXPORT_SYMBOL(kmem_cache_create);
>  
> 
> -- 
> 2.45.2
> 

-- 
Sincerely yours,
Mike.

[PATCH v2 04/15] slab: port kmem_cache_create_rcu() to struct kmem_cache_args

[Christian Brauner]

Port kmem_cache_create_rcu() to struct kmem_cache_args.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 mm/slab_common.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/mm/slab_common.c b/mm/slab_common.c
index ac0832dac01e..da62ed30f95d 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -481,9 +481,13 @@ struct kmem_cache *kmem_cache_create_rcu(const char *name, unsigned int size,
 					 unsigned int freeptr_offset,
 					 slab_flags_t flags)
 {
-	return do_kmem_cache_create_usercopy(name, size, freeptr_offset, 0,
-					     flags | SLAB_TYPESAFE_BY_RCU, 0, 0,
-					     NULL);
+	struct kmem_cache_args kmem_args = {
+		.freeptr_offset		= freeptr_offset,
+		.use_freeptr_offset	= true,
+	};
+
+	return __kmem_cache_create_args(name, size, &kmem_args,
+					flags | SLAB_TYPESAFE_BY_RCU);
 }
 EXPORT_SYMBOL(kmem_cache_create_rcu);
 

-- 
2.45.2

Re: [PATCH v2 04/15] slab: port kmem_cache_create_rcu() to struct kmem_cache_args

[Mike Rapoport]

On Tue, Sep 03, 2024 at 04:20:45PM +0200, Christian Brauner wrote:
> Port kmem_cache_create_rcu() to struct kmem_cache_args.
> 
> Signed-off-by: Christian Brauner <brauner@kernel.org>

Reviewed-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

> ---
>  mm/slab_common.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index ac0832dac01e..da62ed30f95d 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -481,9 +481,13 @@ struct kmem_cache *kmem_cache_create_rcu(const char *name, unsigned int size,
>  					 unsigned int freeptr_offset,
>  					 slab_flags_t flags)
>  {
> -	return do_kmem_cache_create_usercopy(name, size, freeptr_offset, 0,
> -					     flags | SLAB_TYPESAFE_BY_RCU, 0, 0,
> -					     NULL);
> +	struct kmem_cache_args kmem_args = {
> +		.freeptr_offset		= freeptr_offset,
> +		.use_freeptr_offset	= true,
> +	};
> +
> +	return __kmem_cache_create_args(name, size, &kmem_args,
> +					flags | SLAB_TYPESAFE_BY_RCU);
>  }
>  EXPORT_SYMBOL(kmem_cache_create_rcu);
>  
> 
> -- 
> 2.45.2
> 

-- 
Sincerely yours,
Mike.

[PATCH v2 05/15] slab: port kmem_cache_create_usercopy() to struct kmem_cache_args

[Christian Brauner]

Pprt kmem_cache_create_usercopy() to struct kmem_cache_args and remove
the now unused do_kmem_cache_create_usercopy() helper.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 mm/slab_common.c | 30 ++++++++----------------------
 1 file changed, 8 insertions(+), 22 deletions(-)

diff --git a/mm/slab_common.c b/mm/slab_common.c
index da62ed30f95d..16c36a946135 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -351,26 +351,6 @@ struct kmem_cache *__kmem_cache_create_args(const char *name,
 }
 EXPORT_SYMBOL(__kmem_cache_create_args);
 
-static struct kmem_cache *
-do_kmem_cache_create_usercopy(const char *name,
-                 unsigned int size, unsigned int freeptr_offset,
-                 unsigned int align, slab_flags_t flags,
-                 unsigned int useroffset, unsigned int usersize,
-                 void (*ctor)(void *))
-{
-	struct kmem_cache_args kmem_args = {
-		.align			= align,
-		.use_freeptr_offset	= freeptr_offset != UINT_MAX,
-		.freeptr_offset		= freeptr_offset,
-		.useroffset		= useroffset,
-		.usersize		= usersize,
-		.ctor			= ctor,
-	};
-
-	return __kmem_cache_create_args(name, size, &kmem_args, flags);
-}
-
-
 /**
  * kmem_cache_create_usercopy - Create a cache with a region suitable
  * for copying to userspace
@@ -405,8 +385,14 @@ kmem_cache_create_usercopy(const char *name, unsigned int size,
 			   unsigned int useroffset, unsigned int usersize,
 			   void (*ctor)(void *))
 {
-	return do_kmem_cache_create_usercopy(name, size, UINT_MAX, align, flags,
-					     useroffset, usersize, ctor);
+	struct kmem_cache_args kmem_args = {
+		.align		= align,
+		.ctor		= ctor,
+		.useroffset	= useroffset,
+		.usersize	= usersize,
+	};
+
+	return __kmem_cache_create_args(name, size, &kmem_args, flags);
 }
 EXPORT_SYMBOL(kmem_cache_create_usercopy);
 

-- 
2.45.2

Re: [PATCH v2 05/15] slab: port kmem_cache_create_usercopy() to struct kmem_cache_args

[Mike Rapoport]

On Tue, Sep 03, 2024 at 04:20:46PM +0200, Christian Brauner wrote:
> Pprt kmem_cache_create_usercopy() to struct kmem_cache_args and remove
> the now unused do_kmem_cache_create_usercopy() helper.
> 
> Signed-off-by: Christian Brauner <brauner@kernel.org>

Reviewed-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

> ---
>  mm/slab_common.c | 30 ++++++++----------------------
>  1 file changed, 8 insertions(+), 22 deletions(-)
> 
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index da62ed30f95d..16c36a946135 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -351,26 +351,6 @@ struct kmem_cache *__kmem_cache_create_args(const char *name,
>  }
>  EXPORT_SYMBOL(__kmem_cache_create_args);
>  
> -static struct kmem_cache *
> -do_kmem_cache_create_usercopy(const char *name,
> -                 unsigned int size, unsigned int freeptr_offset,
> -                 unsigned int align, slab_flags_t flags,
> -                 unsigned int useroffset, unsigned int usersize,
> -                 void (*ctor)(void *))
> -{
> -	struct kmem_cache_args kmem_args = {
> -		.align			= align,
> -		.use_freeptr_offset	= freeptr_offset != UINT_MAX,
> -		.freeptr_offset		= freeptr_offset,
> -		.useroffset		= useroffset,
> -		.usersize		= usersize,
> -		.ctor			= ctor,
> -	};
> -
> -	return __kmem_cache_create_args(name, size, &kmem_args, flags);
> -}
> -
> -
>  /**
>   * kmem_cache_create_usercopy - Create a cache with a region suitable
>   * for copying to userspace
> @@ -405,8 +385,14 @@ kmem_cache_create_usercopy(const char *name, unsigned int size,
>  			   unsigned int useroffset, unsigned int usersize,
>  			   void (*ctor)(void *))
>  {
> -	return do_kmem_cache_create_usercopy(name, size, UINT_MAX, align, flags,
> -					     useroffset, usersize, ctor);
> +	struct kmem_cache_args kmem_args = {
> +		.align		= align,
> +		.ctor		= ctor,
> +		.useroffset	= useroffset,
> +		.usersize	= usersize,
> +	};
> +
> +	return __kmem_cache_create_args(name, size, &kmem_args, flags);
>  }
>  EXPORT_SYMBOL(kmem_cache_create_usercopy);
>  
> 
> -- 
> 2.45.2
> 

-- 
Sincerely yours,
Mike.

Re: [PATCH v2 05/15] slab: port kmem_cache_create_usercopy() to struct kmem_cache_args

[Vlastimil Babka]

On 9/3/24 16:20, Christian Brauner wrote:
> Pprt kmem_cache_create_usercopy() to struct kmem_cache_args and remove

Typo

> the now unused do_kmem_cache_create_usercopy() helper.
> 
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
>  mm/slab_common.c | 30 ++++++++----------------------
>  1 file changed, 8 insertions(+), 22 deletions(-)
> 
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index da62ed30f95d..16c36a946135 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -351,26 +351,6 @@ struct kmem_cache *__kmem_cache_create_args(const char *name,
>  }
>  EXPORT_SYMBOL(__kmem_cache_create_args);
>  
> -static struct kmem_cache *
> -do_kmem_cache_create_usercopy(const char *name,
> -                 unsigned int size, unsigned int freeptr_offset,
> -                 unsigned int align, slab_flags_t flags,
> -                 unsigned int useroffset, unsigned int usersize,
> -                 void (*ctor)(void *))
> -{
> -	struct kmem_cache_args kmem_args = {
> -		.align			= align,
> -		.use_freeptr_offset	= freeptr_offset != UINT_MAX,
> -		.freeptr_offset		= freeptr_offset,
> -		.useroffset		= useroffset,
> -		.usersize		= usersize,
> -		.ctor			= ctor,
> -	};
> -
> -	return __kmem_cache_create_args(name, size, &kmem_args, flags);
> -}
> -
> -
>  /**
>   * kmem_cache_create_usercopy - Create a cache with a region suitable
>   * for copying to userspace
> @@ -405,8 +385,14 @@ kmem_cache_create_usercopy(const char *name, unsigned int size,
>  			   unsigned int useroffset, unsigned int usersize,
>  			   void (*ctor)(void *))
>  {
> -	return do_kmem_cache_create_usercopy(name, size, UINT_MAX, align, flags,
> -					     useroffset, usersize, ctor);
> +	struct kmem_cache_args kmem_args = {
> +		.align		= align,
> +		.ctor		= ctor,
> +		.useroffset	= useroffset,
> +		.usersize	= usersize,
> +	};
> +
> +	return __kmem_cache_create_args(name, size, &kmem_args, flags);
>  }
>  EXPORT_SYMBOL(kmem_cache_create_usercopy);
>  
> 

Re: [PATCH v2 05/15] slab: port kmem_cache_create_usercopy() to struct kmem_cache_args

[Christian Brauner]

On Wed, Sep 04, 2024 at 10:14:37AM GMT, Vlastimil Babka wrote:
> On 9/3/24 16:20, Christian Brauner wrote:
> > Pprt kmem_cache_create_usercopy() to struct kmem_cache_args and remove
> 
> Typo

Fixed on the work.kmem_cache_args branch.

[PATCH v2 06/15] slab: pass struct kmem_cache_args to create_cache()

[Christian Brauner]

Pass struct kmem_cache_args to create_cache() so that we can later
simplify further helpers.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 mm/slab_common.c | 39 +++++++++++++++++++--------------------
 1 file changed, 19 insertions(+), 20 deletions(-)

diff --git a/mm/slab_common.c b/mm/slab_common.c
index 16c36a946135..9baa61c9c670 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -202,22 +202,22 @@ struct kmem_cache *find_mergeable(unsigned int size, unsigned int align,
 }
 
 static struct kmem_cache *create_cache(const char *name,
-		unsigned int object_size, unsigned int freeptr_offset,
-		unsigned int align, slab_flags_t flags,
-		unsigned int useroffset, unsigned int usersize,
-		void (*ctor)(void *))
+				       unsigned int object_size,
+				       struct kmem_cache_args *args,
+				       slab_flags_t flags)
 {
 	struct kmem_cache *s;
 	int err;
 
-	if (WARN_ON(useroffset + usersize > object_size))
-		useroffset = usersize = 0;
+	if (WARN_ON(args->useroffset + args->usersize > object_size))
+		args->useroffset = args->usersize = 0;
 
 	/* If a custom freelist pointer is requested make sure it's sane. */
 	err = -EINVAL;
-	if (freeptr_offset != UINT_MAX &&
-	    (freeptr_offset >= object_size || !(flags & SLAB_TYPESAFE_BY_RCU) ||
-	     !IS_ALIGNED(freeptr_offset, sizeof(freeptr_t))))
+	if (args->use_freeptr_offset &&
+	    (args->freeptr_offset >= object_size ||
+	     !(flags & SLAB_TYPESAFE_BY_RCU) ||
+	     !IS_ALIGNED(args->freeptr_offset, sizeof(freeptr_t))))
 		goto out;
 
 	err = -ENOMEM;
@@ -227,12 +227,15 @@ static struct kmem_cache *create_cache(const char *name,
 
 	s->name = name;
 	s->size = s->object_size = object_size;
-	s->rcu_freeptr_offset = freeptr_offset;
-	s->align = align;
-	s->ctor = ctor;
+	if (args->use_freeptr_offset)
+		s->rcu_freeptr_offset = args->freeptr_offset;
+	else
+		s->rcu_freeptr_offset = UINT_MAX;
+	s->align = args->align;
+	s->ctor = args->ctor;
 #ifdef CONFIG_HARDENED_USERCOPY
-	s->useroffset = useroffset;
-	s->usersize = usersize;
+	s->useroffset = args->useroffset;
+	s->usersize = args->usersize;
 #endif
 	err = do_kmem_cache_create(s, flags);
 	if (err)
@@ -265,7 +268,6 @@ struct kmem_cache *__kmem_cache_create_args(const char *name,
 					    slab_flags_t flags)
 {
 	struct kmem_cache *s = NULL;
-	unsigned int freeptr_offset = UINT_MAX;
 	const char *cache_name;
 	int err;
 
@@ -323,11 +325,8 @@ struct kmem_cache *__kmem_cache_create_args(const char *name,
 		goto out_unlock;
 	}
 
-	if (args->use_freeptr_offset)
-		freeptr_offset = args->freeptr_offset;
-	s = create_cache(cache_name, object_size, freeptr_offset,
-			 calculate_alignment(flags, args->align, object_size),
-			 flags, args->useroffset, args->usersize, args->ctor);
+	args->align = calculate_alignment(flags, args->align, object_size);
+	s = create_cache(cache_name, object_size, args, flags);
 	if (IS_ERR(s)) {
 		err = PTR_ERR(s);
 		kfree_const(cache_name);

-- 
2.45.2

Re: [PATCH v2 06/15] slab: pass struct kmem_cache_args to create_cache()

[Mike Rapoport]

On Tue, Sep 03, 2024 at 04:20:47PM +0200, Christian Brauner wrote:
> Pass struct kmem_cache_args to create_cache() so that we can later
> simplify further helpers.
> 
> Signed-off-by: Christian Brauner <brauner@kernel.org>

Reviewed-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

> ---
>  mm/slab_common.c | 39 +++++++++++++++++++--------------------
>  1 file changed, 19 insertions(+), 20 deletions(-)
> 
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 16c36a946135..9baa61c9c670 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -202,22 +202,22 @@ struct kmem_cache *find_mergeable(unsigned int size, unsigned int align,
>  }
>  
>  static struct kmem_cache *create_cache(const char *name,
> -		unsigned int object_size, unsigned int freeptr_offset,
> -		unsigned int align, slab_flags_t flags,
> -		unsigned int useroffset, unsigned int usersize,
> -		void (*ctor)(void *))
> +				       unsigned int object_size,
> +				       struct kmem_cache_args *args,
> +				       slab_flags_t flags)
>  {
>  	struct kmem_cache *s;
>  	int err;
>  
> -	if (WARN_ON(useroffset + usersize > object_size))
> -		useroffset = usersize = 0;
> +	if (WARN_ON(args->useroffset + args->usersize > object_size))
> +		args->useroffset = args->usersize = 0;
>  
>  	/* If a custom freelist pointer is requested make sure it's sane. */
>  	err = -EINVAL;
> -	if (freeptr_offset != UINT_MAX &&
> -	    (freeptr_offset >= object_size || !(flags & SLAB_TYPESAFE_BY_RCU) ||
> -	     !IS_ALIGNED(freeptr_offset, sizeof(freeptr_t))))
> +	if (args->use_freeptr_offset &&
> +	    (args->freeptr_offset >= object_size ||
> +	     !(flags & SLAB_TYPESAFE_BY_RCU) ||
> +	     !IS_ALIGNED(args->freeptr_offset, sizeof(freeptr_t))))
>  		goto out;
>  
>  	err = -ENOMEM;
> @@ -227,12 +227,15 @@ static struct kmem_cache *create_cache(const char *name,
>  
>  	s->name = name;
>  	s->size = s->object_size = object_size;
> -	s->rcu_freeptr_offset = freeptr_offset;
> -	s->align = align;
> -	s->ctor = ctor;
> +	if (args->use_freeptr_offset)
> +		s->rcu_freeptr_offset = args->freeptr_offset;
> +	else
> +		s->rcu_freeptr_offset = UINT_MAX;
> +	s->align = args->align;
> +	s->ctor = args->ctor;
>  #ifdef CONFIG_HARDENED_USERCOPY
> -	s->useroffset = useroffset;
> -	s->usersize = usersize;
> +	s->useroffset = args->useroffset;
> +	s->usersize = args->usersize;
>  #endif
>  	err = do_kmem_cache_create(s, flags);
>  	if (err)
> @@ -265,7 +268,6 @@ struct kmem_cache *__kmem_cache_create_args(const char *name,
>  					    slab_flags_t flags)
>  {
>  	struct kmem_cache *s = NULL;
> -	unsigned int freeptr_offset = UINT_MAX;
>  	const char *cache_name;
>  	int err;
>  
> @@ -323,11 +325,8 @@ struct kmem_cache *__kmem_cache_create_args(const char *name,
>  		goto out_unlock;
>  	}
>  
> -	if (args->use_freeptr_offset)
> -		freeptr_offset = args->freeptr_offset;
> -	s = create_cache(cache_name, object_size, freeptr_offset,
> -			 calculate_alignment(flags, args->align, object_size),
> -			 flags, args->useroffset, args->usersize, args->ctor);
> +	args->align = calculate_alignment(flags, args->align, object_size);
> +	s = create_cache(cache_name, object_size, args, flags);
>  	if (IS_ERR(s)) {
>  		err = PTR_ERR(s);
>  		kfree_const(cache_name);
> 
> -- 
> 2.45.2
> 

-- 
Sincerely yours,
Mike.

[PATCH v2 07/15] slub: pull kmem_cache_open() into do_kmem_cache_create()

[Christian Brauner]

do_kmem_cache_create() is the only caller and we're going to pass down
struct kmem_cache_args in a follow-up patch.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 mm/slub.c | 132 +++++++++++++++++++++++++++++---------------------------------
 1 file changed, 62 insertions(+), 70 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 23d9d783ff26..30f4ca6335c7 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -5290,65 +5290,6 @@ static int calculate_sizes(struct kmem_cache *s)
 	return !!oo_objects(s->oo);
 }
 
-static int kmem_cache_open(struct kmem_cache *s, slab_flags_t flags)
-{
-	s->flags = kmem_cache_flags(flags, s->name);
-#ifdef CONFIG_SLAB_FREELIST_HARDENED
-	s->random = get_random_long();
-#endif
-
-	if (!calculate_sizes(s))
-		goto error;
-	if (disable_higher_order_debug) {
-		/*
-		 * Disable debugging flags that store metadata if the min slab
-		 * order increased.
-		 */
-		if (get_order(s->size) > get_order(s->object_size)) {
-			s->flags &= ~DEBUG_METADATA_FLAGS;
-			s->offset = 0;
-			if (!calculate_sizes(s))
-				goto error;
-		}
-	}
-
-#ifdef system_has_freelist_aba
-	if (system_has_freelist_aba() && !(s->flags & SLAB_NO_CMPXCHG)) {
-		/* Enable fast mode */
-		s->flags |= __CMPXCHG_DOUBLE;
-	}
-#endif
-
-	/*
-	 * The larger the object size is, the more slabs we want on the partial
-	 * list to avoid pounding the page allocator excessively.
-	 */
-	s->min_partial = min_t(unsigned long, MAX_PARTIAL, ilog2(s->size) / 2);
-	s->min_partial = max_t(unsigned long, MIN_PARTIAL, s->min_partial);
-
-	set_cpu_partial(s);
-
-#ifdef CONFIG_NUMA
-	s->remote_node_defrag_ratio = 1000;
-#endif
-
-	/* Initialize the pre-computed randomized freelist if slab is up */
-	if (slab_state >= UP) {
-		if (init_cache_random_seq(s))
-			goto error;
-	}
-
-	if (!init_kmem_cache_nodes(s))
-		goto error;
-
-	if (alloc_kmem_cache_cpus(s))
-		return 0;
-
-error:
-	__kmem_cache_release(s);
-	return -EINVAL;
-}
-
 static void list_slab_objects(struct kmem_cache *s, struct slab *slab,
 			      const char *text)
 {
@@ -5904,26 +5845,77 @@ __kmem_cache_alias(const char *name, unsigned int size, unsigned int align,
 
 int do_kmem_cache_create(struct kmem_cache *s, slab_flags_t flags)
 {
-	int err;
+	int err = -EINVAL;
 
-	err = kmem_cache_open(s, flags);
-	if (err)
-		return err;
+	s->flags = kmem_cache_flags(flags, s->name);
+#ifdef CONFIG_SLAB_FREELIST_HARDENED
+	s->random = get_random_long();
+#endif
+
+	if (!calculate_sizes(s))
+		goto out;
+	if (disable_higher_order_debug) {
+		/*
+		 * Disable debugging flags that store metadata if the min slab
+		 * order increased.
+		 */
+		if (get_order(s->size) > get_order(s->object_size)) {
+			s->flags &= ~DEBUG_METADATA_FLAGS;
+			s->offset = 0;
+			if (!calculate_sizes(s))
+				goto out;
+		}
+	}
+
+#ifdef system_has_freelist_aba
+	if (system_has_freelist_aba() && !(s->flags & SLAB_NO_CMPXCHG)) {
+		/* Enable fast mode */
+		s->flags |= __CMPXCHG_DOUBLE;
+	}
+#endif
+
+	/*
+	 * The larger the object size is, the more slabs we want on the partial
+	 * list to avoid pounding the page allocator excessively.
+	 */
+	s->min_partial = min_t(unsigned long, MAX_PARTIAL, ilog2(s->size) / 2);
+	s->min_partial = max_t(unsigned long, MIN_PARTIAL, s->min_partial);
+
+	set_cpu_partial(s);
+
+#ifdef CONFIG_NUMA
+	s->remote_node_defrag_ratio = 1000;
+#endif
+
+	/* Initialize the pre-computed randomized freelist if slab is up */
+	if (slab_state >= UP) {
+		if (init_cache_random_seq(s))
+			goto out;
+	}
+
+	if (!init_kmem_cache_nodes(s))
+		goto out;
+
+	if (!alloc_kmem_cache_cpus(s))
+		goto out;
 
 	/* Mutex is not taken during early boot */
-	if (slab_state <= UP)
-		return 0;
+	if (slab_state <= UP) {
+		err = 0;
+		goto out;
+	}
 
 	err = sysfs_slab_add(s);
-	if (err) {
-		__kmem_cache_release(s);
-		return err;
-	}
+	if (err)
+		goto out;
 
 	if (s->flags & SLAB_STORE_USER)
 		debugfs_slab_add(s);
 
-	return 0;
+out:
+	if (err)
+		__kmem_cache_release(s);
+	return err;
 }
 
 #ifdef SLAB_SUPPORTS_SYSFS

-- 
2.45.2

Re: [PATCH v2 07/15] slub: pull kmem_cache_open() into do_kmem_cache_create()

[Mike Rapoport]

On Tue, Sep 03, 2024 at 04:20:48PM +0200, Christian Brauner wrote:
> do_kmem_cache_create() is the only caller and we're going to pass down
> struct kmem_cache_args in a follow-up patch.
> 
> Signed-off-by: Christian Brauner <brauner@kernel.org>

Error handling in kmem_cache_open begs for improvement, but that's not
related to this patch.

Reviewed-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

> ---
>  mm/slub.c | 132 +++++++++++++++++++++++++++++---------------------------------
>  1 file changed, 62 insertions(+), 70 deletions(-)
> 
> diff --git a/mm/slub.c b/mm/slub.c
> index 23d9d783ff26..30f4ca6335c7 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -5290,65 +5290,6 @@ static int calculate_sizes(struct kmem_cache *s)
>  	return !!oo_objects(s->oo);
>  }
>  
> -static int kmem_cache_open(struct kmem_cache *s, slab_flags_t flags)
> -{
> -	s->flags = kmem_cache_flags(flags, s->name);
> -#ifdef CONFIG_SLAB_FREELIST_HARDENED
> -	s->random = get_random_long();
> -#endif
> -
> -	if (!calculate_sizes(s))
> -		goto error;
> -	if (disable_higher_order_debug) {
> -		/*
> -		 * Disable debugging flags that store metadata if the min slab
> -		 * order increased.
> -		 */
> -		if (get_order(s->size) > get_order(s->object_size)) {
> -			s->flags &= ~DEBUG_METADATA_FLAGS;
> -			s->offset = 0;
> -			if (!calculate_sizes(s))
> -				goto error;
> -		}
> -	}
> -
> -#ifdef system_has_freelist_aba
> -	if (system_has_freelist_aba() && !(s->flags & SLAB_NO_CMPXCHG)) {
> -		/* Enable fast mode */
> -		s->flags |= __CMPXCHG_DOUBLE;
> -	}
> -#endif
> -
> -	/*
> -	 * The larger the object size is, the more slabs we want on the partial
> -	 * list to avoid pounding the page allocator excessively.
> -	 */
> -	s->min_partial = min_t(unsigned long, MAX_PARTIAL, ilog2(s->size) / 2);
> -	s->min_partial = max_t(unsigned long, MIN_PARTIAL, s->min_partial);
> -
> -	set_cpu_partial(s);
> -
> -#ifdef CONFIG_NUMA
> -	s->remote_node_defrag_ratio = 1000;
> -#endif
> -
> -	/* Initialize the pre-computed randomized freelist if slab is up */
> -	if (slab_state >= UP) {
> -		if (init_cache_random_seq(s))
> -			goto error;
> -	}
> -
> -	if (!init_kmem_cache_nodes(s))
> -		goto error;
> -
> -	if (alloc_kmem_cache_cpus(s))
> -		return 0;
> -
> -error:
> -	__kmem_cache_release(s);
> -	return -EINVAL;
> -}
> -
>  static void list_slab_objects(struct kmem_cache *s, struct slab *slab,
>  			      const char *text)
>  {
> @@ -5904,26 +5845,77 @@ __kmem_cache_alias(const char *name, unsigned int size, unsigned int align,
>  
>  int do_kmem_cache_create(struct kmem_cache *s, slab_flags_t flags)
>  {
> -	int err;
> +	int err = -EINVAL;
>  
> -	err = kmem_cache_open(s, flags);
> -	if (err)
> -		return err;
> +	s->flags = kmem_cache_flags(flags, s->name);
> +#ifdef CONFIG_SLAB_FREELIST_HARDENED
> +	s->random = get_random_long();
> +#endif
> +
> +	if (!calculate_sizes(s))
> +		goto out;
> +	if (disable_higher_order_debug) {
> +		/*
> +		 * Disable debugging flags that store metadata if the min slab
> +		 * order increased.
> +		 */
> +		if (get_order(s->size) > get_order(s->object_size)) {
> +			s->flags &= ~DEBUG_METADATA_FLAGS;
> +			s->offset = 0;
> +			if (!calculate_sizes(s))
> +				goto out;
> +		}
> +	}
> +
> +#ifdef system_has_freelist_aba
> +	if (system_has_freelist_aba() && !(s->flags & SLAB_NO_CMPXCHG)) {
> +		/* Enable fast mode */
> +		s->flags |= __CMPXCHG_DOUBLE;
> +	}
> +#endif
> +
> +	/*
> +	 * The larger the object size is, the more slabs we want on the partial
> +	 * list to avoid pounding the page allocator excessively.
> +	 */
> +	s->min_partial = min_t(unsigned long, MAX_PARTIAL, ilog2(s->size) / 2);
> +	s->min_partial = max_t(unsigned long, MIN_PARTIAL, s->min_partial);
> +
> +	set_cpu_partial(s);
> +
> +#ifdef CONFIG_NUMA
> +	s->remote_node_defrag_ratio = 1000;
> +#endif
> +
> +	/* Initialize the pre-computed randomized freelist if slab is up */
> +	if (slab_state >= UP) {
> +		if (init_cache_random_seq(s))
> +			goto out;
> +	}
> +
> +	if (!init_kmem_cache_nodes(s))
> +		goto out;
> +
> +	if (!alloc_kmem_cache_cpus(s))
> +		goto out;
>  
>  	/* Mutex is not taken during early boot */
> -	if (slab_state <= UP)
> -		return 0;
> +	if (slab_state <= UP) {
> +		err = 0;
> +		goto out;
> +	}
>  
>  	err = sysfs_slab_add(s);
> -	if (err) {
> -		__kmem_cache_release(s);
> -		return err;
> -	}
> +	if (err)
> +		goto out;
>  
>  	if (s->flags & SLAB_STORE_USER)
>  		debugfs_slab_add(s);
>  
> -	return 0;
> +out:
> +	if (err)
> +		__kmem_cache_release(s);
> +	return err;
>  }
>  
>  #ifdef SLAB_SUPPORTS_SYSFS
> 
> -- 
> 2.45.2
> 

-- 
Sincerely yours,
Mike.

[PATCH v2 08/15] slab: pass struct kmem_cache_args to do_kmem_cache_create()

[Christian Brauner]

and initialize most things in do_kmem_cache_create(). In a follow-up
patch we'll remove rcu_freeptr_offset from struct kmem_cache.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 mm/slab.h        |  4 +++-
 mm/slab_common.c | 27 ++++++---------------------
 mm/slub.c        | 17 ++++++++++++++++-
 3 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/mm/slab.h b/mm/slab.h
index 684bb48c4f39..c7a4e0fc3cf1 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -424,7 +424,9 @@ kmalloc_slab(size_t size, kmem_buckets *b, gfp_t flags, unsigned long caller)
 gfp_t kmalloc_fix_flags(gfp_t flags);
 
 /* Functions provided by the slab allocators */
-int do_kmem_cache_create(struct kmem_cache *, slab_flags_t flags);
+int do_kmem_cache_create(struct kmem_cache *s, const char *name,
+			 unsigned int size, struct kmem_cache_args *args,
+			 slab_flags_t flags);
 
 void __init kmem_cache_init(void);
 extern void create_boot_cache(struct kmem_cache *, const char *name,
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 9baa61c9c670..19ae3dd6e36f 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -224,20 +224,7 @@ static struct kmem_cache *create_cache(const char *name,
 	s = kmem_cache_zalloc(kmem_cache, GFP_KERNEL);
 	if (!s)
 		goto out;
-
-	s->name = name;
-	s->size = s->object_size = object_size;
-	if (args->use_freeptr_offset)
-		s->rcu_freeptr_offset = args->freeptr_offset;
-	else
-		s->rcu_freeptr_offset = UINT_MAX;
-	s->align = args->align;
-	s->ctor = args->ctor;
-#ifdef CONFIG_HARDENED_USERCOPY
-	s->useroffset = args->useroffset;
-	s->usersize = args->usersize;
-#endif
-	err = do_kmem_cache_create(s, flags);
+	err = do_kmem_cache_create(s, name, object_size, args, flags);
 	if (err)
 		goto out_free_cache;
 
@@ -788,9 +775,7 @@ void __init create_boot_cache(struct kmem_cache *s, const char *name,
 {
 	int err;
 	unsigned int align = ARCH_KMALLOC_MINALIGN;
-
-	s->name = name;
-	s->size = s->object_size = size;
+	struct kmem_cache_args kmem_args = {};
 
 	/*
 	 * kmalloc caches guarantee alignment of at least the largest
@@ -799,14 +784,14 @@ void __init create_boot_cache(struct kmem_cache *s, const char *name,
 	 */
 	if (flags & SLAB_KMALLOC)
 		align = max(align, 1U << (ffs(size) - 1));
-	s->align = calculate_alignment(flags, align, size);
+	kmem_args.align = calculate_alignment(flags, align, size);
 
 #ifdef CONFIG_HARDENED_USERCOPY
-	s->useroffset = useroffset;
-	s->usersize = usersize;
+	kmem_args.useroffset = useroffset;
+	kmem_args.usersize = usersize;
 #endif
 
-	err = do_kmem_cache_create(s, flags);
+	err = do_kmem_cache_create(s, name, size, &kmem_args, flags);
 
 	if (err)
 		panic("Creation of kmalloc slab %s size=%u failed. Reason %d\n",
diff --git a/mm/slub.c b/mm/slub.c
index 30f4ca6335c7..4719b60215b8 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -5843,14 +5843,29 @@ __kmem_cache_alias(const char *name, unsigned int size, unsigned int align,
 	return s;
 }
 
-int do_kmem_cache_create(struct kmem_cache *s, slab_flags_t flags)
+int do_kmem_cache_create(struct kmem_cache *s, const char *name,
+			 unsigned int size, struct kmem_cache_args *args,
+			 slab_flags_t flags)
 {
 	int err = -EINVAL;
 
+	s->name = name;
+	s->size = s->object_size = size;
+
 	s->flags = kmem_cache_flags(flags, s->name);
 #ifdef CONFIG_SLAB_FREELIST_HARDENED
 	s->random = get_random_long();
 #endif
+	if (args->use_freeptr_offset)
+		s->rcu_freeptr_offset = args->freeptr_offset;
+	else
+		s->rcu_freeptr_offset = UINT_MAX;
+	s->align = args->align;
+	s->ctor = args->ctor;
+#ifdef CONFIG_HARDENED_USERCOPY
+	s->useroffset = args->useroffset;
+	s->usersize = args->usersize;
+#endif
 
 	if (!calculate_sizes(s))
 		goto out;

-- 
2.45.2

Re: [PATCH v2 08/15] slab: pass struct kmem_cache_args to do_kmem_cache_create()

[Mike Rapoport]

On Tue, Sep 03, 2024 at 04:20:49PM +0200, Christian Brauner wrote:
> and initialize most things in do_kmem_cache_create(). In a follow-up
> patch we'll remove rcu_freeptr_offset from struct kmem_cache.
> 
> Signed-off-by: Christian Brauner <brauner@kernel.org>

Reviewed-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

> ---
>  mm/slab.h        |  4 +++-
>  mm/slab_common.c | 27 ++++++---------------------
>  mm/slub.c        | 17 ++++++++++++++++-
>  3 files changed, 25 insertions(+), 23 deletions(-)
> 
> diff --git a/mm/slab.h b/mm/slab.h
> index 684bb48c4f39..c7a4e0fc3cf1 100644
> --- a/mm/slab.h
> +++ b/mm/slab.h
> @@ -424,7 +424,9 @@ kmalloc_slab(size_t size, kmem_buckets *b, gfp_t flags, unsigned long caller)
>  gfp_t kmalloc_fix_flags(gfp_t flags);
>  
>  /* Functions provided by the slab allocators */
> -int do_kmem_cache_create(struct kmem_cache *, slab_flags_t flags);
> +int do_kmem_cache_create(struct kmem_cache *s, const char *name,
> +			 unsigned int size, struct kmem_cache_args *args,
> +			 slab_flags_t flags);
>  
>  void __init kmem_cache_init(void);
>  extern void create_boot_cache(struct kmem_cache *, const char *name,
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 9baa61c9c670..19ae3dd6e36f 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -224,20 +224,7 @@ static struct kmem_cache *create_cache(const char *name,
>  	s = kmem_cache_zalloc(kmem_cache, GFP_KERNEL);
>  	if (!s)
>  		goto out;
> -
> -	s->name = name;
> -	s->size = s->object_size = object_size;
> -	if (args->use_freeptr_offset)
> -		s->rcu_freeptr_offset = args->freeptr_offset;
> -	else
> -		s->rcu_freeptr_offset = UINT_MAX;
> -	s->align = args->align;
> -	s->ctor = args->ctor;
> -#ifdef CONFIG_HARDENED_USERCOPY
> -	s->useroffset = args->useroffset;
> -	s->usersize = args->usersize;
> -#endif
> -	err = do_kmem_cache_create(s, flags);
> +	err = do_kmem_cache_create(s, name, object_size, args, flags);
>  	if (err)
>  		goto out_free_cache;
>  
> @@ -788,9 +775,7 @@ void __init create_boot_cache(struct kmem_cache *s, const char *name,
>  {
>  	int err;
>  	unsigned int align = ARCH_KMALLOC_MINALIGN;
> -
> -	s->name = name;
> -	s->size = s->object_size = size;
> +	struct kmem_cache_args kmem_args = {};
>  
>  	/*
>  	 * kmalloc caches guarantee alignment of at least the largest
> @@ -799,14 +784,14 @@ void __init create_boot_cache(struct kmem_cache *s, const char *name,
>  	 */
>  	if (flags & SLAB_KMALLOC)
>  		align = max(align, 1U << (ffs(size) - 1));
> -	s->align = calculate_alignment(flags, align, size);
> +	kmem_args.align = calculate_alignment(flags, align, size);
>  
>  #ifdef CONFIG_HARDENED_USERCOPY
> -	s->useroffset = useroffset;
> -	s->usersize = usersize;
> +	kmem_args.useroffset = useroffset;
> +	kmem_args.usersize = usersize;
>  #endif
>  
> -	err = do_kmem_cache_create(s, flags);
> +	err = do_kmem_cache_create(s, name, size, &kmem_args, flags);
>  
>  	if (err)
>  		panic("Creation of kmalloc slab %s size=%u failed. Reason %d\n",
> diff --git a/mm/slub.c b/mm/slub.c
> index 30f4ca6335c7..4719b60215b8 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -5843,14 +5843,29 @@ __kmem_cache_alias(const char *name, unsigned int size, unsigned int align,
>  	return s;
>  }
>  
> -int do_kmem_cache_create(struct kmem_cache *s, slab_flags_t flags)
> +int do_kmem_cache_create(struct kmem_cache *s, const char *name,
> +			 unsigned int size, struct kmem_cache_args *args,
> +			 slab_flags_t flags)
>  {
>  	int err = -EINVAL;
>  
> +	s->name = name;
> +	s->size = s->object_size = size;
> +
>  	s->flags = kmem_cache_flags(flags, s->name);
>  #ifdef CONFIG_SLAB_FREELIST_HARDENED
>  	s->random = get_random_long();
>  #endif
> +	if (args->use_freeptr_offset)
> +		s->rcu_freeptr_offset = args->freeptr_offset;
> +	else
> +		s->rcu_freeptr_offset = UINT_MAX;
> +	s->align = args->align;
> +	s->ctor = args->ctor;
> +#ifdef CONFIG_HARDENED_USERCOPY
> +	s->useroffset = args->useroffset;
> +	s->usersize = args->usersize;
> +#endif
>  
>  	if (!calculate_sizes(s))
>  		goto out;
> 
> -- 
> 2.45.2
> 

-- 
Sincerely yours,
Mike.

[PATCH v2 09/15] sl*b: remove rcu_freeptr_offset from struct kmem_cache

[Christian Brauner]

Now that we pass down struct kmem_cache_args to calculate_sizes() we
don't need it anymore.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 mm/slab.h |  2 --
 mm/slub.c | 25 +++++++------------------
 2 files changed, 7 insertions(+), 20 deletions(-)

diff --git a/mm/slab.h b/mm/slab.h
index c7a4e0fc3cf1..36ac38e21fcb 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -261,8 +261,6 @@ struct kmem_cache {
 	unsigned int object_size;	/* Object size without metadata */
 	struct reciprocal_value reciprocal_size;
 	unsigned int offset;		/* Free pointer offset */
-	/* Specific free pointer requested (if not UINT_MAX) */
-	unsigned int rcu_freeptr_offset;
 #ifdef CONFIG_SLUB_CPU_PARTIAL
 	/* Number of per cpu partial objects to keep around */
 	unsigned int cpu_partial;
diff --git a/mm/slub.c b/mm/slub.c
index 4719b60215b8..a23c7036cd61 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -3916,8 +3916,7 @@ static void *__slab_alloc_node(struct kmem_cache *s,
  * If the object has been wiped upon free, make sure it's fully initialized by
  * zeroing out freelist pointer.
  *
- * Note that we also wipe custom freelist pointers specified via
- * s->rcu_freeptr_offset.
+ * Note that we also wipe custom freelist pointers.
  */
 static __always_inline void maybe_wipe_obj_freeptr(struct kmem_cache *s,
 						   void *obj)
@@ -5141,17 +5140,11 @@ static void set_cpu_partial(struct kmem_cache *s)
 #endif
 }
 
-/* Was a valid freeptr offset requested? */
-static inline bool has_freeptr_offset(const struct kmem_cache *s)
-{
-	return s->rcu_freeptr_offset != UINT_MAX;
-}
-
 /*
  * calculate_sizes() determines the order and the distribution of data within
  * a slab object.
  */
-static int calculate_sizes(struct kmem_cache *s)
+static int calculate_sizes(struct kmem_cache_args *args, struct kmem_cache *s)
 {
 	slab_flags_t flags = s->flags;
 	unsigned int size = s->object_size;
@@ -5192,7 +5185,7 @@ static int calculate_sizes(struct kmem_cache *s)
 	 */
 	s->inuse = size;
 
-	if (((flags & SLAB_TYPESAFE_BY_RCU) && !has_freeptr_offset(s)) ||
+	if (((flags & SLAB_TYPESAFE_BY_RCU) && !args->use_freeptr_offset) ||
 	    (flags & SLAB_POISON) || s->ctor ||
 	    ((flags & SLAB_RED_ZONE) &&
 	     (s->object_size < sizeof(void *) || slub_debug_orig_size(s)))) {
@@ -5214,8 +5207,8 @@ static int calculate_sizes(struct kmem_cache *s)
 		 */
 		s->offset = size;
 		size += sizeof(void *);
-	} else if ((flags & SLAB_TYPESAFE_BY_RCU) && has_freeptr_offset(s)) {
-		s->offset = s->rcu_freeptr_offset;
+	} else if ((flags & SLAB_TYPESAFE_BY_RCU) && args->use_freeptr_offset) {
+		s->offset = args->freeptr_offset;
 	} else {
 		/*
 		 * Store freelist pointer near middle of object to keep
@@ -5856,10 +5849,6 @@ int do_kmem_cache_create(struct kmem_cache *s, const char *name,
 #ifdef CONFIG_SLAB_FREELIST_HARDENED
 	s->random = get_random_long();
 #endif
-	if (args->use_freeptr_offset)
-		s->rcu_freeptr_offset = args->freeptr_offset;
-	else
-		s->rcu_freeptr_offset = UINT_MAX;
 	s->align = args->align;
 	s->ctor = args->ctor;
 #ifdef CONFIG_HARDENED_USERCOPY
@@ -5867,7 +5856,7 @@ int do_kmem_cache_create(struct kmem_cache *s, const char *name,
 	s->usersize = args->usersize;
 #endif
 
-	if (!calculate_sizes(s))
+	if (!calculate_sizes(args, s))
 		goto out;
 	if (disable_higher_order_debug) {
 		/*
@@ -5877,7 +5866,7 @@ int do_kmem_cache_create(struct kmem_cache *s, const char *name,
 		if (get_order(s->size) > get_order(s->object_size)) {
 			s->flags &= ~DEBUG_METADATA_FLAGS;
 			s->offset = 0;
-			if (!calculate_sizes(s))
+			if (!calculate_sizes(args, s))
 				goto out;
 		}
 	}

-- 
2.45.2

Re: [PATCH v2 09/15] sl*b: remove rcu_freeptr_offset from struct kmem_cache

[Mike Rapoport]

On Tue, Sep 03, 2024 at 04:20:50PM +0200, Christian Brauner wrote:
> Now that we pass down struct kmem_cache_args to calculate_sizes() we
> don't need it anymore.
> 
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
>  mm/slab.h |  2 --
>  mm/slub.c | 25 +++++++------------------
>  2 files changed, 7 insertions(+), 20 deletions(-)
> 
> diff --git a/mm/slab.h b/mm/slab.h
> index c7a4e0fc3cf1..36ac38e21fcb 100644
> --- a/mm/slab.h
> +++ b/mm/slab.h
> @@ -261,8 +261,6 @@ struct kmem_cache {
>  	unsigned int object_size;	/* Object size without metadata */
>  	struct reciprocal_value reciprocal_size;
>  	unsigned int offset;		/* Free pointer offset */
> -	/* Specific free pointer requested (if not UINT_MAX) */
> -	unsigned int rcu_freeptr_offset;
>  #ifdef CONFIG_SLUB_CPU_PARTIAL
>  	/* Number of per cpu partial objects to keep around */
>  	unsigned int cpu_partial;
> diff --git a/mm/slub.c b/mm/slub.c
> index 4719b60215b8..a23c7036cd61 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -3916,8 +3916,7 @@ static void *__slab_alloc_node(struct kmem_cache *s,
>   * If the object has been wiped upon free, make sure it's fully initialized by
>   * zeroing out freelist pointer.
>   *
> - * Note that we also wipe custom freelist pointers specified via
> - * s->rcu_freeptr_offset.
> + * Note that we also wipe custom freelist pointers.
>   */
>  static __always_inline void maybe_wipe_obj_freeptr(struct kmem_cache *s,
>  						   void *obj)
> @@ -5141,17 +5140,11 @@ static void set_cpu_partial(struct kmem_cache *s)
>  #endif
>  }
>  
> -/* Was a valid freeptr offset requested? */
> -static inline bool has_freeptr_offset(const struct kmem_cache *s)
> -{
> -	return s->rcu_freeptr_offset != UINT_MAX;
> -}
> -
>  /*
>   * calculate_sizes() determines the order and the distribution of data within
>   * a slab object.
>   */
> -static int calculate_sizes(struct kmem_cache *s)
> +static int calculate_sizes(struct kmem_cache_args *args, struct kmem_cache *s)

I'd keep kmem_cache the first argument.
As for the rest

Reviewed-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

>  {
>  	slab_flags_t flags = s->flags;
>  	unsigned int size = s->object_size;
> @@ -5192,7 +5185,7 @@ static int calculate_sizes(struct kmem_cache *s)
>  	 */
>  	s->inuse = size;
>  
> -	if (((flags & SLAB_TYPESAFE_BY_RCU) && !has_freeptr_offset(s)) ||
> +	if (((flags & SLAB_TYPESAFE_BY_RCU) && !args->use_freeptr_offset) ||
>  	    (flags & SLAB_POISON) || s->ctor ||
>  	    ((flags & SLAB_RED_ZONE) &&
>  	     (s->object_size < sizeof(void *) || slub_debug_orig_size(s)))) {
> @@ -5214,8 +5207,8 @@ static int calculate_sizes(struct kmem_cache *s)
>  		 */
>  		s->offset = size;
>  		size += sizeof(void *);
> -	} else if ((flags & SLAB_TYPESAFE_BY_RCU) && has_freeptr_offset(s)) {
> -		s->offset = s->rcu_freeptr_offset;
> +	} else if ((flags & SLAB_TYPESAFE_BY_RCU) && args->use_freeptr_offset) {
> +		s->offset = args->freeptr_offset;
>  	} else {
>  		/*
>  		 * Store freelist pointer near middle of object to keep
> @@ -5856,10 +5849,6 @@ int do_kmem_cache_create(struct kmem_cache *s, const char *name,
>  #ifdef CONFIG_SLAB_FREELIST_HARDENED
>  	s->random = get_random_long();
>  #endif
> -	if (args->use_freeptr_offset)
> -		s->rcu_freeptr_offset = args->freeptr_offset;
> -	else
> -		s->rcu_freeptr_offset = UINT_MAX;
>  	s->align = args->align;
>  	s->ctor = args->ctor;
>  #ifdef CONFIG_HARDENED_USERCOPY
> @@ -5867,7 +5856,7 @@ int do_kmem_cache_create(struct kmem_cache *s, const char *name,
>  	s->usersize = args->usersize;
>  #endif
>  
> -	if (!calculate_sizes(s))
> +	if (!calculate_sizes(args, s))
>  		goto out;
>  	if (disable_higher_order_debug) {
>  		/*
> @@ -5877,7 +5866,7 @@ int do_kmem_cache_create(struct kmem_cache *s, const char *name,
>  		if (get_order(s->size) > get_order(s->object_size)) {
>  			s->flags &= ~DEBUG_METADATA_FLAGS;
>  			s->offset = 0;
> -			if (!calculate_sizes(s))
> +			if (!calculate_sizes(args, s))
>  				goto out;
>  		}
>  	}
> 
> -- 
> 2.45.2
> 

-- 
Sincerely yours,
Mike.

Re: [PATCH v2 09/15] sl*b: remove rcu_freeptr_offset from struct kmem_cache

[Vlastimil Babka]

On 9/3/24 16:20, Christian Brauner wrote:
> Now that we pass down struct kmem_cache_args to calculate_sizes() we
> don't need it anymore.

Nit: that sounds like a previous patch did the "pass down" part? Fine to do
both at once but maybe adjust description that we do both here?

> 
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
>  mm/slab.h |  2 --
>  mm/slub.c | 25 +++++++------------------
>  2 files changed, 7 insertions(+), 20 deletions(-)
> 
> diff --git a/mm/slab.h b/mm/slab.h
> index c7a4e0fc3cf1..36ac38e21fcb 100644
> --- a/mm/slab.h
> +++ b/mm/slab.h
> @@ -261,8 +261,6 @@ struct kmem_cache {
>  	unsigned int object_size;	/* Object size without metadata */
>  	struct reciprocal_value reciprocal_size;
>  	unsigned int offset;		/* Free pointer offset */
> -	/* Specific free pointer requested (if not UINT_MAX) */
> -	unsigned int rcu_freeptr_offset;
>  #ifdef CONFIG_SLUB_CPU_PARTIAL
>  	/* Number of per cpu partial objects to keep around */
>  	unsigned int cpu_partial;
> diff --git a/mm/slub.c b/mm/slub.c
> index 4719b60215b8..a23c7036cd61 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -3916,8 +3916,7 @@ static void *__slab_alloc_node(struct kmem_cache *s,
>   * If the object has been wiped upon free, make sure it's fully initialized by
>   * zeroing out freelist pointer.
>   *
> - * Note that we also wipe custom freelist pointers specified via
> - * s->rcu_freeptr_offset.
> + * Note that we also wipe custom freelist pointers.
>   */
>  static __always_inline void maybe_wipe_obj_freeptr(struct kmem_cache *s,
>  						   void *obj)
> @@ -5141,17 +5140,11 @@ static void set_cpu_partial(struct kmem_cache *s)
>  #endif
>  }
>  
> -/* Was a valid freeptr offset requested? */
> -static inline bool has_freeptr_offset(const struct kmem_cache *s)
> -{
> -	return s->rcu_freeptr_offset != UINT_MAX;
> -}
> -
>  /*
>   * calculate_sizes() determines the order and the distribution of data within
>   * a slab object.
>   */
> -static int calculate_sizes(struct kmem_cache *s)
> +static int calculate_sizes(struct kmem_cache_args *args, struct kmem_cache *s)
>  {
>  	slab_flags_t flags = s->flags;
>  	unsigned int size = s->object_size;
> @@ -5192,7 +5185,7 @@ static int calculate_sizes(struct kmem_cache *s)
>  	 */
>  	s->inuse = size;
>  
> -	if (((flags & SLAB_TYPESAFE_BY_RCU) && !has_freeptr_offset(s)) ||
> +	if (((flags & SLAB_TYPESAFE_BY_RCU) && !args->use_freeptr_offset) ||
>  	    (flags & SLAB_POISON) || s->ctor ||
>  	    ((flags & SLAB_RED_ZONE) &&
>  	     (s->object_size < sizeof(void *) || slub_debug_orig_size(s)))) {
> @@ -5214,8 +5207,8 @@ static int calculate_sizes(struct kmem_cache *s)
>  		 */
>  		s->offset = size;
>  		size += sizeof(void *);
> -	} else if ((flags & SLAB_TYPESAFE_BY_RCU) && has_freeptr_offset(s)) {
> -		s->offset = s->rcu_freeptr_offset;
> +	} else if ((flags & SLAB_TYPESAFE_BY_RCU) && args->use_freeptr_offset) {
> +		s->offset = args->freeptr_offset;
>  	} else {
>  		/*
>  		 * Store freelist pointer near middle of object to keep
> @@ -5856,10 +5849,6 @@ int do_kmem_cache_create(struct kmem_cache *s, const char *name,
>  #ifdef CONFIG_SLAB_FREELIST_HARDENED
>  	s->random = get_random_long();
>  #endif
> -	if (args->use_freeptr_offset)
> -		s->rcu_freeptr_offset = args->freeptr_offset;
> -	else
> -		s->rcu_freeptr_offset = UINT_MAX;
>  	s->align = args->align;
>  	s->ctor = args->ctor;
>  #ifdef CONFIG_HARDENED_USERCOPY
> @@ -5867,7 +5856,7 @@ int do_kmem_cache_create(struct kmem_cache *s, const char *name,
>  	s->usersize = args->usersize;
>  #endif
>  
> -	if (!calculate_sizes(s))
> +	if (!calculate_sizes(args, s))
>  		goto out;
>  	if (disable_higher_order_debug) {
>  		/*
> @@ -5877,7 +5866,7 @@ int do_kmem_cache_create(struct kmem_cache *s, const char *name,
>  		if (get_order(s->size) > get_order(s->object_size)) {
>  			s->flags &= ~DEBUG_METADATA_FLAGS;
>  			s->offset = 0;
> -			if (!calculate_sizes(s))
> +			if (!calculate_sizes(args, s))
>  				goto out;
>  		}
>  	}
> 

Re: [PATCH v2 09/15] sl*b: remove rcu_freeptr_offset from struct kmem_cache

[Christian Brauner]

On Wed, Sep 04, 2024 at 10:16:17AM GMT, Vlastimil Babka wrote:
> On 9/3/24 16:20, Christian Brauner wrote:
> > Now that we pass down struct kmem_cache_args to calculate_sizes() we
> > don't need it anymore.
> 
> Nit: that sounds like a previous patch did the "pass down" part? Fine to do
> both at once but maybe adjust description that we do both here?

Hm, maybe that was misleadingly phrased but my intention was to express
that this patch passes it down and removes the now unneeded member from
struct kmem_cache. I've rephrased this to:

"Pass down struct kmem_cache_args to calculate_sizes() so we can use
args->{use}_freeptr_offset directly. This allows us to remove
->rcu_freeptr_offset from struct kmem_cache."

on the work.kmem_cache_args branch.

[PATCH v2 10/15] slab: port KMEM_CACHE() to struct kmem_cache_args

[Christian Brauner]

Make KMEM_CACHE() use struct kmem_cache_args.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 include/linux/slab.h | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/include/linux/slab.h b/include/linux/slab.h
index 79d8c8bca4a4..d9c2ed5bc02f 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -283,9 +283,12 @@ int kmem_cache_shrink(struct kmem_cache *s);
  * f.e. add ____cacheline_aligned_in_smp to the struct declaration
  * then the objects will be properly aligned in SMP configurations.
  */
-#define KMEM_CACHE(__struct, __flags)					\
-		kmem_cache_create(#__struct, sizeof(struct __struct),	\
-			__alignof__(struct __struct), (__flags), NULL)
+#define KMEM_CACHE(__struct, __flags)                                   \
+	__kmem_cache_create_args(#__struct, sizeof(struct __struct),    \
+			&(struct kmem_cache_args) {			\
+				.align	= __alignof__(struct __struct), \
+				.ctor	= NULL,                         \
+			}, (__flags))
 
 /*
  * To whitelist a single field for copying to/from usercopy, use this

-- 
2.45.2

Re: [PATCH v2 10/15] slab: port KMEM_CACHE() to struct kmem_cache_args

[Mike Rapoport]

On Tue, Sep 03, 2024 at 04:20:51PM +0200, Christian Brauner wrote:
> Make KMEM_CACHE() use struct kmem_cache_args.
> 
> Signed-off-by: Christian Brauner <brauner@kernel.org>

Reviewed-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

> ---
>  include/linux/slab.h | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index 79d8c8bca4a4..d9c2ed5bc02f 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -283,9 +283,12 @@ int kmem_cache_shrink(struct kmem_cache *s);
>   * f.e. add ____cacheline_aligned_in_smp to the struct declaration
>   * then the objects will be properly aligned in SMP configurations.
>   */
> -#define KMEM_CACHE(__struct, __flags)					\
> -		kmem_cache_create(#__struct, sizeof(struct __struct),	\
> -			__alignof__(struct __struct), (__flags), NULL)
> +#define KMEM_CACHE(__struct, __flags)                                   \
> +	__kmem_cache_create_args(#__struct, sizeof(struct __struct),    \
> +			&(struct kmem_cache_args) {			\
> +				.align	= __alignof__(struct __struct), \
> +				.ctor	= NULL,                         \
> +			}, (__flags))
>  
>  /*
>   * To whitelist a single field for copying to/from usercopy, use this
> 
> -- 
> 2.45.2
> 

-- 
Sincerely yours,
Mike.

[PATCH v2 11/15] slab: port KMEM_CACHE_USERCOPY() to struct kmem_cache_args

[Christian Brauner]

Make KMEM_CACHE_USERCOPY() use struct kmem_cache_args.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 include/linux/slab.h | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/include/linux/slab.h b/include/linux/slab.h
index d9c2ed5bc02f..aced16a08700 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -294,12 +294,14 @@ int kmem_cache_shrink(struct kmem_cache *s);
  * To whitelist a single field for copying to/from usercopy, use this
  * macro instead for KMEM_CACHE() above.
  */
-#define KMEM_CACHE_USERCOPY(__struct, __flags, __field)			\
-		kmem_cache_create_usercopy(#__struct,			\
-			sizeof(struct __struct),			\
-			__alignof__(struct __struct), (__flags),	\
-			offsetof(struct __struct, __field),		\
-			sizeof_field(struct __struct, __field), NULL)
+#define KMEM_CACHE_USERCOPY(__struct, __flags, __field)						\
+	__kmem_cache_create_args(#__struct, sizeof(struct __struct),				\
+			&(struct kmem_cache_args) {						\
+				.align		= __alignof__(struct __struct),			\
+				.useroffset	= offsetof(struct __struct, __field),		\
+				.usersize	= sizeof_field(struct __struct, __field),	\
+				.ctor		= NULL,						\
+			}, (__flags))
 
 /*
  * Common kmalloc functions provided by all allocators

-- 
2.45.2

Re: [PATCH v2 11/15] slab: port KMEM_CACHE_USERCOPY() to struct kmem_cache_args

[Mike Rapoport]

On Tue, Sep 03, 2024 at 04:20:52PM +0200, Christian Brauner wrote:
> Make KMEM_CACHE_USERCOPY() use struct kmem_cache_args.
> 
> Signed-off-by: Christian Brauner <brauner@kernel.org>

Reviewed-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

> ---
>  include/linux/slab.h | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index d9c2ed5bc02f..aced16a08700 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -294,12 +294,14 @@ int kmem_cache_shrink(struct kmem_cache *s);
>   * To whitelist a single field for copying to/from usercopy, use this
>   * macro instead for KMEM_CACHE() above.
>   */
> -#define KMEM_CACHE_USERCOPY(__struct, __flags, __field)			\
> -		kmem_cache_create_usercopy(#__struct,			\
> -			sizeof(struct __struct),			\
> -			__alignof__(struct __struct), (__flags),	\
> -			offsetof(struct __struct, __field),		\
> -			sizeof_field(struct __struct, __field), NULL)
> +#define KMEM_CACHE_USERCOPY(__struct, __flags, __field)						\
> +	__kmem_cache_create_args(#__struct, sizeof(struct __struct),				\
> +			&(struct kmem_cache_args) {						\
> +				.align		= __alignof__(struct __struct),			\
> +				.useroffset	= offsetof(struct __struct, __field),		\
> +				.usersize	= sizeof_field(struct __struct, __field),	\
> +				.ctor		= NULL,						\
> +			}, (__flags))
>  
>  /*
>   * Common kmalloc functions provided by all allocators
> 
> -- 
> 2.45.2
> 

-- 
Sincerely yours,
Mike.

[PATCH v2 12/15] slab: create kmem_cache_create() compatibility layer

[Christian Brauner]

Use _Generic() to create a compatibility layer that type switches on the
third argument to either call __kmem_cache_create() or
__kmem_cache_create_args(). This can be kept in place until all callers
have been ported to struct kmem_cache_args.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 include/linux/slab.h | 13 ++++++++++---
 mm/slab_common.c     | 10 +++++-----
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/include/linux/slab.h b/include/linux/slab.h
index aced16a08700..4292d67094c3 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -261,9 +261,10 @@ struct kmem_cache *__kmem_cache_create_args(const char *name,
 					    unsigned int object_size,
 					    struct kmem_cache_args *args,
 					    slab_flags_t flags);
-struct kmem_cache *kmem_cache_create(const char *name, unsigned int size,
-			unsigned int align, slab_flags_t flags,
-			void (*ctor)(void *));
+
+struct kmem_cache *__kmem_cache_create(const char *name, unsigned int size,
+				       unsigned int align, slab_flags_t flags,
+				       void (*ctor)(void *));
 struct kmem_cache *kmem_cache_create_usercopy(const char *name,
 			unsigned int size, unsigned int align,
 			slab_flags_t flags,
@@ -272,6 +273,12 @@ struct kmem_cache *kmem_cache_create_usercopy(const char *name,
 struct kmem_cache *kmem_cache_create_rcu(const char *name, unsigned int size,
 					 unsigned int freeptr_offset,
 					 slab_flags_t flags);
+
+#define kmem_cache_create(__name, __object_size, __args, ...)           \
+	_Generic((__args),                                              \
+		struct kmem_cache_args *: __kmem_cache_create_args,	\
+		default: __kmem_cache_create)(__name, __object_size, __args, __VA_ARGS__)
+
 void kmem_cache_destroy(struct kmem_cache *s);
 int kmem_cache_shrink(struct kmem_cache *s);
 
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 19ae3dd6e36f..418459927670 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -383,7 +383,7 @@ kmem_cache_create_usercopy(const char *name, unsigned int size,
 EXPORT_SYMBOL(kmem_cache_create_usercopy);
 
 /**
- * kmem_cache_create - Create a cache.
+ * __kmem_cache_create - Create a cache.
  * @name: A string which is used in /proc/slabinfo to identify this cache.
  * @size: The size of objects to be created in this cache.
  * @align: The required alignment for the objects.
@@ -407,9 +407,9 @@ EXPORT_SYMBOL(kmem_cache_create_usercopy);
  *
  * Return: a pointer to the cache on success, NULL on failure.
  */
-struct kmem_cache *
-kmem_cache_create(const char *name, unsigned int size, unsigned int align,
-		slab_flags_t flags, void (*ctor)(void *))
+struct kmem_cache *__kmem_cache_create(const char *name, unsigned int size,
+				       unsigned int align, slab_flags_t flags,
+				       void (*ctor)(void *))
 {
 	struct kmem_cache_args kmem_args = {
 		.align	= align,
@@ -418,7 +418,7 @@ kmem_cache_create(const char *name, unsigned int size, unsigned int align,
 
 	return __kmem_cache_create_args(name, size, &kmem_args, flags);
 }
-EXPORT_SYMBOL(kmem_cache_create);
+EXPORT_SYMBOL(__kmem_cache_create);
 
 /**
  * kmem_cache_create_rcu - Create a SLAB_TYPESAFE_BY_RCU cache.

-- 
2.45.2

Re: [PATCH v2 12/15] slab: create kmem_cache_create() compatibility layer

[Mike Rapoport]

On Tue, Sep 03, 2024 at 04:20:53PM +0200, Christian Brauner wrote:
> Use _Generic() to create a compatibility layer that type switches on the
> third argument to either call __kmem_cache_create() or
> __kmem_cache_create_args(). This can be kept in place until all callers
> have been ported to struct kmem_cache_args.
> 
> Signed-off-by: Christian Brauner <brauner@kernel.org>

Reviewed-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

> ---
>  include/linux/slab.h | 13 ++++++++++---
>  mm/slab_common.c     | 10 +++++-----
>  2 files changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index aced16a08700..4292d67094c3 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -261,9 +261,10 @@ struct kmem_cache *__kmem_cache_create_args(const char *name,
>  					    unsigned int object_size,
>  					    struct kmem_cache_args *args,
>  					    slab_flags_t flags);
> -struct kmem_cache *kmem_cache_create(const char *name, unsigned int size,
> -			unsigned int align, slab_flags_t flags,
> -			void (*ctor)(void *));
> +
> +struct kmem_cache *__kmem_cache_create(const char *name, unsigned int size,
> +				       unsigned int align, slab_flags_t flags,
> +				       void (*ctor)(void *));

As I said earlier, this can become _kmem_cache_create and
__kmem_cache_create_args can be __kmem_cache_create from the beginning.

And as a followup cleanup both kmem_cache_create_usercopy() and
kmem_cache_create() can be made static inlines.

>  struct kmem_cache *kmem_cache_create_usercopy(const char *name,
>  			unsigned int size, unsigned int align,
>  			slab_flags_t flags,
> @@ -272,6 +273,12 @@ struct kmem_cache *kmem_cache_create_usercopy(const char *name,
>  struct kmem_cache *kmem_cache_create_rcu(const char *name, unsigned int size,
>  					 unsigned int freeptr_offset,
>  					 slab_flags_t flags);
> +
> +#define kmem_cache_create(__name, __object_size, __args, ...)           \
> +	_Generic((__args),                                              \
> +		struct kmem_cache_args *: __kmem_cache_create_args,	\
> +		default: __kmem_cache_create)(__name, __object_size, __args, __VA_ARGS__)
> +
>  void kmem_cache_destroy(struct kmem_cache *s);
>  int kmem_cache_shrink(struct kmem_cache *s);
>  
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 19ae3dd6e36f..418459927670 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -383,7 +383,7 @@ kmem_cache_create_usercopy(const char *name, unsigned int size,
>  EXPORT_SYMBOL(kmem_cache_create_usercopy);
>  
>  /**
> - * kmem_cache_create - Create a cache.
> + * __kmem_cache_create - Create a cache.
>   * @name: A string which is used in /proc/slabinfo to identify this cache.
>   * @size: The size of objects to be created in this cache.
>   * @align: The required alignment for the objects.
> @@ -407,9 +407,9 @@ EXPORT_SYMBOL(kmem_cache_create_usercopy);
>   *
>   * Return: a pointer to the cache on success, NULL on failure.
>   */
> -struct kmem_cache *
> -kmem_cache_create(const char *name, unsigned int size, unsigned int align,
> -		slab_flags_t flags, void (*ctor)(void *))
> +struct kmem_cache *__kmem_cache_create(const char *name, unsigned int size,
> +				       unsigned int align, slab_flags_t flags,
> +				       void (*ctor)(void *))
>  {
>  	struct kmem_cache_args kmem_args = {
>  		.align	= align,
> @@ -418,7 +418,7 @@ kmem_cache_create(const char *name, unsigned int size, unsigned int align,
>  
>  	return __kmem_cache_create_args(name, size, &kmem_args, flags);
>  }
> -EXPORT_SYMBOL(kmem_cache_create);
> +EXPORT_SYMBOL(__kmem_cache_create);
>  
>  /**
>   * kmem_cache_create_rcu - Create a SLAB_TYPESAFE_BY_RCU cache.
> 
> -- 
> 2.45.2
> 

-- 
Sincerely yours,
Mike.

[PATCH 17/16] slab: make kmem_cache_create_usercopy() static inline

[Christian Brauner]

Make kmem_cache_create_usercopy() a static inline function.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---

Based on a suggestion by Mike.

---
 include/linux/slab.h | 49 +++++++++++++++++++++++++++++++++++++++-----
 mm/slab_common.c     | 45 ----------------------------------------
 2 files changed, 44 insertions(+), 50 deletions(-)

diff --git a/include/linux/slab.h b/include/linux/slab.h
index 1176b30cd4b2..e224a1a9bcbc 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -265,11 +265,50 @@ struct kmem_cache *__kmem_cache_create_args(const char *name,
 struct kmem_cache *__kmem_cache_create(const char *name, unsigned int size,
 				       unsigned int align, slab_flags_t flags,
 				       void (*ctor)(void *));
-struct kmem_cache *kmem_cache_create_usercopy(const char *name,
-			unsigned int size, unsigned int align,
-			slab_flags_t flags,
-			unsigned int useroffset, unsigned int usersize,
-			void (*ctor)(void *));
+
+/**
+ * kmem_cache_create_usercopy - Create a cache with a region suitable
+ * for copying to userspace
+ * @name: A string which is used in /proc/slabinfo to identify this cache.
+ * @size: The size of objects to be created in this cache.
+ * @align: The required alignment for the objects.
+ * @flags: SLAB flags
+ * @useroffset: Usercopy region offset
+ * @usersize: Usercopy region size
+ * @ctor: A constructor for the objects.
+ *
+ * Cannot be called within a interrupt, but can be interrupted.
+ * The @ctor is run when new pages are allocated by the cache.
+ *
+ * The flags are
+ *
+ * %SLAB_POISON - Poison the slab with a known test pattern (a5a5a5a5)
+ * to catch references to uninitialised memory.
+ *
+ * %SLAB_RED_ZONE - Insert `Red` zones around the allocated memory to check
+ * for buffer overruns.
+ *
+ * %SLAB_HWCACHE_ALIGN - Align the objects in this cache to a hardware
+ * cacheline.  This can be beneficial if you're counting cycles as closely
+ * as davem.
+ *
+ * Return: a pointer to the cache on success, NULL on failure.
+ */
+static inline struct kmem_cache *
+kmem_cache_create_usercopy(const char *name, unsigned int size,
+			   unsigned int align, slab_flags_t flags,
+			   unsigned int useroffset, unsigned int usersize,
+			   void (*ctor)(void *))
+{
+	struct kmem_cache_args kmem_args = {
+		.align		= align,
+		.ctor		= ctor,
+		.useroffset	= useroffset,
+		.usersize	= usersize,
+	};
+
+	return __kmem_cache_create_args(name, size, &kmem_args, flags);
+}
 
 #define kmem_cache_create(__name, __object_size, __args, ...)           \
 	_Generic((__args),                                              \
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 9133b9fafcb1..3477a3918afd 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -337,51 +337,6 @@ struct kmem_cache *__kmem_cache_create_args(const char *name,
 }
 EXPORT_SYMBOL(__kmem_cache_create_args);
 
-/**
- * kmem_cache_create_usercopy - Create a cache with a region suitable
- * for copying to userspace
- * @name: A string which is used in /proc/slabinfo to identify this cache.
- * @size: The size of objects to be created in this cache.
- * @align: The required alignment for the objects.
- * @flags: SLAB flags
- * @useroffset: Usercopy region offset
- * @usersize: Usercopy region size
- * @ctor: A constructor for the objects.
- *
- * Cannot be called within a interrupt, but can be interrupted.
- * The @ctor is run when new pages are allocated by the cache.
- *
- * The flags are
- *
- * %SLAB_POISON - Poison the slab with a known test pattern (a5a5a5a5)
- * to catch references to uninitialised memory.
- *
- * %SLAB_RED_ZONE - Insert `Red` zones around the allocated memory to check
- * for buffer overruns.
- *
- * %SLAB_HWCACHE_ALIGN - Align the objects in this cache to a hardware
- * cacheline.  This can be beneficial if you're counting cycles as closely
- * as davem.
- *
- * Return: a pointer to the cache on success, NULL on failure.
- */
-struct kmem_cache *
-kmem_cache_create_usercopy(const char *name, unsigned int size,
-			   unsigned int align, slab_flags_t flags,
-			   unsigned int useroffset, unsigned int usersize,
-			   void (*ctor)(void *))
-{
-	struct kmem_cache_args kmem_args = {
-		.align		= align,
-		.ctor		= ctor,
-		.useroffset	= useroffset,
-		.usersize	= usersize,
-	};
-
-	return __kmem_cache_create_args(name, size, &kmem_args, flags);
-}
-EXPORT_SYMBOL(kmem_cache_create_usercopy);
-
 /**
  * __kmem_cache_create - Create a cache.
  * @name: A string which is used in /proc/slabinfo to identify this cache.
-- 
2.45.2

[PATCH 18/16] slab: make __kmem_cache_create() static inline

[Christian Brauner]

Make __kmem_cache_create() a static inline function.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 include/linux/slab.h | 39 ++++++++++++++++++++++++++++++++++++---
 mm/slab_common.c     | 38 --------------------------------------
 2 files changed, 36 insertions(+), 41 deletions(-)

diff --git a/include/linux/slab.h b/include/linux/slab.h
index e224a1a9bcbc..3e16392fa37f 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -262,9 +262,42 @@ struct kmem_cache *__kmem_cache_create_args(const char *name,
 					    struct kmem_cache_args *args,
 					    slab_flags_t flags);
 
-struct kmem_cache *__kmem_cache_create(const char *name, unsigned int size,
-				       unsigned int align, slab_flags_t flags,
-				       void (*ctor)(void *));
+/**
+ * __kmem_cache_create - Create a cache.
+ * @name: A string which is used in /proc/slabinfo to identify this cache.
+ * @size: The size of objects to be created in this cache.
+ * @align: The required alignment for the objects.
+ * @flags: SLAB flags
+ * @ctor: A constructor for the objects.
+ *
+ * Cannot be called within a interrupt, but can be interrupted.
+ * The @ctor is run when new pages are allocated by the cache.
+ *
+ * The flags are
+ *
+ * %SLAB_POISON - Poison the slab with a known test pattern (a5a5a5a5)
+ * to catch references to uninitialised memory.
+ *
+ * %SLAB_RED_ZONE - Insert `Red` zones around the allocated memory to check
+ * for buffer overruns.
+ *
+ * %SLAB_HWCACHE_ALIGN - Align the objects in this cache to a hardware
+ * cacheline.  This can be beneficial if you're counting cycles as closely
+ * as davem.
+ *
+ * Return: a pointer to the cache on success, NULL on failure.
+ */
+static inline struct kmem_cache *
+__kmem_cache_create(const char *name, unsigned int size, unsigned int align,
+		    slab_flags_t flags, void (*ctor)(void *))
+{
+	struct kmem_cache_args kmem_args = {
+		.align	= align,
+		.ctor	= ctor,
+	};
+
+	return __kmem_cache_create_args(name, size, &kmem_args, flags);
+}
 
 /**
  * kmem_cache_create_usercopy - Create a cache with a region suitable
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 3477a3918afd..30000dcf0736 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -337,44 +337,6 @@ struct kmem_cache *__kmem_cache_create_args(const char *name,
 }
 EXPORT_SYMBOL(__kmem_cache_create_args);
 
-/**
- * __kmem_cache_create - Create a cache.
- * @name: A string which is used in /proc/slabinfo to identify this cache.
- * @size: The size of objects to be created in this cache.
- * @align: The required alignment for the objects.
- * @flags: SLAB flags
- * @ctor: A constructor for the objects.
- *
- * Cannot be called within a interrupt, but can be interrupted.
- * The @ctor is run when new pages are allocated by the cache.
- *
- * The flags are
- *
- * %SLAB_POISON - Poison the slab with a known test pattern (a5a5a5a5)
- * to catch references to uninitialised memory.
- *
- * %SLAB_RED_ZONE - Insert `Red` zones around the allocated memory to check
- * for buffer overruns.
- *
- * %SLAB_HWCACHE_ALIGN - Align the objects in this cache to a hardware
- * cacheline.  This can be beneficial if you're counting cycles as closely
- * as davem.
- *
- * Return: a pointer to the cache on success, NULL on failure.
- */
-struct kmem_cache *__kmem_cache_create(const char *name, unsigned int size,
-				       unsigned int align, slab_flags_t flags,
-				       void (*ctor)(void *))
-{
-	struct kmem_cache_args kmem_args = {
-		.align	= align,
-		.ctor	= ctor,
-	};
-
-	return __kmem_cache_create_args(name, size, &kmem_args, flags);
-}
-EXPORT_SYMBOL(__kmem_cache_create);
-
 static struct kmem_cache *kmem_buckets_cache __ro_after_init;
 
 /**
-- 
2.45.2

Re: [PATCH v2 12/15] slab: create kmem_cache_create() compatibility layer

[Christian Brauner]

On Wed, Sep 04, 2024 at 08:14:24AM GMT, Mike Rapoport wrote:
> On Tue, Sep 03, 2024 at 04:20:53PM +0200, Christian Brauner wrote:
> > Use _Generic() to create a compatibility layer that type switches on the
> > third argument to either call __kmem_cache_create() or
> > __kmem_cache_create_args(). This can be kept in place until all callers
> > have been ported to struct kmem_cache_args.
> > 
> > Signed-off-by: Christian Brauner <brauner@kernel.org>
> 
> Reviewed-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> 
> > ---
> >  include/linux/slab.h | 13 ++++++++++---
> >  mm/slab_common.c     | 10 +++++-----
> >  2 files changed, 15 insertions(+), 8 deletions(-)
> > 
> > diff --git a/include/linux/slab.h b/include/linux/slab.h
> > index aced16a08700..4292d67094c3 100644
> > --- a/include/linux/slab.h
> > +++ b/include/linux/slab.h
> > @@ -261,9 +261,10 @@ struct kmem_cache *__kmem_cache_create_args(const char *name,
> >  					    unsigned int object_size,
> >  					    struct kmem_cache_args *args,
> >  					    slab_flags_t flags);
> > -struct kmem_cache *kmem_cache_create(const char *name, unsigned int size,
> > -			unsigned int align, slab_flags_t flags,
> > -			void (*ctor)(void *));
> > +
> > +struct kmem_cache *__kmem_cache_create(const char *name, unsigned int size,
> > +				       unsigned int align, slab_flags_t flags,
> > +				       void (*ctor)(void *));
> 
> As I said earlier, this can become _kmem_cache_create and
> __kmem_cache_create_args can be __kmem_cache_create from the beginning.
> 
> And as a followup cleanup both kmem_cache_create_usercopy() and
> kmem_cache_create() can be made static inlines.

Seems an ok suggestion to me. See the two patches I sent out now.

Re: [PATCH v2 12/15] slab: create kmem_cache_create() compatibility layer

[Vlastimil Babka]

On 9/4/24 11:45, Christian Brauner wrote:
> On Wed, Sep 04, 2024 at 08:14:24AM GMT, Mike Rapoport wrote:
>> On Tue, Sep 03, 2024 at 04:20:53PM +0200, Christian Brauner wrote:
>> > Use _Generic() to create a compatibility layer that type switches on the
>> > third argument to either call __kmem_cache_create() or
>> > __kmem_cache_create_args(). This can be kept in place until all callers
>> > have been ported to struct kmem_cache_args.
>> > 
>> > Signed-off-by: Christian Brauner <brauner@kernel.org>
>> 
>> Reviewed-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
>> 
>> > ---
>> >  include/linux/slab.h | 13 ++++++++++---
>> >  mm/slab_common.c     | 10 +++++-----
>> >  2 files changed, 15 insertions(+), 8 deletions(-)
>> > 
>> > diff --git a/include/linux/slab.h b/include/linux/slab.h
>> > index aced16a08700..4292d67094c3 100644
>> > --- a/include/linux/slab.h
>> > +++ b/include/linux/slab.h
>> > @@ -261,9 +261,10 @@ struct kmem_cache *__kmem_cache_create_args(const char *name,
>> >  					    unsigned int object_size,
>> >  					    struct kmem_cache_args *args,
>> >  					    slab_flags_t flags);
>> > -struct kmem_cache *kmem_cache_create(const char *name, unsigned int size,
>> > -			unsigned int align, slab_flags_t flags,
>> > -			void (*ctor)(void *));
>> > +
>> > +struct kmem_cache *__kmem_cache_create(const char *name, unsigned int size,
>> > +				       unsigned int align, slab_flags_t flags,
>> > +				       void (*ctor)(void *));
>> 
>> As I said earlier, this can become _kmem_cache_create and
>> __kmem_cache_create_args can be __kmem_cache_create from the beginning.

I didn't notice an answer to this suggestion? Even if it's just that you
don't think it's worth the rewrite, or it's not possible because X Y Z.
Thanks.

>> And as a followup cleanup both kmem_cache_create_usercopy() and
>> kmem_cache_create() can be made static inlines.
> 
> Seems an ok suggestion to me. See the two patches I sent out now.

Re: [PATCH v2 12/15] slab: create kmem_cache_create() compatibility layer

[Christian Brauner]

On Wed, Sep 04, 2024 at 12:50:28PM GMT, Vlastimil Babka wrote:
> On 9/4/24 11:45, Christian Brauner wrote:
> > On Wed, Sep 04, 2024 at 08:14:24AM GMT, Mike Rapoport wrote:
> >> On Tue, Sep 03, 2024 at 04:20:53PM +0200, Christian Brauner wrote:
> >> > Use _Generic() to create a compatibility layer that type switches on the
> >> > third argument to either call __kmem_cache_create() or
> >> > __kmem_cache_create_args(). This can be kept in place until all callers
> >> > have been ported to struct kmem_cache_args.
> >> > 
> >> > Signed-off-by: Christian Brauner <brauner@kernel.org>
> >> 
> >> Reviewed-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> >> 
> >> > ---
> >> >  include/linux/slab.h | 13 ++++++++++---
> >> >  mm/slab_common.c     | 10 +++++-----
> >> >  2 files changed, 15 insertions(+), 8 deletions(-)
> >> > 
> >> > diff --git a/include/linux/slab.h b/include/linux/slab.h
> >> > index aced16a08700..4292d67094c3 100644
> >> > --- a/include/linux/slab.h
> >> > +++ b/include/linux/slab.h
> >> > @@ -261,9 +261,10 @@ struct kmem_cache *__kmem_cache_create_args(const char *name,
> >> >  					    unsigned int object_size,
> >> >  					    struct kmem_cache_args *args,
> >> >  					    slab_flags_t flags);
> >> > -struct kmem_cache *kmem_cache_create(const char *name, unsigned int size,
> >> > -			unsigned int align, slab_flags_t flags,
> >> > -			void (*ctor)(void *));
> >> > +
> >> > +struct kmem_cache *__kmem_cache_create(const char *name, unsigned int size,
> >> > +				       unsigned int align, slab_flags_t flags,
> >> > +				       void (*ctor)(void *));
> >> 
> >> As I said earlier, this can become _kmem_cache_create and
> >> __kmem_cache_create_args can be __kmem_cache_create from the beginning.
> 
> I didn't notice an answer to this suggestion? Even if it's just that you
> don't think it's worth the rewrite, or it's not possible because X Y Z.
> Thanks.

I'm confused. I sent two patches as a reply to the thread plus the
answer below and there's two patches in v3 that you can use or drop.

> 
> >> And as a followup cleanup both kmem_cache_create_usercopy() and
> >> kmem_cache_create() can be made static inlines.
> > 
> > Seems an ok suggestion to me. See the two patches I sent out now.
> 

Re: [PATCH v2 12/15] slab: create kmem_cache_create() compatibility layer

[Vlastimil Babka]

On 9/4/24 13:38, Christian Brauner wrote:
> On Wed, Sep 04, 2024 at 12:50:28PM GMT, Vlastimil Babka wrote:
>> On 9/4/24 11:45, Christian Brauner wrote:
>> > On Wed, Sep 04, 2024 at 08:14:24AM GMT, Mike Rapoport wrote:
>> >> On Tue, Sep 03, 2024 at 04:20:53PM +0200, Christian Brauner wrote:
>> >> > Use _Generic() to create a compatibility layer that type switches on the
>> >> > third argument to either call __kmem_cache_create() or
>> >> > __kmem_cache_create_args(). This can be kept in place until all callers
>> >> > have been ported to struct kmem_cache_args.
>> >> > 
>> >> > Signed-off-by: Christian Brauner <brauner@kernel.org>
>> >> 
>> >> Reviewed-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
>> >> 
>> >> > ---
>> >> >  include/linux/slab.h | 13 ++++++++++---
>> >> >  mm/slab_common.c     | 10 +++++-----
>> >> >  2 files changed, 15 insertions(+), 8 deletions(-)
>> >> > 
>> >> > diff --git a/include/linux/slab.h b/include/linux/slab.h
>> >> > index aced16a08700..4292d67094c3 100644
>> >> > --- a/include/linux/slab.h
>> >> > +++ b/include/linux/slab.h
>> >> > @@ -261,9 +261,10 @@ struct kmem_cache *__kmem_cache_create_args(const char *name,
>> >> >  					    unsigned int object_size,
>> >> >  					    struct kmem_cache_args *args,
>> >> >  					    slab_flags_t flags);
>> >> > -struct kmem_cache *kmem_cache_create(const char *name, unsigned int size,
>> >> > -			unsigned int align, slab_flags_t flags,
>> >> > -			void (*ctor)(void *));
>> >> > +
>> >> > +struct kmem_cache *__kmem_cache_create(const char *name, unsigned int size,
>> >> > +				       unsigned int align, slab_flags_t flags,
>> >> > +				       void (*ctor)(void *));
>> >> 
>> >> As I said earlier, this can become _kmem_cache_create and
>> >> __kmem_cache_create_args can be __kmem_cache_create from the beginning.
>> 
>> I didn't notice an answer to this suggestion? Even if it's just that you
>> don't think it's worth the rewrite, or it's not possible because X Y Z.
>> Thanks.
> 
> I'm confused. I sent two patches as a reply to the thread plus the
> answer below and there's two patches in v3 that you can use or drop.

Right, that's the part below. But the suggestion above, and also in Mike's
reply to 02/12 was AFAICS to rename __kmem_cache_create_args to
__kmem_cache_create (since patch 02) and here __kmem_cache_create to
_kmem_cache_create. It just seemed odd to see no reaction to that (did I
miss or not receive it?).

>> 
>> >> And as a followup cleanup both kmem_cache_create_usercopy() and
>> >> kmem_cache_create() can be made static inlines.
>> > 
>> > Seems an ok suggestion to me. See the two patches I sent out now.
>> 

Re: [PATCH v2 12/15] slab: create kmem_cache_create() compatibility layer

[Christian Brauner]

On Wed, Sep 04, 2024 at 03:33:30PM GMT, Vlastimil Babka wrote:
> On 9/4/24 13:38, Christian Brauner wrote:
> > On Wed, Sep 04, 2024 at 12:50:28PM GMT, Vlastimil Babka wrote:
> >> On 9/4/24 11:45, Christian Brauner wrote:
> >> > On Wed, Sep 04, 2024 at 08:14:24AM GMT, Mike Rapoport wrote:
> >> >> On Tue, Sep 03, 2024 at 04:20:53PM +0200, Christian Brauner wrote:
> >> >> > Use _Generic() to create a compatibility layer that type switches on the
> >> >> > third argument to either call __kmem_cache_create() or
> >> >> > __kmem_cache_create_args(). This can be kept in place until all callers
> >> >> > have been ported to struct kmem_cache_args.
> >> >> > 
> >> >> > Signed-off-by: Christian Brauner <brauner@kernel.org>
> >> >> 
> >> >> Reviewed-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> >> >> 
> >> >> > ---
> >> >> >  include/linux/slab.h | 13 ++++++++++---
> >> >> >  mm/slab_common.c     | 10 +++++-----
> >> >> >  2 files changed, 15 insertions(+), 8 deletions(-)
> >> >> > 
> >> >> > diff --git a/include/linux/slab.h b/include/linux/slab.h
> >> >> > index aced16a08700..4292d67094c3 100644
> >> >> > --- a/include/linux/slab.h
> >> >> > +++ b/include/linux/slab.h
> >> >> > @@ -261,9 +261,10 @@ struct kmem_cache *__kmem_cache_create_args(const char *name,
> >> >> >  					    unsigned int object_size,
> >> >> >  					    struct kmem_cache_args *args,
> >> >> >  					    slab_flags_t flags);
> >> >> > -struct kmem_cache *kmem_cache_create(const char *name, unsigned int size,
> >> >> > -			unsigned int align, slab_flags_t flags,
> >> >> > -			void (*ctor)(void *));
> >> >> > +
> >> >> > +struct kmem_cache *__kmem_cache_create(const char *name, unsigned int size,
> >> >> > +				       unsigned int align, slab_flags_t flags,
> >> >> > +				       void (*ctor)(void *));
> >> >> 
> >> >> As I said earlier, this can become _kmem_cache_create and
> >> >> __kmem_cache_create_args can be __kmem_cache_create from the beginning.
> >> 
> >> I didn't notice an answer to this suggestion? Even if it's just that you
> >> don't think it's worth the rewrite, or it's not possible because X Y Z.
> >> Thanks.
> > 
> > I'm confused. I sent two patches as a reply to the thread plus the
> > answer below and there's two patches in v3 that you can use or drop.
> 
> Right, that's the part below. But the suggestion above, and also in Mike's
> reply to 02/12 was AFAICS to rename __kmem_cache_create_args to
> __kmem_cache_create (since patch 02) and here __kmem_cache_create to
> _kmem_cache_create. It just seemed odd to see no reaction to that (did I
> miss or not receive it?).

Oh, I see. I read it as a expressing taste and so I didn't bother
replying. And I really dislike single underscore function names so I
would like to avoid it and it also seems more confusing to me.

Re: [PATCH v2 12/15] slab: create kmem_cache_create() compatibility layer

[Mike Rapoport]

On Wed, Sep 04, 2024 at 04:44:03PM +0200, Christian Brauner wrote:
> On Wed, Sep 04, 2024 at 03:33:30PM GMT, Vlastimil Babka wrote:
> > On 9/4/24 13:38, Christian Brauner wrote:
> > > On Wed, Sep 04, 2024 at 12:50:28PM GMT, Vlastimil Babka wrote:
> > >> On 9/4/24 11:45, Christian Brauner wrote:
> > >> > On Wed, Sep 04, 2024 at 08:14:24AM GMT, Mike Rapoport wrote:
> > >> >> On Tue, Sep 03, 2024 at 04:20:53PM +0200, Christian Brauner wrote:
> > >> >> > Use _Generic() to create a compatibility layer that type switches on the
> > >> >> > third argument to either call __kmem_cache_create() or
> > >> >> > __kmem_cache_create_args(). This can be kept in place until all callers
> > >> >> > have been ported to struct kmem_cache_args.
> > >> >> > 
> > >> >> > Signed-off-by: Christian Brauner <brauner@kernel.org>
> > >> >> 
> > >> >> Reviewed-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> > >> >> 
> > >> >> > ---
> > >> >> >  include/linux/slab.h | 13 ++++++++++---
> > >> >> >  mm/slab_common.c     | 10 +++++-----
> > >> >> >  2 files changed, 15 insertions(+), 8 deletions(-)
> > >> >> > 
> > >> >> > diff --git a/include/linux/slab.h b/include/linux/slab.h
> > >> >> > index aced16a08700..4292d67094c3 100644
> > >> >> > --- a/include/linux/slab.h
> > >> >> > +++ b/include/linux/slab.h
> > >> >> > @@ -261,9 +261,10 @@ struct kmem_cache *__kmem_cache_create_args(const char *name,
> > >> >> >  					    unsigned int object_size,
> > >> >> >  					    struct kmem_cache_args *args,
> > >> >> >  					    slab_flags_t flags);
> > >> >> > -struct kmem_cache *kmem_cache_create(const char *name, unsigned int size,
> > >> >> > -			unsigned int align, slab_flags_t flags,
> > >> >> > -			void (*ctor)(void *));
> > >> >> > +
> > >> >> > +struct kmem_cache *__kmem_cache_create(const char *name, unsigned int size,
> > >> >> > +				       unsigned int align, slab_flags_t flags,
> > >> >> > +				       void (*ctor)(void *));
> > >> >> 
> > >> >> As I said earlier, this can become _kmem_cache_create and
> > >> >> __kmem_cache_create_args can be __kmem_cache_create from the beginning.
> > >> 
> > >> I didn't notice an answer to this suggestion? Even if it's just that you
> > >> don't think it's worth the rewrite, or it's not possible because X Y Z.
> > >> Thanks.
> > > 
> > > I'm confused. I sent two patches as a reply to the thread plus the
> > > answer below and there's two patches in v3 that you can use or drop.
> > 
> > Right, that's the part below. But the suggestion above, and also in Mike's
> > reply to 02/12 was AFAICS to rename __kmem_cache_create_args to
> > __kmem_cache_create (since patch 02) and here __kmem_cache_create to
> > _kmem_cache_create. It just seemed odd to see no reaction to that (did I
> > miss or not receive it?).
> 
> Oh, I see. I read it as a expressing taste and so I didn't bother
> replying. And I really dislike single underscore function names so I
> would like to avoid it and it also seems more confusing to me.

Heh, not quite. I don't like kmem_cache_create_args essentially becoming a
replacement for kmem_cache_create* and I'd prefer __kmem_cache_create
naming.

As for the single underscore, I don't have strong feelings about it, but I
do think that it should be renamed to something else than
__kmem_cache_create to leave __kmem_cache_create for the core function.

-- 
Sincerely yours,
Mike.

Re: [PATCH v2 12/15] slab: create kmem_cache_create() compatibility layer

[Christian Brauner]

On Wed, Sep 04, 2024 at 06:11:10PM GMT, Mike Rapoport wrote:
> On Wed, Sep 04, 2024 at 04:44:03PM +0200, Christian Brauner wrote:
> > On Wed, Sep 04, 2024 at 03:33:30PM GMT, Vlastimil Babka wrote:
> > > On 9/4/24 13:38, Christian Brauner wrote:
> > > > On Wed, Sep 04, 2024 at 12:50:28PM GMT, Vlastimil Babka wrote:
> > > >> On 9/4/24 11:45, Christian Brauner wrote:
> > > >> > On Wed, Sep 04, 2024 at 08:14:24AM GMT, Mike Rapoport wrote:
> > > >> >> On Tue, Sep 03, 2024 at 04:20:53PM +0200, Christian Brauner wrote:
> > > >> >> > Use _Generic() to create a compatibility layer that type switches on the
> > > >> >> > third argument to either call __kmem_cache_create() or
> > > >> >> > __kmem_cache_create_args(). This can be kept in place until all callers
> > > >> >> > have been ported to struct kmem_cache_args.
> > > >> >> > 
> > > >> >> > Signed-off-by: Christian Brauner <brauner@kernel.org>
> > > >> >> 
> > > >> >> Reviewed-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> > > >> >> 
> > > >> >> > ---
> > > >> >> >  include/linux/slab.h | 13 ++++++++++---
> > > >> >> >  mm/slab_common.c     | 10 +++++-----
> > > >> >> >  2 files changed, 15 insertions(+), 8 deletions(-)
> > > >> >> > 
> > > >> >> > diff --git a/include/linux/slab.h b/include/linux/slab.h
> > > >> >> > index aced16a08700..4292d67094c3 100644
> > > >> >> > --- a/include/linux/slab.h
> > > >> >> > +++ b/include/linux/slab.h
> > > >> >> > @@ -261,9 +261,10 @@ struct kmem_cache *__kmem_cache_create_args(const char *name,
> > > >> >> >  					    unsigned int object_size,
> > > >> >> >  					    struct kmem_cache_args *args,
> > > >> >> >  					    slab_flags_t flags);
> > > >> >> > -struct kmem_cache *kmem_cache_create(const char *name, unsigned int size,
> > > >> >> > -			unsigned int align, slab_flags_t flags,
> > > >> >> > -			void (*ctor)(void *));
> > > >> >> > +
> > > >> >> > +struct kmem_cache *__kmem_cache_create(const char *name, unsigned int size,
> > > >> >> > +				       unsigned int align, slab_flags_t flags,
> > > >> >> > +				       void (*ctor)(void *));
> > > >> >> 
> > > >> >> As I said earlier, this can become _kmem_cache_create and
> > > >> >> __kmem_cache_create_args can be __kmem_cache_create from the beginning.
> > > >> 
> > > >> I didn't notice an answer to this suggestion? Even if it's just that you
> > > >> don't think it's worth the rewrite, or it's not possible because X Y Z.
> > > >> Thanks.
> > > > 
> > > > I'm confused. I sent two patches as a reply to the thread plus the
> > > > answer below and there's two patches in v3 that you can use or drop.
> > > 
> > > Right, that's the part below. But the suggestion above, and also in Mike's
> > > reply to 02/12 was AFAICS to rename __kmem_cache_create_args to
> > > __kmem_cache_create (since patch 02) and here __kmem_cache_create to
> > > _kmem_cache_create. It just seemed odd to see no reaction to that (did I
> > > miss or not receive it?).
> > 
> > Oh, I see. I read it as a expressing taste and so I didn't bother
> > replying. And I really dislike single underscore function names so I
> > would like to avoid it and it also seems more confusing to me.
> 
> Heh, not quite. I don't like kmem_cache_create_args essentially becoming a
> replacement for kmem_cache_create* and I'd prefer __kmem_cache_create
> naming.
> 
> As for the single underscore, I don't have strong feelings about it, but I
> do think that it should be renamed to something else than
> __kmem_cache_create to leave __kmem_cache_create for the core function.

I honestly don't care especially because it's not visible outside of the
header. If you care then a simple patch on top of the series to rename
to whatever is fine by me.

But single vs double underscore with fundamentally different parameters
seems ripe for visual confusion and the compatibility layer will go away
anyway.

Re: [PATCH v2 12/15] slab: create kmem_cache_create() compatibility layer

[Vlastimil Babka]

On 9/4/24 17:11, Mike Rapoport wrote:
>> 
>> Oh, I see. I read it as a expressing taste and so I didn't bother
>> replying. And I really dislike single underscore function names so I
>> would like to avoid it and it also seems more confusing to me.
> 
> Heh, not quite. I don't like kmem_cache_create_args essentially becoming a
> replacement for kmem_cache_create* and I'd prefer __kmem_cache_create
> naming.

It's __kmem_cache_create_args(). If it didn't have the underscores, I'd
agree it might be misleading into being used directly. But like this it
doesn't bother me much.

> As for the single underscore, I don't have strong feelings about it, but I
> do think that it should be renamed to something else than
> __kmem_cache_create to leave __kmem_cache_create for the core function.

[PATCH v2 13/15] file: port to struct kmem_cache_args

[Christian Brauner]

Port filp_cache to struct kmem_cache_args.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 fs/file_table.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/fs/file_table.c b/fs/file_table.c
index 3ef558f27a1c..861c03608e83 100644
--- a/fs/file_table.c
+++ b/fs/file_table.c
@@ -511,9 +511,14 @@ EXPORT_SYMBOL(__fput_sync);
 
 void __init files_init(void)
 {
-	filp_cachep = kmem_cache_create_rcu("filp", sizeof(struct file),
-				offsetof(struct file, f_freeptr),
-				SLAB_HWCACHE_ALIGN | SLAB_PANIC | SLAB_ACCOUNT);
+	struct kmem_cache_args args = {
+		.use_freeptr_offset = true,
+		.freeptr_offset = offsetof(struct file, f_freeptr),
+	};
+
+	filp_cachep = kmem_cache_create("filp", sizeof(struct file), &args,
+				SLAB_HWCACHE_ALIGN | SLAB_PANIC |
+				SLAB_ACCOUNT | SLAB_TYPESAFE_BY_RCU);
 	percpu_counter_init(&nr_files, 0, GFP_KERNEL);
 }
 

-- 
2.45.2

Re: [PATCH v2 13/15] file: port to struct kmem_cache_args

[Mike Rapoport]

On Tue, Sep 03, 2024 at 04:20:54PM +0200, Christian Brauner wrote:
> Port filp_cache to struct kmem_cache_args.
> 
> Signed-off-by: Christian Brauner <brauner@kernel.org>

Reviewed-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

> ---
>  fs/file_table.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/file_table.c b/fs/file_table.c
> index 3ef558f27a1c..861c03608e83 100644
> --- a/fs/file_table.c
> +++ b/fs/file_table.c
> @@ -511,9 +511,14 @@ EXPORT_SYMBOL(__fput_sync);
>  
>  void __init files_init(void)
>  {
> -	filp_cachep = kmem_cache_create_rcu("filp", sizeof(struct file),
> -				offsetof(struct file, f_freeptr),
> -				SLAB_HWCACHE_ALIGN | SLAB_PANIC | SLAB_ACCOUNT);
> +	struct kmem_cache_args args = {
> +		.use_freeptr_offset = true,
> +		.freeptr_offset = offsetof(struct file, f_freeptr),
> +	};
> +
> +	filp_cachep = kmem_cache_create("filp", sizeof(struct file), &args,
> +				SLAB_HWCACHE_ALIGN | SLAB_PANIC |
> +				SLAB_ACCOUNT | SLAB_TYPESAFE_BY_RCU);
>  	percpu_counter_init(&nr_files, 0, GFP_KERNEL);
>  }
>  
> 
> -- 
> 2.45.2
> 

-- 
Sincerely yours,
Mike.

[PATCH v2 14/15] slab: remove kmem_cache_create_rcu()

[Christian Brauner]

Since we have kmem_cache_setup() and have ported kmem_cache_create_rcu()
users over to it is unused and can be removed.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 include/linux/slab.h |  3 ---
 mm/slab_common.c     | 43 -------------------------------------------
 2 files changed, 46 deletions(-)

diff --git a/include/linux/slab.h b/include/linux/slab.h
index 4292d67094c3..1176b30cd4b2 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -270,9 +270,6 @@ struct kmem_cache *kmem_cache_create_usercopy(const char *name,
 			slab_flags_t flags,
 			unsigned int useroffset, unsigned int usersize,
 			void (*ctor)(void *));
-struct kmem_cache *kmem_cache_create_rcu(const char *name, unsigned int size,
-					 unsigned int freeptr_offset,
-					 slab_flags_t flags);
 
 #define kmem_cache_create(__name, __object_size, __args, ...)           \
 	_Generic((__args),                                              \
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 418459927670..9133b9fafcb1 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -420,49 +420,6 @@ struct kmem_cache *__kmem_cache_create(const char *name, unsigned int size,
 }
 EXPORT_SYMBOL(__kmem_cache_create);
 
-/**
- * kmem_cache_create_rcu - Create a SLAB_TYPESAFE_BY_RCU cache.
- * @name: A string which is used in /proc/slabinfo to identify this cache.
- * @size: The size of objects to be created in this cache.
- * @freeptr_offset: The offset into the memory to the free pointer
- * @flags: SLAB flags
- *
- * Cannot be called within an interrupt, but can be interrupted.
- *
- * See kmem_cache_create() for an explanation of possible @flags.
- *
- * By default SLAB_TYPESAFE_BY_RCU caches place the free pointer outside
- * of the object. This might cause the object to grow in size. Callers
- * that have a reason to avoid this can specify a custom free pointer
- * offset in their struct where the free pointer will be placed.
- *
- * Note that placing the free pointer inside the object requires the
- * caller to ensure that no fields are invalidated that are required to
- * guard against object recycling (See SLAB_TYPESAFE_BY_RCU for
- * details.).
- *
- * Using zero as a value for @freeptr_offset is valid. To request no
- * offset UINT_MAX must be specified.
- *
- * Note that @ctor isn't supported with custom free pointers as a @ctor
- * requires an external free pointer.
- *
- * Return: a pointer to the cache on success, NULL on failure.
- */
-struct kmem_cache *kmem_cache_create_rcu(const char *name, unsigned int size,
-					 unsigned int freeptr_offset,
-					 slab_flags_t flags)
-{
-	struct kmem_cache_args kmem_args = {
-		.freeptr_offset		= freeptr_offset,
-		.use_freeptr_offset	= true,
-	};
-
-	return __kmem_cache_create_args(name, size, &kmem_args,
-					flags | SLAB_TYPESAFE_BY_RCU);
-}
-EXPORT_SYMBOL(kmem_cache_create_rcu);
-
 static struct kmem_cache *kmem_buckets_cache __ro_after_init;
 
 /**

-- 
2.45.2

Re: [PATCH v2 14/15] slab: remove kmem_cache_create_rcu()

[Mike Rapoport]

On Tue, Sep 03, 2024 at 04:20:55PM +0200, Christian Brauner wrote:
> Since we have kmem_cache_setup() and have ported kmem_cache_create_rcu()
> users over to it is unused and can be removed.
> 
> Signed-off-by: Christian Brauner <brauner@kernel.org>

Reviewed-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

> ---
>  include/linux/slab.h |  3 ---
>  mm/slab_common.c     | 43 -------------------------------------------
>  2 files changed, 46 deletions(-)
> 
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index 4292d67094c3..1176b30cd4b2 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -270,9 +270,6 @@ struct kmem_cache *kmem_cache_create_usercopy(const char *name,
>  			slab_flags_t flags,
>  			unsigned int useroffset, unsigned int usersize,
>  			void (*ctor)(void *));
> -struct kmem_cache *kmem_cache_create_rcu(const char *name, unsigned int size,
> -					 unsigned int freeptr_offset,
> -					 slab_flags_t flags);
>  
>  #define kmem_cache_create(__name, __object_size, __args, ...)           \
>  	_Generic((__args),                                              \
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 418459927670..9133b9fafcb1 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -420,49 +420,6 @@ struct kmem_cache *__kmem_cache_create(const char *name, unsigned int size,
>  }
>  EXPORT_SYMBOL(__kmem_cache_create);
>  
> -/**
> - * kmem_cache_create_rcu - Create a SLAB_TYPESAFE_BY_RCU cache.
> - * @name: A string which is used in /proc/slabinfo to identify this cache.
> - * @size: The size of objects to be created in this cache.
> - * @freeptr_offset: The offset into the memory to the free pointer
> - * @flags: SLAB flags
> - *
> - * Cannot be called within an interrupt, but can be interrupted.
> - *
> - * See kmem_cache_create() for an explanation of possible @flags.
> - *
> - * By default SLAB_TYPESAFE_BY_RCU caches place the free pointer outside
> - * of the object. This might cause the object to grow in size. Callers
> - * that have a reason to avoid this can specify a custom free pointer
> - * offset in their struct where the free pointer will be placed.
> - *
> - * Note that placing the free pointer inside the object requires the
> - * caller to ensure that no fields are invalidated that are required to
> - * guard against object recycling (See SLAB_TYPESAFE_BY_RCU for
> - * details.).
> - *
> - * Using zero as a value for @freeptr_offset is valid. To request no
> - * offset UINT_MAX must be specified.
> - *
> - * Note that @ctor isn't supported with custom free pointers as a @ctor
> - * requires an external free pointer.
> - *
> - * Return: a pointer to the cache on success, NULL on failure.
> - */
> -struct kmem_cache *kmem_cache_create_rcu(const char *name, unsigned int size,
> -					 unsigned int freeptr_offset,
> -					 slab_flags_t flags)
> -{
> -	struct kmem_cache_args kmem_args = {
> -		.freeptr_offset		= freeptr_offset,
> -		.use_freeptr_offset	= true,
> -	};
> -
> -	return __kmem_cache_create_args(name, size, &kmem_args,
> -					flags | SLAB_TYPESAFE_BY_RCU);
> -}
> -EXPORT_SYMBOL(kmem_cache_create_rcu);
> -
>  static struct kmem_cache *kmem_buckets_cache __ro_after_init;
>  
>  /**
> 
> -- 
> 2.45.2
> 

-- 
Sincerely yours,
Mike.

Re: [PATCH v2 14/15] slab: remove kmem_cache_create_rcu()

[Vlastimil Babka]

On 9/3/24 16:20, Christian Brauner wrote:
> Since we have kmem_cache_setup() and have ported kmem_cache_create_rcu()

Nit: it's now kmem_cache_create() variant with kmem_cacheargs, not
kmem_cache_setup()

Re: [PATCH v2 14/15] slab: remove kmem_cache_create_rcu()

[Christian Brauner]

On Wed, Sep 04, 2024 at 10:18:39AM GMT, Vlastimil Babka wrote:
> On 9/3/24 16:20, Christian Brauner wrote:
> > Since we have kmem_cache_setup() and have ported kmem_cache_create_rcu()
> 
> Nit: it's now kmem_cache_create() variant with kmem_cacheargs, not
> kmem_cache_setup()

Thanks. I've changed that to:

"Now that we have ported all users of kmem_cache_create_rcu() to struct
kmem_cache_args the function is unused and can be removed."

on the work.kmem_cache_args branch.

[PATCH v2 15/15] io_uring: port to struct kmem_cache_args

[Christian Brauner]

Port req_cachep to struct kmem_cache_args.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 io_uring/io_uring.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 3942db160f18..d9d721d1424e 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -3638,6 +3638,11 @@ SYSCALL_DEFINE2(io_uring_setup, u32, entries,
 
 static int __init io_uring_init(void)
 {
+	struct kmem_cache_args kmem_args = {
+		.useroffset = offsetof(struct io_kiocb, cmd.data),
+		.usersize = sizeof_field(struct io_kiocb, cmd.data),
+	};
+
 #define __BUILD_BUG_VERIFY_OFFSET_SIZE(stype, eoffset, esize, ename) do { \
 	BUILD_BUG_ON(offsetof(stype, ename) != eoffset); \
 	BUILD_BUG_ON(sizeof_field(stype, ename) != esize); \
@@ -3722,12 +3727,9 @@ static int __init io_uring_init(void)
 	 * range, and HARDENED_USERCOPY will complain if we haven't
 	 * correctly annotated this range.
 	 */
-	req_cachep = kmem_cache_create_usercopy("io_kiocb",
-				sizeof(struct io_kiocb), 0,
-				SLAB_HWCACHE_ALIGN | SLAB_PANIC |
-				SLAB_ACCOUNT | SLAB_TYPESAFE_BY_RCU,
-				offsetof(struct io_kiocb, cmd.data),
-				sizeof_field(struct io_kiocb, cmd.data), NULL);
+	req_cachep = kmem_cache_create("io_kiocb", sizeof(struct io_kiocb), &kmem_args,
+				SLAB_HWCACHE_ALIGN | SLAB_PANIC | SLAB_ACCOUNT |
+				SLAB_TYPESAFE_BY_RCU);
 	io_buf_cachep = KMEM_CACHE(io_buffer,
 					  SLAB_HWCACHE_ALIGN | SLAB_PANIC | SLAB_ACCOUNT);
 

-- 
2.45.2

Re: [PATCH v2 15/15] io_uring: port to struct kmem_cache_args

[Mike Rapoport]

On Tue, Sep 03, 2024 at 04:20:56PM +0200, Christian Brauner wrote:
> Port req_cachep to struct kmem_cache_args.
> 
> Signed-off-by: Christian Brauner <brauner@kernel.org>

Reviewed-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

> ---
>  io_uring/io_uring.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
> index 3942db160f18..d9d721d1424e 100644
> --- a/io_uring/io_uring.c
> +++ b/io_uring/io_uring.c
> @@ -3638,6 +3638,11 @@ SYSCALL_DEFINE2(io_uring_setup, u32, entries,
>  
>  static int __init io_uring_init(void)
>  {
> +	struct kmem_cache_args kmem_args = {
> +		.useroffset = offsetof(struct io_kiocb, cmd.data),
> +		.usersize = sizeof_field(struct io_kiocb, cmd.data),
> +	};
> +
>  #define __BUILD_BUG_VERIFY_OFFSET_SIZE(stype, eoffset, esize, ename) do { \
>  	BUILD_BUG_ON(offsetof(stype, ename) != eoffset); \
>  	BUILD_BUG_ON(sizeof_field(stype, ename) != esize); \
> @@ -3722,12 +3727,9 @@ static int __init io_uring_init(void)
>  	 * range, and HARDENED_USERCOPY will complain if we haven't
>  	 * correctly annotated this range.
>  	 */
> -	req_cachep = kmem_cache_create_usercopy("io_kiocb",
> -				sizeof(struct io_kiocb), 0,
> -				SLAB_HWCACHE_ALIGN | SLAB_PANIC |
> -				SLAB_ACCOUNT | SLAB_TYPESAFE_BY_RCU,
> -				offsetof(struct io_kiocb, cmd.data),
> -				sizeof_field(struct io_kiocb, cmd.data), NULL);
> +	req_cachep = kmem_cache_create("io_kiocb", sizeof(struct io_kiocb), &kmem_args,
> +				SLAB_HWCACHE_ALIGN | SLAB_PANIC | SLAB_ACCOUNT |
> +				SLAB_TYPESAFE_BY_RCU);
>  	io_buf_cachep = KMEM_CACHE(io_buffer,
>  					  SLAB_HWCACHE_ALIGN | SLAB_PANIC | SLAB_ACCOUNT);
>  
> 
> -- 
> 2.45.2
> 

-- 
Sincerely yours,
Mike.

Re: [PATCH v2 15/15] io_uring: port to struct kmem_cache_args

[Vlastimil Babka]

On 9/3/24 16:20, Christian Brauner wrote:
> Port req_cachep to struct kmem_cache_args.

Fine but doesn't bring anything on its own, wouldn't Jens want to supply the
freeptr_offset at the same time to benefit from it?

> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
>  io_uring/io_uring.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
> index 3942db160f18..d9d721d1424e 100644
> --- a/io_uring/io_uring.c
> +++ b/io_uring/io_uring.c
> @@ -3638,6 +3638,11 @@ SYSCALL_DEFINE2(io_uring_setup, u32, entries,
>  
>  static int __init io_uring_init(void)
>  {
> +	struct kmem_cache_args kmem_args = {
> +		.useroffset = offsetof(struct io_kiocb, cmd.data),
> +		.usersize = sizeof_field(struct io_kiocb, cmd.data),
> +	};
> +
>  #define __BUILD_BUG_VERIFY_OFFSET_SIZE(stype, eoffset, esize, ename) do { \
>  	BUILD_BUG_ON(offsetof(stype, ename) != eoffset); \
>  	BUILD_BUG_ON(sizeof_field(stype, ename) != esize); \
> @@ -3722,12 +3727,9 @@ static int __init io_uring_init(void)
>  	 * range, and HARDENED_USERCOPY will complain if we haven't
>  	 * correctly annotated this range.
>  	 */
> -	req_cachep = kmem_cache_create_usercopy("io_kiocb",
> -				sizeof(struct io_kiocb), 0,
> -				SLAB_HWCACHE_ALIGN | SLAB_PANIC |
> -				SLAB_ACCOUNT | SLAB_TYPESAFE_BY_RCU,
> -				offsetof(struct io_kiocb, cmd.data),
> -				sizeof_field(struct io_kiocb, cmd.data), NULL);
> +	req_cachep = kmem_cache_create("io_kiocb", sizeof(struct io_kiocb), &kmem_args,
> +				SLAB_HWCACHE_ALIGN | SLAB_PANIC | SLAB_ACCOUNT |
> +				SLAB_TYPESAFE_BY_RCU);
>  	io_buf_cachep = KMEM_CACHE(io_buffer,
>  					  SLAB_HWCACHE_ALIGN | SLAB_PANIC | SLAB_ACCOUNT);
>  
> 

Re: [PATCH v2 15/15] io_uring: port to struct kmem_cache_args

[Christian Brauner]

On Wed, Sep 04, 2024 at 10:20:31AM GMT, Vlastimil Babka wrote:
> On 9/3/24 16:20, Christian Brauner wrote:
> > Port req_cachep to struct kmem_cache_args.
> 
> Fine but doesn't bring anything on its own, wouldn't Jens want to supply the
> freeptr_offset at the same time to benefit from it?

I just picked two victims that I knew do rely or want to rely on
features that weren't there before. Afterwards, Jens can just select or
add a free pointer to struct io_kiocb.

Re: [PATCH v2 00/15] slab: add struct kmem_cache_args

[Kees Cook]

On Tue, Sep 03, 2024 at 04:20:41PM +0200, Christian Brauner wrote:
> However, I came up with an alternative using _Generic() to create a
> compatibility layer that will call the correct variant of
> kmem_cache_create() depending on whether struct kmem_cache_args is
> passed or not. That compatibility layer can stay in place until we
> updated all calls to be based on struct kmem_cache_args.

Very nice!

Yeah, this looks good to me. I'd been long pondering something like this
for kmalloc itself (though codetags have taken that in a different
direction).

All the patches look like the right way forward to me:

Reviewed-by: Kees Cook <kees@kernel.org>

-- 
Kees Cook

Re: [PATCH v2 00/15] slab: add struct kmem_cache_args

[Jens Axboe]

Hi,

This is a really nice improvement! With fear of offending someone, the
kmem_cache API has really just grown warts over the years, with
additions slapped on top without anyone taking the time to refactor it a
bit and clean it up. It's about time.

Reviewed-by: Jens Axboe <axboe@kernel.dk>

-- 
Jens Axboe

Re: [PATCH v2 00/15] slab: add struct kmem_cache_args

[Christian Brauner]

On Tue, Sep 03, 2024 at 01:25:22PM GMT, Jens Axboe wrote:
> Hi,
> 
> This is a really nice improvement! With fear of offending someone, the
> kmem_cache API has really just grown warts over the years, with
> additions slapped on top without anyone taking the time to refactor it a
> bit and clean it up. It's about time.

Thank you! I'm sure there's more to improve here in the future. :)

Re: [PATCH v2 00/15] slab: add struct kmem_cache_args

[Vlastimil Babka]

On 9/3/24 16:20, Christian Brauner wrote:
> Hey,
> 
> As discussed last week the various kmem_cache_*() functions should be
> replaced by a unified function that is based around a struct, with only
> the basic parameters passed separately.
> 
> Vlastimil already said that he would like to keep core parameters out
> of the struct: name, object size, and flags. I personally don't care
> much and would not object to moving everything into the struct but
> that's a matter of taste and I yield that decision power to the
> maintainer.
> 
> In the first version I pointed out that the choice of name is somewhat
> forced as kmem_cache_create() is taken and the only way to reuse it
> would be to replace all users in one go. Or to do a global
> sed/kmem_cache_create()/kmem_cache_create2()/g. And then introduce
> kmem_cache_setup(). That doesn't strike me as a viable option.
> 
> If we really cared about the *_create() suffix then an alternative might
> be to do a sed/kmem_cache_setup()/kmem_cache_create()/g after every user
> in the kernel is ported. I honestly don't think that's worth it but I
> wanted to at least mention it to highlight the fact that this might lead
> to a naming compromise.
> 
> However, I came up with an alternative using _Generic() to create a
> compatibility layer that will call the correct variant of
> kmem_cache_create() depending on whether struct kmem_cache_args is
> passed or not. That compatibility layer can stay in place until we
> updated all calls to be based on struct kmem_cache_args.
> 
> From a cursory grep (and not excluding Documentation mentions) we will
> have to replace 44 kmem_cache_create_usercopy() calls and about 463
> kmem_cache_create() calls which makes for a bit above 500 calls to port
> to kmem_cache_setup(). That'll probably be good work for people getting
> into kernel development.
> 
> Signed-off-by: Christian Brauner <brauner@kernel.org>

Besides the nits I replied to individual patches, LGTM and thanks for doing
the work. You could add to all:

Reviewed-by: Vlastimil Babka <vbabka@suse.cz>

Too bad it's quite late for 6.12, right? It would have to be vfs tree anyway
for that due to the kmem_cache_create_rcu() prerequisities. Otherwise I can
handle it in the slab tree after the merge window, for 6.13.

Also for any more postings please Cc the SLAB ALLOCATOR section, only a
small subset of that is completely MIA :)

Thanks,
Vlastimil

> ---
> Changes in v2:
> - Remove kmem_cache_setup() and add a compatibility layer built around
>   _Generic() so that we can keep the kmem_cache_create() name and type
>   switch on the third argument to either call __kmem_cache_create() or
>   __kmem_cache_create_args().
> - Link to v1: https://lore.kernel.org/r/20240902-work-kmem_cache_args-v1-0-27d05bc05128@kernel.org
> 
> ---
> Christian Brauner (15):
>       sl*b: s/__kmem_cache_create/do_kmem_cache_create/g
>       slab: add struct kmem_cache_args
>       slab: port kmem_cache_create() to struct kmem_cache_args
>       slab: port kmem_cache_create_rcu() to struct kmem_cache_args
>       slab: port kmem_cache_create_usercopy() to struct kmem_cache_args
>       slab: pass struct kmem_cache_args to create_cache()
>       slub: pull kmem_cache_open() into do_kmem_cache_create()
>       slab: pass struct kmem_cache_args to do_kmem_cache_create()
>       sl*b: remove rcu_freeptr_offset from struct kmem_cache
>       slab: port KMEM_CACHE() to struct kmem_cache_args
>       slab: port KMEM_CACHE_USERCOPY() to struct kmem_cache_args
>       slab: create kmem_cache_create() compatibility layer
>       file: port to struct kmem_cache_args
>       slab: remove kmem_cache_create_rcu()
>       io_uring: port to struct kmem_cache_args
> 
>  fs/file_table.c      |  11 +++-
>  include/linux/slab.h |  60 ++++++++++++++-----
>  io_uring/io_uring.c  |  14 +++--
>  mm/slab.h            |   6 +-
>  mm/slab_common.c     | 150 +++++++++++++++++++----------------------------
>  mm/slub.c            | 162 +++++++++++++++++++++++++--------------------------
>  6 files changed, 203 insertions(+), 200 deletions(-)
> ---
> base-commit: 6e016babce7c845ed015da25c7a097fa3482d95a
> change-id: 20240902-work-kmem_cache_args-e9760972c7d4
> 

Re: [PATCH v2 00/15] slab: add struct kmem_cache_args

[Christian Brauner]

On Wed, Sep 04, 2024 at 10:25:32AM GMT, Vlastimil Babka wrote:
> On 9/3/24 16:20, Christian Brauner wrote:
> > Hey,
> > 
> > As discussed last week the various kmem_cache_*() functions should be
> > replaced by a unified function that is based around a struct, with only
> > the basic parameters passed separately.
> > 
> > Vlastimil already said that he would like to keep core parameters out
> > of the struct: name, object size, and flags. I personally don't care
> > much and would not object to moving everything into the struct but
> > that's a matter of taste and I yield that decision power to the
> > maintainer.
> > 
> > In the first version I pointed out that the choice of name is somewhat
> > forced as kmem_cache_create() is taken and the only way to reuse it
> > would be to replace all users in one go. Or to do a global
> > sed/kmem_cache_create()/kmem_cache_create2()/g. And then introduce
> > kmem_cache_setup(). That doesn't strike me as a viable option.
> > 
> > If we really cared about the *_create() suffix then an alternative might
> > be to do a sed/kmem_cache_setup()/kmem_cache_create()/g after every user
> > in the kernel is ported. I honestly don't think that's worth it but I
> > wanted to at least mention it to highlight the fact that this might lead
> > to a naming compromise.
> > 
> > However, I came up with an alternative using _Generic() to create a
> > compatibility layer that will call the correct variant of
> > kmem_cache_create() depending on whether struct kmem_cache_args is
> > passed or not. That compatibility layer can stay in place until we
> > updated all calls to be based on struct kmem_cache_args.
> > 
> > From a cursory grep (and not excluding Documentation mentions) we will
> > have to replace 44 kmem_cache_create_usercopy() calls and about 463
> > kmem_cache_create() calls which makes for a bit above 500 calls to port
> > to kmem_cache_setup(). That'll probably be good work for people getting
> > into kernel development.
> > 
> > Signed-off-by: Christian Brauner <brauner@kernel.org>
> 
> Besides the nits I replied to individual patches, LGTM and thanks for doing
> the work. You could add to all:
> 
> Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
> 
> Too bad it's quite late for 6.12, right? It would have to be vfs tree anyway
> for that due to the kmem_cache_create_rcu() prerequisities. Otherwise I can

Imho, we can do it and Linus can always just tell us to go away and wait
for v6.13. But if you prefer to wait that's fine for me too.

And I don't even need to take it all through the vfs tree. I mean, I'm
happy to do it but the vfs.file branch in it's current form is stable.
So you could just 

git pull -S --no-ff git@gitolite.kernel.org:pub/scm/linux/kernel/git/vfs/vfs vfs.file

and note in the merge message that you're bringing in the branch as
prerequisites for the rework and then pull this series in.

My pull requests will go out latest on Friday before the final release.
If Linus merges it you can just send yours after.

> handle it in the slab tree after the merge window, for 6.13.
> 
> Also for any more postings please Cc the SLAB ALLOCATOR section, only a
> small subset of that is completely MIA :)

Sure.

Re: [PATCH v2 00/15] slab: add struct kmem_cache_args

[Vlastimil Babka]

On 9/4/24 10:42, Christian Brauner wrote:
> On Wed, Sep 04, 2024 at 10:25:32AM GMT, Vlastimil Babka wrote:
>> On 9/3/24 16:20, Christian Brauner wrote:
>> > Hey,
>> > 
>> > As discussed last week the various kmem_cache_*() functions should be
>> > replaced by a unified function that is based around a struct, with only
>> > the basic parameters passed separately.
>> > 
>> > Vlastimil already said that he would like to keep core parameters out
>> > of the struct: name, object size, and flags. I personally don't care
>> > much and would not object to moving everything into the struct but
>> > that's a matter of taste and I yield that decision power to the
>> > maintainer.
>> > 
>> > In the first version I pointed out that the choice of name is somewhat
>> > forced as kmem_cache_create() is taken and the only way to reuse it
>> > would be to replace all users in one go. Or to do a global
>> > sed/kmem_cache_create()/kmem_cache_create2()/g. And then introduce
>> > kmem_cache_setup(). That doesn't strike me as a viable option.
>> > 
>> > If we really cared about the *_create() suffix then an alternative might
>> > be to do a sed/kmem_cache_setup()/kmem_cache_create()/g after every user
>> > in the kernel is ported. I honestly don't think that's worth it but I
>> > wanted to at least mention it to highlight the fact that this might lead
>> > to a naming compromise.
>> > 
>> > However, I came up with an alternative using _Generic() to create a
>> > compatibility layer that will call the correct variant of
>> > kmem_cache_create() depending on whether struct kmem_cache_args is
>> > passed or not. That compatibility layer can stay in place until we
>> > updated all calls to be based on struct kmem_cache_args.
>> > 
>> > From a cursory grep (and not excluding Documentation mentions) we will
>> > have to replace 44 kmem_cache_create_usercopy() calls and about 463
>> > kmem_cache_create() calls which makes for a bit above 500 calls to port
>> > to kmem_cache_setup(). That'll probably be good work for people getting
>> > into kernel development.
>> > 
>> > Signed-off-by: Christian Brauner <brauner@kernel.org>
>> 
>> Besides the nits I replied to individual patches, LGTM and thanks for doing
>> the work. You could add to all:
>> 
>> Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
>> 
>> Too bad it's quite late for 6.12, right? It would have to be vfs tree anyway
>> for that due to the kmem_cache_create_rcu() prerequisities. Otherwise I can
> 
> Imho, we can do it and Linus can always just tell us to go away and wait
> for v6.13. But if you prefer to wait that's fine for me too.

Right, we can try. Maybe there's rc8 anyway due to conferences overlapping
with merge window otherwise, so there's more soak time in -next possible.

> And I don't even need to take it all through the vfs tree. I mean, I'm
> happy to do it but the vfs.file branch in it's current form is stable.
> So you could just 
> 
> git pull -S --no-ff git@gitolite.kernel.org:pub/scm/linux/kernel/git/vfs/vfs vfs.file
> 
> and note in the merge message that you're bringing in the branch as
> prerequisites for the rework and then pull this series in.

OK I'll do that with a v3! Thanks.

> My pull requests will go out latest on Friday before the final release.
> If Linus merges it you can just send yours after.
> 
>> handle it in the slab tree after the merge window, for 6.13.
>> 
>> Also for any more postings please Cc the SLAB ALLOCATOR section, only a
>> small subset of that is completely MIA :)
> 
> Sure.