public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@kernel.org>
To: Borislav Petkov <bp@alien8.de>
Cc: Uros Bizjak <ubizjak@gmail.com>,
	x86@kernel.org, linux-kernel@vger.kernel.org,
	Thomas Gleixner <tglx@linutronix.de>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [PATCH] x86/asm: Use asm_inline() instead of asm() in __untagged_addr()
Date: Mon, 17 Mar 2025 10:01:12 +0100	[thread overview]
Message-ID: <Z9fk2NMBRHB9Eu5h@gmail.com> (raw)
In-Reply-To: <20250314112504.GBZ9QSEL1hgjp376ey@fat_crate.local>


* Borislav Petkov <bp@alien8.de> wrote:

> On Fri, Mar 14, 2025 at 10:30:55AM +0100, Uros Bizjak wrote:
> > Use asm_inline() to instruct the compiler that the size of asm()
> > is the minimum size of one instruction, ignoring how many instructions
> > the compiler thinks it is. ALTERNATIVE macro that expands to several
> > pseudo directives causes instruction length estimate to count
> > more than 20 instructions.
> > 
> > bloat-o-meter reports minimal code size increase
> 
> If you see an increase and *no* *other* *palpable* improvement, you 
> don't send it. It is that simple.

Sorry, but you wouldn't be saying that eliminating function calls is 
not a 'palpable improvement', had you ever profiled a recent kernel on 
a real system, on modern CPUs ... :-/

The sad reality is that the top profile is dominated by function call + 
return overhead due to CPU bug mitigation workarounds that create per 
function call overhead:

 Overhead  Shared Object               Symbol
   4.57%  [kernel]                    [k] retbleed_return_thunk <============= !!!!!!!!
   4.40%  [kernel]                    [k] unmap_page_range
   4.31%  [kernel]                    [k] _copy_to_iter
   2.46%  [kernel]                    [k] memset_orig
   2.31%  libc.so.6                   [.] __cxa_finalize

That retbleed_return_thunk overhead gets avoided every time we inline a 
simple enough function.

But GCC cannot always do proper inlining decisions due to our 
complicated ALTERNATIVE macro constructs confusing the GCC inliner:

  > > ALTERNATIVE macro that expands to several pseudo directives causes 
  > > instruction length estimate to count more than 20 instructions.
                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Note how the asm_inline() compiler feature was added by GCC at the 
kernel community's request to address such issues. (!)

So for those reasons, in my book, eliminating a function call for 
really simple single instruction inlines is an unconditional 
improvement that doesn't require futile performance measurements - it 
'only' requires assembly level code generation analysis in the 
changelog.

The reason is that requiring measurable effects for really small 
inlining changes is pretty much impossible in practice. I know, because 
I tried, and I'm good at measuring such things and I have the hardware 
to do it. Yet the per function call overhead demonstrated above in the 
profile is very much real and should not be handwaved away.

Note that this policy doesn't apply to other inlining decisions, only 
to single-instruction inline functions.

Also, having said all that, for this particular patch I'd still like to 
see a bit more GCC code generation analysis in this particular 
changelog: could you please cite a single relevant, representative 
example before/after assembly code section that demonstrates the 
effects of the inlined asm versus function call version, including the 
function that gets called?

I'm asking for that because sometimes single instructions can still 
have a halo of half a dozen of instructions that set them up or 
transform their results, so sometimes having a function call is the 
better option. Not all single-instruction asm() statements are 'simple' 
in praxis - but looking at the code generation will very much tell us 
whether it is.

Thanks,

	Ingo

  parent reply	other threads:[~2025-03-17  9:01 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-14  9:30 [PATCH] x86/asm: Use asm_inline() instead of asm() in __untagged_addr() Uros Bizjak
2025-03-14 11:25 ` Borislav Petkov
2025-03-14 13:22   ` Uros Bizjak
2025-03-14 13:34     ` Borislav Petkov
2025-03-17  9:01   ` Ingo Molnar [this message]
2025-03-17  9:28     ` Ingo Molnar
2025-03-17 18:34     ` H. Peter Anvin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Z9fk2NMBRHB9Eu5h@gmail.com \
    --to=mingo@kernel.org \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=ubizjak@gmail.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox