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@linux.ibm.com Signed-off-by: Andrea Parri <andrea.parri@amarulasolutions.com> Cc: "Paul E. McKenney" <paulmck@linux.ibm.com> Cc: Josh Triplett <josh@joshtriplett.org> Cc: Steven Rostedt <rostedt@goodmis.org> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> Cc: Lai Jiangshan <jiangshanlai@gmail.com> Cc: Joel Fernandes <joel@joelfernandes.org> Cc: rcu@vger.kernel.org Cc: Peter Zijlstra <peterz@infradead.org> Cc: Will Deacon <will.deacon@arm.com> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Matthew Wilcox <willy@infradead.org> Cc: Sasha Levin <sashal@kernel.org> --- 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
On Fri, May 24, 2019 at 12:36:37PM +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@linux.ibm.com Thank you! A few comments below. Thanx, Paul > Signed-off-by: Andrea Parri <andrea.parri@amarulasolutions.com> > Cc: "Paul E. McKenney" <paulmck@linux.ibm.com> > Cc: Josh Triplett <josh@joshtriplett.org> > Cc: Steven Rostedt <rostedt@goodmis.org> > Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> > Cc: Lai Jiangshan <jiangshanlai@gmail.com> > Cc: Joel Fernandes <joel@joelfernandes.org> > Cc: rcu@vger.kernel.org > Cc: Peter Zijlstra <peterz@infradead.org> > Cc: Will Deacon <will.deacon@arm.com> > Cc: Mark Rutland <mark.rutland@arm.com> > Cc: Matthew Wilcox <willy@infradead.org> > Cc: Sasha Levin <sashal@kernel.org> > --- > 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); Please add the "void", similar to synchronize_rcu() above. > 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) These two each fit nicely on one line: #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) As does this one. > #endif > -- > 2.7.4 >
On Fri, May 24, 2019 at 12:36:37PM +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@linux.ibm.com
>
> Signed-off-by: Andrea Parri <andrea.parri@amarulasolutions.com>
> Cc: "Paul E. McKenney" <paulmck@linux.ibm.com>
> Cc: Josh Triplett <josh@joshtriplett.org>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Cc: Lai Jiangshan <jiangshanlai@gmail.com>
> Cc: Joel Fernandes <joel@joelfernandes.org>
> Cc: rcu@vger.kernel.org
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: Sasha Levin <sashal@kernel.org>
> ---
> 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
>
Other than Paul's nits, LGTM. Thanks.
On Fri, May 24, 2019 at 06:29:11AM -0700, Paul E. McKenney wrote: > On Fri, May 24, 2019 at 12:36:37PM +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@linux.ibm.com > > Thank you! A few comments below. Thank you for the suggestions, Paul. > > Thanx, Paul > > > Signed-off-by: Andrea Parri <andrea.parri@amarulasolutions.com> > > Cc: "Paul E. McKenney" <paulmck@linux.ibm.com> > > Cc: Josh Triplett <josh@joshtriplett.org> > > Cc: Steven Rostedt <rostedt@goodmis.org> > > Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> > > Cc: Lai Jiangshan <jiangshanlai@gmail.com> > > Cc: Joel Fernandes <joel@joelfernandes.org> > > Cc: rcu@vger.kernel.org > > Cc: Peter Zijlstra <peterz@infradead.org> > > Cc: Will Deacon <will.deacon@arm.com> > > Cc: Mark Rutland <mark.rutland@arm.com> > > Cc: Matthew Wilcox <willy@infradead.org> > > Cc: Sasha Levin <sashal@kernel.org> > > --- > > 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); > > Please add the "void", similar to synchronize_rcu() above. Sure, will do in v2. > > > 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) > > These two each fit nicely on one line: > > #define rcu_assign_pointer(p, v) do { (p) = (v); } while (0) > #define RCU_INIT_POINTER(p, v) do { (p) = (v); } while (0) Same here. > > > > > #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) > > As does this one. ... And here. Thanks, Andrea
On Fri, May 24, 2019 at 12:45:09PM -0400, Joel Fernandes wrote:
> On Fri, May 24, 2019 at 12:36:37PM +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@linux.ibm.com
> >
> > Signed-off-by: Andrea Parri <andrea.parri@amarulasolutions.com>
> > Cc: "Paul E. McKenney" <paulmck@linux.ibm.com>
> > Cc: Josh Triplett <josh@joshtriplett.org>
> > Cc: Steven Rostedt <rostedt@goodmis.org>
> > Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> > Cc: Lai Jiangshan <jiangshanlai@gmail.com>
> > Cc: Joel Fernandes <joel@joelfernandes.org>
> > Cc: rcu@vger.kernel.org
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Will Deacon <will.deacon@arm.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Matthew Wilcox <willy@infradead.org>
> > Cc: Sasha Levin <sashal@kernel.org>
> > ---
> > 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
> >
>
> Other than Paul's nits, LGTM. Thanks.
Thank you, Joel! Will fix those and resend, most likely next week.
Andrea