Re: [PATCH] [RFC] mm: add kmem_cache_create_rcu()

[Christian Brauner <brauner@kernel.org>]

On Tue, Aug 27, 2024 at 05:33:54PM GMT, Jann Horn wrote:
> On Tue, Aug 27, 2024 at 4:05 PM Vlastimil Babka <vbabka@suse.cz> wrote:
> > On 8/26/24 18:04, Christian Brauner wrote:
> > > diff --git a/include/linux/slab.h b/include/linux/slab.h
> > > index eb2bf4629157..fc3c3cc9f689 100644
> > > --- a/include/linux/slab.h
> > > +++ b/include/linux/slab.h
> > > @@ -242,6 +242,10 @@ 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 offset,
> > > +                                      slab_flags_t flags,
> > > +                                      void (*ctor)(void *));
> >
> > I wonder if there's a way to do this in a more generic way, we'd now have 3
> > variants and neither supports everything (what about both rcu offset and
> > usercopy?).
> 
> The "pass all arguments as a struct" pattern might not look too bad
> (or alternatively, "pass all optional arguments as a struct"), as long
> as it's fine for unused arguments to be zero-initialized? Like:
> 
> struct kcr_options {
>     const char *name;
>     unsigned int size;
>     ...
> };
> 
> struct kmem_cache *kmem_cache_create(struct kcr_options opt);
> 
> void blah() {
>   kmem_cache_create((struct kcr_options){
>     .name = "blah",
>     .size = ...,
>     ...
>   });
> }
> 
> I think maybe we can do this now that the kernel is C11?
> 
> But I guess if we want to allow leaving out the freeptr_offset
> parameter, we'd have to have a flag to say whether the freeptr_offset
> parameter value should be used... Maybe my proposal is too overly
> fancy...

Fwiw, I suspect that most users would probably just do

// top of file

static struct kmem_cache_args filp_cache = {
	.name = "filp",
};

and then 

void __init files_init(void)
{
	filp_cachep = kmem_cache_create(&filp_cache);
	percpu_counter_init(&nr_files, 0, GFP_KERNEL);
}

but I really would love to leave that intense refactoring to the
maintainers.