From: Andrea Parri <parri.andrea@gmail.com> To: Alan Stern <stern@rowland.harvard.edu> Cc: Palmer Dabbelt <palmer@sifive.com>, Albert Ou <albert@sifive.com>, Daniel Lustig <dlustig@nvidia.com>, Will Deacon <will.deacon@arm.com>, Peter Zijlstra <peterz@infradead.org>, 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>, Paul McKenney <paulmck@linux.vnet.ibm.com>, Akira Yokosawa <akiyks@gmail.com>, Ingo Molnar <mingo@kernel.org>, Linus Torvalds <torvalds@linux-foundation.org>, linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 2/2] riscv/atomic: Strengthen implementations with fences Date: Fri, 9 Mar 2018 17:57:58 +0100 [thread overview] Message-ID: <20180309165758.GA24626@andrea> (raw) In-Reply-To: <Pine.LNX.4.44L0.1803091129420.2256-100000@iolanthe.rowland.org> On Fri, Mar 09, 2018 at 11:39:11AM -0500, Alan Stern wrote: > On Fri, 9 Mar 2018, Andrea Parri wrote: > > > Atomics present the same issue with locking: release and acquire > > variants need to be strengthened to meet the constraints defined > > by the Linux-kernel memory consistency model [1]. > > > > Atomics present a further issue: implementations of atomics such > > as atomic_cmpxchg() and atomic_add_unless() rely on LR/SC pairs, > > which do not give full-ordering with .aqrl; for example, current > > implementations allow the "lr-sc-aqrl-pair-vs-full-barrier" test > > below to end up with the state indicated in the "exists" clause. > > > > In order to "synchronize" LKMM and RISC-V's implementation, this > > commit strengthens the implementations of the atomics operations > > by replacing .rl and .aq with the use of ("lightweigth") fences, > > and by replacing .aqrl LR/SC pairs in sequences such as: > > > > 0: lr.w.aqrl %0, %addr > > bne %0, %old, 1f > > ... > > sc.w.aqrl %1, %new, %addr > > bnez %1, 0b > > 1: > > > > with sequences of the form: > > > > 0: lr.w %0, %addr > > bne %0, %old, 1f > > ... > > sc.w.rl %1, %new, %addr /* SC-release */ > > bnez %1, 0b > > fence rw, rw /* "full" fence */ > > 1: > > > > following Daniel's suggestion. > > > > These modifications were validated with simulation of the RISC-V > > memory consistency model. > > > > C lr-sc-aqrl-pair-vs-full-barrier > > > > {} > > > > P0(int *x, int *y, atomic_t *u) > > { > > int r0; > > int r1; > > > > WRITE_ONCE(*x, 1); > > r0 = atomic_cmpxchg(u, 0, 1); > > r1 = READ_ONCE(*y); > > } > > > > P1(int *x, int *y, atomic_t *v) > > { > > int r0; > > int r1; > > > > WRITE_ONCE(*y, 1); > > r0 = atomic_cmpxchg(v, 0, 1); > > r1 = READ_ONCE(*x); > > } > > > > exists (u=1 /\ v=1 /\ 0:r1=0 /\ 1:r1=0) > > There's another aspect to this imposed by the LKMM, and I'm not sure > whether your patch addresses it. You add a fence after the cmpxchg > operation but nothing before it. So what would happen with the > following litmus test (which the LKMM forbids)? Available RISC-V memory model formalizations forbid it; an intuitive explanation could probably be derived by paralleling the argument for arm64, as pointed out by Daniel at: https://marc.info/?l=linux-kernel&m=151994289015267&w=2 Andrea > > C SB-atomic_cmpxchg-mb > > {} > > P0(int *x, int *y) > { > int r0; > > WRITE_ONCE(*x, 1); > r0 = atomic_cmpxchg(y, 0, 0); > } > > P1(int *x, int *y) > { > int r1; > > WRITE_ONCE(*y, 1); > smp_mb(); > r1 = READ_ONCE(*x); > } > > exists (0:r0=0 /\ 1:r1=0) > > This is yet another illustration showing that full fences are stronger > than cominations of release + acquire. > > Alan Stern >
WARNING: multiple messages have this Message-ID (diff)
From: parri.andrea@gmail.com (Andrea Parri) To: linux-riscv@lists.infradead.org Subject: [PATCH v2 2/2] riscv/atomic: Strengthen implementations with fences Date: Fri, 9 Mar 2018 17:57:58 +0100 [thread overview] Message-ID: <20180309165758.GA24626@andrea> (raw) In-Reply-To: <Pine.LNX.4.44L0.1803091129420.2256-100000@iolanthe.rowland.org> On Fri, Mar 09, 2018 at 11:39:11AM -0500, Alan Stern wrote: > On Fri, 9 Mar 2018, Andrea Parri wrote: > > > Atomics present the same issue with locking: release and acquire > > variants need to be strengthened to meet the constraints defined > > by the Linux-kernel memory consistency model [1]. > > > > Atomics present a further issue: implementations of atomics such > > as atomic_cmpxchg() and atomic_add_unless() rely on LR/SC pairs, > > which do not give full-ordering with .aqrl; for example, current > > implementations allow the "lr-sc-aqrl-pair-vs-full-barrier" test > > below to end up with the state indicated in the "exists" clause. > > > > In order to "synchronize" LKMM and RISC-V's implementation, this > > commit strengthens the implementations of the atomics operations > > by replacing .rl and .aq with the use of ("lightweigth") fences, > > and by replacing .aqrl LR/SC pairs in sequences such as: > > > > 0: lr.w.aqrl %0, %addr > > bne %0, %old, 1f > > ... > > sc.w.aqrl %1, %new, %addr > > bnez %1, 0b > > 1: > > > > with sequences of the form: > > > > 0: lr.w %0, %addr > > bne %0, %old, 1f > > ... > > sc.w.rl %1, %new, %addr /* SC-release */ > > bnez %1, 0b > > fence rw, rw /* "full" fence */ > > 1: > > > > following Daniel's suggestion. > > > > These modifications were validated with simulation of the RISC-V > > memory consistency model. > > > > C lr-sc-aqrl-pair-vs-full-barrier > > > > {} > > > > P0(int *x, int *y, atomic_t *u) > > { > > int r0; > > int r1; > > > > WRITE_ONCE(*x, 1); > > r0 = atomic_cmpxchg(u, 0, 1); > > r1 = READ_ONCE(*y); > > } > > > > P1(int *x, int *y, atomic_t *v) > > { > > int r0; > > int r1; > > > > WRITE_ONCE(*y, 1); > > r0 = atomic_cmpxchg(v, 0, 1); > > r1 = READ_ONCE(*x); > > } > > > > exists (u=1 /\ v=1 /\ 0:r1=0 /\ 1:r1=0) > > There's another aspect to this imposed by the LKMM, and I'm not sure > whether your patch addresses it. You add a fence after the cmpxchg > operation but nothing before it. So what would happen with the > following litmus test (which the LKMM forbids)? Available RISC-V memory model formalizations forbid it; an intuitive explanation could probably be derived by paralleling the argument for arm64, as pointed out by Daniel at: https://marc.info/?l=linux-kernel&m=151994289015267&w=2 Andrea > > C SB-atomic_cmpxchg-mb > > {} > > P0(int *x, int *y) > { > int r0; > > WRITE_ONCE(*x, 1); > r0 = atomic_cmpxchg(y, 0, 0); > } > > P1(int *x, int *y) > { > int r1; > > WRITE_ONCE(*y, 1); > smp_mb(); > r1 = READ_ONCE(*x); > } > > exists (0:r0=0 /\ 1:r1=0) > > This is yet another illustration showing that full fences are stronger > than cominations of release + acquire. > > Alan Stern >
next prev parent reply other threads:[~2018-03-09 16:58 UTC|newest] Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-03-09 12:13 [PATCH v2 2/2] riscv/atomic: Strengthen implementations with fences Andrea Parri 2018-03-09 12:13 ` Andrea Parri 2018-03-09 16:39 ` Alan Stern 2018-03-09 16:39 ` Alan Stern 2018-03-09 16:57 ` Andrea Parri [this message] 2018-03-09 16:57 ` Andrea Parri 2018-03-09 17:56 ` Palmer Dabbelt 2018-03-09 17:56 ` Palmer Dabbelt 2018-03-09 18:36 ` Andrea Parri 2018-03-09 18:36 ` Andrea Parri 2018-03-09 18:54 ` Palmer Dabbelt 2018-03-09 18:54 ` Palmer Dabbelt 2018-03-09 21:30 ` Andrea Parri 2018-03-09 21:30 ` Andrea Parri 2018-03-09 22:57 ` Palmer Dabbelt 2018-03-09 22:57 ` Palmer Dabbelt 2018-03-10 0:21 ` Daniel Lustig 2018-03-10 0:21 ` Daniel Lustig 2018-03-10 14:18 ` Andrea Parri 2018-03-10 14:18 ` Andrea Parri 2018-03-12 6:13 ` Boqun Feng 2018-03-12 6:13 ` Boqun Feng 2018-03-13 13:27 ` Luc Maranget 2018-03-13 13:27 ` Luc Maranget
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=20180309165758.GA24626@andrea \ --to=parri.andrea@gmail.com \ --cc=akiyks@gmail.com \ --cc=albert@sifive.com \ --cc=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=linux-riscv@lists.infradead.org \ --cc=luc.maranget@inria.fr \ --cc=mingo@kernel.org \ --cc=npiggin@gmail.com \ --cc=palmer@sifive.com \ --cc=paulmck@linux.vnet.ibm.com \ --cc=peterz@infradead.org \ --cc=stern@rowland.harvard.edu \ --cc=torvalds@linux-foundation.org \ --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: linkBe 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.