public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Dave Hansen <dave.hansen@intel.com>
To: Uros Bizjak <ubizjak@gmail.com>
Cc: x86@kernel.org, linux-kernel@vger.kernel.org,
	Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@kernel.org>, Borislav Petkov <bp@alien8.de>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	"H. Peter Anvin" <hpa@zytor.com>
Subject: Re: [PATCH -tip] x86/locking/atomic: Use asm_inline for atomic locking insns
Date: Fri, 28 Feb 2025 14:58:47 -0800	[thread overview]
Message-ID: <c4aca08a-95c1-48ee-b4da-55a69b74101c@intel.com> (raw)
In-Reply-To: <CAFULd4bpHGE83qc37sbh=rpGj+SFqQrsNDLzL_-NQpo6pQH3jw@mail.gmail.com>

On 2/28/25 14:31, Uros Bizjak wrote:
> On Fri, Feb 28, 2025 at 5:48 PM Dave Hansen <dave.hansen@intel.com> wrote:
>>
>> On 2/28/25 04:35, Uros Bizjak wrote:
>>> The code size of the resulting x86_64 defconfig object file increases
>>> for 33.264 kbytes, representing 1.2% code size increase:
>>>
>>>    text    data     bss     dec     hex filename
>>> 27450107        4633332  814148 32897587        1f5fa33 vmlinux-old.o
>>> 27483371        4633784  814148 32931303        1f67de7 vmlinux-new.o
>>
>> So, first of all, thank you for including some objective measurement of
>> the impact if your patches. It's much appreciated.
>>
>> But I think the patches need to come with a solid theory of why they're
>> good. The minimum bar for that, I think, is *some* kind of actual
>> real-world performance test. I'm not picky. Just *something* that spends
>> a lot of time in the kernel and ideally where a profile points at some
>> of the code you're poking here.
>>
>> I'm seriously not picky: will-it-scale, lmbench, dbench, kernel
>> compiles. *ANYTHING*. *ANY* hardware. Run it on your laptop.
>>
>> But performance patches need to come with performance *numbers*.
> 
> I don't consider this patch a performance patch, it is more a patch
> that fixes a correctness issue. The compiler estimates the number of
> instructions in the asm template wrong, so the patch instructs the
> compiler that everything in the template in fact results in a single
> instruction, no matter the pseudos there. The correct estimation then
> allows the compiler to do its job better (e.g. better scheduling,
> better inlining decisions, etc...).

Why does it matter if the compiler does its job better?

I'll let the other folks who maintain this code chime in if they think
I'm off my rocker. But, *I* consider this -- and all of these, frankly
-- performance patches.

> I don't expect some noticeable performance changes from the
> non-algorithmic patch like this. TBH, I would be surprised if they
> were outside the measurement noise. Nevertheless, I'll try to provide
> some performance numbers.

That would be appreciated, thanks!

  reply	other threads:[~2025-02-28 22:58 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-28 12:35 [PATCH -tip] x86/locking/atomic: Use asm_inline for atomic locking insns Uros Bizjak
2025-02-28 13:13 ` Uros Bizjak
2025-02-28 16:48 ` Dave Hansen
2025-02-28 22:31   ` Uros Bizjak
2025-02-28 22:58     ` Dave Hansen [this message]
2025-03-01  9:05       ` Uros Bizjak
2025-03-01 12:38         ` Borislav Petkov
2025-03-05  8:54           ` Uros Bizjak
2025-03-05 17:04             ` Linus Torvalds
2025-03-05 19:40               ` Peter Zijlstra
2025-03-05 19:47               ` Uros Bizjak
2025-03-05 22:18                 ` David Laight
2025-03-05 20:14               ` David Laight
2025-03-06 10:45                 ` Uros Bizjak
2025-03-06 13:07                   ` Uros Bizjak
2025-03-06 22:19                     ` Ingo Molnar
2025-03-08  7:22                       ` Uros Bizjak
2025-03-08 19:15               ` H. Peter Anvin
2025-03-05 19:55             ` Ingo Molnar
2025-03-05 20:13               ` Uros Bizjak
2025-03-05 20:21                 ` Ingo Molnar
2025-03-06  9:38                   ` Uros Bizjak
2025-03-05 20:20               ` Ingo Molnar
2025-03-06 10:52                 ` Dirk Gouders
2025-03-06 10:59                   ` Ingo Molnar
2025-03-05 20:36             ` Borislav Petkov
2025-03-05 21:26               ` Peter Zijlstra
2025-03-06  9:01                 ` Uros Bizjak
2025-03-06  9:43                   ` kernel: Current status of CONFIG_CC_OPTIMIZE_FOR_SIZE=y (was: Re: [PATCH -tip] x86/locking/atomic: Use asm_inline for atomic locking insns) Ingo Molnar
2025-03-06 10:37                     ` Arnd Bergmann
2025-03-06 20:37                     ` David Laight
2025-03-03 13:12       ` [PATCH -tip] x86/locking/atomic: Use asm_inline for atomic locking insns David Laight
2025-03-02 20:56   ` Uros Bizjak
2025-03-03 12:23     ` Uros Bizjak
2025-03-08 19:08   ` H. Peter Anvin
2025-03-09  7:50     ` Uros Bizjak
2025-03-09  9:46       ` David Laight
2025-03-09  9:57         ` Uros Bizjak
2025-03-06  9:57 ` Ingo Molnar
2025-03-06 10:26   ` Uros Bizjak
2025-03-06 10:38     ` Ingo Molnar
2025-03-06 10:50       ` Ingo Molnar
2025-03-06 13:56   ` Uros Bizjak

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=c4aca08a-95c1-48ee-b4da-55a69b74101c@intel.com \
    --to=dave.hansen@intel.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --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