Re: [PATCH 0/3] Remove volatile from arch_raw_cpu_ptr() and revert the hacks.

Linus Torvalds
2022-03-24T18:28:43Z

On Thu, Mar 24, 2022 at 10:39 AM Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
>
> The following series does that. The assembly code looks okay. In a few
> simple test cases the this_cpu_ptr() usage is always created and is not
> moved passed preempt_enable() statement.

Hmm. I didn't think things through, and you're very correct in having
thought about preempt_enable/disable as needing barriers for that asm.

So the "volatile" does have that potential significance for that
arch_raw_cpu_ptr() in case it was what ordered it wrt preemption.

It turns out that it's all ok, because

- even without the 'volatile', arch_raw_cpu_ptr() has an *input* that
is a memory location:

: "m" (this_cpu_off)

- and prempt_{dis,en}able() fundamentally has a 'barrier()' in it

and as such it's not just random luck that the percpu thing is never
moved around preemption points. They are properly serialized even
without any 'volatile'.

But that "it's not just random luck" might be worth a comment.

So Ack on your series, but that additional comment might be worth it,
since I didn't even think of it until you mentioned it. Clearly much
too subtle.

Thanks,
Linus