linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH tip/core/rcu 0/21] De-emphasize {smp_,}read_barrier_depends
@ 2017-12-01 19:50 Paul E. McKenney
  2017-12-01 19:50 ` [PATCH tip/core/rcu 01/21] doc: READ_ONCE() now implies smp_barrier_depends() Paul E. McKenney
                   ` (21 more replies)
  0 siblings, 22 replies; 60+ messages in thread
From: Paul E. McKenney @ 2017-12-01 19:50 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, jiangshanlai, dipankar, akpm, mathieu.desnoyers, josh,
	tglx, peterz, rostedt, dhowells, edumazet, fweisbec, oleg

Hello!

Now that READ_ONCE() includes smp_read_barrier_depends(), almost nothing
else needs to, the exceptions being DEC Alpha architecture-specific
code and of course READ_ONCE() itself.  This series therefore removes
smp_read_barrier_depends() and read_barrier_depends() from elsewhere,
as they no longer have any effect.  Note that this patch series also
prohibits use of InfiniBand on DEC Alpha: (1) A poll of DEC Alpha
users revealed no use of InfiniBand and (2) It is not clear that
InfiniBand's memory ordering correctly handles DEC Alpha, even with the
smp_read_barrier_depends() invocations.

Note also that patch 4 moves to release-acquire ordering to ensure that
the name length is properly synchronized.  An alternative approach would
be to place the length in with the name, so that dependency ordering
would cover both the length and the name, but doing so seemed a bit
intrusive for this series.

							Thanx, Paul

------------------------------------------------------------------------

 Documentation/RCU/Design/Requirements/Requirements.html |    3 -
 Documentation/RCU/rcu_dereference.txt                   |    6 --
 Documentation/RCU/whatisRCU.txt                         |    3 -
 Documentation/circular-buffers.txt                      |    3 -
 Documentation/memory-barriers.txt                       |   18 ++++---
 arch/mn10300/kernel/mn10300-serial.c                    |    7 ++-
 drivers/dma/ioat/dma.c                                  |    2 
 drivers/infiniband/Kconfig                              |    1 
 drivers/infiniband/hw/hfi1/rc.c                         |    4 -
 drivers/infiniband/hw/hfi1/ruc.c                        |    1 
 drivers/infiniband/hw/hfi1/sdma.c                       |    1 
 drivers/infiniband/hw/hfi1/uc.c                         |    2 
 drivers/infiniband/hw/hfi1/ud.c                         |    2 
 drivers/infiniband/hw/qib/qib_rc.c                      |    3 -
 drivers/infiniband/hw/qib/qib_ruc.c                     |    1 
 drivers/infiniband/hw/qib/qib_uc.c                      |    2 
 drivers/infiniband/hw/qib/qib_ud.c                      |    2 
 drivers/infiniband/sw/rdmavt/qp.c                       |    1 
 drivers/net/ethernet/qlogic/qed/qed_spq.c               |    4 -
 drivers/vhost/vhost.c                                   |    7 ---
 fs/dcache.c                                             |   10 +---
 include/linux/genetlink.h                               |    3 -
 include/linux/netfilter/nfnetlink.h                     |    3 -
 include/linux/percpu-refcount.h                         |    6 +-
 include/linux/rcupdate.h                                |   23 ++++-----
 include/linux/rtnetlink.h                               |    3 -
 include/linux/seqlock.h                                 |    3 -
 kernel/events/uprobes.c                                 |   12 ++---
 kernel/locking/qspinlock.c                              |   12 ++---
 kernel/tracepoint.c                                     |    9 +--
 lib/assoc_array.c                                       |   37 +++++-----------
 lib/percpu-refcount.c                                   |    8 +--
 mm/ksm.c                                                |    9 ---
 net/ipv4/netfilter/arp_tables.c                         |    7 ---
 net/ipv4/netfilter/ip_tables.c                          |    7 ---
 net/ipv6/netfilter/ip6_tables.c                         |    7 ---
 scripts/checkpatch.pl                                   |    6 ++
 security/keys/keyring.c                                 |    7 ---
 38 files changed, 85 insertions(+), 160 deletions(-)

^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH tip/core/rcu 01/21] doc: READ_ONCE() now implies smp_barrier_depends()
  2017-12-01 19:50 [PATCH tip/core/rcu 0/21] De-emphasize {smp_,}read_barrier_depends Paul E. McKenney
@ 2017-12-01 19:50 ` Paul E. McKenney
  2017-12-01 19:50 ` [PATCH tip/core/rcu 02/21] mn10300: READ_ONCE() now implies smp_read_barrier_depends() Paul E. McKenney
                   ` (20 subsequent siblings)
  21 siblings, 0 replies; 60+ messages in thread
From: Paul E. McKenney @ 2017-12-01 19:50 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, jiangshanlai, dipankar, akpm, mathieu.desnoyers, josh,
	tglx, peterz, rostedt, dhowells, edumazet, fweisbec, oleg,
	Paul E. McKenney

This commit updates an example in memory-barriers.txt to account for
the fact that READ_ONCE() now implies smp_barrier_depends().

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 Documentation/memory-barriers.txt | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/Documentation/memory-barriers.txt b/Documentation/memory-barriers.txt
index 479ecec80593..2269e58fa073 100644
--- a/Documentation/memory-barriers.txt
+++ b/Documentation/memory-barriers.txt
@@ -227,17 +227,18 @@ There are some minimal guarantees that may be expected of a CPU:
  (*) On any given CPU, dependent memory accesses will be issued in order, with
      respect to itself.  This means that for:
 
-	Q = READ_ONCE(P); smp_read_barrier_depends(); D = READ_ONCE(*Q);
+	Q = READ_ONCE(P); D = READ_ONCE(*Q);
 
      the CPU will issue the following memory operations:
 
 	Q = LOAD P, D = LOAD *Q
 
      and always in that order.  On most systems, smp_read_barrier_depends()
-     does nothing, but it is required for DEC Alpha.  The READ_ONCE()
-     is required to prevent compiler mischief.  Please note that you
-     should normally use something like rcu_dereference() instead of
-     open-coding smp_read_barrier_depends().
+     does nothing, but it is required for DEC Alpha, and is supplied by
+     READ_ONCE().  The READ_ONCE() is also required to prevent compiler
+     mischief.  Please note that you should normally use something
+     like READ_ONCE() or rcu_dereference() instead of open-coding
+     smp_read_barrier_depends().
 
  (*) Overlapping loads and stores within a particular CPU will appear to be
      ordered within that CPU.  This means that for:
-- 
2.5.2

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH tip/core/rcu 02/21] mn10300: READ_ONCE() now implies smp_read_barrier_depends()
  2017-12-01 19:50 [PATCH tip/core/rcu 0/21] De-emphasize {smp_,}read_barrier_depends Paul E. McKenney
  2017-12-01 19:50 ` [PATCH tip/core/rcu 01/21] doc: READ_ONCE() now implies smp_barrier_depends() Paul E. McKenney
@ 2017-12-01 19:50 ` Paul E. McKenney
  2017-12-01 19:50 ` [PATCH tip/core/rcu 03/21] drivers/net/ethernet/qlogic/qed: Fix __qed_spq_block() ordering Paul E. McKenney
                   ` (19 subsequent siblings)
  21 siblings, 0 replies; 60+ messages in thread
From: Paul E. McKenney @ 2017-12-01 19:50 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, jiangshanlai, dipankar, akpm, mathieu.desnoyers, josh,
	tglx, peterz, rostedt, dhowells, edumazet, fweisbec, oleg,
	Paul E. McKenney, Mark Rutland, linux-am33-list

Given that READ_ONCE() now implies smp_read_barrier_depends(),
there is no need for the open-coded smp_read_barrier_depends() in
mn10300_serial_receive_interrupt() and mn10300_serial_poll_get_char().
This commit therefore removes them, but replaces them with comments
calling out that carrying dependencies through non-pointers is quite
dangerous.  Compilers simply know too much about integers.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: David Howells <dhowells@redhat.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: <linux-am33-list@redhat.com>
---
 arch/mn10300/kernel/mn10300-serial.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/mn10300/kernel/mn10300-serial.c b/arch/mn10300/kernel/mn10300-serial.c
index d7ef1232a82a..4994b570dfd9 100644
--- a/arch/mn10300/kernel/mn10300-serial.c
+++ b/arch/mn10300/kernel/mn10300-serial.c
@@ -550,7 +550,7 @@ static void mn10300_serial_receive_interrupt(struct mn10300_serial_port *port)
 		return;
 	}
 
-	smp_read_barrier_depends();
+	/* READ_ONCE() enforces dependency, but dangerous through integer!!! */
 	ch = port->rx_buffer[ix++];
 	st = port->rx_buffer[ix++];
 	smp_mb();
@@ -1728,7 +1728,10 @@ static int mn10300_serial_poll_get_char(struct uart_port *_port)
 			if (CIRC_CNT(port->rx_inp, ix, MNSC_BUFFER_SIZE) == 0)
 				return NO_POLL_CHAR;
 
-			smp_read_barrier_depends();
+			/*
+			 * READ_ONCE() enforces dependency, but dangerous
+			 * through integer!!!
+			 */
 			ch = port->rx_buffer[ix++];
 			st = port->rx_buffer[ix++];
 			smp_mb();
-- 
2.5.2

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH tip/core/rcu 03/21] drivers/net/ethernet/qlogic/qed: Fix __qed_spq_block() ordering
  2017-12-01 19:50 [PATCH tip/core/rcu 0/21] De-emphasize {smp_,}read_barrier_depends Paul E. McKenney
  2017-12-01 19:50 ` [PATCH tip/core/rcu 01/21] doc: READ_ONCE() now implies smp_barrier_depends() Paul E. McKenney
  2017-12-01 19:50 ` [PATCH tip/core/rcu 02/21] mn10300: READ_ONCE() now implies smp_read_barrier_depends() Paul E. McKenney
@ 2017-12-01 19:50 ` Paul E. McKenney
  2017-12-01 19:50 ` [PATCH tip/core/rcu 04/21] fs/dcache: Use release-acquire for name/length update Paul E. McKenney
                   ` (18 subsequent siblings)
  21 siblings, 0 replies; 60+ messages in thread
From: Paul E. McKenney @ 2017-12-01 19:50 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, jiangshanlai, dipankar, akpm, mathieu.desnoyers, josh,
	tglx, peterz, rostedt, dhowells, edumazet, fweisbec, oleg,
	Paul E. McKenney, Ariel Elior, everest-linux-l2, netdev

The __qed_spq_block() function expects an smp_read_barrier_depends()
to order a prior READ_ONCE() against a later load that does not depend
on the prior READ_ONCE(), an expectation that can fail to be met.
This commit therefore replaces the READ_ONCE() with smp_load_acquire()
and removes the smp_read_barrier_depends().

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Ariel Elior <Ariel.Elior@cavium.com>
Cc: <everest-linux-l2@cavium.com>
Cc: <netdev@vger.kernel.org>
---
 drivers/net/ethernet/qlogic/qed/qed_spq.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_spq.c b/drivers/net/ethernet/qlogic/qed/qed_spq.c
index be48d9abd001..c1237ec58b6c 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_spq.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_spq.c
@@ -97,9 +97,7 @@ static int __qed_spq_block(struct qed_hwfn *p_hwfn,
 
 	while (iter_cnt--) {
 		/* Validate we receive completion update */
-		if (READ_ONCE(comp_done->done) == 1) {
-			/* Read updated FW return value */
-			smp_read_barrier_depends();
+		if (smp_load_acquire(&comp_done->done) == 1) { /* ^^^ */
 			if (p_fw_ret)
 				*p_fw_ret = comp_done->fw_return_code;
 			return 0;
-- 
2.5.2

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH tip/core/rcu 04/21] fs/dcache: Use release-acquire for name/length update
  2017-12-01 19:50 [PATCH tip/core/rcu 0/21] De-emphasize {smp_,}read_barrier_depends Paul E. McKenney
                   ` (2 preceding siblings ...)
  2017-12-01 19:50 ` [PATCH tip/core/rcu 03/21] drivers/net/ethernet/qlogic/qed: Fix __qed_spq_block() ordering Paul E. McKenney
@ 2017-12-01 19:50 ` Paul E. McKenney
  2017-12-01 19:51 ` [PATCH tip/core/rcu 05/21] percpu: READ_ONCE() now implies smp_read_barrier_depends() Paul E. McKenney
                   ` (17 subsequent siblings)
  21 siblings, 0 replies; 60+ messages in thread
From: Paul E. McKenney @ 2017-12-01 19:50 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, jiangshanlai, dipankar, akpm, mathieu.desnoyers, josh,
	tglx, peterz, rostedt, dhowells, edumazet, fweisbec, oleg,
	Paul E. McKenney, Alexander Viro, linux-fsdevel

The code in __d_alloc() carefully orders filling in the NUL character
of the name (and the length, hash, and the name itself) with assigning
of the name itself.  However, prepend_name() does not order the accesses
to the ->name and ->len fields, other than on TSO systems.  This commit
therefore replaces prepend_name()'s READ_ONCE() of ->name with an
smp_load_acquire(), which orders against the subsequent READ_ONCE() of
->len.  Because READ_ONCE() now incorporates smp_read_barrier_depends(),
prepend_name()'s smp_read_barrier_depends() is removed.  Finally,
to save a line, the smp_wmb()/store pair in __d_alloc() is replaced
by smp_store_release().

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: <linux-fsdevel@vger.kernel.org>
---
 fs/dcache.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 5c7df1df81ff..379dce86f001 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -1636,8 +1636,7 @@ struct dentry *__d_alloc(struct super_block *sb, const struct qstr *name)
 	dname[name->len] = 0;
 
 	/* Make sure we always see the terminating NUL character */
-	smp_wmb();
-	dentry->d_name.name = dname;
+	smp_store_release(&dentry->d_name.name, dname); /* ^^^ */
 
 	dentry->d_lockref.count = 1;
 	dentry->d_flags = 0;
@@ -3047,17 +3046,14 @@ static int prepend(char **buffer, int *buflen, const char *str, int namelen)
  * retry it again when a d_move() does happen. So any garbage in the buffer
  * due to mismatched pointer and length will be discarded.
  *
- * Data dependency barrier is needed to make sure that we see that terminating
- * NUL.  Alpha strikes again, film at 11...
+ * Load acquire is needed to make sure that we see that terminating NUL.
  */
 static int prepend_name(char **buffer, int *buflen, const struct qstr *name)
 {
-	const char *dname = READ_ONCE(name->name);
+	const char *dname = smp_load_acquire(&name->name); /* ^^^ */
 	u32 dlen = READ_ONCE(name->len);
 	char *p;
 
-	smp_read_barrier_depends();
-
 	*buflen -= dlen + 1;
 	if (*buflen < 0)
 		return -ENAMETOOLONG;
-- 
2.5.2

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH tip/core/rcu 05/21] percpu: READ_ONCE() now implies smp_read_barrier_depends()
  2017-12-01 19:50 [PATCH tip/core/rcu 0/21] De-emphasize {smp_,}read_barrier_depends Paul E. McKenney
                   ` (3 preceding siblings ...)
  2017-12-01 19:50 ` [PATCH tip/core/rcu 04/21] fs/dcache: Use release-acquire for name/length update Paul E. McKenney
@ 2017-12-01 19:51 ` Paul E. McKenney
  2017-12-01 19:51 ` [PATCH tip/core/rcu 06/21] rcu: Adjust read-side accessor comments for READ_ONCE() Paul E. McKenney
                   ` (16 subsequent siblings)
  21 siblings, 0 replies; 60+ messages in thread
From: Paul E. McKenney @ 2017-12-01 19:51 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, jiangshanlai, dipankar, akpm, mathieu.desnoyers, josh,
	tglx, peterz, rostedt, dhowells, edumazet, fweisbec, oleg,
	Paul E. McKenney, Christoph Lameter

Because READ_ONCE() now implies smp_read_barrier_depends(), this commit
removes the now-redundant smp_read_barrier_depends() following the
READ_ONCE() in __ref_is_percpu().

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Acked-by: Tejun Heo <tj@kernel.org>
Cc: Christoph Lameter <cl@linux.com>
---
 include/linux/percpu-refcount.h | 6 +++---
 lib/percpu-refcount.c           | 8 ++++----
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/linux/percpu-refcount.h b/include/linux/percpu-refcount.h
index 6658d9ee5257..864d167a1073 100644
--- a/include/linux/percpu-refcount.h
+++ b/include/linux/percpu-refcount.h
@@ -139,12 +139,12 @@ static inline bool __ref_is_percpu(struct percpu_ref *ref,
 	 * when using it as a pointer, __PERCPU_REF_ATOMIC may be set in
 	 * between contaminating the pointer value, meaning that
 	 * READ_ONCE() is required when fetching it.
+	 *
+	 * The smp_read_barrier_depends() implied by READ_ONCE() pairs
+	 * with smp_store_release() in __percpu_ref_switch_to_percpu().
 	 */
 	percpu_ptr = READ_ONCE(ref->percpu_count_ptr);
 
-	/* paired with smp_store_release() in __percpu_ref_switch_to_percpu() */
-	smp_read_barrier_depends();
-
 	/*
 	 * Theoretically, the following could test just ATOMIC; however,
 	 * then we'd have to mask off DEAD separately as DEAD may be
diff --git a/lib/percpu-refcount.c b/lib/percpu-refcount.c
index fe03c6d52761..30e7dd88148b 100644
--- a/lib/percpu-refcount.c
+++ b/lib/percpu-refcount.c
@@ -197,10 +197,10 @@ static void __percpu_ref_switch_to_percpu(struct percpu_ref *ref)
 	atomic_long_add(PERCPU_COUNT_BIAS, &ref->count);
 
 	/*
-	 * Restore per-cpu operation.  smp_store_release() is paired with
-	 * smp_read_barrier_depends() in __ref_is_percpu() and guarantees
-	 * that the zeroing is visible to all percpu accesses which can see
-	 * the following __PERCPU_REF_ATOMIC clearing.
+	 * Restore per-cpu operation.  smp_store_release() is paired
+	 * with READ_ONCE() in __ref_is_percpu() and guarantees that the
+	 * zeroing is visible to all percpu accesses which can see the
+	 * following __PERCPU_REF_ATOMIC clearing.
 	 */
 	for_each_possible_cpu(cpu)
 		*per_cpu_ptr(percpu_count, cpu) = 0;
-- 
2.5.2

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH tip/core/rcu 06/21] rcu: Adjust read-side accessor comments for READ_ONCE()
  2017-12-01 19:50 [PATCH tip/core/rcu 0/21] De-emphasize {smp_,}read_barrier_depends Paul E. McKenney
                   ` (4 preceding siblings ...)
  2017-12-01 19:51 ` [PATCH tip/core/rcu 05/21] percpu: READ_ONCE() now implies smp_read_barrier_depends() Paul E. McKenney
@ 2017-12-01 19:51 ` Paul E. McKenney
  2017-12-01 19:51 ` [PATCH tip/core/rcu 07/21] rtnetlink: Update now-misleading smp_read_barrier_depends() comment Paul E. McKenney
                   ` (15 subsequent siblings)
  21 siblings, 0 replies; 60+ messages in thread
From: Paul E. McKenney @ 2017-12-01 19:51 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, jiangshanlai, dipankar, akpm, mathieu.desnoyers, josh,
	tglx, peterz, rostedt, dhowells, edumazet, fweisbec, oleg,
	Paul E. McKenney

Now that READ_ONCE() implies smp_read_barrier_depends(), the commit
updates now-misleading comments to account for this change.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 include/linux/rcupdate.h | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index a6ddc42f87a5..000432b87e5a 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -433,12 +433,12 @@ static inline void rcu_preempt_sleep_check(void) { }
  * @p: The pointer to read
  *
  * Return the value of the specified RCU-protected pointer, but omit the
- * smp_read_barrier_depends() and keep the READ_ONCE().  This is useful
- * when the value of this pointer is accessed, but the pointer is not
- * dereferenced, for example, when testing an RCU-protected pointer against
- * NULL.  Although rcu_access_pointer() may also be used in cases where
- * update-side locks prevent the value of the pointer from changing, you
- * should instead use rcu_dereference_protected() for this use case.
+ * lockdep checks for being in an RCU read-side critical section.  This is
+ * useful when the value of this pointer is accessed, but the pointer is
+ * not dereferenced, for example, when testing an RCU-protected pointer
+ * against NULL.  Although rcu_access_pointer() may also be used in cases
+ * where update-side locks prevent the value of the pointer from changing,
+ * you should instead use rcu_dereference_protected() for this use case.
  *
  * It is also permissible to use rcu_access_pointer() when read-side
  * access to the pointer was removed at least one grace period ago, as
@@ -521,12 +521,11 @@ static inline void rcu_preempt_sleep_check(void) { }
  * @c: The conditions under which the dereference will take place
  *
  * Return the value of the specified RCU-protected pointer, but omit
- * both the smp_read_barrier_depends() and the READ_ONCE().  This
- * is useful in cases where update-side locks prevent the value of the
- * pointer from changing.  Please note that this primitive does *not*
- * prevent the compiler from repeating this reference or combining it
- * with other references, so it should not be used without protection
- * of appropriate locks.
+ * the READ_ONCE().  This is useful in cases where update-side locks
+ * prevent the value of the pointer from changing.  Please note that this
+ * primitive does *not* prevent the compiler from repeating this reference
+ * or combining it with other references, so it should not be used without
+ * protection of appropriate locks.
  *
  * This function is only for update-side use.  Using this function
  * when protected only by rcu_read_lock() will result in infrequent
-- 
2.5.2

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH tip/core/rcu 07/21] rtnetlink: Update now-misleading smp_read_barrier_depends() comment
  2017-12-01 19:50 [PATCH tip/core/rcu 0/21] De-emphasize {smp_,}read_barrier_depends Paul E. McKenney
                   ` (5 preceding siblings ...)
  2017-12-01 19:51 ` [PATCH tip/core/rcu 06/21] rcu: Adjust read-side accessor comments for READ_ONCE() Paul E. McKenney
@ 2017-12-01 19:51 ` Paul E. McKenney
  2017-12-01 19:51 ` [PATCH tip/core/rcu 08/21] seqlock: Remove now-redundant smp_read_barrier_depends() Paul E. McKenney
                   ` (14 subsequent siblings)
  21 siblings, 0 replies; 60+ messages in thread
From: Paul E. McKenney @ 2017-12-01 19:51 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, jiangshanlai, dipankar, akpm, mathieu.desnoyers, josh,
	tglx, peterz, rostedt, dhowells, edumazet, fweisbec, oleg,
	Paul E. McKenney, David S. Miller, Vladislav Yasevich,
	Mark Rutland, David Ahern, Vlad Yasevich

Now that READ_ONCE() implies smp_read_barrier_depends(), update the
rtnl_dereference() header comment accordingly.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Vladislav Yasevich <vyasevic@redhat.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Vlad Yasevich <vyasevich@gmail.com>
---
 include/linux/rtnetlink.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index 2032ce2eb20b..1eadec3fc228 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -70,8 +70,7 @@ static inline bool lockdep_rtnl_is_held(void)
  * @p: The pointer to read, prior to dereferencing
  *
  * Return the value of the specified RCU-protected pointer, but omit
- * both the smp_read_barrier_depends() and the READ_ONCE(), because
- * caller holds RTNL.
+ * the READ_ONCE(), because caller holds RTNL.
  */
 #define rtnl_dereference(p)					\
 	rcu_dereference_protected(p, lockdep_rtnl_is_held())
-- 
2.5.2

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH tip/core/rcu 08/21] seqlock: Remove now-redundant smp_read_barrier_depends()
  2017-12-01 19:50 [PATCH tip/core/rcu 0/21] De-emphasize {smp_,}read_barrier_depends Paul E. McKenney
                   ` (6 preceding siblings ...)
  2017-12-01 19:51 ` [PATCH tip/core/rcu 07/21] rtnetlink: Update now-misleading smp_read_barrier_depends() comment Paul E. McKenney
@ 2017-12-01 19:51 ` Paul E. McKenney
  2017-12-01 19:51 ` [PATCH tip/core/rcu 09/21] uprobes: " Paul E. McKenney
                   ` (13 subsequent siblings)
  21 siblings, 0 replies; 60+ messages in thread
From: Paul E. McKenney @ 2017-12-01 19:51 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, jiangshanlai, dipankar, akpm, mathieu.desnoyers, josh,
	tglx, peterz, rostedt, dhowells, edumazet, fweisbec, oleg,
	Paul E. McKenney, Ingo Molnar

READ_ONCE() now implies smp_read_barrier_depends(), so this patch
removes the now-redundant smp_read_barrier_depends() from
raw_read_seqcount_latch().

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
---
 include/linux/seqlock.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/linux/seqlock.h b/include/linux/seqlock.h
index f189a8a3bbb8..bcf4cf26b8c8 100644
--- a/include/linux/seqlock.h
+++ b/include/linux/seqlock.h
@@ -278,9 +278,8 @@ static inline void raw_write_seqcount_barrier(seqcount_t *s)
 
 static inline int raw_read_seqcount_latch(seqcount_t *s)
 {
-	int seq = READ_ONCE(s->sequence);
 	/* Pairs with the first smp_wmb() in raw_write_seqcount_latch() */
-	smp_read_barrier_depends();
+	int seq = READ_ONCE(s->sequence); /* ^^^ */
 	return seq;
 }
 
-- 
2.5.2

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH tip/core/rcu 09/21] uprobes: Remove now-redundant smp_read_barrier_depends()
  2017-12-01 19:50 [PATCH tip/core/rcu 0/21] De-emphasize {smp_,}read_barrier_depends Paul E. McKenney
                   ` (7 preceding siblings ...)
  2017-12-01 19:51 ` [PATCH tip/core/rcu 08/21] seqlock: Remove now-redundant smp_read_barrier_depends() Paul E. McKenney
@ 2017-12-01 19:51 ` Paul E. McKenney
  2017-12-01 19:51 ` [PATCH tip/core/rcu 10/21] locking: Remove smp_read_barrier_depends() from queued_spin_lock_slowpath() Paul E. McKenney
                   ` (12 subsequent siblings)
  21 siblings, 0 replies; 60+ messages in thread
From: Paul E. McKenney @ 2017-12-01 19:51 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, jiangshanlai, dipankar, akpm, mathieu.desnoyers, josh,
	tglx, peterz, rostedt, dhowells, edumazet, fweisbec, oleg,
	Paul E. McKenney, Ingo Molnar, Arnaldo Carvalho de Melo,
	Alexander Shishkin

Now that READ_ONCE() implies smp_read_barrier_depends(), the
get_xol_area() and get_trampoline_vaddr() no longer need their
smp_read_barrier_depends() calls, which this commit removes.
While we are here, convert the corresponding smp_wmb() to an
smp_store_release().

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 kernel/events/uprobes.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 267f6ef91d97..ce6848e46e94 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -1167,8 +1167,8 @@ static int xol_add_vma(struct mm_struct *mm, struct xol_area *area)
 	}
 
 	ret = 0;
-	smp_wmb();	/* pairs with get_xol_area() */
-	mm->uprobes_state.xol_area = area;
+	/* pairs with get_xol_area() */
+	smp_store_release(&mm->uprobes_state.xol_area, area); /* ^^^ */
  fail:
 	up_write(&mm->mmap_sem);
 
@@ -1230,8 +1230,8 @@ static struct xol_area *get_xol_area(void)
 	if (!mm->uprobes_state.xol_area)
 		__create_xol_area(0);
 
-	area = mm->uprobes_state.xol_area;
-	smp_read_barrier_depends();	/* pairs with wmb in xol_add_vma() */
+	/* Pairs with xol_add_vma() smp_store_release() */
+	area = READ_ONCE(mm->uprobes_state.xol_area); /* ^^^ */
 	return area;
 }
 
@@ -1528,8 +1528,8 @@ static unsigned long get_trampoline_vaddr(void)
 	struct xol_area *area;
 	unsigned long trampoline_vaddr = -1;
 
-	area = current->mm->uprobes_state.xol_area;
-	smp_read_barrier_depends();
+	/* Pairs with xol_add_vma() smp_store_release() */
+	area = READ_ONCE(current->mm->uprobes_state.xol_area); /* ^^^ */
 	if (area)
 		trampoline_vaddr = area->vaddr;
 
-- 
2.5.2

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH tip/core/rcu 10/21] locking: Remove smp_read_barrier_depends() from queued_spin_lock_slowpath()
  2017-12-01 19:50 [PATCH tip/core/rcu 0/21] De-emphasize {smp_,}read_barrier_depends Paul E. McKenney
                   ` (8 preceding siblings ...)
  2017-12-01 19:51 ` [PATCH tip/core/rcu 09/21] uprobes: " Paul E. McKenney
@ 2017-12-01 19:51 ` Paul E. McKenney
  2017-12-01 19:51 ` [PATCH tip/core/rcu 11/21] tracepoint: Remove smp_read_barrier_depends() from comment Paul E. McKenney
                   ` (11 subsequent siblings)
  21 siblings, 0 replies; 60+ messages in thread
From: Paul E. McKenney @ 2017-12-01 19:51 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, jiangshanlai, dipankar, akpm, mathieu.desnoyers, josh,
	tglx, peterz, rostedt, dhowells, edumazet, fweisbec, oleg,
	Paul E. McKenney, Ingo Molnar

Queued spinlocks are not used by DEC Alpha, and furthermore operations
such as READ_ONCE() and release/relaxed RMW atomics are being changed
to imply smp_read_barrier_depends().  This commit therefore removes the
now-redundant smp_read_barrier_depends() from queued_spin_lock_slowpath(),
and adjusts the comments accordingly.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
---
 kernel/locking/qspinlock.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/kernel/locking/qspinlock.c b/kernel/locking/qspinlock.c
index 294294c71ba4..38ece035039e 100644
--- a/kernel/locking/qspinlock.c
+++ b/kernel/locking/qspinlock.c
@@ -170,7 +170,7 @@ static __always_inline void clear_pending_set_locked(struct qspinlock *lock)
  * @tail : The new queue tail code word
  * Return: The previous queue tail code word
  *
- * xchg(lock, tail)
+ * xchg(lock, tail), which heads an address dependency
  *
  * p,*,* -> n,*,* ; prev = xchg(lock, node)
  */
@@ -409,13 +409,11 @@ void queued_spin_lock_slowpath(struct qspinlock *lock, u32 val)
 	if (old & _Q_TAIL_MASK) {
 		prev = decode_tail(old);
 		/*
-		 * The above xchg_tail() is also a load of @lock which generates,
-		 * through decode_tail(), a pointer.
-		 *
-		 * The address dependency matches the RELEASE of xchg_tail()
-		 * such that the access to @prev must happen after.
+		 * The above xchg_tail() is also a load of @lock which
+		 * generates, through decode_tail(), a pointer.  The address
+		 * dependency matches the RELEASE of xchg_tail() such that
+		 * the subsequent access to @prev happens after.
 		 */
-		smp_read_barrier_depends();
 
 		WRITE_ONCE(prev->next, node);
 
-- 
2.5.2

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH tip/core/rcu 11/21] tracepoint: Remove smp_read_barrier_depends() from comment
  2017-12-01 19:50 [PATCH tip/core/rcu 0/21] De-emphasize {smp_,}read_barrier_depends Paul E. McKenney
                   ` (9 preceding siblings ...)
  2017-12-01 19:51 ` [PATCH tip/core/rcu 10/21] locking: Remove smp_read_barrier_depends() from queued_spin_lock_slowpath() Paul E. McKenney
@ 2017-12-01 19:51 ` Paul E. McKenney
  2017-12-01 19:51 ` [PATCH tip/core/rcu 12/21] lib/assoc_array: Remove smp_read_barrier_depends() Paul E. McKenney
                   ` (10 subsequent siblings)
  21 siblings, 0 replies; 60+ messages in thread
From: Paul E. McKenney @ 2017-12-01 19:51 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, jiangshanlai, dipankar, akpm, mathieu.desnoyers, josh,
	tglx, peterz, rostedt, dhowells, edumazet, fweisbec, oleg,
	Paul E. McKenney

The comment in tracepoint_add_func() mentions smp_read_barrier_depends(),
whose use should be quite restricted.  This commit updates the comment
to instead mention the smp_store_release() and rcu_dereference_sched()
that the current code actually uses.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@kernel.org>
Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Acked-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
 kernel/tracepoint.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
index 685c50ae6300..671b13457387 100644
--- a/kernel/tracepoint.c
+++ b/kernel/tracepoint.c
@@ -212,11 +212,10 @@ static int tracepoint_add_func(struct tracepoint *tp,
 	}
 
 	/*
-	 * rcu_assign_pointer has a smp_wmb() which makes sure that the new
-	 * probe callbacks array is consistent before setting a pointer to it.
-	 * This array is referenced by __DO_TRACE from
-	 * include/linux/tracepoints.h. A matching smp_read_barrier_depends()
-	 * is used.
+	 * rcu_assign_pointer has as smp_store_release() which makes sure
+	 * that the new probe callbacks array is consistent before setting
+	 * a pointer to it.  This array is referenced by __DO_TRACE from
+	 * include/linux/tracepoint.h using rcu_dereference_sched().
 	 */
 	rcu_assign_pointer(tp->funcs, tp_funcs);
 	if (!static_key_enabled(&tp->key))
-- 
2.5.2

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH tip/core/rcu 12/21] lib/assoc_array: Remove smp_read_barrier_depends()
  2017-12-01 19:50 [PATCH tip/core/rcu 0/21] De-emphasize {smp_,}read_barrier_depends Paul E. McKenney
                   ` (10 preceding siblings ...)
  2017-12-01 19:51 ` [PATCH tip/core/rcu 11/21] tracepoint: Remove smp_read_barrier_depends() from comment Paul E. McKenney
@ 2017-12-01 19:51 ` Paul E. McKenney
  2017-12-01 19:51 ` [PATCH tip/core/rcu 13/21] mm/ksm: Remove now-redundant smp_read_barrier_depends() Paul E. McKenney
                   ` (9 subsequent siblings)
  21 siblings, 0 replies; 60+ messages in thread
From: Paul E. McKenney @ 2017-12-01 19:51 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, jiangshanlai, dipankar, akpm, mathieu.desnoyers, josh,
	tglx, peterz, rostedt, dhowells, edumazet, fweisbec, oleg,
	Paul E. McKenney, Jonathan Corbet, Mark Rutland,
	Alexander Kuleshov

Now that smp_read_barrier_depends() is implied by READ_ONCE(), the several
smp_read_barrier_depends() calls may be removed from lib/assoc_array.c.
This commit makes this change and marks the READ_ONCE() calls that head
address dependencies.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Alexander Kuleshov <kuleshovmail@gmail.com>
Cc: David Howells <dhowells@redhat.com>
---
 lib/assoc_array.c | 37 ++++++++++++-------------------------
 1 file changed, 12 insertions(+), 25 deletions(-)

diff --git a/lib/assoc_array.c b/lib/assoc_array.c
index b77d51da8c73..c6659cb37033 100644
--- a/lib/assoc_array.c
+++ b/lib/assoc_array.c
@@ -38,12 +38,10 @@ static int assoc_array_subtree_iterate(const struct assoc_array_ptr *root,
 	if (assoc_array_ptr_is_shortcut(cursor)) {
 		/* Descend through a shortcut */
 		shortcut = assoc_array_ptr_to_shortcut(cursor);
-		smp_read_barrier_depends();
-		cursor = READ_ONCE(shortcut->next_node);
+		cursor = READ_ONCE(shortcut->next_node); /* Address dependency. */
 	}
 
 	node = assoc_array_ptr_to_node(cursor);
-	smp_read_barrier_depends();
 	slot = 0;
 
 	/* We perform two passes of each node.
@@ -55,15 +53,12 @@ static int assoc_array_subtree_iterate(const struct assoc_array_ptr *root,
 	 */
 	has_meta = 0;
 	for (; slot < ASSOC_ARRAY_FAN_OUT; slot++) {
-		ptr = READ_ONCE(node->slots[slot]);
+		ptr = READ_ONCE(node->slots[slot]); /* Address dependency. */
 		has_meta |= (unsigned long)ptr;
 		if (ptr && assoc_array_ptr_is_leaf(ptr)) {
-			/* We need a barrier between the read of the pointer
-			 * and dereferencing the pointer - but only if we are
-			 * actually going to dereference it.
+			/* We need a barrier between the read of the pointer,
+			 * which is supplied by the above READ_ONCE().
 			 */
-			smp_read_barrier_depends();
-
 			/* Invoke the callback */
 			ret = iterator(assoc_array_ptr_to_leaf(ptr),
 				       iterator_data);
@@ -86,10 +81,8 @@ static int assoc_array_subtree_iterate(const struct assoc_array_ptr *root,
 
 continue_node:
 	node = assoc_array_ptr_to_node(cursor);
-	smp_read_barrier_depends();
-
 	for (; slot < ASSOC_ARRAY_FAN_OUT; slot++) {
-		ptr = READ_ONCE(node->slots[slot]);
+		ptr = READ_ONCE(node->slots[slot]); /* Address dependency. */
 		if (assoc_array_ptr_is_meta(ptr)) {
 			cursor = ptr;
 			goto begin_node;
@@ -98,16 +91,15 @@ static int assoc_array_subtree_iterate(const struct assoc_array_ptr *root,
 
 finished_node:
 	/* Move up to the parent (may need to skip back over a shortcut) */
-	parent = READ_ONCE(node->back_pointer);
+	parent = READ_ONCE(node->back_pointer); /* Address dependency. */
 	slot = node->parent_slot;
 	if (parent == stop)
 		return 0;
 
 	if (assoc_array_ptr_is_shortcut(parent)) {
 		shortcut = assoc_array_ptr_to_shortcut(parent);
-		smp_read_barrier_depends();
 		cursor = parent;
-		parent = READ_ONCE(shortcut->back_pointer);
+		parent = READ_ONCE(shortcut->back_pointer); /* Address dependency. */
 		slot = shortcut->parent_slot;
 		if (parent == stop)
 			return 0;
@@ -147,7 +139,7 @@ int assoc_array_iterate(const struct assoc_array *array,
 					void *iterator_data),
 			void *iterator_data)
 {
-	struct assoc_array_ptr *root = READ_ONCE(array->root);
+	struct assoc_array_ptr *root = READ_ONCE(array->root); /* Address dependency. */
 
 	if (!root)
 		return 0;
@@ -194,7 +186,7 @@ assoc_array_walk(const struct assoc_array *array,
 
 	pr_devel("-->%s()\n", __func__);
 
-	cursor = READ_ONCE(array->root);
+	cursor = READ_ONCE(array->root);  /* Address dependency. */
 	if (!cursor)
 		return assoc_array_walk_tree_empty;
 
@@ -216,11 +208,9 @@ assoc_array_walk(const struct assoc_array *array,
 
 consider_node:
 	node = assoc_array_ptr_to_node(cursor);
-	smp_read_barrier_depends();
-
 	slot = segments >> (level & ASSOC_ARRAY_KEY_CHUNK_MASK);
 	slot &= ASSOC_ARRAY_FAN_MASK;
-	ptr = READ_ONCE(node->slots[slot]);
+	ptr = READ_ONCE(node->slots[slot]); /* Address dependency. */
 
 	pr_devel("consider slot %x [ix=%d type=%lu]\n",
 		 slot, level, (unsigned long)ptr & 3);
@@ -254,7 +244,6 @@ assoc_array_walk(const struct assoc_array *array,
 	cursor = ptr;
 follow_shortcut:
 	shortcut = assoc_array_ptr_to_shortcut(cursor);
-	smp_read_barrier_depends();
 	pr_devel("shortcut to %d\n", shortcut->skip_to_level);
 	sc_level = level + ASSOC_ARRAY_LEVEL_STEP;
 	BUG_ON(sc_level > shortcut->skip_to_level);
@@ -294,7 +283,7 @@ assoc_array_walk(const struct assoc_array *array,
 	} while (sc_level < shortcut->skip_to_level);
 
 	/* The shortcut matches the leaf's index to this point. */
-	cursor = READ_ONCE(shortcut->next_node);
+	cursor = READ_ONCE(shortcut->next_node); /* Address dependency. */
 	if (((level ^ sc_level) & ~ASSOC_ARRAY_KEY_CHUNK_MASK) != 0) {
 		level = sc_level;
 		goto jumped;
@@ -331,20 +320,18 @@ void *assoc_array_find(const struct assoc_array *array,
 		return NULL;
 
 	node = result.terminal_node.node;
-	smp_read_barrier_depends();
 
 	/* If the target key is available to us, it's has to be pointed to by
 	 * the terminal node.
 	 */
 	for (slot = 0; slot < ASSOC_ARRAY_FAN_OUT; slot++) {
-		ptr = READ_ONCE(node->slots[slot]);
+		ptr = READ_ONCE(node->slots[slot]); /* Address dependency. */
 		if (ptr && assoc_array_ptr_is_leaf(ptr)) {
 			/* We need a barrier between the read of the pointer
 			 * and dereferencing the pointer - but only if we are
 			 * actually going to dereference it.
 			 */
 			leaf = assoc_array_ptr_to_leaf(ptr);
-			smp_read_barrier_depends();
 			if (ops->compare_object(leaf, index_key))
 				return (void *)leaf;
 		}
-- 
2.5.2

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH tip/core/rcu 13/21] mm/ksm: Remove now-redundant smp_read_barrier_depends()
  2017-12-01 19:50 [PATCH tip/core/rcu 0/21] De-emphasize {smp_,}read_barrier_depends Paul E. McKenney
                   ` (11 preceding siblings ...)
  2017-12-01 19:51 ` [PATCH tip/core/rcu 12/21] lib/assoc_array: Remove smp_read_barrier_depends() Paul E. McKenney
@ 2017-12-01 19:51 ` Paul E. McKenney
  2017-12-01 19:51 ` [PATCH tip/core/rcu 14/21] netfilter: " Paul E. McKenney
                   ` (8 subsequent siblings)
  21 siblings, 0 replies; 60+ messages in thread
From: Paul E. McKenney @ 2017-12-01 19:51 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, jiangshanlai, dipankar, akpm, mathieu.desnoyers, josh,
	tglx, peterz, rostedt, dhowells, edumazet, fweisbec, oleg,
	Paul E. McKenney, Andrea Arcangeli, Minchan Kim, Michal Hocko,
	Kirill A. Shutemov, Aneesh Kumar K.V, Claudio Imbrenda, linux-mm

Because READ_ONCE() now implies smp_read_barrier_depends(), the
smp_read_barrier_depends() in get_ksm_page() is now redundant.
This commit removes it and updates the comments.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: Minchan Kim <minchan@kernel.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
Cc: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
Cc: <linux-mm@kvack.org>
---
 mm/ksm.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/mm/ksm.c b/mm/ksm.c
index be8f4576f842..c406f75957ad 100644
--- a/mm/ksm.c
+++ b/mm/ksm.c
@@ -675,15 +675,8 @@ static struct page *get_ksm_page(struct stable_node *stable_node, bool lock_it)
 	expected_mapping = (void *)((unsigned long)stable_node |
 					PAGE_MAPPING_KSM);
 again:
-	kpfn = READ_ONCE(stable_node->kpfn);
+	kpfn = READ_ONCE(stable_node->kpfn); /* Address dependency. */
 	page = pfn_to_page(kpfn);
-
-	/*
-	 * page is computed from kpfn, so on most architectures reading
-	 * page->mapping is naturally ordered after reading node->kpfn,
-	 * but on Alpha we need to be more careful.
-	 */
-	smp_read_barrier_depends();
 	if (READ_ONCE(page->mapping) != expected_mapping)
 		goto stale;
 
-- 
2.5.2

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH tip/core/rcu 14/21] netfilter: Remove now-redundant smp_read_barrier_depends()
  2017-12-01 19:50 [PATCH tip/core/rcu 0/21] De-emphasize {smp_,}read_barrier_depends Paul E. McKenney
                   ` (12 preceding siblings ...)
  2017-12-01 19:51 ` [PATCH tip/core/rcu 13/21] mm/ksm: Remove now-redundant smp_read_barrier_depends() Paul E. McKenney
@ 2017-12-01 19:51 ` Paul E. McKenney
  2017-12-01 19:51 ` [PATCH tip/core/rcu 15/21] keyring: " Paul E. McKenney
                   ` (7 subsequent siblings)
  21 siblings, 0 replies; 60+ messages in thread
From: Paul E. McKenney @ 2017-12-01 19:51 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, jiangshanlai, dipankar, akpm, mathieu.desnoyers, josh,
	tglx, peterz, rostedt, dhowells, edumazet, fweisbec, oleg,
	Paul E. McKenney, Pablo Neira Ayuso, Jozsef Kadlecsik,
	Florian Westphal, David S. Miller, netfilter-devel, coreteam,
	netdev

READ_ONCE() now implies smp_read_barrier_depends(), which means that
the instances in arpt_do_table(), ipt_do_table(), and ip6t_do_table()
are now redundant.  This commit removes them and adjusts the comments.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
Cc: Florian Westphal <fw@strlen.de>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: <netfilter-devel@vger.kernel.org>
Cc: <coreteam@netfilter.org>
Cc: <netdev@vger.kernel.org>
---
 net/ipv4/netfilter/arp_tables.c | 7 +------
 net/ipv4/netfilter/ip_tables.c  | 7 +------
 net/ipv6/netfilter/ip6_tables.c | 7 +------
 3 files changed, 3 insertions(+), 18 deletions(-)

diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index f88221aebc9d..d242c2d29161 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -202,13 +202,8 @@ unsigned int arpt_do_table(struct sk_buff *skb,
 
 	local_bh_disable();
 	addend = xt_write_recseq_begin();
-	private = table->private;
+	private = READ_ONCE(table->private); /* Address dependency. */
 	cpu     = smp_processor_id();
-	/*
-	 * Ensure we load private-> members after we've fetched the base
-	 * pointer.
-	 */
-	smp_read_barrier_depends();
 	table_base = private->entries;
 	jumpstack  = (struct arpt_entry **)private->jumpstack[cpu];
 
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index 4cbe5e80f3bf..46866cc24a84 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -260,13 +260,8 @@ ipt_do_table(struct sk_buff *skb,
 	WARN_ON(!(table->valid_hooks & (1 << hook)));
 	local_bh_disable();
 	addend = xt_write_recseq_begin();
-	private = table->private;
+	private = READ_ONCE(table->private); /* Address dependency. */
 	cpu        = smp_processor_id();
-	/*
-	 * Ensure we load private-> members after we've fetched the base
-	 * pointer.
-	 */
-	smp_read_barrier_depends();
 	table_base = private->entries;
 	jumpstack  = (struct ipt_entry **)private->jumpstack[cpu];
 
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index f06e25065a34..ac1db84722a7 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -282,12 +282,7 @@ ip6t_do_table(struct sk_buff *skb,
 
 	local_bh_disable();
 	addend = xt_write_recseq_begin();
-	private = table->private;
-	/*
-	 * Ensure we load private-> members after we've fetched the base
-	 * pointer.
-	 */
-	smp_read_barrier_depends();
+	private = READ_ONCE(table->private); /* Address dependency. */
 	cpu        = smp_processor_id();
 	table_base = private->entries;
 	jumpstack  = (struct ip6t_entry **)private->jumpstack[cpu];
-- 
2.5.2

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH tip/core/rcu 15/21] keyring: Remove now-redundant smp_read_barrier_depends()
  2017-12-01 19:50 [PATCH tip/core/rcu 0/21] De-emphasize {smp_,}read_barrier_depends Paul E. McKenney
                   ` (13 preceding siblings ...)
  2017-12-01 19:51 ` [PATCH tip/core/rcu 14/21] netfilter: " Paul E. McKenney
@ 2017-12-01 19:51 ` Paul E. McKenney
  2017-12-04  0:59   ` James Morris
  2017-12-01 19:51 ` [PATCH tip/core/rcu 16/21] drivers/infiniband: " Paul E. McKenney
                   ` (6 subsequent siblings)
  21 siblings, 1 reply; 60+ messages in thread
From: Paul E. McKenney @ 2017-12-01 19:51 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, jiangshanlai, dipankar, akpm, mathieu.desnoyers, josh,
	tglx, peterz, rostedt, dhowells, edumazet, fweisbec, oleg,
	Paul E. McKenney, James Morris, Serge E. Hallyn, keyrings,
	linux-security-module

Now that the associative-array library properly heads dependency chains,
the various smp_read_barrier_depends() calls in security/keys/keyring.c
are no longer needed.  This commit therefore removes them.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: David Howells <dhowells@redhat.com>
Cc: James Morris <james.l.morris@oracle.com>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: <keyrings@vger.kernel.org>
Cc: <linux-security-module@vger.kernel.org>
---
 security/keys/keyring.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index d0bccebbd3b5..41bcf57e96f2 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -713,7 +713,6 @@ static bool search_nested_keyrings(struct key *keyring,
 		 * doesn't contain any keyring pointers.
 		 */
 		shortcut = assoc_array_ptr_to_shortcut(ptr);
-		smp_read_barrier_depends();
 		if ((shortcut->index_key[0] & ASSOC_ARRAY_FAN_MASK) != 0)
 			goto not_this_keyring;
 
@@ -723,8 +722,6 @@ static bool search_nested_keyrings(struct key *keyring,
 	}
 
 	node = assoc_array_ptr_to_node(ptr);
-	smp_read_barrier_depends();
-
 	ptr = node->slots[0];
 	if (!assoc_array_ptr_is_meta(ptr))
 		goto begin_node;
@@ -736,7 +733,6 @@ static bool search_nested_keyrings(struct key *keyring,
 	kdebug("descend");
 	if (assoc_array_ptr_is_shortcut(ptr)) {
 		shortcut = assoc_array_ptr_to_shortcut(ptr);
-		smp_read_barrier_depends();
 		ptr = READ_ONCE(shortcut->next_node);
 		BUG_ON(!assoc_array_ptr_is_node(ptr));
 	}
@@ -744,7 +740,6 @@ static bool search_nested_keyrings(struct key *keyring,
 
 begin_node:
 	kdebug("begin_node");
-	smp_read_barrier_depends();
 	slot = 0;
 ascend_to_node:
 	/* Go through the slots in a node */
@@ -792,14 +787,12 @@ static bool search_nested_keyrings(struct key *keyring,
 
 	if (ptr && assoc_array_ptr_is_shortcut(ptr)) {
 		shortcut = assoc_array_ptr_to_shortcut(ptr);
-		smp_read_barrier_depends();
 		ptr = READ_ONCE(shortcut->back_pointer);
 		slot = shortcut->parent_slot;
 	}
 	if (!ptr)
 		goto not_this_keyring;
 	node = assoc_array_ptr_to_node(ptr);
-	smp_read_barrier_depends();
 	slot++;
 
 	/* If we've ascended to the root (zero backpointer), we must have just
-- 
2.5.2

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH tip/core/rcu 16/21] drivers/infiniband: Remove now-redundant smp_read_barrier_depends()
  2017-12-01 19:50 [PATCH tip/core/rcu 0/21] De-emphasize {smp_,}read_barrier_depends Paul E. McKenney
                   ` (14 preceding siblings ...)
  2017-12-01 19:51 ` [PATCH tip/core/rcu 15/21] keyring: " Paul E. McKenney
@ 2017-12-01 19:51 ` Paul E. McKenney
  2017-12-02  0:11   ` Jason Gunthorpe
  2017-12-01 19:51 ` [PATCH tip/core/rcu 17/21] doc: De-emphasize smp_read_barrier_depends Paul E. McKenney
                   ` (5 subsequent siblings)
  21 siblings, 1 reply; 60+ messages in thread
From: Paul E. McKenney @ 2017-12-01 19:51 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, jiangshanlai, dipankar, akpm, mathieu.desnoyers, josh,
	tglx, peterz, rostedt, dhowells, edumazet, fweisbec, oleg,
	Paul E. McKenney, Doug Ledford, Jason Gunthorpe,
	Richard Henderson, Ivan Kokshaysky, Matt Turner, Michael Cree,
	Andrea Parri, linux-rdma, linux-alpha

The smp_read_barrier_depends() does nothing at all except on DEC Alpha,
and no current DEC Alpha systems use Infiniband:

	lkml.kernel.org/r/20171023085921.jwbntptn6ictbnvj@tower

This commit therefore makes Infiniband depend on !ALPHA and removes
the now-ineffective invocations of smp_read_barrier_depends() from
the InfiniBand driver.

Please note that this patch should not be construed as my saying that
InfiniBand's memory ordering is correct, but rather that this patch does
not in any way affect InfiniBand's correctness.  In other words, the
result of applying this patch is bug-for-bug compatible with the original.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Doug Ledford <dledford@redhat.com>
Cc: Jason Gunthorpe <jgg@mellanox.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc: Matt Turner <mattst88@gmail.com>
Cc: Michael Cree <mcree@orcon.net.nz>
Cc: Andrea Parri <parri.andrea@gmail.com>
Cc: <linux-rdma@vger.kernel.org>
Cc: <linux-alpha@vger.kernel.org>
---
 drivers/dma/ioat/dma.c              | 2 --
 drivers/infiniband/Kconfig          | 1 +
 drivers/infiniband/hw/hfi1/rc.c     | 4 ----
 drivers/infiniband/hw/hfi1/ruc.c    | 1 -
 drivers/infiniband/hw/hfi1/sdma.c   | 1 -
 drivers/infiniband/hw/hfi1/uc.c     | 2 --
 drivers/infiniband/hw/hfi1/ud.c     | 2 --
 drivers/infiniband/hw/qib/qib_rc.c  | 3 ---
 drivers/infiniband/hw/qib/qib_ruc.c | 1 -
 drivers/infiniband/hw/qib/qib_uc.c  | 2 --
 drivers/infiniband/hw/qib/qib_ud.c  | 2 --
 drivers/infiniband/sw/rdmavt/qp.c   | 1 -
 12 files changed, 1 insertion(+), 21 deletions(-)

diff --git a/drivers/dma/ioat/dma.c b/drivers/dma/ioat/dma.c
index 58d4ccd33672..8b5b23a8ace9 100644
--- a/drivers/dma/ioat/dma.c
+++ b/drivers/dma/ioat/dma.c
@@ -597,7 +597,6 @@ static void __cleanup(struct ioatdma_chan *ioat_chan, dma_addr_t phys_complete)
 	for (i = 0; i < active && !seen_current; i++) {
 		struct dma_async_tx_descriptor *tx;
 
-		smp_read_barrier_depends();
 		prefetch(ioat_get_ring_ent(ioat_chan, idx + i + 1));
 		desc = ioat_get_ring_ent(ioat_chan, idx + i);
 		dump_desc_dbg(ioat_chan, desc);
@@ -715,7 +714,6 @@ static void ioat_abort_descs(struct ioatdma_chan *ioat_chan)
 	for (i = 1; i < active; i++) {
 		struct dma_async_tx_descriptor *tx;
 
-		smp_read_barrier_depends();
 		prefetch(ioat_get_ring_ent(ioat_chan, idx + i + 1));
 		desc = ioat_get_ring_ent(ioat_chan, idx + i);
 
diff --git a/drivers/infiniband/Kconfig b/drivers/infiniband/Kconfig
index 98ac46ed7214..3bb6e35b0bbf 100644
--- a/drivers/infiniband/Kconfig
+++ b/drivers/infiniband/Kconfig
@@ -4,6 +4,7 @@ menuconfig INFINIBAND
 	depends on NET
 	depends on INET
 	depends on m || IPV6 != m
+	depends on !ALPHA
 	select IRQ_POLL
 	---help---
 	  Core support for InfiniBand (IB).  Make sure to also select
diff --git a/drivers/infiniband/hw/hfi1/rc.c b/drivers/infiniband/hw/hfi1/rc.c
index fd01a760259f..f527bcda4650 100644
--- a/drivers/infiniband/hw/hfi1/rc.c
+++ b/drivers/infiniband/hw/hfi1/rc.c
@@ -302,7 +302,6 @@ int hfi1_make_rc_req(struct rvt_qp *qp, struct hfi1_pkt_state *ps)
 		if (!(ib_rvt_state_ops[qp->state] & RVT_FLUSH_SEND))
 			goto bail;
 		/* We are in the error state, flush the work request. */
-		smp_read_barrier_depends(); /* see post_one_send() */
 		if (qp->s_last == READ_ONCE(qp->s_head))
 			goto bail;
 		/* If DMAs are in progress, we can't flush immediately. */
@@ -346,7 +345,6 @@ int hfi1_make_rc_req(struct rvt_qp *qp, struct hfi1_pkt_state *ps)
 		newreq = 0;
 		if (qp->s_cur == qp->s_tail) {
 			/* Check if send work queue is empty. */
-			smp_read_barrier_depends(); /* see post_one_send() */
 			if (qp->s_tail == READ_ONCE(qp->s_head)) {
 				clear_ahg(qp);
 				goto bail;
@@ -900,7 +898,6 @@ void hfi1_send_rc_ack(struct hfi1_ctxtdata *rcd,
 	}
 
 	/* Ensure s_rdma_ack_cnt changes are committed */
-	smp_read_barrier_depends();
 	if (qp->s_rdma_ack_cnt) {
 		hfi1_queue_rc_ack(qp, is_fecn);
 		return;
@@ -1562,7 +1559,6 @@ static void rc_rcv_resp(struct hfi1_packet *packet)
 	trace_hfi1_ack(qp, psn);
 
 	/* Ignore invalid responses. */
-	smp_read_barrier_depends(); /* see post_one_send */
 	if (cmp_psn(psn, READ_ONCE(qp->s_next_psn)) >= 0)
 		goto ack_done;
 
diff --git a/drivers/infiniband/hw/hfi1/ruc.c b/drivers/infiniband/hw/hfi1/ruc.c
index 2c7fc6e331ea..13b994738f41 100644
--- a/drivers/infiniband/hw/hfi1/ruc.c
+++ b/drivers/infiniband/hw/hfi1/ruc.c
@@ -362,7 +362,6 @@ static void ruc_loopback(struct rvt_qp *sqp)
 	sqp->s_flags |= RVT_S_BUSY;
 
 again:
-	smp_read_barrier_depends(); /* see post_one_send() */
 	if (sqp->s_last == READ_ONCE(sqp->s_head))
 		goto clr_busy;
 	wqe = rvt_get_swqe_ptr(sqp, sqp->s_last);
diff --git a/drivers/infiniband/hw/hfi1/sdma.c b/drivers/infiniband/hw/hfi1/sdma.c
index 31c8f89b5fc8..61c130dbed10 100644
--- a/drivers/infiniband/hw/hfi1/sdma.c
+++ b/drivers/infiniband/hw/hfi1/sdma.c
@@ -553,7 +553,6 @@ static void sdma_hw_clean_up_task(unsigned long opaque)
 
 static inline struct sdma_txreq *get_txhead(struct sdma_engine *sde)
 {
-	smp_read_barrier_depends(); /* see sdma_update_tail() */
 	return sde->tx_ring[sde->tx_head & sde->sdma_mask];
 }
 
diff --git a/drivers/infiniband/hw/hfi1/uc.c b/drivers/infiniband/hw/hfi1/uc.c
index 991bbee04821..132b63e787d1 100644
--- a/drivers/infiniband/hw/hfi1/uc.c
+++ b/drivers/infiniband/hw/hfi1/uc.c
@@ -79,7 +79,6 @@ int hfi1_make_uc_req(struct rvt_qp *qp, struct hfi1_pkt_state *ps)
 		if (!(ib_rvt_state_ops[qp->state] & RVT_FLUSH_SEND))
 			goto bail;
 		/* We are in the error state, flush the work request. */
-		smp_read_barrier_depends(); /* see post_one_send() */
 		if (qp->s_last == READ_ONCE(qp->s_head))
 			goto bail;
 		/* If DMAs are in progress, we can't flush immediately. */
@@ -119,7 +118,6 @@ int hfi1_make_uc_req(struct rvt_qp *qp, struct hfi1_pkt_state *ps)
 		    RVT_PROCESS_NEXT_SEND_OK))
 			goto bail;
 		/* Check if send work queue is empty. */
-		smp_read_barrier_depends(); /* see post_one_send() */
 		if (qp->s_cur == READ_ONCE(qp->s_head)) {
 			clear_ahg(qp);
 			goto bail;
diff --git a/drivers/infiniband/hw/hfi1/ud.c b/drivers/infiniband/hw/hfi1/ud.c
index beb5091eccca..deb184574395 100644
--- a/drivers/infiniband/hw/hfi1/ud.c
+++ b/drivers/infiniband/hw/hfi1/ud.c
@@ -486,7 +486,6 @@ int hfi1_make_ud_req(struct rvt_qp *qp, struct hfi1_pkt_state *ps)
 		if (!(ib_rvt_state_ops[qp->state] & RVT_FLUSH_SEND))
 			goto bail;
 		/* We are in the error state, flush the work request. */
-		smp_read_barrier_depends(); /* see post_one_send */
 		if (qp->s_last == READ_ONCE(qp->s_head))
 			goto bail;
 		/* If DMAs are in progress, we can't flush immediately. */
@@ -500,7 +499,6 @@ int hfi1_make_ud_req(struct rvt_qp *qp, struct hfi1_pkt_state *ps)
 	}
 
 	/* see post_one_send() */
-	smp_read_barrier_depends();
 	if (qp->s_cur == READ_ONCE(qp->s_head))
 		goto bail;
 
diff --git a/drivers/infiniband/hw/qib/qib_rc.c b/drivers/infiniband/hw/qib/qib_rc.c
index 8f5754fb8579..1a785c37ad0a 100644
--- a/drivers/infiniband/hw/qib/qib_rc.c
+++ b/drivers/infiniband/hw/qib/qib_rc.c
@@ -246,7 +246,6 @@ int qib_make_rc_req(struct rvt_qp *qp, unsigned long *flags)
 		if (!(ib_rvt_state_ops[qp->state] & RVT_FLUSH_SEND))
 			goto bail;
 		/* We are in the error state, flush the work request. */
-		smp_read_barrier_depends(); /* see post_one_send() */
 		if (qp->s_last == READ_ONCE(qp->s_head))
 			goto bail;
 		/* If DMAs are in progress, we can't flush immediately. */
@@ -293,7 +292,6 @@ int qib_make_rc_req(struct rvt_qp *qp, unsigned long *flags)
 		newreq = 0;
 		if (qp->s_cur == qp->s_tail) {
 			/* Check if send work queue is empty. */
-			smp_read_barrier_depends(); /* see post_one_send() */
 			if (qp->s_tail == READ_ONCE(qp->s_head))
 				goto bail;
 			/*
@@ -1340,7 +1338,6 @@ static void qib_rc_rcv_resp(struct qib_ibport *ibp,
 		goto ack_done;
 
 	/* Ignore invalid responses. */
-	smp_read_barrier_depends(); /* see post_one_send */
 	if (qib_cmp24(psn, READ_ONCE(qp->s_next_psn)) >= 0)
 		goto ack_done;
 
diff --git a/drivers/infiniband/hw/qib/qib_ruc.c b/drivers/infiniband/hw/qib/qib_ruc.c
index 9a37e844d4c8..4662cc7bde92 100644
--- a/drivers/infiniband/hw/qib/qib_ruc.c
+++ b/drivers/infiniband/hw/qib/qib_ruc.c
@@ -367,7 +367,6 @@ static void qib_ruc_loopback(struct rvt_qp *sqp)
 	sqp->s_flags |= RVT_S_BUSY;
 
 again:
-	smp_read_barrier_depends(); /* see post_one_send() */
 	if (sqp->s_last == READ_ONCE(sqp->s_head))
 		goto clr_busy;
 	wqe = rvt_get_swqe_ptr(sqp, sqp->s_last);
diff --git a/drivers/infiniband/hw/qib/qib_uc.c b/drivers/infiniband/hw/qib/qib_uc.c
index bddcc37ace44..70c58b88192c 100644
--- a/drivers/infiniband/hw/qib/qib_uc.c
+++ b/drivers/infiniband/hw/qib/qib_uc.c
@@ -60,7 +60,6 @@ int qib_make_uc_req(struct rvt_qp *qp, unsigned long *flags)
 		if (!(ib_rvt_state_ops[qp->state] & RVT_FLUSH_SEND))
 			goto bail;
 		/* We are in the error state, flush the work request. */
-		smp_read_barrier_depends(); /* see post_one_send() */
 		if (qp->s_last == READ_ONCE(qp->s_head))
 			goto bail;
 		/* If DMAs are in progress, we can't flush immediately. */
@@ -90,7 +89,6 @@ int qib_make_uc_req(struct rvt_qp *qp, unsigned long *flags)
 		    RVT_PROCESS_NEXT_SEND_OK))
 			goto bail;
 		/* Check if send work queue is empty. */
-		smp_read_barrier_depends(); /* see post_one_send() */
 		if (qp->s_cur == READ_ONCE(qp->s_head))
 			goto bail;
 		/*
diff --git a/drivers/infiniband/hw/qib/qib_ud.c b/drivers/infiniband/hw/qib/qib_ud.c
index 15962ed193ce..386c3c4da0c7 100644
--- a/drivers/infiniband/hw/qib/qib_ud.c
+++ b/drivers/infiniband/hw/qib/qib_ud.c
@@ -252,7 +252,6 @@ int qib_make_ud_req(struct rvt_qp *qp, unsigned long *flags)
 		if (!(ib_rvt_state_ops[qp->state] & RVT_FLUSH_SEND))
 			goto bail;
 		/* We are in the error state, flush the work request. */
-		smp_read_barrier_depends(); /* see post_one_send */
 		if (qp->s_last == READ_ONCE(qp->s_head))
 			goto bail;
 		/* If DMAs are in progress, we can't flush immediately. */
@@ -266,7 +265,6 @@ int qib_make_ud_req(struct rvt_qp *qp, unsigned long *flags)
 	}
 
 	/* see post_one_send() */
-	smp_read_barrier_depends();
 	if (qp->s_cur == READ_ONCE(qp->s_head))
 		goto bail;
 
diff --git a/drivers/infiniband/sw/rdmavt/qp.c b/drivers/infiniband/sw/rdmavt/qp.c
index 9177df60742a..eae84c216e2f 100644
--- a/drivers/infiniband/sw/rdmavt/qp.c
+++ b/drivers/infiniband/sw/rdmavt/qp.c
@@ -1684,7 +1684,6 @@ static inline int rvt_qp_is_avail(
 	/* non-reserved operations */
 	if (likely(qp->s_avail))
 		return 0;
-	smp_read_barrier_depends(); /* see rc.c */
 	slast = READ_ONCE(qp->s_last);
 	if (qp->s_head >= slast)
 		avail = qp->s_size - (qp->s_head - slast);
-- 
2.5.2

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH tip/core/rcu 17/21] doc: De-emphasize smp_read_barrier_depends
  2017-12-01 19:50 [PATCH tip/core/rcu 0/21] De-emphasize {smp_,}read_barrier_depends Paul E. McKenney
                   ` (15 preceding siblings ...)
  2017-12-01 19:51 ` [PATCH tip/core/rcu 16/21] drivers/infiniband: " Paul E. McKenney
@ 2017-12-01 19:51 ` Paul E. McKenney
  2017-12-01 19:51 ` [PATCH tip/core/rcu 18/21] genetlink: Remove smp_read_barrier_depends() from comment Paul E. McKenney
                   ` (4 subsequent siblings)
  21 siblings, 0 replies; 60+ messages in thread
From: Paul E. McKenney @ 2017-12-01 19:51 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, jiangshanlai, dipankar, akpm, mathieu.desnoyers, josh,
	tglx, peterz, rostedt, dhowells, edumazet, fweisbec, oleg,
	Paul E. McKenney

This commit keeps only the historical and low-level discussion of
smp_read_barrier_depends().

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 Documentation/RCU/Design/Requirements/Requirements.html | 3 ++-
 Documentation/RCU/rcu_dereference.txt                   | 6 +-----
 Documentation/RCU/whatisRCU.txt                         | 3 +--
 Documentation/circular-buffers.txt                      | 3 +--
 Documentation/memory-barriers.txt                       | 7 ++++---
 5 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/Documentation/RCU/Design/Requirements/Requirements.html b/Documentation/RCU/Design/Requirements/Requirements.html
index 62e847bcdcdd..571c3d75510f 100644
--- a/Documentation/RCU/Design/Requirements/Requirements.html
+++ b/Documentation/RCU/Design/Requirements/Requirements.html
@@ -581,7 +581,8 @@ This guarantee was only partially premeditated.
 DYNIX/ptx used an explicit memory barrier for publication, but had nothing
 resembling <tt>rcu_dereference()</tt> for subscription, nor did it
 have anything resembling the <tt>smp_read_barrier_depends()</tt>
-that was later subsumed into <tt>rcu_dereference()</tt>.
+that was later subsumed into <tt>rcu_dereference()</tt> and later
+still into <tt>READ_ONCE()</tt>.
 The need for these operations made itself known quite suddenly at a
 late-1990s meeting with the DEC Alpha architects, back in the days when
 DEC was still a free-standing company.
diff --git a/Documentation/RCU/rcu_dereference.txt b/Documentation/RCU/rcu_dereference.txt
index 1acb26b09b48..ab96227bad42 100644
--- a/Documentation/RCU/rcu_dereference.txt
+++ b/Documentation/RCU/rcu_dereference.txt
@@ -122,11 +122,7 @@ o	Be very careful about comparing pointers obtained from
 		Note that if checks for being within an RCU read-side
 		critical section are not required and the pointer is never
 		dereferenced, rcu_access_pointer() should be used in place
-		of rcu_dereference(). The rcu_access_pointer() primitive
-		does not require an enclosing read-side critical section,
-		and also omits the smp_read_barrier_depends() included in
-		rcu_dereference(), which in turn should provide a small
-		performance gain in some CPUs (e.g., the DEC Alpha).
+		of rcu_dereference().
 
 	o	The comparison is against a pointer that references memory
 		that was initialized "a long time ago."  The reason
diff --git a/Documentation/RCU/whatisRCU.txt b/Documentation/RCU/whatisRCU.txt
index df62466da4e0..a27fbfb0efb8 100644
--- a/Documentation/RCU/whatisRCU.txt
+++ b/Documentation/RCU/whatisRCU.txt
@@ -600,8 +600,7 @@ don't forget about them when submitting patches making use of RCU!]
 
 	#define rcu_dereference(p) \
 	({ \
-		typeof(p) _________p1 = p; \
-		smp_read_barrier_depends(); \
+		typeof(p) _________p1 = READ_ONCE(p); \
 		(_________p1); \
 	})
 
diff --git a/Documentation/circular-buffers.txt b/Documentation/circular-buffers.txt
index d4628174b7c5..53e51caa3347 100644
--- a/Documentation/circular-buffers.txt
+++ b/Documentation/circular-buffers.txt
@@ -220,8 +220,7 @@ before it writes the new tail pointer, which will erase the item.
 
 Note the use of READ_ONCE() and smp_load_acquire() to read the
 opposition index.  This prevents the compiler from discarding and
-reloading its cached value - which some compilers will do across
-smp_read_barrier_depends().  This isn't strictly needed if you can
+reloading its cached value.  This isn't strictly needed if you can
 be sure that the opposition index will _only_ be used the once.
 The smp_load_acquire() additionally forces the CPU to order against
 subsequent memory references.  Similarly, smp_store_release() is used
diff --git a/Documentation/memory-barriers.txt b/Documentation/memory-barriers.txt
index 2269e58fa073..c21a9b25cacf 100644
--- a/Documentation/memory-barriers.txt
+++ b/Documentation/memory-barriers.txt
@@ -236,9 +236,10 @@ There are some minimal guarantees that may be expected of a CPU:
      and always in that order.  On most systems, smp_read_barrier_depends()
      does nothing, but it is required for DEC Alpha, and is supplied by
      READ_ONCE().  The READ_ONCE() is also required to prevent compiler
-     mischief.  Please note that you should normally use something
+     mischief.  Please note that you should almost always use something
      like READ_ONCE() or rcu_dereference() instead of open-coding
-     smp_read_barrier_depends().
+     smp_read_barrier_depends(), the only exceptions being in DEC Alpha
+     architecture-specific code and in ACCESS_ONCE itself.
 
  (*) Overlapping loads and stores within a particular CPU will appear to be
      ordered within that CPU.  This means that for:
@@ -1816,7 +1817,7 @@ The Linux kernel has eight basic CPU memory barriers:
 	GENERAL		mb()			smp_mb()
 	WRITE		wmb()			smp_wmb()
 	READ		rmb()			smp_rmb()
-	DATA DEPENDENCY	read_barrier_depends()	smp_read_barrier_depends()
+	DATA DEPENDENCY				READ_ONCE()
 
 
 All memory barriers except the data dependency barriers imply a compiler
-- 
2.5.2

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH tip/core/rcu 18/21] genetlink: Remove smp_read_barrier_depends() from comment
  2017-12-01 19:50 [PATCH tip/core/rcu 0/21] De-emphasize {smp_,}read_barrier_depends Paul E. McKenney
                   ` (16 preceding siblings ...)
  2017-12-01 19:51 ` [PATCH tip/core/rcu 17/21] doc: De-emphasize smp_read_barrier_depends Paul E. McKenney
@ 2017-12-01 19:51 ` Paul E. McKenney
  2017-12-01 19:51 ` [PATCH tip/core/rcu 19/21] netlink: " Paul E. McKenney
                   ` (3 subsequent siblings)
  21 siblings, 0 replies; 60+ messages in thread
From: Paul E. McKenney @ 2017-12-01 19:51 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, jiangshanlai, dipankar, akpm, mathieu.desnoyers, josh,
	tglx, peterz, rostedt, dhowells, edumazet, fweisbec, oleg,
	Paul E. McKenney, Mark Rutland, Kate Stewart,
	Philippe Ombredanne, Greg Kroah-Hartman

Now that smp_read_barrier_depends() has been de-emphasized, the less
said about it, the better.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Kate Stewart <kstewart@linuxfoundation.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Philippe Ombredanne <pombredanne@nexb.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 include/linux/genetlink.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/linux/genetlink.h b/include/linux/genetlink.h
index ecc2928e8046..bc738504ab4a 100644
--- a/include/linux/genetlink.h
+++ b/include/linux/genetlink.h
@@ -31,8 +31,7 @@ extern wait_queue_head_t genl_sk_destructing_waitq;
  * @p: The pointer to read, prior to dereferencing
  *
  * Return the value of the specified RCU-protected pointer, but omit
- * both the smp_read_barrier_depends() and the READ_ONCE(), because
- * caller holds genl mutex.
+ * the READ_ONCE(), because caller holds genl mutex.
  */
 #define genl_dereference(p)					\
 	rcu_dereference_protected(p, lockdep_genl_is_held())
-- 
2.5.2

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH tip/core/rcu 19/21] netlink: Remove smp_read_barrier_depends() from comment
  2017-12-01 19:50 [PATCH tip/core/rcu 0/21] De-emphasize {smp_,}read_barrier_depends Paul E. McKenney
                   ` (17 preceding siblings ...)
  2017-12-01 19:51 ` [PATCH tip/core/rcu 18/21] genetlink: Remove smp_read_barrier_depends() from comment Paul E. McKenney
@ 2017-12-01 19:51 ` Paul E. McKenney
  2017-12-01 19:51 ` [PATCH tip/core/rcu 20/21] checkpatch: Add warnings for {smp_,}read_barrier_depends() Paul E. McKenney
                   ` (2 subsequent siblings)
  21 siblings, 0 replies; 60+ messages in thread
From: Paul E. McKenney @ 2017-12-01 19:51 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, jiangshanlai, dipankar, akpm, mathieu.desnoyers, josh,
	tglx, peterz, rostedt, dhowells, edumazet, fweisbec, oleg,
	Paul E. McKenney, Pablo Neira Ayuso, Jozsef Kadlecsik,
	Florian Westphal, netfilter-devel, coreteam

Now that smp_read_barrier_depends() has been de-emphasized, the less
said about it, the better.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
Cc: Florian Westphal <fw@strlen.de>
Cc: <netfilter-devel@vger.kernel.org>
Cc: <coreteam@netfilter.org>
---
 include/linux/netfilter/nfnetlink.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/linux/netfilter/nfnetlink.h b/include/linux/netfilter/nfnetlink.h
index 495ba4dd9da5..34551f8aaf9d 100644
--- a/include/linux/netfilter/nfnetlink.h
+++ b/include/linux/netfilter/nfnetlink.h
@@ -67,8 +67,7 @@ static inline bool lockdep_nfnl_is_held(__u8 subsys_id)
  * @ss: The nfnetlink subsystem ID
  *
  * Return the value of the specified RCU-protected pointer, but omit
- * both the smp_read_barrier_depends() and the READ_ONCE(), because
- * caller holds the NFNL subsystem mutex.
+ * the READ_ONCE(), because caller holds the NFNL subsystem mutex.
  */
 #define nfnl_dereference(p, ss)					\
 	rcu_dereference_protected(p, lockdep_nfnl_is_held(ss))
-- 
2.5.2

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH tip/core/rcu 20/21] checkpatch: Add warnings for {smp_,}read_barrier_depends()
  2017-12-01 19:50 [PATCH tip/core/rcu 0/21] De-emphasize {smp_,}read_barrier_depends Paul E. McKenney
                   ` (18 preceding siblings ...)
  2017-12-01 19:51 ` [PATCH tip/core/rcu 19/21] netlink: " Paul E. McKenney
@ 2017-12-01 19:51 ` Paul E. McKenney
  2017-12-01 20:14   ` Joe Perches
  2017-12-01 19:51 ` [PATCH tip/core/rcu 21/21] drivers/vhost: Remove now-redundant read_barrier_depends() Paul E. McKenney
  2017-12-04 15:38 ` [PATCH tip/core/rcu 01/21] doc: READ_ONCE() now implies smp_barrier_depends() David Howells
  21 siblings, 1 reply; 60+ messages in thread
From: Paul E. McKenney @ 2017-12-01 19:51 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, jiangshanlai, dipankar, akpm, mathieu.desnoyers, josh,
	tglx, peterz, rostedt, dhowells, edumazet, fweisbec, oleg,
	Paul E. McKenney, Andy Whitcroft, Joe Perches

Now that both smp_read_barrier_depends() and read_barrier_depends()
are being de-emphasized, warn if any are added.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Andy Whitcroft <apw@canonical.com>
Cc: Joe Perches <joe@perches.com>
---
 scripts/checkpatch.pl | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 95cda3ecc66b..25f7098e2ad3 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -5586,6 +5586,12 @@ sub process {
 			}
 		}
 
+# check for smp_read_barrier_depends and read_barrier_depends
+		if ($line =~ /\b(smp_|)read_barrier_depends\(/) {
+			WARN("READ_BARRIER_DEPENDS",
+			     "Dependency barriers should only be used in READ_ONCE or DEC Alpha code" . $herecurr);
+		}
+
 # check of hardware specific defines
 		if ($line =~ m@^.\s*\#\s*if.*\b(__i386__|__powerpc64__|__sun__|__s390x__)\b@ && $realfile !~ m@include/asm-@) {
 			CHK("ARCH_DEFINES",
-- 
2.5.2

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH tip/core/rcu 21/21] drivers/vhost: Remove now-redundant read_barrier_depends()
  2017-12-01 19:50 [PATCH tip/core/rcu 0/21] De-emphasize {smp_,}read_barrier_depends Paul E. McKenney
                   ` (19 preceding siblings ...)
  2017-12-01 19:51 ` [PATCH tip/core/rcu 20/21] checkpatch: Add warnings for {smp_,}read_barrier_depends() Paul E. McKenney
@ 2017-12-01 19:51 ` Paul E. McKenney
  2017-12-05 18:31   ` Michael S. Tsirkin
  2017-12-04 15:38 ` [PATCH tip/core/rcu 01/21] doc: READ_ONCE() now implies smp_barrier_depends() David Howells
  21 siblings, 1 reply; 60+ messages in thread
From: Paul E. McKenney @ 2017-12-01 19:51 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, jiangshanlai, dipankar, akpm, mathieu.desnoyers, josh,
	tglx, peterz, rostedt, dhowells, edumazet, fweisbec, oleg,
	Paul E. McKenney, Michael S. Tsirkin, Jason Wang, kvm,
	virtualization, netdev

Because READ_ONCE() now implies read_barrier_depends(), the
read_barrier_depends() in next_desc() is now redundant.  This commit
therefore removes it and the related comments.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: <kvm@vger.kernel.org>
Cc: <virtualization@lists.linux-foundation.org>
Cc: <netdev@vger.kernel.org>
---
 drivers/vhost/vhost.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 33ac2b186b85..78b5940a415a 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1877,12 +1877,7 @@ static unsigned next_desc(struct vhost_virtqueue *vq, struct vring_desc *desc)
 		return -1U;
 
 	/* Check they're not leading us off end of descriptors. */
-	next = vhost16_to_cpu(vq, desc->next);
-	/* Make sure compiler knows to grab that: we don't want it changing! */
-	/* We will use the result as an index in an array, so most
-	 * architectures only need a compiler barrier here. */
-	read_barrier_depends();
-
+	next = vhost16_to_cpu(vq, READ_ONCE(desc->next));
 	return next;
 }
 
-- 
2.5.2

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* Re: [PATCH tip/core/rcu 20/21] checkpatch: Add warnings for {smp_,}read_barrier_depends()
  2017-12-01 19:51 ` [PATCH tip/core/rcu 20/21] checkpatch: Add warnings for {smp_,}read_barrier_depends() Paul E. McKenney
@ 2017-12-01 20:14   ` Joe Perches
  2017-12-01 21:44     ` Paul E. McKenney
  0 siblings, 1 reply; 60+ messages in thread
From: Joe Perches @ 2017-12-01 20:14 UTC (permalink / raw)
  To: Paul E. McKenney, linux-kernel
  Cc: mingo, jiangshanlai, dipankar, akpm, mathieu.desnoyers, josh,
	tglx, peterz, rostedt, dhowells, edumazet, fweisbec, oleg,
	Andy Whitcroft

On Fri, 2017-12-01 at 11:51 -0800, Paul E. McKenney wrote:
> Now that both smp_read_barrier_depends() and read_barrier_depends()
> are being de-emphasized, warn if any are added.

This would also warn on existing files when run
with ./scripts/checkpatch.pl -f <file>

Do you want it to check new patches only?

If so the test could become "if (!$file && etc...)

> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 95cda3ecc66b..25f7098e2ad3 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -5586,6 +5586,12 @@ sub process {
>  			}
>  		}
>  
> +# check for smp_read_barrier_depends and read_barrier_depends
> +		if ($line =~ /\b(smp_|)read_barrier_depends\(/) {

Must become

+		if ($line =~ /\b(smp_|)read_barrier_depends\s*\(/) {

similar to the lines above this as there are sometimes
spaces between function name and argument parentheses.

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH tip/core/rcu 20/21] checkpatch: Add warnings for {smp_,}read_barrier_depends()
  2017-12-01 20:14   ` Joe Perches
@ 2017-12-01 21:44     ` Paul E. McKenney
  2017-12-02  4:45       ` Joe Perches
  0 siblings, 1 reply; 60+ messages in thread
From: Paul E. McKenney @ 2017-12-01 21:44 UTC (permalink / raw)
  To: Joe Perches
  Cc: linux-kernel, mingo, jiangshanlai, dipankar, akpm,
	mathieu.desnoyers, josh, tglx, peterz, rostedt, dhowells,
	edumazet, fweisbec, oleg, Andy Whitcroft

On Fri, Dec 01, 2017 at 12:14:17PM -0800, Joe Perches wrote:
> On Fri, 2017-12-01 at 11:51 -0800, Paul E. McKenney wrote:
> > Now that both smp_read_barrier_depends() and read_barrier_depends()
> > are being de-emphasized, warn if any are added.
> 
> This would also warn on existing files when run
> with ./scripts/checkpatch.pl -f <file>
> 
> Do you want it to check new patches only?
> 
> If so the test could become "if (!$file && etc...)
> 
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > index 95cda3ecc66b..25f7098e2ad3 100755
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -5586,6 +5586,12 @@ sub process {
> >  			}
> >  		}
> >  
> > +# check for smp_read_barrier_depends and read_barrier_depends
> > +		if ($line =~ /\b(smp_|)read_barrier_depends\(/) {
> 
> Must become
> 
> +		if ($line =~ /\b(smp_|)read_barrier_depends\s*\(/) {
> 
> similar to the lines above this as there are sometimes
> spaces between function name and argument parentheses.

Good points!  Like this?

							Thanx, Paul

------------------------------------------------------------------------

commit ff155ce179aab891dbe2ca80f82a453383fd165a
Author: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Date:   Mon Nov 27 09:37:35 2017 -0800

    checkpatch: Add warnings for {smp_,}read_barrier_depends()
    
    Now that both smp_read_barrier_depends() and read_barrier_depends()
    are being de-emphasized, warn if any are added.
    
    Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
    Cc: Andy Whitcroft <apw@canonical.com>
    Cc: Joe Perches <joe@perches.com>
    [ paulmck: Skipped checking files and handled whitespace per Joe Perches. ]

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 95cda3ecc66b..0d90518d4147 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -5586,6 +5586,12 @@ sub process {
 			}
 		}
 
+# check for smp_read_barrier_depends and read_barrier_depends
+		if (!$file && $line =~ /\b(smp_|)read_barrier_depends\s*\(/) {
+			WARN("READ_BARRIER_DEPENDS",
+			     "Dependency barriers should only be used in READ_ONCE or DEC Alpha code" . $herecurr);
+		}
+
 # check of hardware specific defines
 		if ($line =~ m@^.\s*\#\s*if.*\b(__i386__|__powerpc64__|__sun__|__s390x__)\b@ && $realfile !~ m@include/asm-@) {
 			CHK("ARCH_DEFINES",

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* Re: [PATCH tip/core/rcu 16/21] drivers/infiniband: Remove now-redundant smp_read_barrier_depends()
  2017-12-01 19:51 ` [PATCH tip/core/rcu 16/21] drivers/infiniband: " Paul E. McKenney
@ 2017-12-02  0:11   ` Jason Gunthorpe
  2017-12-02  1:08     ` Paul E. McKenney
  0 siblings, 1 reply; 60+ messages in thread
From: Jason Gunthorpe @ 2017-12-02  0:11 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: linux-kernel, mingo, jiangshanlai, dipankar, akpm,
	mathieu.desnoyers, josh, tglx, peterz, rostedt, dhowells,
	edumazet, fweisbec, oleg, Doug Ledford, Richard Henderson,
	Ivan Kokshaysky, Matt Turner, Michael Cree, Andrea Parri,
	linux-rdma, linux-alpha

On Fri, Dec 01, 2017 at 11:51:11AM -0800, Paul E. McKenney wrote:
> The smp_read_barrier_depends() does nothing at all except on DEC Alpha,
> and no current DEC Alpha systems use Infiniband:
> 
> 	lkml.kernel.org/r/20171023085921.jwbntptn6ictbnvj@tower

I understand DEC Alpha has PCI, and we continue to support several
RDMA devices that used the PCI bus.

However, the hif1, rdma vt and qib drivers modified in this patch are
PCI-Express ONLY. So I think this idea is fine, but would revise the
commit message to focus on PCI-E not Infiniband.

>  drivers/dma/ioat/dma.c              | 2 --
>  drivers/infiniband/Kconfig          | 1 +

Did you mean to have ioat/dma.c in this patch?

Jason

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH tip/core/rcu 16/21] drivers/infiniband: Remove now-redundant smp_read_barrier_depends()
  2017-12-02  0:11   ` Jason Gunthorpe
@ 2017-12-02  1:08     ` Paul E. McKenney
  2017-12-05 15:08       ` Jason Gunthorpe
  0 siblings, 1 reply; 60+ messages in thread
From: Paul E. McKenney @ 2017-12-02  1:08 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: linux-kernel, mingo, jiangshanlai, dipankar, akpm,
	mathieu.desnoyers, josh, tglx, peterz, rostedt, dhowells,
	edumazet, fweisbec, oleg, Doug Ledford, Richard Henderson,
	Ivan Kokshaysky, Matt Turner, Michael Cree, Andrea Parri,
	linux-rdma, linux-alpha

On Fri, Dec 01, 2017 at 05:11:09PM -0700, Jason Gunthorpe wrote:
> On Fri, Dec 01, 2017 at 11:51:11AM -0800, Paul E. McKenney wrote:
> > The smp_read_barrier_depends() does nothing at all except on DEC Alpha,
> > and no current DEC Alpha systems use Infiniband:
> > 
> > 	lkml.kernel.org/r/20171023085921.jwbntptn6ictbnvj@tower
> 
> I understand DEC Alpha has PCI, and we continue to support several
> RDMA devices that used the PCI bus.
> 
> However, the hif1, rdma vt and qib drivers modified in this patch are
> PCI-Express ONLY. So I think this idea is fine, but would revise the
> commit message to focus on PCI-E not Infiniband.

Hmmm...  It is not just the commit log, but also the Kconfig change.
I am not as worried as I might be about the few museum-piece DEC
Alpha systems suddenly sporting new RDMA PCI devices, but I am worried
about getting a more complex change right.

And if someone really does add a PCI RDMA device to their DEC Alpha,
and this patch causes them trouble, we can work out what to do about
it when and if that happens.

> >  drivers/dma/ioat/dma.c              | 2 --
> >  drivers/infiniband/Kconfig          | 1 +
> 
> Did you mean to have ioat/dma.c in this patch?

Indeed I did not, thank you for catching this!  Please see below for
updated ioat-free patch.

							Thanx, Paul

------------------------------------------------------------------------

commit c389c98ec5f4a7aa4c36853e89801eb5ea81870e
Author: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Date:   Mon Nov 27 09:04:22 2017 -0800

    drivers/infiniband: Remove now-redundant smp_read_barrier_depends()
    
    The smp_read_barrier_depends() does nothing at all except on DEC Alpha,
    and no current DEC Alpha systems use Infiniband:
    
    	lkml.kernel.org/r/20171023085921.jwbntptn6ictbnvj@tower
    
    This commit therefore makes Infiniband depend on !ALPHA and removes
    the now-ineffective invocations of smp_read_barrier_depends() from
    the InfiniBand driver.
    
    Please note that this patch should not be construed as my saying that
    InfiniBand's memory ordering is correct, but rather that this patch does
    not in any way affect InfiniBand's correctness.  In other words, the
    result of applying this patch is bug-for-bug compatible with the original.
    
    Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
    Cc: Doug Ledford <dledford@redhat.com>
    Cc: Jason Gunthorpe <jgg@mellanox.com>
    Cc: Richard Henderson <rth@twiddle.net>
    Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
    Cc: Matt Turner <mattst88@gmail.com>
    Cc: Michael Cree <mcree@orcon.net.nz>
    Cc: Andrea Parri <parri.andrea@gmail.com>
    Cc: <linux-rdma@vger.kernel.org>
    Cc: <linux-alpha@vger.kernel.org>
    [ paulmck: Removed drivers/dma/ioat/dma.c per Jason Gunthorpe's feedback. ]

diff --git a/drivers/infiniband/Kconfig b/drivers/infiniband/Kconfig
index 98ac46ed7214..3bb6e35b0bbf 100644
--- a/drivers/infiniband/Kconfig
+++ b/drivers/infiniband/Kconfig
@@ -4,6 +4,7 @@ menuconfig INFINIBAND
 	depends on NET
 	depends on INET
 	depends on m || IPV6 != m
+	depends on !ALPHA
 	select IRQ_POLL
 	---help---
 	  Core support for InfiniBand (IB).  Make sure to also select
diff --git a/drivers/infiniband/hw/hfi1/rc.c b/drivers/infiniband/hw/hfi1/rc.c
index fd01a760259f..f527bcda4650 100644
--- a/drivers/infiniband/hw/hfi1/rc.c
+++ b/drivers/infiniband/hw/hfi1/rc.c
@@ -302,7 +302,6 @@ int hfi1_make_rc_req(struct rvt_qp *qp, struct hfi1_pkt_state *ps)
 		if (!(ib_rvt_state_ops[qp->state] & RVT_FLUSH_SEND))
 			goto bail;
 		/* We are in the error state, flush the work request. */
-		smp_read_barrier_depends(); /* see post_one_send() */
 		if (qp->s_last == READ_ONCE(qp->s_head))
 			goto bail;
 		/* If DMAs are in progress, we can't flush immediately. */
@@ -346,7 +345,6 @@ int hfi1_make_rc_req(struct rvt_qp *qp, struct hfi1_pkt_state *ps)
 		newreq = 0;
 		if (qp->s_cur == qp->s_tail) {
 			/* Check if send work queue is empty. */
-			smp_read_barrier_depends(); /* see post_one_send() */
 			if (qp->s_tail == READ_ONCE(qp->s_head)) {
 				clear_ahg(qp);
 				goto bail;
@@ -900,7 +898,6 @@ void hfi1_send_rc_ack(struct hfi1_ctxtdata *rcd,
 	}
 
 	/* Ensure s_rdma_ack_cnt changes are committed */
-	smp_read_barrier_depends();
 	if (qp->s_rdma_ack_cnt) {
 		hfi1_queue_rc_ack(qp, is_fecn);
 		return;
@@ -1562,7 +1559,6 @@ static void rc_rcv_resp(struct hfi1_packet *packet)
 	trace_hfi1_ack(qp, psn);
 
 	/* Ignore invalid responses. */
-	smp_read_barrier_depends(); /* see post_one_send */
 	if (cmp_psn(psn, READ_ONCE(qp->s_next_psn)) >= 0)
 		goto ack_done;
 
diff --git a/drivers/infiniband/hw/hfi1/ruc.c b/drivers/infiniband/hw/hfi1/ruc.c
index 2c7fc6e331ea..13b994738f41 100644
--- a/drivers/infiniband/hw/hfi1/ruc.c
+++ b/drivers/infiniband/hw/hfi1/ruc.c
@@ -362,7 +362,6 @@ static void ruc_loopback(struct rvt_qp *sqp)
 	sqp->s_flags |= RVT_S_BUSY;
 
 again:
-	smp_read_barrier_depends(); /* see post_one_send() */
 	if (sqp->s_last == READ_ONCE(sqp->s_head))
 		goto clr_busy;
 	wqe = rvt_get_swqe_ptr(sqp, sqp->s_last);
diff --git a/drivers/infiniband/hw/hfi1/sdma.c b/drivers/infiniband/hw/hfi1/sdma.c
index 31c8f89b5fc8..61c130dbed10 100644
--- a/drivers/infiniband/hw/hfi1/sdma.c
+++ b/drivers/infiniband/hw/hfi1/sdma.c
@@ -553,7 +553,6 @@ static void sdma_hw_clean_up_task(unsigned long opaque)
 
 static inline struct sdma_txreq *get_txhead(struct sdma_engine *sde)
 {
-	smp_read_barrier_depends(); /* see sdma_update_tail() */
 	return sde->tx_ring[sde->tx_head & sde->sdma_mask];
 }
 
diff --git a/drivers/infiniband/hw/hfi1/uc.c b/drivers/infiniband/hw/hfi1/uc.c
index 991bbee04821..132b63e787d1 100644
--- a/drivers/infiniband/hw/hfi1/uc.c
+++ b/drivers/infiniband/hw/hfi1/uc.c
@@ -79,7 +79,6 @@ int hfi1_make_uc_req(struct rvt_qp *qp, struct hfi1_pkt_state *ps)
 		if (!(ib_rvt_state_ops[qp->state] & RVT_FLUSH_SEND))
 			goto bail;
 		/* We are in the error state, flush the work request. */
-		smp_read_barrier_depends(); /* see post_one_send() */
 		if (qp->s_last == READ_ONCE(qp->s_head))
 			goto bail;
 		/* If DMAs are in progress, we can't flush immediately. */
@@ -119,7 +118,6 @@ int hfi1_make_uc_req(struct rvt_qp *qp, struct hfi1_pkt_state *ps)
 		    RVT_PROCESS_NEXT_SEND_OK))
 			goto bail;
 		/* Check if send work queue is empty. */
-		smp_read_barrier_depends(); /* see post_one_send() */
 		if (qp->s_cur == READ_ONCE(qp->s_head)) {
 			clear_ahg(qp);
 			goto bail;
diff --git a/drivers/infiniband/hw/hfi1/ud.c b/drivers/infiniband/hw/hfi1/ud.c
index beb5091eccca..deb184574395 100644
--- a/drivers/infiniband/hw/hfi1/ud.c
+++ b/drivers/infiniband/hw/hfi1/ud.c
@@ -486,7 +486,6 @@ int hfi1_make_ud_req(struct rvt_qp *qp, struct hfi1_pkt_state *ps)
 		if (!(ib_rvt_state_ops[qp->state] & RVT_FLUSH_SEND))
 			goto bail;
 		/* We are in the error state, flush the work request. */
-		smp_read_barrier_depends(); /* see post_one_send */
 		if (qp->s_last == READ_ONCE(qp->s_head))
 			goto bail;
 		/* If DMAs are in progress, we can't flush immediately. */
@@ -500,7 +499,6 @@ int hfi1_make_ud_req(struct rvt_qp *qp, struct hfi1_pkt_state *ps)
 	}
 
 	/* see post_one_send() */
-	smp_read_barrier_depends();
 	if (qp->s_cur == READ_ONCE(qp->s_head))
 		goto bail;
 
diff --git a/drivers/infiniband/hw/qib/qib_rc.c b/drivers/infiniband/hw/qib/qib_rc.c
index 8f5754fb8579..1a785c37ad0a 100644
--- a/drivers/infiniband/hw/qib/qib_rc.c
+++ b/drivers/infiniband/hw/qib/qib_rc.c
@@ -246,7 +246,6 @@ int qib_make_rc_req(struct rvt_qp *qp, unsigned long *flags)
 		if (!(ib_rvt_state_ops[qp->state] & RVT_FLUSH_SEND))
 			goto bail;
 		/* We are in the error state, flush the work request. */
-		smp_read_barrier_depends(); /* see post_one_send() */
 		if (qp->s_last == READ_ONCE(qp->s_head))
 			goto bail;
 		/* If DMAs are in progress, we can't flush immediately. */
@@ -293,7 +292,6 @@ int qib_make_rc_req(struct rvt_qp *qp, unsigned long *flags)
 		newreq = 0;
 		if (qp->s_cur == qp->s_tail) {
 			/* Check if send work queue is empty. */
-			smp_read_barrier_depends(); /* see post_one_send() */
 			if (qp->s_tail == READ_ONCE(qp->s_head))
 				goto bail;
 			/*
@@ -1340,7 +1338,6 @@ static void qib_rc_rcv_resp(struct qib_ibport *ibp,
 		goto ack_done;
 
 	/* Ignore invalid responses. */
-	smp_read_barrier_depends(); /* see post_one_send */
 	if (qib_cmp24(psn, READ_ONCE(qp->s_next_psn)) >= 0)
 		goto ack_done;
 
diff --git a/drivers/infiniband/hw/qib/qib_ruc.c b/drivers/infiniband/hw/qib/qib_ruc.c
index 9a37e844d4c8..4662cc7bde92 100644
--- a/drivers/infiniband/hw/qib/qib_ruc.c
+++ b/drivers/infiniband/hw/qib/qib_ruc.c
@@ -367,7 +367,6 @@ static void qib_ruc_loopback(struct rvt_qp *sqp)
 	sqp->s_flags |= RVT_S_BUSY;
 
 again:
-	smp_read_barrier_depends(); /* see post_one_send() */
 	if (sqp->s_last == READ_ONCE(sqp->s_head))
 		goto clr_busy;
 	wqe = rvt_get_swqe_ptr(sqp, sqp->s_last);
diff --git a/drivers/infiniband/hw/qib/qib_uc.c b/drivers/infiniband/hw/qib/qib_uc.c
index bddcc37ace44..70c58b88192c 100644
--- a/drivers/infiniband/hw/qib/qib_uc.c
+++ b/drivers/infiniband/hw/qib/qib_uc.c
@@ -60,7 +60,6 @@ int qib_make_uc_req(struct rvt_qp *qp, unsigned long *flags)
 		if (!(ib_rvt_state_ops[qp->state] & RVT_FLUSH_SEND))
 			goto bail;
 		/* We are in the error state, flush the work request. */
-		smp_read_barrier_depends(); /* see post_one_send() */
 		if (qp->s_last == READ_ONCE(qp->s_head))
 			goto bail;
 		/* If DMAs are in progress, we can't flush immediately. */
@@ -90,7 +89,6 @@ int qib_make_uc_req(struct rvt_qp *qp, unsigned long *flags)
 		    RVT_PROCESS_NEXT_SEND_OK))
 			goto bail;
 		/* Check if send work queue is empty. */
-		smp_read_barrier_depends(); /* see post_one_send() */
 		if (qp->s_cur == READ_ONCE(qp->s_head))
 			goto bail;
 		/*
diff --git a/drivers/infiniband/hw/qib/qib_ud.c b/drivers/infiniband/hw/qib/qib_ud.c
index 15962ed193ce..386c3c4da0c7 100644
--- a/drivers/infiniband/hw/qib/qib_ud.c
+++ b/drivers/infiniband/hw/qib/qib_ud.c
@@ -252,7 +252,6 @@ int qib_make_ud_req(struct rvt_qp *qp, unsigned long *flags)
 		if (!(ib_rvt_state_ops[qp->state] & RVT_FLUSH_SEND))
 			goto bail;
 		/* We are in the error state, flush the work request. */
-		smp_read_barrier_depends(); /* see post_one_send */
 		if (qp->s_last == READ_ONCE(qp->s_head))
 			goto bail;
 		/* If DMAs are in progress, we can't flush immediately. */
@@ -266,7 +265,6 @@ int qib_make_ud_req(struct rvt_qp *qp, unsigned long *flags)
 	}
 
 	/* see post_one_send() */
-	smp_read_barrier_depends();
 	if (qp->s_cur == READ_ONCE(qp->s_head))
 		goto bail;
 
diff --git a/drivers/infiniband/sw/rdmavt/qp.c b/drivers/infiniband/sw/rdmavt/qp.c
index 9177df60742a..eae84c216e2f 100644
--- a/drivers/infiniband/sw/rdmavt/qp.c
+++ b/drivers/infiniband/sw/rdmavt/qp.c
@@ -1684,7 +1684,6 @@ static inline int rvt_qp_is_avail(
 	/* non-reserved operations */
 	if (likely(qp->s_avail))
 		return 0;
-	smp_read_barrier_depends(); /* see rc.c */
 	slast = READ_ONCE(qp->s_last);
 	if (qp->s_head >= slast)
 		avail = qp->s_size - (qp->s_head - slast);

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* Re: [PATCH tip/core/rcu 20/21] checkpatch: Add warnings for {smp_,}read_barrier_depends()
  2017-12-01 21:44     ` Paul E. McKenney
@ 2017-12-02  4:45       ` Joe Perches
  2017-12-04 19:06         ` Paul E. McKenney
  0 siblings, 1 reply; 60+ messages in thread
From: Joe Perches @ 2017-12-02  4:45 UTC (permalink / raw)
  To: paulmck
  Cc: linux-kernel, mingo, jiangshanlai, dipankar, akpm,
	mathieu.desnoyers, josh, tglx, peterz, rostedt, dhowells,
	edumazet, fweisbec, oleg, Andy Whitcroft

On Fri, 2017-12-01 at 13:44 -0800, Paul E. McKenney wrote:
> On Fri, Dec 01, 2017 at 12:14:17PM -0800, Joe Perches wrote:
> > On Fri, 2017-12-01 at 11:51 -0800, Paul E. McKenney wrote:
> > > Now that both smp_read_barrier_depends() and read_barrier_depends()
> > > are being de-emphasized, warn if any are added.
> > 
> > This would also warn on existing files when run
> > with ./scripts/checkpatch.pl -f <file>
> > 
> > Do you want it to check new patches only?
> > 
> > If so the test could become "if (!$file && etc...)
> > 
> > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> > > @@ -5586,6 +5586,12 @@ sub process {
> > >  			}
> > >  		}
> > >  
> > > +# check for smp_read_barrier_depends and read_barrier_depends
> > > +		if ($line =~ /\b(smp_|)read_barrier_depends\(/) {
> > 
> > Must become
> > 
> > +		if ($line =~ /\b(smp_|)read_barrier_depends\s*\(/) {
> > 
> > similar to the lines above this as there are sometimes
> > spaces between function name and argument parentheses.
> 
> Good points!  Like this?
[]
> commit ff155ce179aab891dbe2ca80f82a453383fd165a
> Author: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Date:   Mon Nov 27 09:37:35 2017 -0800
> 
>     checkpatch: Add warnings for {smp_,}read_barrier_depends()
>     
>     Now that both smp_read_barrier_depends() and read_barrier_depends()
>     are being de-emphasized, warn if any are added.
>     
>     Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
>     Cc: Andy Whitcroft <apw@canonical.com>
>     Cc: Joe Perches <joe@perches.com>
>     [ paulmck: Skipped checking files and handled whitespace per Joe Perches. ]
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -5586,6 +5586,12 @@ sub process {
>  			}
>  		}
>  
> +# check for smp_read_barrier_depends and read_barrier_depends
> +		if (!$file && $line =~ /\b(smp_|)read_barrier_depends\s*\(/) {
> +			WARN("READ_BARRIER_DEPENDS",
> +			     "Dependency barriers should only be used in READ_ONCE or DEC Alpha code" . $herecurr);

Almost right.  Missing a '\n' after "DEC Alpha code".
Without the '\n', running checkpatch with --terse outputs badly.

If you really wanted to optimize, you could make the first (smp_|)
become (?:smp_|) to avoid the unused capture group.
Or perhaps this could emit the actual type in the message like:

		if (!$file && $line =~ /\b((?:smp_|)read_barrier_depends)\s*\(/ {
			WARN("READ_BARRIER_DEPENDS",
			     "$1 should only be used in READ_ONCE or DEC Alpha code\n" . $herecurr);

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH tip/core/rcu 15/21] keyring: Remove now-redundant smp_read_barrier_depends()
  2017-12-01 19:51 ` [PATCH tip/core/rcu 15/21] keyring: " Paul E. McKenney
@ 2017-12-04  0:59   ` James Morris
  2017-12-04 18:54     ` Paul E. McKenney
  0 siblings, 1 reply; 60+ messages in thread
From: James Morris @ 2017-12-04  0:59 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: linux-kernel, mingo, jiangshanlai, dipankar, akpm,
	mathieu.desnoyers, josh, tglx, peterz, rostedt, dhowells,
	edumazet, fweisbec, oleg, Serge E. Hallyn, keyrings,
	linux-security-module

On Fri, 1 Dec 2017, Paul E. McKenney wrote:

> Now that the associative-array library properly heads dependency chains,
> the various smp_read_barrier_depends() calls in security/keys/keyring.c
> are no longer needed.  This commit therefore removes them.
> 
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Cc: David Howells <dhowells@redhat.com>
> Cc: James Morris <james.l.morris@oracle.com>
> Cc: "Serge E. Hallyn" <serge@hallyn.com>
> Cc: <keyrings@vger.kernel.org>
> Cc: <linux-security-module@vger.kernel.org>


Reviewed-by: James Morris <james.l.morris@oracle.com>


-- 
James Morris
<james.l.morris@oracle.com>

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH tip/core/rcu 01/21] doc: READ_ONCE() now implies smp_barrier_depends()
  2017-12-01 19:50 [PATCH tip/core/rcu 0/21] De-emphasize {smp_,}read_barrier_depends Paul E. McKenney
                   ` (20 preceding siblings ...)
  2017-12-01 19:51 ` [PATCH tip/core/rcu 21/21] drivers/vhost: Remove now-redundant read_barrier_depends() Paul E. McKenney
@ 2017-12-04 15:38 ` David Howells
  2017-12-04 18:52   ` Paul E. McKenney
  21 siblings, 1 reply; 60+ messages in thread
From: David Howells @ 2017-12-04 15:38 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: dhowells, linux-kernel, mingo, jiangshanlai, dipankar, akpm,
	mathieu.desnoyers, josh, tglx, peterz, rostedt, edumazet,
	fweisbec, oleg

Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:

> -	Q = READ_ONCE(P); smp_read_barrier_depends(); D = READ_ONCE(*Q);
> +	Q = READ_ONCE(P); D = READ_ONCE(*Q);
>  
>       the CPU will issue the following memory operations:
>  
>  	Q = LOAD P, D = LOAD *Q

The CPU may now issue two barriers in addition to the loads, so should we show
this?  E.g.:

	Q = LOAD P, BARRIER, D = LOAD *Q, BARRIER

David

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH tip/core/rcu 01/21] doc: READ_ONCE() now implies smp_barrier_depends()
  2017-12-04 15:38 ` [PATCH tip/core/rcu 01/21] doc: READ_ONCE() now implies smp_barrier_depends() David Howells
@ 2017-12-04 18:52   ` Paul E. McKenney
  2017-12-04 21:54     ` Peter Zijlstra
  2017-12-04 22:39     ` David Howells
  0 siblings, 2 replies; 60+ messages in thread
From: Paul E. McKenney @ 2017-12-04 18:52 UTC (permalink / raw)
  To: David Howells
  Cc: linux-kernel, mingo, jiangshanlai, dipankar, akpm,
	mathieu.desnoyers, josh, tglx, peterz, rostedt, edumazet,
	fweisbec, oleg

On Mon, Dec 04, 2017 at 03:38:56PM +0000, David Howells wrote:
> Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:
> 
> > -	Q = READ_ONCE(P); smp_read_barrier_depends(); D = READ_ONCE(*Q);
> > +	Q = READ_ONCE(P); D = READ_ONCE(*Q);
> >  
> >       the CPU will issue the following memory operations:
> >  
> >  	Q = LOAD P, D = LOAD *Q
> 
> The CPU may now issue two barriers in addition to the loads, so should we show
> this?  E.g.:
> 
> 	Q = LOAD P, BARRIER, D = LOAD *Q, BARRIER

Good point!  How about as shown in the updated patch below?

							Thanx, Paul

------------------------------------------------------------------------

commit 40555946447a394889243e4393e312f65d847e1e
Author: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Date:   Mon Oct 9 09:15:21 2017 -0700

    doc: READ_ONCE() now implies smp_barrier_depends()
    
    This commit updates an example in memory-barriers.txt to account for
    the fact that READ_ONCE() now implies smp_barrier_depends().
    
    Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
    [ paulmck: Added MEMORY_BARRIER instructions from DEC Alpha from
      READ_ONCE(), per David Howells's feedback. ]

diff --git a/Documentation/memory-barriers.txt b/Documentation/memory-barriers.txt
index 479ecec80593..13fd35b6a597 100644
--- a/Documentation/memory-barriers.txt
+++ b/Documentation/memory-barriers.txt
@@ -227,17 +227,20 @@ There are some minimal guarantees that may be expected of a CPU:
  (*) On any given CPU, dependent memory accesses will be issued in order, with
      respect to itself.  This means that for:
 
-	Q = READ_ONCE(P); smp_read_barrier_depends(); D = READ_ONCE(*Q);
+	Q = READ_ONCE(P); D = READ_ONCE(*Q);
 
      the CPU will issue the following memory operations:
 
 	Q = LOAD P, D = LOAD *Q
 
-     and always in that order.  On most systems, smp_read_barrier_depends()
-     does nothing, but it is required for DEC Alpha.  The READ_ONCE()
-     is required to prevent compiler mischief.  Please note that you
-     should normally use something like rcu_dereference() instead of
-     open-coding smp_read_barrier_depends().
+     and always in that order.  However, on DEC Alpha, READ_ONCE() also
+     emits a memory-barrier instruction, so that a DEC Alpha CPU will
+     instead issue the following memory operations:
+
+	Q = LOAD P, MEMORY_BARRIER, D = LOAD *Q, MEMORY_BARRIER
+
+     Whether on DEC Alpha or not, the READ_ONCE() also prevents compiler
+     mischief.
 
  (*) Overlapping loads and stores within a particular CPU will appear to be
      ordered within that CPU.  This means that for:

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* Re: [PATCH tip/core/rcu 15/21] keyring: Remove now-redundant smp_read_barrier_depends()
  2017-12-04  0:59   ` James Morris
@ 2017-12-04 18:54     ` Paul E. McKenney
  0 siblings, 0 replies; 60+ messages in thread
From: Paul E. McKenney @ 2017-12-04 18:54 UTC (permalink / raw)
  To: James Morris
  Cc: linux-kernel, mingo, jiangshanlai, dipankar, akpm,
	mathieu.desnoyers, josh, tglx, peterz, rostedt, dhowells,
	edumazet, fweisbec, oleg, Serge E. Hallyn, keyrings,
	linux-security-module

On Mon, Dec 04, 2017 at 11:59:22AM +1100, James Morris wrote:
> On Fri, 1 Dec 2017, Paul E. McKenney wrote:
> 
> > Now that the associative-array library properly heads dependency chains,
> > the various smp_read_barrier_depends() calls in security/keys/keyring.c
> > are no longer needed.  This commit therefore removes them.
> > 
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > Cc: David Howells <dhowells@redhat.com>
> > Cc: James Morris <james.l.morris@oracle.com>
> > Cc: "Serge E. Hallyn" <serge@hallyn.com>
> > Cc: <keyrings@vger.kernel.org>
> > Cc: <linux-security-module@vger.kernel.org>
> 
> Reviewed-by: James Morris <james.l.morris@oracle.com>

Applied, thank you for the review!

							Thanx, Paul

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH tip/core/rcu 20/21] checkpatch: Add warnings for {smp_,}read_barrier_depends()
  2017-12-02  4:45       ` Joe Perches
@ 2017-12-04 19:06         ` Paul E. McKenney
  2017-12-04 19:12           ` Joe Perches
  0 siblings, 1 reply; 60+ messages in thread
From: Paul E. McKenney @ 2017-12-04 19:06 UTC (permalink / raw)
  To: Joe Perches
  Cc: linux-kernel, mingo, jiangshanlai, dipankar, akpm,
	mathieu.desnoyers, josh, tglx, peterz, rostedt, dhowells,
	edumazet, fweisbec, oleg, Andy Whitcroft

On Fri, Dec 01, 2017 at 08:45:13PM -0800, Joe Perches wrote:
> On Fri, 2017-12-01 at 13:44 -0800, Paul E. McKenney wrote:

[ . . . ]

> > Good points!  Like this?
> []
> > commit ff155ce179aab891dbe2ca80f82a453383fd165a
> > Author: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > Date:   Mon Nov 27 09:37:35 2017 -0800
> > 
> >     checkpatch: Add warnings for {smp_,}read_barrier_depends()
> >     
> >     Now that both smp_read_barrier_depends() and read_barrier_depends()
> >     are being de-emphasized, warn if any are added.
> >     
> >     Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> >     Cc: Andy Whitcroft <apw@canonical.com>
> >     Cc: Joe Perches <joe@perches.com>
> >     [ paulmck: Skipped checking files and handled whitespace per Joe Perches. ]
> > 
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> []
> > @@ -5586,6 +5586,12 @@ sub process {
> >  			}
> >  		}
> >  
> > +# check for smp_read_barrier_depends and read_barrier_depends
> > +		if (!$file && $line =~ /\b(smp_|)read_barrier_depends\s*\(/) {
> > +			WARN("READ_BARRIER_DEPENDS",
> > +			     "Dependency barriers should only be used in READ_ONCE or DEC Alpha code" . $herecurr);
> 
> Almost right.  Missing a '\n' after "DEC Alpha code".
> Without the '\n', running checkpatch with --terse outputs badly.
> 
> If you really wanted to optimize, you could make the first (smp_|)
> become (?:smp_|) to avoid the unused capture group.
> Or perhaps this could emit the actual type in the message like:
> 
> 		if (!$file && $line =~ /\b((?:smp_|)read_barrier_depends)\s*\(/ {
> 			WARN("READ_BARRIER_DEPENDS",
> 			     "$1 should only be used in READ_ONCE or DEC Alpha code\n" . $herecurr);

How about like this?  (s/$1/$1read_barrier_depends/).

							Thanx, Paul

------------------------------------------------------------------------

commit e4f1d781e33c150547c68e55099794b98ee14d5e
Author: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Date:   Mon Nov 27 09:37:35 2017 -0800

    checkpatch: Add warnings for {smp_,}read_barrier_depends()
    
    Now that both smp_read_barrier_depends() and read_barrier_depends()
    are being de-emphasized, warn if any are added.
    
    Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
    Cc: Andy Whitcroft <apw@canonical.com>
    Cc: Joe Perches <joe@perches.com>
    [ paulmck: Skipped checking files and handled whitespace per Joe Perches. ]

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 95cda3ecc66b..9a384bfe2bd5 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -5586,6 +5586,12 @@ sub process {
 			}
 		}
 
+# check for smp_read_barrier_depends and read_barrier_depends
+		if (!$file && $line =~ /\b(smp_|)read_barrier_depends\s*\(/) {
+			WARN("READ_BARRIER_DEPENDS",
+			     "$1read_barrier_depends should only be used in READ_ONCE or DEC Alpha code\n" . $herecurr);
+		}
+
 # check of hardware specific defines
 		if ($line =~ m@^.\s*\#\s*if.*\b(__i386__|__powerpc64__|__sun__|__s390x__)\b@ && $realfile !~ m@include/asm-@) {
 			CHK("ARCH_DEFINES",

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* Re: [PATCH tip/core/rcu 20/21] checkpatch: Add warnings for {smp_,}read_barrier_depends()
  2017-12-04 19:06         ` Paul E. McKenney
@ 2017-12-04 19:12           ` Joe Perches
  0 siblings, 0 replies; 60+ messages in thread
From: Joe Perches @ 2017-12-04 19:12 UTC (permalink / raw)
  To: paulmck
  Cc: linux-kernel, mingo, jiangshanlai, dipankar, akpm,
	mathieu.desnoyers, josh, tglx, peterz, rostedt, dhowells,
	edumazet, fweisbec, oleg, Andy Whitcroft

On Mon, 2017-12-04 at 11:06 -0800, Paul E. McKenney wrote:
> On Fri, Dec 01, 2017 at 08:45:13PM -0800, Joe Perches wrote:
> > On Fri, 2017-12-01 at 13:44 -0800, Paul E. McKenney wrote:
> 
> > If you really wanted to optimize, you could make the first (smp_|)
> > become (?:smp_|) to avoid the unused capture group.
> > Or perhaps this could emit the actual type in the message like:
> > 
> > 		if (!$file && $line =~ /\b((?:smp_|)read_barrier_depends)\s*\(/ {
> > 			WARN("READ_BARRIER_DEPENDS",
> > 			     "$1 should only be used in READ_ONCE or DEC Alpha code\n" . $herecurr);
> 
> How about like this?  (s/$1/$1read_barrier_depends/).

Works for me.

Acked-by: Joe Perches <joe@perches.com>

Are you going to merge this?
Normally akpm does checkpatch merging but whatever
works for you is OK by me.

cheers, Joe

> commit e4f1d781e33c150547c68e55099794b98ee14d5e
> Author: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Date:   Mon Nov 27 09:37:35 2017 -0800
> {smp_,}read_barrier_depends()
>     
>     Now that both smp_read_barrier_depends() and read_barrier_depends()
>     are being de-emphasized, warn if any are added.
>     
>     Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
>     Cc: Andy Whitcroft <apw@canonical.com>
>     Cc: Joe Perches <joe@perches.com>
>     [ paulmck: Skipped checking files and handled whitespace per Joe Perches.
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 95cda3ecc66b..9a384bfe2bd5 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -5586,6 +5586,12 @@ sub process {
>  			}
>  		}
>  
> +# check for smp_read_barrier_depends and read_barrier_depends
> +		if (!$file && $line =~ /\b(smp_|)read_barrier_depends\s*\(/) {
> +			WARN("READ_BARRIER_DEPENDS",
> +			     "$1read_barrier_depends should only be used in READ_ONCE or DEC Alpha code\n" . $herecurr);
> +		}
> +
>  # check of hardware specific defines
>  		if ($line =~ m@^.\s*\#\s*if.*\b(__i386__|__powerpc64__|__sun__|__s390x__)\b@ && $realfile !~ m@include/asm-@) {
>  			CHK("ARCH_DEFINES",
> 

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH tip/core/rcu 01/21] doc: READ_ONCE() now implies smp_barrier_depends()
  2017-12-04 18:52   ` Paul E. McKenney
@ 2017-12-04 21:54     ` Peter Zijlstra
  2017-12-04 22:15       ` Paul E. McKenney
  2017-12-04 22:39     ` David Howells
  1 sibling, 1 reply; 60+ messages in thread
From: Peter Zijlstra @ 2017-12-04 21:54 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: David Howells, linux-kernel, mingo, jiangshanlai, dipankar, akpm,
	mathieu.desnoyers, josh, tglx, rostedt, edumazet, fweisbec, oleg

On Mon, Dec 04, 2017 at 10:52:15AM -0800, Paul E. McKenney wrote:
> On Mon, Dec 04, 2017 at 03:38:56PM +0000, David Howells wrote:
> > Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:
> > 
> > > -	Q = READ_ONCE(P); smp_read_barrier_depends(); D = READ_ONCE(*Q);
> > > +	Q = READ_ONCE(P); D = READ_ONCE(*Q);
> > >  
> > >       the CPU will issue the following memory operations:
> > >  
> > >  	Q = LOAD P, D = LOAD *Q
> > 
> > The CPU may now issue two barriers in addition to the loads, so should we show
> > this?  E.g.:
> > 
> > 	Q = LOAD P, BARRIER, D = LOAD *Q, BARRIER
> 
> Good point!  How about as shown in the updated patch below?

Humm, I thought the idea was to completely remove read_barrier_depends
from the lkmm and memory-barriers.txt, making it an Alpha implementation
detail.

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH tip/core/rcu 01/21] doc: READ_ONCE() now implies smp_barrier_depends()
  2017-12-04 21:54     ` Peter Zijlstra
@ 2017-12-04 22:15       ` Paul E. McKenney
  0 siblings, 0 replies; 60+ messages in thread
From: Paul E. McKenney @ 2017-12-04 22:15 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: David Howells, linux-kernel, mingo, jiangshanlai, dipankar, akpm,
	mathieu.desnoyers, josh, tglx, rostedt, edumazet, fweisbec, oleg

On Mon, Dec 04, 2017 at 10:54:48PM +0100, Peter Zijlstra wrote:
> On Mon, Dec 04, 2017 at 10:52:15AM -0800, Paul E. McKenney wrote:
> > On Mon, Dec 04, 2017 at 03:38:56PM +0000, David Howells wrote:
> > > Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:
> > > 
> > > > -	Q = READ_ONCE(P); smp_read_barrier_depends(); D = READ_ONCE(*Q);
> > > > +	Q = READ_ONCE(P); D = READ_ONCE(*Q);
> > > >  
> > > >       the CPU will issue the following memory operations:
> > > >  
> > > >  	Q = LOAD P, D = LOAD *Q
> > > 
> > > The CPU may now issue two barriers in addition to the loads, so should we show
> > > this?  E.g.:
> > > 
> > > 	Q = LOAD P, BARRIER, D = LOAD *Q, BARRIER
> > 
> > Good point!  How about as shown in the updated patch below?
> 
> Humm, I thought the idea was to completely remove read_barrier_depends
> from the lkmm and memory-barriers.txt, making it an Alpha implementation
> detail.

That was indeed my hope, but a too-abrupt departure of DEC Alpha seemed
to be causing some confusion, so I jumped on David's suggested change.  My
hope now is to slowly remove mention of DEC Alpha from the documentation.

Hmmm...  Maybe we need an LWN article on how we are weaning the memory
model from its historical DEC Alpha influences?  That might get the word
out more effectively.

							Thanx, Paul

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH tip/core/rcu 01/21] doc: READ_ONCE() now implies smp_barrier_depends()
  2017-12-04 18:52   ` Paul E. McKenney
  2017-12-04 21:54     ` Peter Zijlstra
@ 2017-12-04 22:39     ` David Howells
  2017-12-04 22:57       ` Peter Zijlstra
  1 sibling, 1 reply; 60+ messages in thread
From: David Howells @ 2017-12-04 22:39 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: dhowells, Paul E. McKenney, linux-kernel, mingo, jiangshanlai,
	dipankar, akpm, mathieu.desnoyers, josh, tglx, rostedt, edumazet,
	fweisbec, oleg

Peter Zijlstra <peterz@infradead.org> wrote:

> > Good point!  How about as shown in the updated patch below?
> 
> Humm, I thought the idea was to completely remove read_barrier_depends
> from the lkmm and memory-barriers.txt, making it an Alpha implementation
> detail.

memory-barriers.txt explains how the barriers used by the kernel work, amongst
other things.

Don't forget, btw, that Blackfin uses it also.

David

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH tip/core/rcu 01/21] doc: READ_ONCE() now implies smp_barrier_depends()
  2017-12-04 22:39     ` David Howells
@ 2017-12-04 22:57       ` Peter Zijlstra
  0 siblings, 0 replies; 60+ messages in thread
From: Peter Zijlstra @ 2017-12-04 22:57 UTC (permalink / raw)
  To: David Howells
  Cc: Paul E. McKenney, linux-kernel, mingo, jiangshanlai, dipankar,
	akpm, mathieu.desnoyers, josh, tglx, rostedt, edumazet, fweisbec,
	oleg

On Mon, Dec 04, 2017 at 10:39:05PM +0000, David Howells wrote:
> Peter Zijlstra <peterz@infradead.org> wrote:
> 
> > > Good point!  How about as shown in the updated patch below?
> > 
> > Humm, I thought the idea was to completely remove read_barrier_depends
> > from the lkmm and memory-barriers.txt, making it an Alpha implementation
> > detail.
> 
> memory-barriers.txt explains how the barriers used by the kernel work, amongst
> other things.

The thing is, read-barrier-depends will no longer be used. read-read
dependencies will henceforth be assumed to 'just work'.

> Don't forget, btw, that Blackfin uses it also.

We should delete that blackfin code, its beyond insane.

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH tip/core/rcu 16/21] drivers/infiniband: Remove now-redundant smp_read_barrier_depends()
  2017-12-02  1:08     ` Paul E. McKenney
@ 2017-12-05 15:08       ` Jason Gunthorpe
  2017-12-05 20:06         ` Paul E. McKenney
  0 siblings, 1 reply; 60+ messages in thread
From: Jason Gunthorpe @ 2017-12-05 15:08 UTC (permalink / raw)
  To: Paul E. McKenney, Dennis Dalessandro
  Cc: linux-kernel, mingo, jiangshanlai, dipankar, akpm,
	mathieu.desnoyers, josh, tglx, peterz, rostedt, dhowells,
	edumazet, fweisbec, oleg, Doug Ledford, Richard Henderson,
	Ivan Kokshaysky, Matt Turner, Michael Cree, Andrea Parri,
	linux-rdma, linux-alpha

> commit c389c98ec5f4a7aa4c36853e89801eb5ea81870e
> Author: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Date:   Mon Nov 27 09:04:22 2017 -0800
> 
>     drivers/infiniband: Remove now-redundant smp_read_barrier_depends()
>     
>     The smp_read_barrier_depends() does nothing at all except on DEC Alpha,
>     and no current DEC Alpha systems use Infiniband:
>     
>     	lkml.kernel.org/r/20171023085921.jwbntptn6ictbnvj@tower
>     
>     This commit therefore makes Infiniband depend on !ALPHA and removes
>     the now-ineffective invocations of smp_read_barrier_depends() from
>     the InfiniBand driver.
>     
>     Please note that this patch should not be construed as my saying that
>     InfiniBand's memory ordering is correct, but rather that this patch does
>     not in any way affect InfiniBand's correctness.  In other words, the
>     result of applying this patch is bug-for-bug compatible with the original.
>     
>     Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
>     Cc: Doug Ledford <dledford@redhat.com>
>     Cc: Jason Gunthorpe <jgg@mellanox.com>
>     Cc: Richard Henderson <rth@twiddle.net>
>     Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
>     Cc: Matt Turner <mattst88@gmail.com>
>     Cc: Michael Cree <mcree@orcon.net.nz>
>     Cc: Andrea Parri <parri.andrea@gmail.com>
>     Cc: <linux-rdma@vger.kernel.org>
>     Cc: <linux-alpha@vger.kernel.org>
>     [ paulmck: Removed drivers/dma/ioat/dma.c per Jason Gunthorpe's feedback. ]

Let me know if you want this patch to flow through the rdma tree..

Acked-by: Jason Gunthorpe <jgg@mellanox.com>

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH tip/core/rcu 21/21] drivers/vhost: Remove now-redundant read_barrier_depends()
  2017-12-01 19:51 ` [PATCH tip/core/rcu 21/21] drivers/vhost: Remove now-redundant read_barrier_depends() Paul E. McKenney
@ 2017-12-05 18:31   ` Michael S. Tsirkin
  2017-12-05 18:39     ` Peter Zijlstra
  0 siblings, 1 reply; 60+ messages in thread
From: Michael S. Tsirkin @ 2017-12-05 18:31 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: linux-kernel, mingo, jiangshanlai, dipankar, akpm,
	mathieu.desnoyers, josh, tglx, peterz, rostedt, dhowells,
	edumazet, fweisbec, oleg, Jason Wang, kvm, virtualization,
	netdev

On Fri, Dec 01, 2017 at 11:51:16AM -0800, Paul E. McKenney wrote:
> Because READ_ONCE() now implies read_barrier_depends(), the
> read_barrier_depends() in next_desc() is now redundant.  This commit
> therefore removes it and the related comments.
> 
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: <kvm@vger.kernel.org>
> Cc: <virtualization@lists.linux-foundation.org>
> Cc: <netdev@vger.kernel.org>

Apropos, READ_ONCE is now asymmetrical with WRITE_ONCE.

I can read a pointer with READ_ONCE and be sure the value
is sane, but only if I also remember to put in smp_wmb before
WRITE_ONCE. Otherwise the pointer is ok but no guarantees
about the data pointed to.

It would be better if the API reflected he assymetry somehow.

> ---
>  drivers/vhost/vhost.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 33ac2b186b85..78b5940a415a 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -1877,12 +1877,7 @@ static unsigned next_desc(struct vhost_virtqueue *vq, struct vring_desc *desc)
>  		return -1U;
>  
>  	/* Check they're not leading us off end of descriptors. */
> -	next = vhost16_to_cpu(vq, desc->next);
> -	/* Make sure compiler knows to grab that: we don't want it changing! */
> -	/* We will use the result as an index in an array, so most
> -	 * architectures only need a compiler barrier here. */
> -	read_barrier_depends();
> -
> +	next = vhost16_to_cpu(vq, READ_ONCE(desc->next));
>  	return next;
>  }
>  
> -- 
> 2.5.2

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH tip/core/rcu 21/21] drivers/vhost: Remove now-redundant read_barrier_depends()
  2017-12-05 18:31   ` Michael S. Tsirkin
@ 2017-12-05 18:39     ` Peter Zijlstra
  2017-12-05 18:57       ` Michael S. Tsirkin
  0 siblings, 1 reply; 60+ messages in thread
From: Peter Zijlstra @ 2017-12-05 18:39 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Paul E. McKenney, linux-kernel, mingo, jiangshanlai, dipankar,
	akpm, mathieu.desnoyers, josh, tglx, rostedt, dhowells, edumazet,
	fweisbec, oleg, Jason Wang, kvm, virtualization, netdev

On Tue, Dec 05, 2017 at 08:31:20PM +0200, Michael S. Tsirkin wrote:

> Apropos, READ_ONCE is now asymmetrical with WRITE_ONCE.
> 
> I can read a pointer with READ_ONCE and be sure the value
> is sane, but only if I also remember to put in smp_wmb before
> WRITE_ONCE. Otherwise the pointer is ok but no guarantees
> about the data pointed to.

That was already the case on everything except Alpha. And the canonical
match do the data dependency is store_release, not wmb.

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH tip/core/rcu 21/21] drivers/vhost: Remove now-redundant read_barrier_depends()
  2017-12-05 18:39     ` Peter Zijlstra
@ 2017-12-05 18:57       ` Michael S. Tsirkin
  2017-12-05 19:17         ` Peter Zijlstra
  0 siblings, 1 reply; 60+ messages in thread
From: Michael S. Tsirkin @ 2017-12-05 18:57 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Paul E. McKenney, linux-kernel, mingo, jiangshanlai, dipankar,
	akpm, mathieu.desnoyers, josh, tglx, rostedt, dhowells, edumazet,
	fweisbec, oleg, Jason Wang, kvm, virtualization, netdev

On Tue, Dec 05, 2017 at 07:39:46PM +0100, Peter Zijlstra wrote:
> On Tue, Dec 05, 2017 at 08:31:20PM +0200, Michael S. Tsirkin wrote:
> 
> > Apropos, READ_ONCE is now asymmetrical with WRITE_ONCE.
> > 
> > I can read a pointer with READ_ONCE and be sure the value
> > is sane, but only if I also remember to put in smp_wmb before
> > WRITE_ONCE. Otherwise the pointer is ok but no guarantees
> > about the data pointed to.
> 
> That was already the case on everything except Alpha. And the canonical
> match do the data dependency is store_release, not wmb.

Oh, interesting

static __always_inline void __write_once_size(volatile void *p, void *res, int size)
{
        switch (size) {
        case 1: *(volatile __u8 *)p = *(__u8 *)res; break;
        case 2: *(volatile __u16 *)p = *(__u16 *)res; break;
        case 4: *(volatile __u32 *)p = *(__u32 *)res; break;
        case 8: *(volatile __u64 *)p = *(__u64 *)res; break;
        default:
                barrier();
                __builtin_memcpy((void *)p, (const void *)res, size);
                barrier();
        }
}

#define WRITE_ONCE(x, val) \
({                                                      \
        union { typeof(x) __val; char __c[1]; } __u =   \
                { .__val = (__force typeof(x)) (val) }; \
        __write_once_size(&(x), __u.__c, sizeof(x));    \
        __u.__val;                                      \
})

I don't see WRITE_ONCE inserting any barriers, release or
write.

So it seems that on an architecture where writes can be reordered,
if I do

*pointer = 0xa;
WRITE_ONCE(array[x], pointer);

array write might bypass the pointer write,
and readers will read a stale value.




-- 
MST

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH tip/core/rcu 21/21] drivers/vhost: Remove now-redundant read_barrier_depends()
  2017-12-05 18:57       ` Michael S. Tsirkin
@ 2017-12-05 19:17         ` Peter Zijlstra
  2017-12-05 19:24           ` Michael S. Tsirkin
  0 siblings, 1 reply; 60+ messages in thread
From: Peter Zijlstra @ 2017-12-05 19:17 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Paul E. McKenney, linux-kernel, mingo, jiangshanlai, dipankar,
	akpm, mathieu.desnoyers, josh, tglx, rostedt, dhowells, edumazet,
	fweisbec, oleg, Jason Wang, kvm, virtualization, netdev

On Tue, Dec 05, 2017 at 08:57:46PM +0200, Michael S. Tsirkin wrote:

> I don't see WRITE_ONCE inserting any barriers, release or
> write.

Correct, never claimed there was.

Just saying that:

	obj = READ_ONCE(*foo);
	val = READ_ONCE(obj->val);

Never needs a barrier (except on Alpha and we want to make that go
away). Simply because a CPU needs to complete the load of @obj before it
can compute the address &obj->val. Thus the second load _must_ come
after the first load and we get LOAD-LOAD ordering.

Alpha messing that up is a royal pain, and Alpha not being an
active/living architecture is just not worth the pain of keeping this in
the generic model.

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH tip/core/rcu 21/21] drivers/vhost: Remove now-redundant read_barrier_depends()
  2017-12-05 19:17         ` Peter Zijlstra
@ 2017-12-05 19:24           ` Michael S. Tsirkin
  2017-12-05 19:33             ` Paul E. McKenney
  2017-12-05 19:55             ` Peter Zijlstra
  0 siblings, 2 replies; 60+ messages in thread
From: Michael S. Tsirkin @ 2017-12-05 19:24 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Paul E. McKenney, linux-kernel, mingo, jiangshanlai, dipankar,
	akpm, mathieu.desnoyers, josh, tglx, rostedt, dhowells, edumazet,
	fweisbec, oleg, Jason Wang, kvm, virtualization, netdev

On Tue, Dec 05, 2017 at 08:17:33PM +0100, Peter Zijlstra wrote:
> On Tue, Dec 05, 2017 at 08:57:46PM +0200, Michael S. Tsirkin wrote:
> 
> > I don't see WRITE_ONCE inserting any barriers, release or
> > write.
> 
> Correct, never claimed there was.
> 
> Just saying that:
> 
> 	obj = READ_ONCE(*foo);
> 	val = READ_ONCE(obj->val);
> 
> Never needs a barrier (except on Alpha and we want to make that go
> away). Simply because a CPU needs to complete the load of @obj before it
> can compute the address &obj->val. Thus the second load _must_ come
> after the first load and we get LOAD-LOAD ordering.
> 
> Alpha messing that up is a royal pain, and Alpha not being an
> active/living architecture is just not worth the pain of keeping this in
> the generic model.
> 

Right. What I am saying is that for writes you need

WRITE_ONCE(obj->val, 1);
smp_wmb();
WRITE_ONCE(*foo, obj);

and this barrier is no longer paired with anything until
you realize there's a dependency barrier within READ_ONCE.

Barrier pairing was a useful tool to check code validity,
maybe there are other, better tools now.


-- 
MST

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH tip/core/rcu 21/21] drivers/vhost: Remove now-redundant read_barrier_depends()
  2017-12-05 19:24           ` Michael S. Tsirkin
@ 2017-12-05 19:33             ` Paul E. McKenney
  2017-12-05 19:51               ` Michael S. Tsirkin
  2017-12-05 19:55             ` Peter Zijlstra
  1 sibling, 1 reply; 60+ messages in thread
From: Paul E. McKenney @ 2017-12-05 19:33 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Peter Zijlstra, linux-kernel, mingo, jiangshanlai, dipankar,
	akpm, mathieu.desnoyers, josh, tglx, rostedt, dhowells, edumazet,
	fweisbec, oleg, Jason Wang, kvm, virtualization, netdev

On Tue, Dec 05, 2017 at 09:24:21PM +0200, Michael S. Tsirkin wrote:
> On Tue, Dec 05, 2017 at 08:17:33PM +0100, Peter Zijlstra wrote:
> > On Tue, Dec 05, 2017 at 08:57:46PM +0200, Michael S. Tsirkin wrote:
> > 
> > > I don't see WRITE_ONCE inserting any barriers, release or
> > > write.
> > 
> > Correct, never claimed there was.
> > 
> > Just saying that:
> > 
> > 	obj = READ_ONCE(*foo);
> > 	val = READ_ONCE(obj->val);
> > 
> > Never needs a barrier (except on Alpha and we want to make that go
> > away). Simply because a CPU needs to complete the load of @obj before it
> > can compute the address &obj->val. Thus the second load _must_ come
> > after the first load and we get LOAD-LOAD ordering.
> > 
> > Alpha messing that up is a royal pain, and Alpha not being an
> > active/living architecture is just not worth the pain of keeping this in
> > the generic model.
> > 
> 
> Right. What I am saying is that for writes you need
> 
> WRITE_ONCE(obj->val, 1);
> smp_wmb();
> WRITE_ONCE(*foo, obj);

I believe Peter was instead suggesting:

WRITE_ONCE(obj->val, 1);
smp_store_release(foo, obj);

> and this barrier is no longer paired with anything until
> you realize there's a dependency barrier within READ_ONCE.
> 
> Barrier pairing was a useful tool to check code validity,
> maybe there are other, better tools now.

There are quite a few people who say that smp_store_release() is
easier for the tools to analyze than is smp_wmb().  My experience with
smp_read_barrier_depends() and rcu_dereference() leads me to believe
that they are correct.

							Thanx, Paul

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH tip/core/rcu 21/21] drivers/vhost: Remove now-redundant read_barrier_depends()
  2017-12-05 19:33             ` Paul E. McKenney
@ 2017-12-05 19:51               ` Michael S. Tsirkin
  2017-12-05 19:57                 ` Peter Zijlstra
  2017-12-05 20:08                 ` Paul E. McKenney
  0 siblings, 2 replies; 60+ messages in thread
From: Michael S. Tsirkin @ 2017-12-05 19:51 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: Peter Zijlstra, linux-kernel, mingo, jiangshanlai, dipankar,
	akpm, mathieu.desnoyers, josh, tglx, rostedt, dhowells, edumazet,
	fweisbec, oleg, Jason Wang, kvm, virtualization, netdev

On Tue, Dec 05, 2017 at 11:33:39AM -0800, Paul E. McKenney wrote:
> On Tue, Dec 05, 2017 at 09:24:21PM +0200, Michael S. Tsirkin wrote:
> > On Tue, Dec 05, 2017 at 08:17:33PM +0100, Peter Zijlstra wrote:
> > > On Tue, Dec 05, 2017 at 08:57:46PM +0200, Michael S. Tsirkin wrote:
> > > 
> > > > I don't see WRITE_ONCE inserting any barriers, release or
> > > > write.
> > > 
> > > Correct, never claimed there was.
> > > 
> > > Just saying that:
> > > 
> > > 	obj = READ_ONCE(*foo);
> > > 	val = READ_ONCE(obj->val);
> > > 
> > > Never needs a barrier (except on Alpha and we want to make that go
> > > away). Simply because a CPU needs to complete the load of @obj before it
> > > can compute the address &obj->val. Thus the second load _must_ come
> > > after the first load and we get LOAD-LOAD ordering.
> > > 
> > > Alpha messing that up is a royal pain, and Alpha not being an
> > > active/living architecture is just not worth the pain of keeping this in
> > > the generic model.
> > > 
> > 
> > Right. What I am saying is that for writes you need
> > 
> > WRITE_ONCE(obj->val, 1);
> > smp_wmb();
> > WRITE_ONCE(*foo, obj);
> 
> I believe Peter was instead suggesting:
> 
> WRITE_ONCE(obj->val, 1);
> smp_store_release(foo, obj);

Isn't that more expensive though?


> > and this barrier is no longer paired with anything until
> > you realize there's a dependency barrier within READ_ONCE.
> > 
> > Barrier pairing was a useful tool to check code validity,
> > maybe there are other, better tools now.
> 
> There are quite a few people who say that smp_store_release() is
> easier for the tools to analyze than is smp_wmb().  My experience with
> smp_read_barrier_depends() and rcu_dereference() leads me to believe
> that they are correct.
> 
> 							Thanx, Paul

OK, but smp_store_release is still not paired with anything since we
rely on READ_ONCE to include the implicit dpendendency barrier.

-- 
MST

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH tip/core/rcu 21/21] drivers/vhost: Remove now-redundant read_barrier_depends()
  2017-12-05 19:24           ` Michael S. Tsirkin
  2017-12-05 19:33             ` Paul E. McKenney
@ 2017-12-05 19:55             ` Peter Zijlstra
  1 sibling, 0 replies; 60+ messages in thread
From: Peter Zijlstra @ 2017-12-05 19:55 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Paul E. McKenney, linux-kernel, mingo, jiangshanlai, dipankar,
	akpm, mathieu.desnoyers, josh, tglx, rostedt, dhowells, edumazet,
	fweisbec, oleg, Jason Wang, kvm, virtualization, netdev

On Tue, Dec 05, 2017 at 09:24:21PM +0200, Michael S. Tsirkin wrote:
> On Tue, Dec 05, 2017 at 08:17:33PM +0100, Peter Zijlstra wrote:
> > On Tue, Dec 05, 2017 at 08:57:46PM +0200, Michael S. Tsirkin wrote:
> > 
> > > I don't see WRITE_ONCE inserting any barriers, release or
> > > write.
> > 
> > Correct, never claimed there was.
> > 
> > Just saying that:
> > 
> > 	obj = READ_ONCE(*foo);
> > 	val = READ_ONCE(obj->val);
> > 
> > Never needs a barrier (except on Alpha and we want to make that go
> > away). Simply because a CPU needs to complete the load of @obj before it
> > can compute the address &obj->val. Thus the second load _must_ come
> > after the first load and we get LOAD-LOAD ordering.
> > 
> > Alpha messing that up is a royal pain, and Alpha not being an
> > active/living architecture is just not worth the pain of keeping this in
> > the generic model.
> > 
> 
> Right. What I am saying is that for writes you need
> 
> WRITE_ONCE(obj->val, 1);
> smp_wmb();
> WRITE_ONCE(*foo, obj);

You really should use smp_store_release() here instead of the smp_wmb().
But yes.

> and this barrier is no longer paired with anything until
> you realize there's a dependency barrier within READ_ONCE.

No, there isn't. read_dependecy barriers are no more. They don't exist.
They never did, except on Alpha anyway.

There were a ton of sites that relied on this but never had the
smp_read_barrier_depends() annotation, similarly there were quite a few
sites that had the barrier but nobody could explain wtf for.

What these patches do is return the sane rule that dependent loads are
ordered.

And like all memory ordering; it should come with comments. Any piece of
code that relies on memory ordering and doesn't have big fat comments
that explain the required ordering are broken per definition. Maybe not
now, but they will be after a few years because someone changed it and
didn't know.

> Barrier pairing was a useful tool to check code validity,
> maybe there are other, better tools now.

Same is true for the closely related control dependency, you can pair
against those just fine but they don't have an explicit barrier
annotation.

There are no tools, use brain.

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH tip/core/rcu 21/21] drivers/vhost: Remove now-redundant read_barrier_depends()
  2017-12-05 19:51               ` Michael S. Tsirkin
@ 2017-12-05 19:57                 ` Peter Zijlstra
  2017-12-05 20:28                   ` Michael S. Tsirkin
  2017-12-05 20:08                 ` Paul E. McKenney
  1 sibling, 1 reply; 60+ messages in thread
From: Peter Zijlstra @ 2017-12-05 19:57 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Paul E. McKenney, linux-kernel, mingo, jiangshanlai, dipankar,
	akpm, mathieu.desnoyers, josh, tglx, rostedt, dhowells, edumazet,
	fweisbec, oleg, Jason Wang, kvm, virtualization, netdev

On Tue, Dec 05, 2017 at 09:51:48PM +0200, Michael S. Tsirkin wrote:
> > > WRITE_ONCE(obj->val, 1);
> > > smp_wmb();
> > > WRITE_ONCE(*foo, obj);
> > 
> > I believe Peter was instead suggesting:
> > 
> > WRITE_ONCE(obj->val, 1);
> > smp_store_release(foo, obj);
> 
> Isn't that more expensive though?

Depends on the architecture. The only architecture where it is more
expensive and people actually still care about is ARM I think.

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH tip/core/rcu 16/21] drivers/infiniband: Remove now-redundant smp_read_barrier_depends()
  2017-12-05 15:08       ` Jason Gunthorpe
@ 2017-12-05 20:06         ` Paul E. McKenney
  0 siblings, 0 replies; 60+ messages in thread
From: Paul E. McKenney @ 2017-12-05 20:06 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Dennis Dalessandro, linux-kernel, mingo, jiangshanlai, dipankar,
	akpm, mathieu.desnoyers, josh, tglx, peterz, rostedt, dhowells,
	edumazet, fweisbec, oleg, Doug Ledford, Richard Henderson,
	Ivan Kokshaysky, Matt Turner, Michael Cree, Andrea Parri,
	linux-rdma, linux-alpha

On Tue, Dec 05, 2017 at 08:08:32AM -0700, Jason Gunthorpe wrote:
> > commit c389c98ec5f4a7aa4c36853e89801eb5ea81870e
> > Author: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > Date:   Mon Nov 27 09:04:22 2017 -0800
> > 
> >     drivers/infiniband: Remove now-redundant smp_read_barrier_depends()
> >     
> >     The smp_read_barrier_depends() does nothing at all except on DEC Alpha,
> >     and no current DEC Alpha systems use Infiniband:
> >     
> >     	lkml.kernel.org/r/20171023085921.jwbntptn6ictbnvj@tower
> >     
> >     This commit therefore makes Infiniband depend on !ALPHA and removes
> >     the now-ineffective invocations of smp_read_barrier_depends() from
> >     the InfiniBand driver.
> >     
> >     Please note that this patch should not be construed as my saying that
> >     InfiniBand's memory ordering is correct, but rather that this patch does
> >     not in any way affect InfiniBand's correctness.  In other words, the
> >     result of applying this patch is bug-for-bug compatible with the original.
> >     
> >     Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> >     Cc: Doug Ledford <dledford@redhat.com>
> >     Cc: Jason Gunthorpe <jgg@mellanox.com>
> >     Cc: Richard Henderson <rth@twiddle.net>
> >     Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
> >     Cc: Matt Turner <mattst88@gmail.com>
> >     Cc: Michael Cree <mcree@orcon.net.nz>
> >     Cc: Andrea Parri <parri.andrea@gmail.com>
> >     Cc: <linux-rdma@vger.kernel.org>
> >     Cc: <linux-alpha@vger.kernel.org>
> >     [ paulmck: Removed drivers/dma/ioat/dma.c per Jason Gunthorpe's feedback. ]
> 
> Let me know if you want this patch to flow through the rdma tree..
> 
> Acked-by: Jason Gunthorpe <jgg@mellanox.com>

I applied your ack, thank you!

Your choice as to what tree it flows through.  By default, and if all
goes well, I will push this series through -rcu and -tip during the next
merge window.

							Thanx, Paul

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH tip/core/rcu 21/21] drivers/vhost: Remove now-redundant read_barrier_depends()
  2017-12-05 19:51               ` Michael S. Tsirkin
  2017-12-05 19:57                 ` Peter Zijlstra
@ 2017-12-05 20:08                 ` Paul E. McKenney
  2017-12-05 21:24                   ` Michael S. Tsirkin
  1 sibling, 1 reply; 60+ messages in thread
From: Paul E. McKenney @ 2017-12-05 20:08 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Peter Zijlstra, linux-kernel, mingo, jiangshanlai, dipankar,
	akpm, mathieu.desnoyers, josh, tglx, rostedt, dhowells, edumazet,
	fweisbec, oleg, Jason Wang, kvm, virtualization, netdev

On Tue, Dec 05, 2017 at 09:51:48PM +0200, Michael S. Tsirkin wrote:
> On Tue, Dec 05, 2017 at 11:33:39AM -0800, Paul E. McKenney wrote:
> > On Tue, Dec 05, 2017 at 09:24:21PM +0200, Michael S. Tsirkin wrote:

[ . . . ]

> > > and this barrier is no longer paired with anything until
> > > you realize there's a dependency barrier within READ_ONCE.
> > > 
> > > Barrier pairing was a useful tool to check code validity,
> > > maybe there are other, better tools now.
> > 
> > There are quite a few people who say that smp_store_release() is
> > easier for the tools to analyze than is smp_wmb().  My experience with
> > smp_read_barrier_depends() and rcu_dereference() leads me to believe
> > that they are correct.
> 
> OK, but smp_store_release is still not paired with anything since we
> rely on READ_ONCE to include the implicit dpendendency barrier.

Why wouldn't you consider the smp_store_release() to be paired with
the new improved READ_ONCE()?

							Thanx, Paul

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH tip/core/rcu 21/21] drivers/vhost: Remove now-redundant read_barrier_depends()
  2017-12-05 19:57                 ` Peter Zijlstra
@ 2017-12-05 20:28                   ` Michael S. Tsirkin
  2017-12-05 21:17                     ` Peter Zijlstra
  0 siblings, 1 reply; 60+ messages in thread
From: Michael S. Tsirkin @ 2017-12-05 20:28 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Paul E. McKenney, linux-kernel, mingo, jiangshanlai, dipankar,
	akpm, mathieu.desnoyers, josh, tglx, rostedt, dhowells, edumazet,
	fweisbec, oleg, Jason Wang, kvm, virtualization, netdev

On Tue, Dec 05, 2017 at 08:57:52PM +0100, Peter Zijlstra wrote:
> On Tue, Dec 05, 2017 at 09:51:48PM +0200, Michael S. Tsirkin wrote:
> > > > WRITE_ONCE(obj->val, 1);
> > > > smp_wmb();
> > > > WRITE_ONCE(*foo, obj);
> > > 
> > > I believe Peter was instead suggesting:
> > > 
> > > WRITE_ONCE(obj->val, 1);
> > > smp_store_release(foo, obj);
> > 
> > Isn't that more expensive though?
> 
> Depends on the architecture. The only architecture where it is more
> expensive and people actually still care about is ARM I think.

Right. Why should I use the more expensive smp_store_release then?

-- 
MST

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH tip/core/rcu 21/21] drivers/vhost: Remove now-redundant read_barrier_depends()
  2017-12-05 20:28                   ` Michael S. Tsirkin
@ 2017-12-05 21:17                     ` Peter Zijlstra
  2017-12-05 21:42                       ` Michael S. Tsirkin
  0 siblings, 1 reply; 60+ messages in thread
From: Peter Zijlstra @ 2017-12-05 21:17 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Paul E. McKenney, linux-kernel, mingo, jiangshanlai, dipankar,
	akpm, mathieu.desnoyers, josh, tglx, rostedt, dhowells, edumazet,
	fweisbec, oleg, Jason Wang, kvm, virtualization, netdev

On Tue, Dec 05, 2017 at 10:28:38PM +0200, Michael S. Tsirkin wrote:
> On Tue, Dec 05, 2017 at 08:57:52PM +0100, Peter Zijlstra wrote:
> > On Tue, Dec 05, 2017 at 09:51:48PM +0200, Michael S. Tsirkin wrote:
> > > > > WRITE_ONCE(obj->val, 1);
> > > > > smp_wmb();
> > > > > WRITE_ONCE(*foo, obj);
> > > > 
> > > > I believe Peter was instead suggesting:
> > > > 
> > > > WRITE_ONCE(obj->val, 1);
> > > > smp_store_release(foo, obj);
> > > 
> > > Isn't that more expensive though?
> > 
> > Depends on the architecture. The only architecture where it is more
> > expensive and people actually still care about is ARM I think.
> 
> Right. Why should I use the more expensive smp_store_release then?

Because it makes more sense. Memory ordering is hard enough, don't make
it harder still if you don't have to.

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH tip/core/rcu 21/21] drivers/vhost: Remove now-redundant read_barrier_depends()
  2017-12-05 20:08                 ` Paul E. McKenney
@ 2017-12-05 21:24                   ` Michael S. Tsirkin
  2017-12-05 21:36                     ` Paul E. McKenney
  2017-12-05 21:57                     ` Peter Zijlstra
  0 siblings, 2 replies; 60+ messages in thread
From: Michael S. Tsirkin @ 2017-12-05 21:24 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: Peter Zijlstra, linux-kernel, mingo, jiangshanlai, dipankar,
	akpm, mathieu.desnoyers, josh, tglx, rostedt, dhowells, edumazet,
	fweisbec, oleg, Jason Wang, kvm, virtualization, netdev

On Tue, Dec 05, 2017 at 12:08:01PM -0800, Paul E. McKenney wrote:
> On Tue, Dec 05, 2017 at 09:51:48PM +0200, Michael S. Tsirkin wrote:
> > On Tue, Dec 05, 2017 at 11:33:39AM -0800, Paul E. McKenney wrote:
> > > On Tue, Dec 05, 2017 at 09:24:21PM +0200, Michael S. Tsirkin wrote:
> 
> [ . . . ]
> 
> > > > and this barrier is no longer paired with anything until
> > > > you realize there's a dependency barrier within READ_ONCE.
> > > > 
> > > > Barrier pairing was a useful tool to check code validity,
> > > > maybe there are other, better tools now.
> > > 
> > > There are quite a few people who say that smp_store_release() is
> > > easier for the tools to analyze than is smp_wmb().  My experience with
> > > smp_read_barrier_depends() and rcu_dereference() leads me to believe
> > > that they are correct.
> > 
> > OK, but smp_store_release is still not paired with anything since we
> > rely on READ_ONCE to include the implicit dpendendency barrier.
> 
> Why wouldn't you consider the smp_store_release() to be paired with
> the new improved READ_ONCE()?
> 
> 							Thanx, Paul

READ_ONCE is really all over the place (some code literally replaced all
memory accesses with READ/WRITE ONCE).

And I also prefer smp_wmb as it seems to be cheaper on ARM.

Would an API like WRITE_POINTER()/smp_store_pointer make sense,
and READ_POINTER for symmetry?

-- 
MST

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH tip/core/rcu 21/21] drivers/vhost: Remove now-redundant read_barrier_depends()
  2017-12-05 21:24                   ` Michael S. Tsirkin
@ 2017-12-05 21:36                     ` Paul E. McKenney
  2017-12-05 21:43                       ` Michael S. Tsirkin
  2017-12-05 22:09                       ` Peter Zijlstra
  2017-12-05 21:57                     ` Peter Zijlstra
  1 sibling, 2 replies; 60+ messages in thread
From: Paul E. McKenney @ 2017-12-05 21:36 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Peter Zijlstra, linux-kernel, mingo, jiangshanlai, dipankar,
	akpm, mathieu.desnoyers, josh, tglx, rostedt, dhowells, edumazet,
	fweisbec, oleg, Jason Wang, kvm, virtualization, netdev

On Tue, Dec 05, 2017 at 11:24:49PM +0200, Michael S. Tsirkin wrote:
> On Tue, Dec 05, 2017 at 12:08:01PM -0800, Paul E. McKenney wrote:
> > On Tue, Dec 05, 2017 at 09:51:48PM +0200, Michael S. Tsirkin wrote:
> > > On Tue, Dec 05, 2017 at 11:33:39AM -0800, Paul E. McKenney wrote:
> > > > On Tue, Dec 05, 2017 at 09:24:21PM +0200, Michael S. Tsirkin wrote:
> > 
> > [ . . . ]
> > 
> > > > > and this barrier is no longer paired with anything until
> > > > > you realize there's a dependency barrier within READ_ONCE.
> > > > > 
> > > > > Barrier pairing was a useful tool to check code validity,
> > > > > maybe there are other, better tools now.
> > > > 
> > > > There are quite a few people who say that smp_store_release() is
> > > > easier for the tools to analyze than is smp_wmb().  My experience with
> > > > smp_read_barrier_depends() and rcu_dereference() leads me to believe
> > > > that they are correct.
> > > 
> > > OK, but smp_store_release is still not paired with anything since we
> > > rely on READ_ONCE to include the implicit dpendendency barrier.
> > 
> > Why wouldn't you consider the smp_store_release() to be paired with
> > the new improved READ_ONCE()?
> 
> READ_ONCE is really all over the place (some code literally replaced all
> memory accesses with READ/WRITE ONCE).
> 
> And I also prefer smp_wmb as it seems to be cheaper on ARM.
> 
> Would an API like WRITE_POINTER()/smp_store_pointer make sense,
> and READ_POINTER for symmetry?

What we do in some code is to comment the pairings, allowing the other
side of the pairing to be easily located.  Would that work for you?

							Thanx, Paul

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH tip/core/rcu 21/21] drivers/vhost: Remove now-redundant read_barrier_depends()
  2017-12-05 21:17                     ` Peter Zijlstra
@ 2017-12-05 21:42                       ` Michael S. Tsirkin
  0 siblings, 0 replies; 60+ messages in thread
From: Michael S. Tsirkin @ 2017-12-05 21:42 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Paul E. McKenney, linux-kernel, mingo, jiangshanlai, dipankar,
	akpm, mathieu.desnoyers, josh, tglx, rostedt, dhowells, edumazet,
	fweisbec, oleg, Jason Wang, kvm, virtualization, netdev

On Tue, Dec 05, 2017 at 10:17:35PM +0100, Peter Zijlstra wrote:
> On Tue, Dec 05, 2017 at 10:28:38PM +0200, Michael S. Tsirkin wrote:
> > On Tue, Dec 05, 2017 at 08:57:52PM +0100, Peter Zijlstra wrote:
> > > On Tue, Dec 05, 2017 at 09:51:48PM +0200, Michael S. Tsirkin wrote:
> > > > > > WRITE_ONCE(obj->val, 1);
> > > > > > smp_wmb();
> > > > > > WRITE_ONCE(*foo, obj);
> > > > > 
> > > > > I believe Peter was instead suggesting:
> > > > > 
> > > > > WRITE_ONCE(obj->val, 1);
> > > > > smp_store_release(foo, obj);
> > > > 
> > > > Isn't that more expensive though?
> > > 
> > > Depends on the architecture. The only architecture where it is more
> > > expensive and people actually still care about is ARM I think.
> > 
> > Right. Why should I use the more expensive smp_store_release then?
> 
> Because it makes more sense. Memory ordering is hard enough, don't make
> it harder still if you don't have to.

I suspect I have to -  ptr_ring is a very low level construct used by
netowrking on data path so making it a bit more complicated for
a bit of performance is probably justified.

-- 
MST

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH tip/core/rcu 21/21] drivers/vhost: Remove now-redundant read_barrier_depends()
  2017-12-05 21:36                     ` Paul E. McKenney
@ 2017-12-05 21:43                       ` Michael S. Tsirkin
  2017-12-05 22:02                         ` Paul E. McKenney
  2017-12-05 22:09                       ` Peter Zijlstra
  1 sibling, 1 reply; 60+ messages in thread
From: Michael S. Tsirkin @ 2017-12-05 21:43 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: Peter Zijlstra, linux-kernel, mingo, jiangshanlai, dipankar,
	akpm, mathieu.desnoyers, josh, tglx, rostedt, dhowells, edumazet,
	fweisbec, oleg, Jason Wang, kvm, virtualization, netdev

On Tue, Dec 05, 2017 at 01:36:44PM -0800, Paul E. McKenney wrote:
> On Tue, Dec 05, 2017 at 11:24:49PM +0200, Michael S. Tsirkin wrote:
> > On Tue, Dec 05, 2017 at 12:08:01PM -0800, Paul E. McKenney wrote:
> > > On Tue, Dec 05, 2017 at 09:51:48PM +0200, Michael S. Tsirkin wrote:
> > > > On Tue, Dec 05, 2017 at 11:33:39AM -0800, Paul E. McKenney wrote:
> > > > > On Tue, Dec 05, 2017 at 09:24:21PM +0200, Michael S. Tsirkin wrote:
> > > 
> > > [ . . . ]
> > > 
> > > > > > and this barrier is no longer paired with anything until
> > > > > > you realize there's a dependency barrier within READ_ONCE.
> > > > > > 
> > > > > > Barrier pairing was a useful tool to check code validity,
> > > > > > maybe there are other, better tools now.
> > > > > 
> > > > > There are quite a few people who say that smp_store_release() is
> > > > > easier for the tools to analyze than is smp_wmb().  My experience with
> > > > > smp_read_barrier_depends() and rcu_dereference() leads me to believe
> > > > > that they are correct.
> > > > 
> > > > OK, but smp_store_release is still not paired with anything since we
> > > > rely on READ_ONCE to include the implicit dpendendency barrier.
> > > 
> > > Why wouldn't you consider the smp_store_release() to be paired with
> > > the new improved READ_ONCE()?
> > 
> > READ_ONCE is really all over the place (some code literally replaced all
> > memory accesses with READ/WRITE ONCE).
> > 
> > And I also prefer smp_wmb as it seems to be cheaper on ARM.
> > 
> > Would an API like WRITE_POINTER()/smp_store_pointer make sense,
> > and READ_POINTER for symmetry?
> 
> What we do in some code is to comment the pairings, allowing the other
> side of the pairing to be easily located.  Would that work for you?
> 
> 							Thanx, Paul

Yes, that's exactly what I did for now.

Thanks!

-- 
MST

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH tip/core/rcu 21/21] drivers/vhost: Remove now-redundant read_barrier_depends()
  2017-12-05 21:24                   ` Michael S. Tsirkin
  2017-12-05 21:36                     ` Paul E. McKenney
@ 2017-12-05 21:57                     ` Peter Zijlstra
  2017-12-05 22:09                       ` Michael S. Tsirkin
  1 sibling, 1 reply; 60+ messages in thread
From: Peter Zijlstra @ 2017-12-05 21:57 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Paul E. McKenney, linux-kernel, mingo, jiangshanlai, dipankar,
	akpm, mathieu.desnoyers, josh, tglx, rostedt, dhowells, edumazet,
	fweisbec, oleg, Jason Wang, kvm, virtualization, netdev

On Tue, Dec 05, 2017 at 11:24:49PM +0200, Michael S. Tsirkin wrote:
> READ_ONCE is really all over the place (some code literally replaced all
> memory accesses with READ/WRITE ONCE).

Yeah, so? Complain to the compiler people for forcing us into that.

> Would an API like WRITE_POINTER()/smp_store_pointer make sense,
> and READ_POINTER for symmetry?

No, the whole point of the exercise was to get away from the fact that
dependent loads are special.

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH tip/core/rcu 21/21] drivers/vhost: Remove now-redundant read_barrier_depends()
  2017-12-05 21:43                       ` Michael S. Tsirkin
@ 2017-12-05 22:02                         ` Paul E. McKenney
  0 siblings, 0 replies; 60+ messages in thread
From: Paul E. McKenney @ 2017-12-05 22:02 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Peter Zijlstra, linux-kernel, mingo, jiangshanlai, dipankar,
	akpm, mathieu.desnoyers, josh, tglx, rostedt, dhowells, edumazet,
	fweisbec, oleg, Jason Wang, kvm, virtualization, netdev

On Tue, Dec 05, 2017 at 11:43:41PM +0200, Michael S. Tsirkin wrote:
> On Tue, Dec 05, 2017 at 01:36:44PM -0800, Paul E. McKenney wrote:
> > On Tue, Dec 05, 2017 at 11:24:49PM +0200, Michael S. Tsirkin wrote:
> > > On Tue, Dec 05, 2017 at 12:08:01PM -0800, Paul E. McKenney wrote:
> > > > On Tue, Dec 05, 2017 at 09:51:48PM +0200, Michael S. Tsirkin wrote:
> > > > > On Tue, Dec 05, 2017 at 11:33:39AM -0800, Paul E. McKenney wrote:
> > > > > > On Tue, Dec 05, 2017 at 09:24:21PM +0200, Michael S. Tsirkin wrote:
> > > > 
> > > > [ . . . ]
> > > > 
> > > > > > > and this barrier is no longer paired with anything until
> > > > > > > you realize there's a dependency barrier within READ_ONCE.
> > > > > > > 
> > > > > > > Barrier pairing was a useful tool to check code validity,
> > > > > > > maybe there are other, better tools now.
> > > > > > 
> > > > > > There are quite a few people who say that smp_store_release() is
> > > > > > easier for the tools to analyze than is smp_wmb().  My experience with
> > > > > > smp_read_barrier_depends() and rcu_dereference() leads me to believe
> > > > > > that they are correct.
> > > > > 
> > > > > OK, but smp_store_release is still not paired with anything since we
> > > > > rely on READ_ONCE to include the implicit dpendendency barrier.
> > > > 
> > > > Why wouldn't you consider the smp_store_release() to be paired with
> > > > the new improved READ_ONCE()?
> > > 
> > > READ_ONCE is really all over the place (some code literally replaced all
> > > memory accesses with READ/WRITE ONCE).
> > > 
> > > And I also prefer smp_wmb as it seems to be cheaper on ARM.
> > > 
> > > Would an API like WRITE_POINTER()/smp_store_pointer make sense,
> > > and READ_POINTER for symmetry?
> > 
> > What we do in some code is to comment the pairings, allowing the other
> > side of the pairing to be easily located.  Would that work for you?
> 
> Yes, that's exactly what I did for now.

Very good, thank you!

							Thanx, Paul

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH tip/core/rcu 21/21] drivers/vhost: Remove now-redundant read_barrier_depends()
  2017-12-05 21:36                     ` Paul E. McKenney
  2017-12-05 21:43                       ` Michael S. Tsirkin
@ 2017-12-05 22:09                       ` Peter Zijlstra
  1 sibling, 0 replies; 60+ messages in thread
From: Peter Zijlstra @ 2017-12-05 22:09 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: Michael S. Tsirkin, linux-kernel, mingo, jiangshanlai, dipankar,
	akpm, mathieu.desnoyers, josh, tglx, rostedt, dhowells, edumazet,
	fweisbec, oleg, Jason Wang, kvm, virtualization, netdev

On Tue, Dec 05, 2017 at 01:36:44PM -0800, Paul E. McKenney wrote:
> What we do in some code is to comment the pairings, allowing the other
> side of the pairing to be easily located.  Would that work for you?

I would say that that is mandatory for any memory ordering code ;-)

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH tip/core/rcu 21/21] drivers/vhost: Remove now-redundant read_barrier_depends()
  2017-12-05 21:57                     ` Peter Zijlstra
@ 2017-12-05 22:09                       ` Michael S. Tsirkin
  2017-12-05 23:39                         ` Paul E. McKenney
  0 siblings, 1 reply; 60+ messages in thread
From: Michael S. Tsirkin @ 2017-12-05 22:09 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Paul E. McKenney, linux-kernel, mingo, jiangshanlai, dipankar,
	akpm, mathieu.desnoyers, josh, tglx, rostedt, dhowells, edumazet,
	fweisbec, oleg, Jason Wang, kvm, virtualization, netdev

On Tue, Dec 05, 2017 at 10:57:00PM +0100, Peter Zijlstra wrote:
> On Tue, Dec 05, 2017 at 11:24:49PM +0200, Michael S. Tsirkin wrote:
> > READ_ONCE is really all over the place (some code literally replaced all
> > memory accesses with READ/WRITE ONCE).
> 
> Yeah, so?

Oh my point was I can't just look for READ_ONCE and go
*that's the pair*. there are too many of these.
At Paul's suggestion I will document the pairing *this read once has a
barrier that is paired with that barrier*.

> Complain to the compiler people for forcing us into that.

In some cases when you end up with all accesses
going through read/write once volatile just might better.

> > Would an API like WRITE_POINTER()/smp_store_pointer make sense,
> > and READ_POINTER for symmetry?
> 
> No, the whole point of the exercise was to get away from the fact that
> dependent loads are special.

It's a pity that dependent stores are still special.

-- 
MST

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH tip/core/rcu 21/21] drivers/vhost: Remove now-redundant read_barrier_depends()
  2017-12-05 22:09                       ` Michael S. Tsirkin
@ 2017-12-05 23:39                         ` Paul E. McKenney
  0 siblings, 0 replies; 60+ messages in thread
From: Paul E. McKenney @ 2017-12-05 23:39 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Peter Zijlstra, linux-kernel, mingo, jiangshanlai, dipankar,
	akpm, mathieu.desnoyers, josh, tglx, rostedt, dhowells, edumazet,
	fweisbec, oleg, Jason Wang, kvm, virtualization, netdev

On Wed, Dec 06, 2017 at 12:09:36AM +0200, Michael S. Tsirkin wrote:
> On Tue, Dec 05, 2017 at 10:57:00PM +0100, Peter Zijlstra wrote:
> > On Tue, Dec 05, 2017 at 11:24:49PM +0200, Michael S. Tsirkin wrote:
> > > READ_ONCE is really all over the place (some code literally replaced all
> > > memory accesses with READ/WRITE ONCE).
> > 
> > Yeah, so?
> 
> Oh my point was I can't just look for READ_ONCE and go
> *that's the pair*. there are too many of these.
> At Paul's suggestion I will document the pairing *this read once has a
> barrier that is paired with that barrier*.
> 
> > Complain to the compiler people for forcing us into that.
> 
> In some cases when you end up with all accesses
> going through read/write once volatile just might better.

That is in fact what the jiffies counter does.  But you lose READ_ONCE()'s
automatic handling of DEC Alpha when you take that approach.

> > > Would an API like WRITE_POINTER()/smp_store_pointer make sense,
> > > and READ_POINTER for symmetry?
> > 
> > No, the whole point of the exercise was to get away from the fact that
> > dependent loads are special.
> 
> It's a pity that dependent stores are still special.

We can make READ_ONCE() not be special at zero cost on non-Alpha
systems, but both smp_wmb() and smp_store_release() are decidedly
not free of added overhead.

							Thanx, Paul

^ permalink raw reply	[flat|nested] 60+ messages in thread

end of thread, other threads:[~2017-12-05 23:40 UTC | newest]

Thread overview: 60+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-01 19:50 [PATCH tip/core/rcu 0/21] De-emphasize {smp_,}read_barrier_depends Paul E. McKenney
2017-12-01 19:50 ` [PATCH tip/core/rcu 01/21] doc: READ_ONCE() now implies smp_barrier_depends() Paul E. McKenney
2017-12-01 19:50 ` [PATCH tip/core/rcu 02/21] mn10300: READ_ONCE() now implies smp_read_barrier_depends() Paul E. McKenney
2017-12-01 19:50 ` [PATCH tip/core/rcu 03/21] drivers/net/ethernet/qlogic/qed: Fix __qed_spq_block() ordering Paul E. McKenney
2017-12-01 19:50 ` [PATCH tip/core/rcu 04/21] fs/dcache: Use release-acquire for name/length update Paul E. McKenney
2017-12-01 19:51 ` [PATCH tip/core/rcu 05/21] percpu: READ_ONCE() now implies smp_read_barrier_depends() Paul E. McKenney
2017-12-01 19:51 ` [PATCH tip/core/rcu 06/21] rcu: Adjust read-side accessor comments for READ_ONCE() Paul E. McKenney
2017-12-01 19:51 ` [PATCH tip/core/rcu 07/21] rtnetlink: Update now-misleading smp_read_barrier_depends() comment Paul E. McKenney
2017-12-01 19:51 ` [PATCH tip/core/rcu 08/21] seqlock: Remove now-redundant smp_read_barrier_depends() Paul E. McKenney
2017-12-01 19:51 ` [PATCH tip/core/rcu 09/21] uprobes: " Paul E. McKenney
2017-12-01 19:51 ` [PATCH tip/core/rcu 10/21] locking: Remove smp_read_barrier_depends() from queued_spin_lock_slowpath() Paul E. McKenney
2017-12-01 19:51 ` [PATCH tip/core/rcu 11/21] tracepoint: Remove smp_read_barrier_depends() from comment Paul E. McKenney
2017-12-01 19:51 ` [PATCH tip/core/rcu 12/21] lib/assoc_array: Remove smp_read_barrier_depends() Paul E. McKenney
2017-12-01 19:51 ` [PATCH tip/core/rcu 13/21] mm/ksm: Remove now-redundant smp_read_barrier_depends() Paul E. McKenney
2017-12-01 19:51 ` [PATCH tip/core/rcu 14/21] netfilter: " Paul E. McKenney
2017-12-01 19:51 ` [PATCH tip/core/rcu 15/21] keyring: " Paul E. McKenney
2017-12-04  0:59   ` James Morris
2017-12-04 18:54     ` Paul E. McKenney
2017-12-01 19:51 ` [PATCH tip/core/rcu 16/21] drivers/infiniband: " Paul E. McKenney
2017-12-02  0:11   ` Jason Gunthorpe
2017-12-02  1:08     ` Paul E. McKenney
2017-12-05 15:08       ` Jason Gunthorpe
2017-12-05 20:06         ` Paul E. McKenney
2017-12-01 19:51 ` [PATCH tip/core/rcu 17/21] doc: De-emphasize smp_read_barrier_depends Paul E. McKenney
2017-12-01 19:51 ` [PATCH tip/core/rcu 18/21] genetlink: Remove smp_read_barrier_depends() from comment Paul E. McKenney
2017-12-01 19:51 ` [PATCH tip/core/rcu 19/21] netlink: " Paul E. McKenney
2017-12-01 19:51 ` [PATCH tip/core/rcu 20/21] checkpatch: Add warnings for {smp_,}read_barrier_depends() Paul E. McKenney
2017-12-01 20:14   ` Joe Perches
2017-12-01 21:44     ` Paul E. McKenney
2017-12-02  4:45       ` Joe Perches
2017-12-04 19:06         ` Paul E. McKenney
2017-12-04 19:12           ` Joe Perches
2017-12-01 19:51 ` [PATCH tip/core/rcu 21/21] drivers/vhost: Remove now-redundant read_barrier_depends() Paul E. McKenney
2017-12-05 18:31   ` Michael S. Tsirkin
2017-12-05 18:39     ` Peter Zijlstra
2017-12-05 18:57       ` Michael S. Tsirkin
2017-12-05 19:17         ` Peter Zijlstra
2017-12-05 19:24           ` Michael S. Tsirkin
2017-12-05 19:33             ` Paul E. McKenney
2017-12-05 19:51               ` Michael S. Tsirkin
2017-12-05 19:57                 ` Peter Zijlstra
2017-12-05 20:28                   ` Michael S. Tsirkin
2017-12-05 21:17                     ` Peter Zijlstra
2017-12-05 21:42                       ` Michael S. Tsirkin
2017-12-05 20:08                 ` Paul E. McKenney
2017-12-05 21:24                   ` Michael S. Tsirkin
2017-12-05 21:36                     ` Paul E. McKenney
2017-12-05 21:43                       ` Michael S. Tsirkin
2017-12-05 22:02                         ` Paul E. McKenney
2017-12-05 22:09                       ` Peter Zijlstra
2017-12-05 21:57                     ` Peter Zijlstra
2017-12-05 22:09                       ` Michael S. Tsirkin
2017-12-05 23:39                         ` Paul E. McKenney
2017-12-05 19:55             ` Peter Zijlstra
2017-12-04 15:38 ` [PATCH tip/core/rcu 01/21] doc: READ_ONCE() now implies smp_barrier_depends() David Howells
2017-12-04 18:52   ` Paul E. McKenney
2017-12-04 21:54     ` Peter Zijlstra
2017-12-04 22:15       ` Paul E. McKenney
2017-12-04 22:39     ` David Howells
2017-12-04 22:57       ` Peter Zijlstra

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).