On Mon, May 27, 2019 at 10:49:57AM +0200, Andrea Parri wrote: > 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 Queued, thank you! Adding the checkpatch maintainers on CC as well. The "do { } while (0)" prevents the return value from being used, by design. Given the checkpatch complaint, is there some better way to achieve this? Thanx, Paul > 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> > --- > Changes in v2: > - Fix documentation and style (Paul E. McKenney) > - Improve subject line (Mark Rutland) > --- > Documentation/RCU/whatisRCU.txt | 8 ++++---- > include/linux/rcupdate.h | 5 ++--- > tools/include/linux/rcu.h | 4 ++-- > tools/testing/radix-tree/linux/rcupdate.h | 2 +- > 4 files changed, 9 insertions(+), 10 deletions(-) > > diff --git a/Documentation/RCU/whatisRCU.txt b/Documentation/RCU/whatisRCU.txt > index 981651a8b65d2..7e1a8721637ab 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); > + void 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 a8ed624da5556..0c9b92799abc7 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..9554d3fa54f33 100644 > --- a/tools/include/linux/rcu.h > +++ b/tools/include/linux/rcu.h > @@ -19,7 +19,7 @@ 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..fed468fb0c78d 100644 > --- a/tools/testing/radix-tree/linux/rcupdate.h > +++ b/tools/testing/radix-tree/linux/rcupdate.h > @@ -7,6 +7,6 @@ > #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 >
