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

[Jens Axboe <axboe@kernel.dk>]

On 8/27/24 9:33 AM, 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?

Yep should work.

I like this approach as a way of unifying the various ways of creating a
slab cache. Particularly as I'd want to use a free ptr myself for an
in-kernel use case that currently creates a cache with
kmem_cache_create_usercopy().

> 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...

I think we'll end up going down that route eventually anyway to cover
all cases, so may as well do a prep patch that does that first. At least
that's my opinion...

-- 
Jens Axboe