linux-kernel - Re: [PATCH] vfs: inline getname()

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAGudoHHT1bULKPy8BGnEM3gshOuLwh=i1nwuJz-vHfX0UP-LZQ@mail.gmail.com>
Date: Thu, 6 Feb 2025 19:03:53 +0100
From: Mateusz Guzik <mjguzik@...il.com>
To: Alexey Dobriyan <adobriyan@...il.com>
Cc: brauner@...nel.org, viro@...iv.linux.org.uk, jack@...e.cz, 
	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH] vfs: inline getname()

On Thu, Feb 6, 2025 at 6:11 PM Alexey Dobriyan <adobriyan@...il.com> 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>