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

[Linus Torvalds]

On Wed, 18 Oct 2023 at 15:40, Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> See for example __count_vm_event() vs count_vm_event().
>
> In fact, that particular use isn't even in an interrupt-safe context,
> that's an example of literally "I'd rather be fast that correct for
> certain statistics that aren't all that important".

.. just to clarify - I don't think the VM statistics code isn't even
updated from interrupts, but it is still incorrect to do
"raw_cpu_add()" even in just process context, because on architectures
where it results in separate load-op-store  instructions, you can get
preempted in the middle, and now your loaded value is some old stale
one. So when you get back, somebody else might have updated the count,
but you'll still end up doing the store using the stale value.

For VM statistics like the BALLOON_MIGRATE, nobody cares. The stats
may be incorrect, but they aren't a correctness issue, and they'll be
in the right ballpark because the race is not generally hit.

So "interrupt safe" here is not necessarily about actual interrupts
themselves directly. You *can* have that too, but it can also be about
just an interrupt causing preemption.

Anyway, again, none of this is relevant on x86, since the
single-instruction rmw percpu sequence is obviously non-interruptible,

The one oddity on x86 is that because 'xchg' always has an implied
lock, so there we *do* have a multi-instruction sequence.

And then - and *ONLY* then - the raw-vs-this matters even on x86:
"raw" just does a "load-store" pair, while "this" does a cmpxchg loop
(the latter of which is safe for both irq use and preemption because
the cmpxchg obviously re-checks the original value).

But even in that xchg case, the "volatile" part of the asm is a
complete red herring and shouldn't exist.

                Linus