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

[Christian Brauner <brauner@kernel.org>]

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.