Messages in this thread | | | From | Mateusz Guzik <> | Date | Fri, 3 Mar 2023 22:09:43 +0100 | Subject | Re: [PATCH v3 2/2] vfs: avoid duplicating creds in faccessat if possible |
| |
On 3/3/23, Linus Torvalds <torvalds@linux-foundation.org> wrote: > On Fri, Mar 3, 2023 at 12:39 PM Mateusz Guzik <mjguzik@gmail.com> 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>
| |