On 3/3/23, Linus Torvalds [off-list ref] wrote:

On Fri, Mar 3, 2023 at 12:39 PM Mateusz Guzik [off-list ref] wrote:

quoted

I think there is a systemic problem which comes with the kzalloc API

Well, it's not necessarily the API that is bad, but the implementation.

We could easily make kzalloc() with a constant size just expand to
kmalloc+memset, and get the behavior you want.

We already do magical things for "find the right slab bucket" part of
kmalloc too for constant sizes. It's changed over the years, but that
policy goes back a long long time. See


https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/commit/?id=95203fe78007f9ab3aebb96606473ae18c00a5a8

from the BK history tree.

Exactly because some things are worth optimizing for when the size is
known at compile time.

Maybe just extending kzalloc() similarly? Trivial and entirely untested
patch:

   --- a/include/linux/slab.h
   +++ b/include/linux/slab.h
   @@ -717,6 +717,12 @@ static inline void *kmem_cache_zalloc(struct
kmem_cache *k, gfp_t flags)
     */
    static inline __alloc_size(1) void *kzalloc(size_t size, gfp_t flags)
    {
   +    if (__builtin_constant_p(size)) {
   +            void *ret = kmalloc(size, flags);
   +            if (ret)
   +                    memset(ret, 0, size);
   +            return ret;
   +    }
        return kmalloc(size, flags | __GFP_ZERO);
    }

This may well be part of what has changed over the years. People have
done a *lot* of pseudo-automated "kmalloc+memset -> kzalloc" code
simplification. And in the process we've lost a lot of good
optimizations.

I was about to write that kzalloc can use automagic treatment. I made
a change of similar sort years back in FreeBSD
https://cgit.freebsd.org/src/commit/?id=34c538c3560591a3856e85988b0b5eefdde53b0c

The crux of the comment though was not about kzalloc (another
brainfart, apologies), but kmem_cache_zalloc -- that one is kind of
screwed as is.

Perhaps it would be unscrewed if calls could be converted to something
in the lines of kmem_cache_zalloc_ptr(cachep, flags, returnobj);

so this from __alloc_file:
        struct file *f;
	int error;

	f = kmem_cache_zalloc(filp_cachep, GFP_KERNEL);
        if (unlikely(!f))
		return ERR_PTR(-ENOMEM);

could be:
	if (unlikely(!kmem_cache_zalloc_ptr(filp_cachep, GFP_KERNEL, f))
		return ERR_PTR(-ENOMEM);

... where the macro rolls with similar treatment to the one you pasted
for kzalloc. and assigns to f.

if this sounds acceptable coccinelle could be used to do a sweep

I don't have a good name for it.

-- 
Mateusz Guzik <mjguzik gmail.com>