Re: [PATCH v2 -tip] x86/percpu: Use C for arch_raw_cpu_ptr()

From: Uros Bizjak <ubizjak@gmail.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: 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>,
	Peter Zijlstra <peterz@infradead.org>,
	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: Wed, 18 Oct 2023 23:40:42 +0200	[thread overview]
Message-ID: <CAFULd4Zj5hTvATZUVYhUGrxH3fiAUWjO9C27UV_USf2H164thQ@mail.gmail.com> (raw)
In-Reply-To: <CAHk-=wia9vFmyCJPkYg0vvegF8eojLy+DxVtpfoDv-UHoWKfqQ@mail.gmail.com>

On Wed, Oct 18, 2023 at 11:11 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Wed, 18 Oct 2023 at 13:52, Uros Bizjak <ubizjak@gmail.com> wrote:
> >
> > FYI: This is the reason RMW instructions in percpu.h are not (blindly)
> > converted to C ops.  They will remain in their (volatile or not) asm
> > form because of the above reason, and due to the fact that they don't
> > combine with anything.
>
> Well, also because converting them to C would be HORRIBYL BUGGY.
>
> They absolutely have to be a single instruction. We have architectures
> that can't do rmw instructions, and then they have to do lots of extra
> horrid crud (disable interrupts or whatever) to make a percpu 'add' be
> a valid thing.
>
>
> > Actually, RMW insns are better written in asm, while simple "mov"
> > should be converted to (volatile or not) memory access.
>
> No.
>
> This remat issue has convinced me that the *only* thing that should be
> converted to a memory access is the "stable" case (which in practice
> is mainly just 'current').
>
> Because if you make them 'volatile' memory instructions, then the
> simple "mov" inline asm is simply better. It still allows CSE on the
> asm (in the "raw" form).

The ones in "raw" form are not IRQ safe and these are implemented
without volatile qualifier.

The safe variant are ones with "this" form. These were implemented as
volatile-asm and are now implemented as volatile reads. They do not
rematerialize, the number of memory accesses stays the same. They do
not CSE (volatile-asm also doesn't), but they can propagate into
follow-up instructions.

> And if you make them memory instructions _without_ the 'volatile', the
> memory access is simply buggy until we have some 'nomaterialize'
> model.

This is the reason that almost all percpu access is implemented using
this_* accessors. raw_* is a relaxed version without IRQ guarantees
that should be (and is) used in a controlled manner in a special
places:

https://elixir.bootlin.com/linux/latest/A/ident/this_cpu_read
https://elixir.bootlin.com/linux/latest/A/ident/raw_cpu_read

>
> So the *only* situation where a memory access is better is that
> 'stable' case. In all other cases they are the same or strictly worse
> than 'asm'.

No, argument propagation is non-existent with "asm" version.

Uros.