On Thu, Feb 6, 2025 at 6:11 PM Alexey Dobriyan <adobriyan@xxxxxxxxx> wrote:
>
> [cc lists and people]
>
> > +static inline struct filename *getname(const char __user *name)
> > +{
> > + return getname_flags(name, 0);
> > +}
>
> This may be misguided. The reason is that if function is used often enough
> then all those clears of the second argument bloat icache at the call sites.
> Uninlining moves all clears in one place, shrinking callers at the cost of
> additional function which (in this case) tail calls into another function.
> And tailcalling is quite efficient (essentially free):
>
> getname:
> xor esi, esi
> jmp getname_flags
Side note is that so happens in this case the compiler had funnier
ideas of pulling out parts of getname_flags into getname itself.
As for the general notion, it is cheaper to xor at the callsite + call
than to call + xor + jmp. Also note the total i-cache footprint absent
sufficiently fewer consumer will also be *lower* without the func.
If the routine was doing anything fancy I would not be proposing the
patch. For something which merely adds a zeroed-out argument I don't
see a legitimate reason to keep a func just to xor. It is merely 2
bytes.
Ultimately this being a minor change which I don't believe is worth
arguing about and I'll have no opinion should the patch get dropped.
This only showed up because I"m looking at whacking atomics in
filename ref handling and *that* is definitely something I'm going to
argue about.
--
Mateusz Guzik <mjguzik gmail.com>
