linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Uros Bizjak <ubizjak@gmail.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: peterz@infradead.org, Nadav Amit <namit@vmware.com>,
	"the arch/x86 maintainers" <x86@kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Andy Lutomirski <luto@kernel.org>,
	Brian Gerst <brgerst@gmail.com>,
	Denys Vlasenko <dvlasenk@redhat.com>,
	"H . Peter Anvin" <hpa@zytor.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	Nick Desaulniers <ndesaulniers@google.com>
Subject: Re: [PATCH v2 -tip] x86/percpu: Use C for arch_raw_cpu_ptr()
Date: Thu, 19 Oct 2023 19:21:22 +0200	[thread overview]
Message-ID: <CAFULd4b91Tr9Q2p4a20eusC+QO6O81gxY+nP-zpFiFKGTmLpYg@mail.gmail.com> (raw)
In-Reply-To: <CAHk-=wijmmRB7-ZeT-sdxCSUoB83Lb5dnN7a7mCcH3cRw_aghQ@mail.gmail.com>

On Thu, Oct 19, 2023 at 7:00 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Thu, 19 Oct 2023 at 00:04, Uros Bizjak <ubizjak@gmail.com> wrote:
> >
> > Let me explain how the compiler handles volatile.
>
> We're talking past each other.
>
> You are talking about the volatile *memory* ops, and the the
> difference that "raw" vs "this" would cause with and without the
> "volatile".
>
> While *I* am now convinced that the memory ops aren't even an option,
> because they will generate worse code, because pretty much all users
> use the "this" version (which would have to use volatile),

Please see [1]. Even with volatile access, with memory ops the
compiler can propagate operands, resulting in ~8k code size reduction,
and many hundreds (if not thousands) MOVs propagated into subsequent
instructions. Please note many code examples in [1]. This is not
possible with the asm variant.

[1] https://lore.kernel.org/lkml/20231004192404.31733-1-ubizjak@gmail.com/

> Because if we just stick with inline asms, the need for "volatile"
> simply goes away.

No, the compiler is then free to remove or duplicate the asm (plus
other unwanted optimizations), please see the end of chapter 6.47.2.1
in [2].

[2] https://gcc.gnu.org/onlinedocs/gcc-13.2.0/gcc/Extended-Asm.html#Volatile-1

> The existing volatile on those percpu inline asms is *wrong*. It's a
> historical mistake.

Please see above.

> And with just a plain non-volatile inline asm, the inline asm wins.

Please see [1] for the code propagation argument.

> It doesn't have the (bad) read-once behavior of a volatile memory op.
>
> And it also doesn't have the (horrible correctness issue)
> rematerialization behavior of a non-volatile memory op.

Unfortunately, it does. Without volatile, asm can be rematerialized in
the same way as it can be CSEd. OTOH, the memory op with memory-ops
approach is casted to volatile in this_* case, so it for sure won't
get rematerialized.

> A compiler that were to rematerializes an inline asm (instead of
> spilling) would be a bad joke. That's not an optimization, that's just
> a crazy bad compiler with a code generation bug.

But that is what the compiler does without volatile.

Thanks,
Uros.

  reply	other threads:[~2023-10-19 17:21 UTC|newest]

Thread overview: 106+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-10 16:42 [PATCH v2 -tip] x86/percpu: Use C for arch_raw_cpu_ptr() Uros Bizjak
2023-10-10 17:32 ` Linus Torvalds
2023-10-10 18:22   ` Uros Bizjak
2023-10-10 18:25     ` Nadav Amit
2023-10-10 18:42       ` Linus Torvalds
2023-10-10 18:37     ` Linus Torvalds
2023-10-10 18:41       ` Uros Bizjak
2023-10-10 18:52         ` Linus Torvalds
2023-10-11  7:27           ` Uros Bizjak
2023-10-11  7:45             ` Uros Bizjak
2023-10-11 19:40               ` Linus Torvalds
2023-10-11 18:42           ` Uros Bizjak
2023-10-11 19:51             ` Linus Torvalds
2023-10-11 19:52               ` Linus Torvalds
2023-10-11 20:00               ` Uros Bizjak
2023-10-11 22:37               ` Ingo Molnar
2023-10-11 23:15                 ` H. Peter Anvin
2023-10-12  1:35                   ` Josh Poimboeuf
2023-10-12  6:19                     ` Ingo Molnar
2023-10-12 16:08                       ` Josh Poimboeuf
2023-10-12 17:59                         ` Ingo Molnar
2023-10-12 21:30                           ` Josh Poimboeuf
2023-10-13 10:52                             ` Ingo Molnar
2023-10-11  7:41       ` Nadav Amit
2023-10-11 19:37         ` Linus Torvalds
2023-10-11 21:32           ` Uros Bizjak
2023-10-11 21:54             ` Linus Torvalds
2023-10-12 15:19               ` Nadav Amit
2023-10-12 16:33                 ` Uros Bizjak
2023-10-12 16:55                   ` Uros Bizjak
2023-10-12 17:10                     ` Linus Torvalds
2023-10-12 17:47                       ` Linus Torvalds
2023-10-12 18:01                         ` Uros Bizjak
2023-10-13  9:38                           ` Uros Bizjak
2023-10-13 11:53                             ` Uros Bizjak
2023-10-13 16:38                               ` Linus Torvalds
2023-10-12 17:52                       ` Uros Bizjak
2023-11-20  9:39                       ` Use %a asm operand modifier to obtain %rip-relative addressing Uros Bizjak
2023-10-12 16:56                   ` [PATCH v2 -tip] x86/percpu: Use C for arch_raw_cpu_ptr() Linus Torvalds
2023-10-12 17:16                 ` Linus Torvalds
2023-10-12 19:32                   ` Nadav Amit
2023-10-12 19:40                     ` Linus Torvalds
2023-10-16 18:52                 ` Uros Bizjak
2023-10-16 19:24                   ` Linus Torvalds
2023-10-16 20:35                     ` Nadav Amit
2023-10-16 20:59                       ` Linus Torvalds
2023-10-16 23:02                       ` Linus Torvalds
2023-10-16 23:14                         ` Linus Torvalds
2023-10-17  7:23                         ` Nadav Amit
2023-10-17 19:00                           ` Linus Torvalds
2023-10-17 19:11                             ` Uros Bizjak
2023-10-17 21:05                               ` Uros Bizjak
2023-10-17 21:53                                 ` Linus Torvalds
2023-10-17 22:06                                   ` Nadav Amit
2023-10-17 22:29                                     ` Nadav Amit
2023-10-18  7:46                                   ` Uros Bizjak
2023-10-18  9:04                                     ` Uros Bizjak
2023-10-18 10:54                                       ` Nadav Amit
2023-10-18 12:14                                         ` Uros Bizjak
2023-10-18 13:15                                           ` Uros Bizjak
2023-10-18 14:46                                             ` Nadav Amit
2023-10-18 15:17                                               ` Uros Bizjak
2023-10-18 16:03                                                 ` Nadav Amit
2023-10-18 16:26                                                   ` Linus Torvalds
2023-10-18 17:23                                                     ` Uros Bizjak
2023-10-18 18:11                                                       ` Linus Torvalds
2023-10-18 18:08                                                     ` Uros Bizjak
2023-10-18 18:15                                                       ` Linus Torvalds
2023-10-18 18:26                                                         ` Uros Bizjak
2023-10-18 19:33                                                           ` Uros Bizjak
2023-10-18 20:17                                                             ` Nadav Amit
2023-10-18 20:22                                                             ` Linus Torvalds
2023-10-18 20:34                                                               ` Linus Torvalds
2023-10-18 20:51                                                                 ` Uros Bizjak
2023-10-18 21:09                                                                   ` Uros Bizjak
2023-10-18 21:10                                                                   ` Linus Torvalds
2023-10-18 21:40                                                                     ` Uros Bizjak
2023-10-18 22:40                                                                       ` Linus Torvalds
2023-10-18 23:06                                                                         ` Linus Torvalds
2023-10-19  7:04                                                                         ` Uros Bizjak
2023-10-19 16:59                                                                           ` Linus Torvalds
2023-10-19 17:21                                                                             ` Uros Bizjak [this message]
2023-10-19 18:06                                                                               ` Linus Torvalds
2023-10-19 18:16                                                                                 ` Uros Bizjak
2023-10-19 18:49                                                                                   ` Linus Torvalds
2023-10-19 19:07                                                                                     ` Linus Torvalds
2023-10-20  7:57                                                                                       ` Uros Bizjak
2023-10-19 21:04                                                                                   ` Linus Torvalds
2023-10-19 22:39                                                                                     ` Linus Torvalds
2023-10-20  8:08                                                                                       ` Uros Bizjak
2023-10-19  8:44                                                                         ` Peter Zijlstra
2023-10-19  8:54                                                                         ` Peter Zijlstra
2023-10-19 17:04                                                                           ` Linus Torvalds
2023-10-19 18:13                                                                             ` Peter Zijlstra
2023-10-19 18:22                                                                               ` Linus Torvalds
2023-10-19 18:37                                                                                 ` Uros Bizjak
2023-10-19  9:07                                                                         ` Peter Zijlstra
2023-10-19  9:23                                                                           ` Uros Bizjak
2023-10-18 20:42                                                               ` Uros Bizjak
2023-10-19 16:32                                                               ` Uros Bizjak
2023-10-19 17:08                                                                 ` Linus Torvalds
2023-10-18 18:29                                                       ` Nadav Amit
2023-10-18 16:12                                             ` Linus Torvalds
2023-10-18 17:07                                               ` Uros Bizjak
2023-10-18 18:01                                                 ` Linus Torvalds
2023-10-16 21:09                   ` 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=CAFULd4b91Tr9Q2p4a20eusC+QO6O81gxY+nP-zpFiFKGTmLpYg@mail.gmail.com \
    --to=ubizjak@gmail.com \
    --cc=brgerst@gmail.com \
    --cc=dvlasenk@redhat.com \
    --cc=hpa@zytor.com \
    --cc=jpoimboe@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=namit@vmware.com \
    --cc=ndesaulniers@google.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --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;
as well as URLs for NNTP newsgroup(s).