[PATCH] rcu: Prevent evaluation of rcu_assign_pointer()

From: Andrea Parri
Date: Fri May 24 2019 - 06:39:49 EST


Quoting Paul [1]:

"Given that a quick (and perhaps error-prone) search of the uses
of rcu_assign_pointer() in v5.1 didn't find a single use of the
return value, let's please instead change the documentation and
implementation to eliminate the return value."

[1] https://lkml.kernel.org/r/20190523135013.GL28207@xxxxxxxxxxxxx

Signed-off-by: Andrea Parri <andrea.parri@xxxxxxxxxxxxxxxxxxxx>
Cc: "Paul E. McKenney" <paulmck@xxxxxxxxxxxxx>
Cc: Josh Triplett <josh@xxxxxxxxxxxxxxxx>
Cc: Steven Rostedt <rostedt@xxxxxxxxxxx>
Cc: Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx>
Cc: Lai Jiangshan <jiangshanlai@xxxxxxxxx>
Cc: Joel Fernandes <joel@xxxxxxxxxxxxxxxxx>
Cc: rcu@xxxxxxxxxxxxxxx
Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
Cc: Will Deacon <will.deacon@xxxxxxx>
Cc: Mark Rutland <mark.rutland@xxxxxxx>
Cc: Matthew Wilcox <willy@xxxxxxxxxxxxx>
Cc: Sasha Levin <sashal@xxxxxxxxxx>
---
Matthew, Sasha:

The patch is based on -rcu/dev; I took the liberty of applying the
same change to your #defines in:

tools/testing/radix-tree/linux/rcupdate.h
tools/include/linux/rcu.h

but I admit that I'm not familiar with their uses: please shout if
you have any objections with it.
---
Documentation/RCU/whatisRCU.txt | 8 ++++----
include/linux/rcupdate.h | 5 ++---
tools/include/linux/rcu.h | 11 +++++++++--
tools/testing/radix-tree/linux/rcupdate.h | 5 ++++-
4 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/Documentation/RCU/whatisRCU.txt b/Documentation/RCU/whatisRCU.txt
index 981651a8b65d2..f99a87b9a88fa 100644
--- a/Documentation/RCU/whatisRCU.txt
+++ b/Documentation/RCU/whatisRCU.txt
@@ -212,7 +212,7 @@ synchronize_rcu()

rcu_assign_pointer()

- typeof(p) rcu_assign_pointer(p, typeof(p) v);
+ rcu_assign_pointer(p, typeof(p) v);

Yes, rcu_assign_pointer() -is- implemented as a macro, though it
would be cool to be able to declare a function in this manner.
@@ -220,9 +220,9 @@ rcu_assign_pointer()

The updater uses this function to assign a new value to an
RCU-protected pointer, in order to safely communicate the change
- in value from the updater to the reader. This function returns
- the new value, and also executes any memory-barrier instructions
- required for a given CPU architecture.
+ in value from the updater to the reader. This macro does not
+ evaluate to an rvalue, but it does execute any memory-barrier
+ instructions required for a given CPU architecture.

Perhaps just as important, it serves to document (1) which
pointers are protected by RCU and (2) the point at which a
diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 915460ec08722..a5f61a08e65fc 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -367,7 +367,7 @@ static inline void rcu_preempt_sleep_check(void) { }
* other macros that it invokes.
*/
#define rcu_assign_pointer(p, v) \
-({ \
+do { \
uintptr_t _r_a_p__v = (uintptr_t)(v); \
rcu_check_sparse(p, __rcu); \
\
@@ -375,8 +375,7 @@ static inline void rcu_preempt_sleep_check(void) { }
WRITE_ONCE((p), (typeof(p))(_r_a_p__v)); \
else \
smp_store_release(&p, RCU_INITIALIZER((typeof(p))_r_a_p__v)); \
- _r_a_p__v; \
-})
+} while (0)

/**
* rcu_swap_protected() - swap an RCU and a regular pointer
diff --git a/tools/include/linux/rcu.h b/tools/include/linux/rcu.h
index 7d02527e5bcea..01a435ee48cd6 100644
--- a/tools/include/linux/rcu.h
+++ b/tools/include/linux/rcu.h
@@ -19,7 +19,14 @@ static inline bool rcu_is_watching(void)
return false;
}

-#define rcu_assign_pointer(p, v) ((p) = (v))
-#define RCU_INIT_POINTER(p, v) p=(v)
+#define rcu_assign_pointer(p, v) \
+do { \
+ (p) = (v); \
+} while (0)
+
+#define RCU_INIT_POINTER(p, v) \
+do { \
+ (p) = (v); \
+} while (0)

#endif
diff --git a/tools/testing/radix-tree/linux/rcupdate.h b/tools/testing/radix-tree/linux/rcupdate.h
index fd280b070fdb1..48212f3a758e6 100644
--- a/tools/testing/radix-tree/linux/rcupdate.h
+++ b/tools/testing/radix-tree/linux/rcupdate.h
@@ -7,6 +7,9 @@
#define rcu_dereference_raw(p) rcu_dereference(p)
#define rcu_dereference_protected(p, cond) rcu_dereference(p)
#define rcu_dereference_check(p, cond) rcu_dereference(p)
-#define RCU_INIT_POINTER(p, v) (p) = (v)
+#define RCU_INIT_POINTER(p, v) \
+do { \
+ (p) = (v); \
+} while (0)

#endif
--
2.7.4