Re: [PATCH 0/2] fix vma->anon_vma check for per-VMA locking; fix anon_vma memory ordering

From: Linus Torvalds <torvalds@linuxfoundation.org>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: Will Deacon <will@kernel.org>, Jann Horn <jannh@google.com>,
	paulmck@kernel.org, Andrew Morton <akpm@linux-foundation.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Suren Baghdasaryan <surenb@google.com>,
	Matthew Wilcox <willy@infradead.org>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	Andrea Parri <parri.andrea@gmail.com>,
	Boqun Feng <boqun.feng@gmail.com>,
	Nicholas Piggin <npiggin@gmail.com>,
	David Howells <dhowells@redhat.com>,
	Jade Alglave <j.alglave@ucl.ac.uk>,
	Luc Maranget <luc.maranget@inria.fr>,
	Akira Yokosawa <akiyks@gmail.com>,
	Daniel Lustig <dlustig@nvidia.com>,
	Joel Fernandes <joel@joelfernandes.org>
Subject: Re: [PATCH 0/2] fix vma->anon_vma check for per-VMA locking; fix anon_vma memory ordering
Date: Thu, 27 Jul 2023 11:01:39 -0700	[thread overview]
Message-ID: <CAHk-=whXP+27F-k7qrkmMYRZLD1Q05expC2x=esgC5qJJS7M1A@mail.gmail.com> (raw)
In-Reply-To: <af1eed90-a1d5-4da0-84a0-05e61767ab37@rowland.harvard.edu>

On Thu, 27 Jul 2023 at 10:41, Alan Stern <stern@rowland.harvard.edu> wrote:
>
> But in the presence of data races (as in the example that Will posted
> earlier), all bets are off.  So if you want to use a plain access rather
> than READ_ONCE, you need to be certain that it won't race with anything.

So in this case, the initial NULL check really is just checking for
"has the smp_store_release() happened". The reason even tearing
wouldn't matter is that seeing *any* value other than all-zeroes (aka
NULL) is already sufficient information.

Because once the smp_store_release() has been seen by this CPU, the
data race no longer exists, and the value is stable.

Which is why I argue that even without READ_ONCE() the code would
*work* due to all the circumstances around it (one of them is that we
just took a lock, after doing an optimistic check that really has no
semantic effect at all except for the "don't even take the lock if it
looks like things are going to fail").

But if we want to have the code be obvious, and not have to refer to
those kinds of arguments, I think smp_load_acquire() is the only
actual "obvious" thing to use. At that point it's no longer some chain
of "because of X, Y and Z".

              Linus