On 3/3/23, Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote: > On Fri, Mar 3, 2023 at 12:39 PM Mateusz Guzik <mjguzik@xxxxxxxxx> wrote: >> >> 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>
