linux-kernel - Re: [PATCH v2 -tip] x86/percpu: Use C for arch_raw_cpu

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHk-=whA4+rg5D548K0fpJ+XpgDug2TvVBfsGpKJUid53FZgEg@mail.gmail.com>
Date:   Wed, 18 Oct 2023 16:06:40 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Uros Bizjak <ubizjak@...il.com>, peterz@...radead.org
Cc:     Nadav Amit <namit@...are.com>,
        "the arch/x86 maintainers" <x86@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Andy Lutomirski <luto@...nel.org>,
        Brian Gerst <brgerst@...il.com>,
        Denys Vlasenko <dvlasenk@...hat.com>,
        "H . Peter Anvin" <hpa@...or.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Nick Desaulniers <ndesaulniers@...gle.com>
Subject: Re: [PATCH v2 -tip] x86/percpu: Use C for arch_raw_cpu_ptr()

On Wed, 18 Oct 2023 at 15:40, Linus Torvalds
<torvalds@...ux-foundation.org> 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