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

[Vlastimil Babka <vbabka@suse.cz>]

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
>