From: Andrea Parri <parri.andrea@gmail.com>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
Akira Yokosawa <akiyks@gmail.com>,
Kernel development list <linux-kernel@vger.kernel.org>,
mingo@kernel.org, Will Deacon <will.deacon@arm.com>,
peterz@infradead.org, boqun.feng@gmail.com, npiggin@gmail.com,
dhowells@redhat.com, Jade Alglave <j.alglave@ucl.ac.uk>,
Luc Maranget <luc.maranget@inria.fr>,
Patrick Bellasi <patrick.bellasi@arm.com>
Subject: Re: [PATCH] tools/memory-model: remove rb-dep, smp_read_barrier_depends, and lockless_dereference
Date: Tue, 20 Feb 2018 10:33:47 +0100 [thread overview]
Message-ID: <20180220093346.GA5505@andrea> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1802191158320.1361-100000@iolanthe.rowland.org>
On Mon, Feb 19, 2018 at 12:14:45PM -0500, Alan Stern wrote:
> On Sat, 17 Feb 2018, Andrea Parri wrote:
>
> > > Akira's observation about READ_ONCE extends to all (annotated) loads. In
> > > fact, it also applies to loads corresponding to unsuccessful RMW operations;
> > > consider, for example, the following variation of MP+onceassign+derefonce:
> > >
> > > C T
> > >
> > > {
> > > y=z;
> > > z=0;
> > > }
> > >
> > > P0(int *x, int **y)
> > > {
> > > WRITE_ONCE(*x, 1);
> > > smp_store_release(y, x);
> > > }
> > >
> > > P1(int **y, int *z)
> > > {
> > > int *r0;
> > > int r1;
> > >
> > > r0 = cmpxchg_relaxed(y, z, z);
> > > r1 = READ_ONCE(*r0);
> > > }
> > >
> > > exists (1:r0=x /\ 1:r1=0)
> > >
> > > The final state is allowed w/o the patch, and forbidden w/ the patch.
> > >
> > > This also reminds me of
> > >
> > > 5a8897cc7631fa544d079c443800f4420d1b173f
> > > ("locking/atomics/alpha: Add smp_read_barrier_depends() to _release()/_relaxed() atomics")
> > >
> > > (that we probably want to mention in the commit message).
> >
> > Please also notice that 5a8897cc7631f only touched alpha's atomic.h:
> > I see no corresponding commit/change on {,cmp}xchg.h (where the "mb"
> > is currently conditionally executed).
>
> This leaves us with a question: Do we want to change the kernel by
> adding memory barriers after unsuccessful RMW operations on Alpha, or
> do we want to change the model by excluding such operations from
> address dependencies?
I'd like to continue to treat R[once] and R*[once] equally if possible.
Given the (unconditional) smp_read_barrier_depends in READ_ONCE and in
atomics, it seems reasonable to have it unconditionally in cmpxchg.
As with the following patch?
Andrea
---
diff --git a/arch/alpha/include/asm/xchg.h b/arch/alpha/include/asm/xchg.h
index 68dfb3cb71454..e2660866ce972 100644
--- a/arch/alpha/include/asm/xchg.h
+++ b/arch/alpha/include/asm/xchg.h
@@ -128,10 +128,9 @@ ____xchg(, volatile void *ptr, unsigned long x, int size)
* store NEW in MEM. Return the initial value in MEM. Success is
* indicated by comparing RETURN with OLD.
*
- * The memory barrier should be placed in SMP only when we actually
- * make the change. If we don't change anything (so if the returned
- * prev is equal to old) then we aren't acquiring anything new and
- * we don't need any memory barrier as far I can tell.
+ * The memory barrier is placed in SMP unconditionally, in order to
+ * guarantee that dependency ordering is preserved when a dependency
+ * is headed by an unsuccessful operation.
*/
static inline unsigned long
@@ -150,8 +149,8 @@ ____cmpxchg(_u8, volatile char *m, unsigned char old, unsigned char new)
" or %1,%2,%2\n"
" stq_c %2,0(%4)\n"
" beq %2,3f\n"
- __ASM__MB
"2:\n"
+ __ASM__MB
".subsection 2\n"
"3: br 1b\n"
".previous"
@@ -177,8 +176,8 @@ ____cmpxchg(_u16, volatile short *m, unsigned short old, unsigned short new)
" or %1,%2,%2\n"
" stq_c %2,0(%4)\n"
" beq %2,3f\n"
- __ASM__MB
"2:\n"
+ __ASM__MB
".subsection 2\n"
"3: br 1b\n"
".previous"
@@ -200,8 +199,8 @@ ____cmpxchg(_u32, volatile int *m, int old, int new)
" mov %4,%1\n"
" stl_c %1,%2\n"
" beq %1,3f\n"
- __ASM__MB
"2:\n"
+ __ASM__MB
".subsection 2\n"
"3: br 1b\n"
".previous"
@@ -223,8 +222,8 @@ ____cmpxchg(_u64, volatile long *m, unsigned long old, unsigned long new)
" mov %4,%1\n"
" stq_c %1,%2\n"
" beq %1,3f\n"
- __ASM__MB
"2:\n"
+ __ASM__MB
".subsection 2\n"
"3: br 1b\n"
".previous"
>
> Note that operations like atomic_add_unless() already include memory
> barriers.
>
> Alan
>
next prev parent reply other threads:[~2018-02-20 9:33 UTC|newest]
Thread overview: 74+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-02-09 14:18 [PATCH RFC tools/lkmm] Miscellaneous fixes Paul E. McKenney
2018-02-09 14:20 ` [PATCH RFC tip/lkmm 01/10] tools/memory-model: Clarify the origin/scope of the tool name Paul E. McKenney
2018-02-09 14:20 ` [PATCH RFC tip/lkmm 02/10] MAINTAINERS: Add the Memory Consistency Model subsystem Paul E. McKenney
2018-02-09 14:20 ` [PATCH RFC tip/lkmm 03/10] MAINTAINERS: List file memory-barriers.txt within the LKMM entry Paul E. McKenney
2018-02-09 14:20 ` [PATCH RFC tip/lkmm 04/10] EXP litmus_tests: Add comments explaining tests' purposes Paul E. McKenney
2018-02-09 18:46 ` Alan Stern
2018-02-10 1:05 ` Paul E. McKenney
2018-02-09 14:20 ` [PATCH RFC tip/lkmm 05/10] README: Fix a couple of punctuation errors Paul E. McKenney
2018-02-09 14:20 ` [PATCH RFC tip/lkmm 06/10] EXP MAINTAINERS: Add the "LKMM" acronym Paul E. McKenney
2018-02-09 14:20 ` [PATCH RFC tip/lkmm 07/10] MAINTAINERS: Add Akira Yokosawa as an LKMM reviewer Paul E. McKenney
2018-02-09 14:20 ` [PATCH RFC tip/lkmm 08/10] EXP Remove understore from smp_mb__before_atomic() workings Paul E. McKenney
2018-02-15 22:30 ` Andrea Parri
2018-02-15 22:49 ` Paul E. McKenney
2018-02-15 23:19 ` Andrea Parri
2018-02-15 23:32 ` Paul E. McKenney
2018-02-09 14:20 ` [PATCH RFC tip/lkmm 09/10] EXP Remove underscore from smp_mb__after_atomic() workings Paul E. McKenney
2018-02-09 14:20 ` [PATCH RFC tip/lkmm 10/10] EXP Remove underscores from smp_mb__after_spinlock() workings Paul E. McKenney
2018-02-09 16:02 ` [PATCH RFC tools/lkmm] Miscellaneous fixes Akira Yokosawa
2018-02-09 16:06 ` [PATCH] tools/memory-model: Restore compat with herd7 7.47 ("-" -> "_") Akira Yokosawa
2018-02-09 23:46 ` [PATCH v2] tools/memory-model: Make " Akira Yokosawa
2018-02-10 1:07 ` Paul E. McKenney
2018-02-10 3:03 ` Akira Yokosawa
2018-02-11 11:51 ` Ingo Molnar
2018-02-13 1:38 ` Paul E. McKenney
2018-02-13 8:32 ` Ingo Molnar
2018-02-14 22:20 ` Akira Yokosawa
2018-02-14 22:52 ` Paul E. McKenney
2018-02-15 15:10 ` Alan Stern
2018-02-15 15:58 ` Trial of conflict resolution of Alan's patch Akira Yokosawa
2018-02-15 17:51 ` Alan Stern
2018-02-15 19:29 ` Paul E. McKenney
2018-02-15 21:51 ` Akira Yokosawa
2018-02-16 15:18 ` Alan Stern
2018-02-16 15:47 ` Andrea Parri
2018-02-16 16:53 ` Paul E. McKenney
2018-02-16 22:22 ` [PATCH] tools/memory-model: remove rb-dep, smp_read_barrier_depends, and lockless_dereference Alan Stern
2018-02-16 23:22 ` Akira Yokosawa
2018-02-17 0:39 ` Paul E. McKenney
2018-02-21 15:00 ` Alan Stern
2018-02-21 16:06 ` Paul E. McKenney
2018-02-21 16:51 ` Ingo Molnar
2018-02-21 17:53 ` Paul E. McKenney
2018-02-21 17:15 ` [PATCH] tools/memory-model: update: " Alan Stern
2018-02-21 17:58 ` Andrea Parri
2018-02-21 18:14 ` Paul E. McKenney
2018-02-21 18:00 ` Paul E. McKenney
2018-02-21 22:29 ` Akira Yokosawa
2018-02-24 3:22 ` Akira Yokosawa
2018-02-24 3:30 ` Paul E. McKenney
2018-02-24 14:36 ` Andrea Parri
2018-02-24 16:49 ` Alan Stern
2018-02-24 18:08 ` Paul E. McKenney
2018-02-24 22:47 ` Akira Yokosawa
2018-02-25 22:33 ` Paul E. McKenney
2018-02-17 3:25 ` [PATCH] tools/memory-model: " Andrea Parri
2018-02-17 15:14 ` Andrea Parri
2018-02-19 17:14 ` Alan Stern
2018-02-19 17:43 ` Peter Zijlstra
2018-02-19 17:44 ` Peter Zijlstra
2018-02-20 14:48 ` Paul E. McKenney
2018-02-20 15:17 ` Andrea Parri
2018-02-20 16:11 ` Paul E. McKenney
2018-02-19 19:41 ` Paul E. McKenney
2018-02-19 20:28 ` Peter Zijlstra
2018-02-20 14:49 ` Paul E. McKenney
2018-02-20 15:11 ` Alan Stern
2018-02-20 16:10 ` Paul E. McKenney
2018-02-20 9:33 ` Andrea Parri [this message]
2018-02-20 9:51 ` Peter Zijlstra
2018-02-20 15:38 ` Alan Stern
2018-02-15 22:05 ` Trial of conflict resolution of Alan's patch Andrea Parri
2018-02-15 22:41 ` Paul E. McKenney
2018-02-18 15:46 ` [PATCH v2] tools/memory-model: Make compat with herd7 7.47 ("-" -> "_") Akira Yokosawa
2018-02-20 14:57 ` Paul E. McKenney
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=20180220093346.GA5505@andrea \
--to=parri.andrea@gmail.com \
--cc=akiyks@gmail.com \
--cc=boqun.feng@gmail.com \
--cc=dhowells@redhat.com \
--cc=j.alglave@ucl.ac.uk \
--cc=linux-kernel@vger.kernel.org \
--cc=luc.maranget@inria.fr \
--cc=mingo@kernel.org \
--cc=npiggin@gmail.com \
--cc=patrick.bellasi@arm.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).