From: Boqun Feng <boqun.feng@gmail.com>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: Daniel Lustig <dlustig@nvidia.com>,
"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
Andrea Parri <parri.andrea@gmail.com>,
Luc Maranget <luc.maranget@inria.fr>,
Jade Alglave <j.alglave@ucl.ac.uk>,
Nicholas Piggin <npiggin@gmail.com>,
Peter Zijlstra <peterz@infradead.org>,
Will Deacon <will.deacon@arm.com>,
David Howells <dhowells@redhat.com>,
Palmer Dabbelt <palmer@dabbelt.com>,
Kernel development list <linux-kernel@vger.kernel.org>
Subject: Re: Unlock-lock questions and the Linux Kernel Memory Model
Date: Thu, 30 Nov 2017 16:55:09 +0800 [thread overview]
Message-ID: <20171130085509.GA9516@tardis> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1711291409440.1369-100000@iolanthe.rowland.org>
[-- Attachment #1: Type: text/plain, Size: 2961 bytes --]
On Wed, Nov 29, 2017 at 02:44:37PM -0500, Alan Stern wrote:
> On Wed, 29 Nov 2017, Daniel Lustig wrote:
>
> > While we're here, let me ask about another test which isn't directly
> > about unlock/lock but which is still somewhat related to this
> > discussion:
> >
> > "MP+wmb+xchg-acq" (or some such)
> >
> > {}
> >
> > P0(int *x, int *y)
> > {
> > WRITE_ONCE(*x, 1);
> > smp_wmb();
> > WRITE_ONCE(*y, 1);
> > }
> >
> > P1(int *x, int *y)
> > {
> > r1 = atomic_xchg_relaxed(y, 2);
> > r2 = smp_load_acquire(y);
> > r3 = READ_ONCE(*x);
> > }
> >
> > exists (1:r1=1 /\ 1:r2=2 /\ 1:r3=0)
> >
> > C/C++ would call the atomic_xchg_relaxed part of a release sequence
> > and hence would forbid this outcome.
> >
> > x86 and Power would forbid this. ARM forbids this via a special-case
> > rule in the memory model, ordering atomics with later load-acquires.
> >
> > RISC-V, however, wouldn't forbid this by default using RCpc or RCsc
> > atomics for smp_load_acquire(). It's an "fri; rfi" type of pattern,
> > because xchg doesn't have an inherent internal data dependency.
> >
> > If the Linux memory model is going to forbid this outcome, then
> > RISC-V would either need to use fences instead, or maybe we'd need to
> > add a special rule to our memory model similarly. This is one detail
> > where RISC-V is still actively deciding what to do.
> >
> > Have you all thought about this test before? Any idea which way you
> > are leaning regarding the outcome above?
>
> Good questions. Currently the LKMM allows this, and I think it should
> because xchg doesn't have a dependency from its read to its write.
>
> On the other hand, herd isn't careful enough in the way it implements
> internal dependencies for RMW operations. If we change
> atomic_xchg_relaxed(y, 2) to atomic_inc(y) and remove r1 from the test:
>
> C MP+wmb+inc-acq
>
> {}
>
> P0(int *x, int *y)
> {
> WRITE_ONCE(*x, 1);
> smp_wmb();
> WRITE_ONCE(*y, 1);
> }
>
> P1(int *x, int *y)
> {
> atomic_inc(y);
> r2 = smp_load_acquire(y);
> r3 = READ_ONCE(*x);
> }
>
> exists (1:r2=2 /\ 1:r3=0)
>
> then the test _should_ be forbidden, but it isn't -- herd doesn't
> realize that all atomic RMW operations other than xchg must have a
> dependency (either data or control) between their internal read and
> write.
>
> (Although the smp_load_acquire is allowed to execute before the write
> part of the atomic_inc, it cannot execute before the read part. I
> think a similar argument applies even on ARM.)
>
But in case of AMOs, which directly send the addition request to memory
controller, so there wouldn't be any read part or even write part of the
atomic_inc() executed by CPU. Would this be allowed then?
Regards,
Boqun
> Luc, consider this a bug report. :-)
>
> Alan
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
next prev parent reply other threads:[~2017-11-30 8:53 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <4118cdbe-c396-08b9-a3e3-a0a6491b82fa@nvidia.com>
2017-11-27 21:16 ` Unlock-lock questions and the Linux Kernel Memory Model Alan Stern
2017-11-27 23:28 ` Daniel Lustig
2017-11-28 9:44 ` Peter Zijlstra
2017-11-28 9:58 ` Peter Zijlstra
2017-11-29 19:04 ` Daniel Lustig
2017-11-29 19:33 ` Paul E. McKenney
2017-11-29 19:44 ` Alan Stern
2017-11-30 8:55 ` Boqun Feng [this message]
2017-11-30 9:15 ` Peter Zijlstra
2017-11-30 15:46 ` Alan Stern
2017-12-01 2:46 ` Boqun Feng
2017-12-01 15:32 ` Alan Stern
2017-12-01 16:17 ` Daniel Lustig
2017-12-01 16:24 ` Will Deacon
2017-12-01 17:18 ` Alan Stern
2017-11-29 19:46 ` Peter Zijlstra
2017-11-29 19:53 ` Alan Stern
2017-11-29 20:42 ` Paul E. McKenney
2017-11-29 22:18 ` Daniel Lustig
2017-11-29 22:59 ` Paul E. McKenney
2017-11-30 15:20 ` Alan Stern
2017-11-30 16:14 ` Paul E. McKenney
2017-11-30 16:25 ` Peter Zijlstra
2017-11-30 16:39 ` Paul E. McKenney
2017-11-30 16:41 ` Will Deacon
2017-11-30 16:54 ` Paul E. McKenney
2017-11-30 17:04 ` Will Deacon
2017-11-30 17:56 ` Alan Stern
2017-11-30 10:02 ` Will Deacon
2017-11-29 19:58 ` Peter Zijlstra
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171130085509.GA9516@tardis \
--to=boqun.feng@gmail.com \
--cc=dhowells@redhat.com \
--cc=dlustig@nvidia.com \
--cc=j.alglave@ucl.ac.uk \
--cc=linux-kernel@vger.kernel.org \
--cc=luc.maranget@inria.fr \
--cc=npiggin@gmail.com \
--cc=palmer@dabbelt.com \
--cc=parri.andrea@gmail.com \
--cc=paulmck@linux.vnet.ibm.com \
--cc=peterz@infradead.org \
--cc=stern@rowland.harvard.edu \
--cc=will.deacon@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.