linux-kernel - Re: [PATCH next 1/1] fs: Mark get_sigset_argpack() __always

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHk-=wicUO4WaEE6b010icQPpq+Gk_ZK5V2hF2iBQe-FqmBc3Q@mail.gmail.com>
Date: Sat, 8 Feb 2025 10:53:38 -0800
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: David Laight <david.laight.linux@...il.com>
Cc: Al Viro <viro@...iv.linux.org.uk>, linux-fsdevel@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>
Subject: Re: [PATCH next 1/1] fs: Mark get_sigset_argpack() __always_inline

On Sat, 8 Feb 2025 at 07:14, David Laight <david.laight.linux@...il.com> wrote:
>
> Since the function is 'hot enough' to worry about avoiding the
> overhead of copy_from_user() it must be worth forcing it to be
> inlined.

Hmm. gcc certainly inlines this one for me regardless.

So it's either a gcc version issue (I have gcc-14.2.1), or it's some
build configuration thing that makes the function big enough in your
case that gcc decides not to inline things. Do you perhaps have some
debugging options enabled? At that point, inlining is the least of all
problems.

> I'd guess that gcc is counting up the number of lines in the asm again.

If that's the case, then we should probably make sure that the
relevant inline asms are marked 'inline', which makes gcc estimate it
to be minimal:

    https://gcc.gnu.org/onlinedocs/gcc/Size-of-an-asm.html

but it turns out that with the compiler bug workarounds we have that
wrapper macro for this case:

   #define asm_goto_output(x...) asm volatile goto(x)

which I guess we could just add the inline to.

Bah. I think we should fix those things, but in the meantime your
patch certainly isn't wrong either. So ack.

And while looking at this, I note that I made get_sigset_argpack() do
the masked user access thing, but didn't do the same pattern for
get_compat_sigset_argpack()

          Linus