Messages in this thread | | | Date | Wed, 12 Dec 2018 18:47:03 -0800 | From | "Paul E. McKenney" <> | Subject | Re: [PATCH RFC] rcu: add sparse check to rcu_assign_pointer |
| |
On Wed, Dec 12, 2018 at 02:37:10PM -0800, Joel Fernandes (Google) wrote: > rcu_assign_pointer currently doesn't do any sparse checking on a pointer > assigned. So its possible that a pointer that is not __rcu annotated is > assigned with rcu_assign_pointer without sparse complainting. > rcu_dereference already does such checking so lets also make > rcu_assign_pointer to do the same. The extra error could be helpful in > cases where an RCU pointer is assigned with rcu_assign_pointer but not > annotated with __rcu. > > This doesn't generate any code in the normal case because __CHECKER__ is > defined only in the context of sparse. > > Also we rename rcu_dereference_sparse to rcu_check_parse since the > checking now happens not only during derereferencing but also during > assignment. > > Test: Introduced an rcu_assign_pointer in code and checked the output of > sparse with and without this change. The change correctly causes sparse > to throw an error. > > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
Applied with some commit-log fixes, thank you!
Thanx, Paul
> --- > include/linux/rcupdate.h | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h > index 4db8bcacc51a..9e6f10da7f26 100644 > --- a/include/linux/rcupdate.h > +++ b/include/linux/rcupdate.h > @@ -309,16 +309,16 @@ static inline void rcu_preempt_sleep_check(void) { } > */ > > #ifdef __CHECKER__ > -#define rcu_dereference_sparse(p, space) \ > +#define rcu_check_sparse(p, space) \ > ((void)(((typeof(*p) space *)p) == p)) > #else /* #ifdef __CHECKER__ */ > -#define rcu_dereference_sparse(p, space) > +#define rcu_check_sparse(p, space) > #endif /* #else #ifdef __CHECKER__ */ > > #define __rcu_access_pointer(p, space) \ > ({ \ > typeof(*p) *_________p1 = (typeof(*p) *__force)READ_ONCE(p); \ > - rcu_dereference_sparse(p, space); \ > + rcu_check_sparse(p, space); \ > ((typeof(*p) __force __kernel *)(_________p1)); \ > }) > #define __rcu_dereference_check(p, c, space) \ > @@ -326,13 +326,13 @@ static inline void rcu_preempt_sleep_check(void) { } > /* Dependency order vs. p above. */ \ > typeof(*p) *________p1 = (typeof(*p) *__force)READ_ONCE(p); \ > RCU_LOCKDEP_WARN(!(c), "suspicious rcu_dereference_check() usage"); \ > - rcu_dereference_sparse(p, space); \ > + rcu_check_sparse(p, space); \ > ((typeof(*p) __force __kernel *)(________p1)); \ > }) > #define __rcu_dereference_protected(p, c, space) \ > ({ \ > RCU_LOCKDEP_WARN(!(c), "suspicious rcu_dereference_protected() usage"); \ > - rcu_dereference_sparse(p, space); \ > + rcu_check_sparse(p, space); \ > ((typeof(*p) __force __kernel *)(p)); \ > }) > #define rcu_dereference_raw(p) \ > @@ -382,6 +382,7 @@ static inline void rcu_preempt_sleep_check(void) { } > #define rcu_assign_pointer(p, v) \ > ({ \ > uintptr_t _r_a_p__v = (uintptr_t)(v); \ > + rcu_check_sparse(p, __rcu); \ > \ > if (__builtin_constant_p(v) && (_r_a_p__v) == (uintptr_t)NULL) \ > WRITE_ONCE((p), (typeof(p))(_r_a_p__v)); \ > @@ -785,7 +786,7 @@ static inline notrace void rcu_read_unlock_sched_notrace(void) > */ > #define RCU_INIT_POINTER(p, v) \ > do { \ > - rcu_dereference_sparse(p, __rcu); \ > + rcu_check_sparse(p, __rcu); \ > WRITE_ONCE(p, RCU_INITIALIZER(v)); \ > } while (0) > > -- > 2.20.0.rc1.387.gf8505762e3-goog >
| |