linux-kernel - Re: [RFC] LKMM: Add volatile

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: <20210604214010.GD4397@paulmck-ThinkPad-P17-Gen-1>
Date:   Fri, 4 Jun 2021 14:40:10 -0700
From:   "Paul E. McKenney" <paulmck@...nel.org>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Alan Stern <stern@...land.harvard.edu>,
        Peter Zijlstra <peterz@...radead.org>,
        Will Deacon <will@...nel.org>,
        Andrea Parri <parri.andrea@...il.com>,
        Boqun Feng <boqun.feng@...il.com>,
        Nick Piggin <npiggin@...il.com>,
        David Howells <dhowells@...hat.com>,
        Jade Alglave <j.alglave@....ac.uk>,
        Luc Maranget <luc.maranget@...ia.fr>,
        Akira Yokosawa <akiyks@...il.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-toolchains@...r.kernel.org,
        linux-arch <linux-arch@...r.kernel.org>
Subject: Re: [RFC] LKMM: Add volatile_if()

On Fri, Jun 04, 2021 at 02:27:49PM -0700, Linus Torvalds wrote:
> On Fri, Jun 4, 2021 at 1:56 PM Paul E. McKenney <paulmck@...nel.org> wrote:
> >
> > The usual way to prevent it is to use WRITE_ONCE().
> 
> The very *documentation example* for "volatile_if()" uses that WRITE_ONCE().

Whew!  ;-)

> IOW, the patch that started this discussion has this comment in it:
> 
> +/**
> + * volatile_if() - Provide a control-dependency
> + *
> + * volatile_if(READ_ONCE(A))
> + *     WRITE_ONCE(B, 1);
> + *
> + * will ensure that the STORE to B happens after the LOAD of A.
> 
> and my point is that I don't see *ANY THEORETICALLY POSSIBLE* way that
> that "volatile_if()" could not be just a perfectly regular "if ()".
> 
> Can you?

I cannot, maybe due to failure of imagination.  But please see below.

> Because we *literally* depend on the fundamental concept of causality
> to make the hardware not re-order those operations.
> 
> That is the WHOLE AND ONLY point of this whole construct: we're
> avoiding a possibly expensive hardware barrier operation, because we
> know we have a more fundamental barrier that is INHERENT TO THE
> OPERATION.
> 
> And I cannot for the life of me see how a compiler can break that
> fundamental concept of causality either.
> 
> Seriously. Tell me how a compiler could _possibly_ turn that into
> something that breaks the fundamental causal relationship. The same
> fundamental causal relationship that is the whole and only reason we
> don't need a memory barrier for the hardware.
> 
> And no, there is not a way in hell that the above can be written with
> some kind of semantically visible speculative store without the
> compiler being a total pile of garbage that wouldn't be usable for
> compiling a kernel with.
> 
> If your argument is that the compiler can magically insert speculative
> stores that can then be overwritten later, then MY argument is that
> such a compiler could do that for *ANYTHING*. "volatile_if()" wouldn't
> save us.
> 
> If that's valid compiler behavior in your opinion, then we have
> exactly two options:
> 
>  (a) give up
> 
>  (b) not use that broken garbage of a compiler.
> 
> So I can certainly accept the patch with the simpler implementation of
> "volatile_if()", but dammit, I want to see an actual real example
> arguing for why it would be relevant and why the compiler would need
> our help.
> 
> Because the EXACT VERY EXAMPLE that was in the patch as-is sure as
> hell is no such thing.
> 
> If the intent is to *document* that "this conditional is part of a
> load-conditional-store memory ordering pattern, then that is one
> thing. But if that's the intent, then we might as well just write it
> as
> 
>     #define volatile_if(x) if (x)
> 
> and add a *comment* about why this kind of sequence doesn't need a
> memory barrier.
> 
> I'd much rather have that kind of documentation, than have barriers
> that are magical for theoretical compiler issues that aren't real, and
> don't have any grounding in reality.
> 
> Without a real and valid example of how this could matter, this is
> just voodoo programming.
> 
> We don't actually need to walk three times widdershins around the
> computer before compiling the kernel.That's not how kernel development
> works.
> 
> And we don't need to add a "volatile_if()" with magical barriers that
> have no possibility of having real semantic meaning.
> 
> So I want to know what the semantic meaning of volatile_if() would be,
> and why it fixes anything that a plain "if()" wouldn't. I want to see
> the sequence where that "volatile_if()" actually *fixes* something.

Here is one use case:

	volatile_if(READ_ONCE(A)) {
		WRITE_ONCE(B, 1);
		do_something();
	} else {
		WRITE_ONCE(B, 1);
		do_something_else();
	}

With plain "if", the compiler is within its rights to do this:

	tmp = READ_ONCE(A);
	WRITE_ONCE(B, 1);
	if (tmp)
		do_something();
	else
		do_something_else();

On x86, still no problem.  But weaker hardware could now reorder the
store to B before the load from A.  With volatile_if(), this reordering
would be prevented.

							Thanx, Paul