On Wed, Sep 04, 2024 at 11:53:05AM GMT, Linus Torvalds wrote:
> On Wed, 4 Sept 2024 at 11:21, Christian Brauner <brauner@xxxxxxxxxx> 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.
