Re: [PATCH 4/4] locking: Introduce smp_cond_acquire()

[Peter Zijlstra <peterz@infradead.org>]

On Mon, Nov 02, 2015 at 11:17:17AM -0800, Linus Torvalds wrote:
> As to READ_ONCE_CTRL - two wrongs don't make a right.
> 
> That smp_read_barrier_depends() there doesn't make any sense either.
> 
> And finally, the alpha architecture manual actually does have the
> notion of "Dependence Constraint" (5.6.1.7) that talks about writes
> that depend on previous reads (where "depends" is explicitly spelled
> out to be about conditionals, write data _or_ write address). They are
> actually constrained on alpha too.
> 
> Note that a "Dependence Constraint" is not a memory barrier, because
> it only affects that particular chain of dependencies. So it doesn't
> order other things in *general*, but it does order a particular read
> with a particular sef of subsequent write. Which is all we guarantee
> on anything else too wrt the whole control dependencies.

Something like so then, Paul?

---
Subject: locking: Alpha honours control dependencies too

The alpha architecture manual actually does have the notion of
"Dependence Constraint" (5.6.1.7) that talks about writes that depend on
previous reads (where "depends" is explicitly spelled out to be about
conditionals, write data _or_ write address). They are actually
constrained on alpha too.

Which means we can remove the smp_read_barrier_depends() abuse from the
various control dependency primitives.

Retain the primitives, as they are a useful documentation aid.

Maybe-Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 include/linux/atomic.h   | 2 --
 include/linux/compiler.h | 1 -
 2 files changed, 3 deletions(-)

diff --git a/include/linux/atomic.h b/include/linux/atomic.h
index 27e580d232ca..f16b1dedd909 100644
--- a/include/linux/atomic.h
+++ b/include/linux/atomic.h
@@ -8,7 +8,6 @@
 static inline int atomic_read_ctrl(const atomic_t *v)
 {
 	int val = atomic_read(v);
-	smp_read_barrier_depends(); /* Enforce control dependency. */
 	return val;
 }
 #endif
@@ -565,7 +564,6 @@ static inline int atomic_dec_if_positive(atomic_t *v)
 static inline long long atomic64_read_ctrl(const atomic64_t *v)
 {
 	long long val = atomic64_read(v);
-	smp_read_barrier_depends(); /* Enforce control dependency. */
 	return val;
 }
 #endif
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 3d7810341b57..1d1f5902189d 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -311,7 +311,6 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s
 #define READ_ONCE_CTRL(x) \
 ({ \
 	typeof(x) __val = READ_ONCE(x); \
-	smp_read_barrier_depends(); /* Enforce control dependency. */ \
 	__val; \
 })