* [PATCH] rcu: replace _________p1 with __UNIQUE_ID(rcu)
@ 2021-09-12 20:41 Chun-Hung Tseng
2021-09-13 22:57 ` kernel test robot
2021-09-13 23:04 ` Paul E. McKenney
0 siblings, 2 replies; 5+ messages in thread
From: Chun-Hung Tseng @ 2021-09-12 20:41 UTC (permalink / raw)
To: paulmck
Cc: josh, rostedt, mathieu.desnoyers, jiangshanlai, joel, rcu,
linux-kernel, Chun-Hung Tseng, Jim Huang
This commit replaced _________p1 with __UNIQUE_ID(rcu), which
generates unique variable names during compilation. Necessary
modifications due to the changes in the RCU macros have also been
reflected in this commit.
The same idea is used for the min/max macros (commit 589a978 and commit
e9092d0), which aims to reduce variable shadowing issues caused by hard
coded variable names.
Signed-off-by: Jim Huang <jserv@ccns.ncku.edu.tw>
Signed-off-by: Chun-Hung Tseng <henrybear327@gmail.com>
---
include/linux/rcupdate.h | 44 +++++++++++++++++++++++-----------------
include/linux/srcu.h | 3 ++-
2 files changed, 27 insertions(+), 20 deletions(-)
diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 434d12fe2d4f..a5ab20822040 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -370,39 +370,41 @@ static inline void rcu_preempt_sleep_check(void) { }
* Converts @p from an __rcu pointer to a __kernel pointer.
* This allows an __rcu pointer to be used with xchg() and friends.
*/
-#define unrcu_pointer(p) \
+#define __unrcu_pointer(p, local) \
({ \
- typeof(*p) *_________p1 = (typeof(*p) *__force)(p); \
+ typeof(*p) *local = (typeof(*p) *__force)(p); \
rcu_check_sparse(p, __rcu); \
- ((typeof(*p) __force __kernel *)(_________p1)); \
+ ((typeof(*p) __force __kernel *)(local)); \
})
+#define unrcu_pointer(p) __unrcu_pointer(p, __UNIQUE_ID(rcu))
-#define __rcu_access_pointer(p, space) \
+#define __rcu_access_pointer(p, local, space) \
({ \
- typeof(*p) *_________p1 = (typeof(*p) *__force)READ_ONCE(p); \
+ typeof(*p) *local = (typeof(*p) *__force)READ_ONCE(p); \
rcu_check_sparse(p, space); \
- ((typeof(*p) __force __kernel *)(_________p1)); \
+ ((typeof(*p) __force __kernel *)(local)); \
})
-#define __rcu_dereference_check(p, c, space) \
+#define __rcu_dereference_check(p, local, c, space) \
({ \
/* Dependency order vs. p above. */ \
- typeof(*p) *________p1 = (typeof(*p) *__force)READ_ONCE(p); \
+ typeof(*p) *local = (typeof(*p) *__force)READ_ONCE(p); \
RCU_LOCKDEP_WARN(!(c), "suspicious rcu_dereference_check() usage"); \
rcu_check_sparse(p, space); \
- ((typeof(*p) __force __kernel *)(________p1)); \
+ ((typeof(*p) __force __kernel *)(local)); \
})
-#define __rcu_dereference_protected(p, c, space) \
+#define __rcu_dereference_protected(p, local, c, space) \
({ \
RCU_LOCKDEP_WARN(!(c), "suspicious rcu_dereference_protected() usage"); \
rcu_check_sparse(p, space); \
((typeof(*p) __force __kernel *)(p)); \
})
-#define rcu_dereference_raw(p) \
+#define __rcu_dereference_raw(p, local) \
({ \
/* Dependency order vs. p above. */ \
- typeof(p) ________p1 = READ_ONCE(p); \
- ((typeof(*p) __force __kernel *)(________p1)); \
+ typeof(p) local = READ_ONCE(p); \
+ ((typeof(*p) __force __kernel *)(local)); \
})
+#define rcu_dereference_raw(p) __rcu_dereference_raw(p, __UNIQUE_ID(rcu))
/**
* RCU_INITIALIZER() - statically initialize an RCU-protected global variable
@@ -489,7 +491,7 @@ do { \
* when tearing down multi-linked structures after a grace period
* has elapsed.
*/
-#define rcu_access_pointer(p) __rcu_access_pointer((p), __rcu)
+#define rcu_access_pointer(p) __rcu_access_pointer((p), __UNIQUE_ID(rcu), __rcu)
/**
* rcu_dereference_check() - rcu_dereference with debug checking
@@ -525,7 +527,8 @@ do { \
* annotated as __rcu.
*/
#define rcu_dereference_check(p, c) \
- __rcu_dereference_check((p), (c) || rcu_read_lock_held(), __rcu)
+ __rcu_dereference_check((p), __UNIQUE_ID(rcu), \
+ (c) || rcu_read_lock_held(), __rcu)
/**
* rcu_dereference_bh_check() - rcu_dereference_bh with debug checking
@@ -540,7 +543,8 @@ do { \
* rcu_read_lock() but also rcu_read_lock_bh() into account.
*/
#define rcu_dereference_bh_check(p, c) \
- __rcu_dereference_check((p), (c) || rcu_read_lock_bh_held(), __rcu)
+ __rcu_dereference_check((p), __UNIQUE_ID(rcu), \
+ (c) || rcu_read_lock_bh_held(), __rcu)
/**
* rcu_dereference_sched_check() - rcu_dereference_sched with debug checking
@@ -555,7 +559,8 @@ do { \
* only rcu_read_lock() but also rcu_read_lock_sched() into account.
*/
#define rcu_dereference_sched_check(p, c) \
- __rcu_dereference_check((p), (c) || rcu_read_lock_sched_held(), \
+ __rcu_dereference_check((p), __UNIQUE_ID(rcu), \
+ (c) || rcu_read_lock_sched_held(), \
__rcu)
/*
@@ -565,7 +570,8 @@ do { \
* The no-tracing version of rcu_dereference_raw() must not call
* rcu_read_lock_held().
*/
-#define rcu_dereference_raw_check(p) __rcu_dereference_check((p), 1, __rcu)
+#define rcu_dereference_raw_check(p) \
+ __rcu_dereference_check((p), __UNIQUE_ID(rcu), 1, __rcu)
/**
* rcu_dereference_protected() - fetch RCU pointer when updates prevented
@@ -584,7 +590,7 @@ do { \
* but very ugly failures.
*/
#define rcu_dereference_protected(p, c) \
- __rcu_dereference_protected((p), (c), __rcu)
+ __rcu_dereference_protected((p), __UNIQUE_ID(rcu), (c), __rcu)
/**
diff --git a/include/linux/srcu.h b/include/linux/srcu.h
index e6011a9975af..01226e4d960a 100644
--- a/include/linux/srcu.h
+++ b/include/linux/srcu.h
@@ -117,7 +117,8 @@ static inline int srcu_read_lock_held(const struct srcu_struct *ssp)
* lockdep_is_held() calls.
*/
#define srcu_dereference_check(p, ssp, c) \
- __rcu_dereference_check((p), (c) || srcu_read_lock_held(ssp), __rcu)
+ __rcu_dereference_check((p), __UNIQUE_ID(rcu), \
+ (c) || srcu_read_lock_held(ssp), __rcu)
/**
* srcu_dereference - fetch SRCU-protected pointer for later dereferencing
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] rcu: replace _________p1 with __UNIQUE_ID(rcu)
2021-09-12 20:41 [PATCH] rcu: replace _________p1 with __UNIQUE_ID(rcu) Chun-Hung Tseng
@ 2021-09-13 22:57 ` kernel test robot
2021-09-13 23:04 ` Paul E. McKenney
1 sibling, 0 replies; 5+ messages in thread
From: kernel test robot @ 2021-09-13 22:57 UTC (permalink / raw)
To: Chun-Hung Tseng, paulmck
Cc: kbuild-all, josh, rostedt, mathieu.desnoyers, jiangshanlai, joel,
rcu, linux-kernel, Chun-Hung Tseng, Jim Huang
[-- Attachment #1: Type: text/plain, Size: 2295 bytes --]
Hi Chun-Hung,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on rcu/dev]
[also build test WARNING on v5.15-rc1 next-20210913]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Chun-Hung-Tseng/rcu-replace-_________p1-with-__UNIQUE_ID-rcu/20210913-044418
base: https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git dev
reproduce: make htmldocs
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
include/linux/rcupdate.h:379: warning: expecting prototype for unrcu_pointer(). Prototype was for __unrcu_pointer() instead
vim +379 include/linux/rcupdate.h
53ecfba259f54b Paul E. McKenney 2010-09-13 366
76c8eaafe4f061 Paul E. McKenney 2021-04-21 367 /**
76c8eaafe4f061 Paul E. McKenney 2021-04-21 368 * unrcu_pointer - mark a pointer as not being RCU protected
76c8eaafe4f061 Paul E. McKenney 2021-04-21 369 * @p: pointer needing to lose its __rcu property
76c8eaafe4f061 Paul E. McKenney 2021-04-21 370 *
76c8eaafe4f061 Paul E. McKenney 2021-04-21 371 * Converts @p from an __rcu pointer to a __kernel pointer.
76c8eaafe4f061 Paul E. McKenney 2021-04-21 372 * This allows an __rcu pointer to be used with xchg() and friends.
76c8eaafe4f061 Paul E. McKenney 2021-04-21 373 */
6744ef711df006 Chun-Hung Tseng 2021-09-13 374 #define __unrcu_pointer(p, local) \
76c8eaafe4f061 Paul E. McKenney 2021-04-21 375 ({ \
6744ef711df006 Chun-Hung Tseng 2021-09-13 376 typeof(*p) *local = (typeof(*p) *__force)(p); \
76c8eaafe4f061 Paul E. McKenney 2021-04-21 377 rcu_check_sparse(p, __rcu); \
6744ef711df006 Chun-Hung Tseng 2021-09-13 378 ((typeof(*p) __force __kernel *)(local)); \
76c8eaafe4f061 Paul E. McKenney 2021-04-21 @379 })
6744ef711df006 Chun-Hung Tseng 2021-09-13 380 #define unrcu_pointer(p) __unrcu_pointer(p, __UNIQUE_ID(rcu))
76c8eaafe4f061 Paul E. McKenney 2021-04-21 381
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 8252 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] rcu: replace _________p1 with __UNIQUE_ID(rcu)
2021-09-12 20:41 [PATCH] rcu: replace _________p1 with __UNIQUE_ID(rcu) Chun-Hung Tseng
2021-09-13 22:57 ` kernel test robot
@ 2021-09-13 23:04 ` Paul E. McKenney
2021-09-13 23:17 ` Paul E. McKenney
1 sibling, 1 reply; 5+ messages in thread
From: Paul E. McKenney @ 2021-09-13 23:04 UTC (permalink / raw)
To: Chun-Hung Tseng
Cc: josh, rostedt, mathieu.desnoyers, jiangshanlai, joel, rcu,
linux-kernel, Jim Huang
On Mon, Sep 13, 2021 at 04:41:31AM +0800, Chun-Hung Tseng wrote:
> This commit replaced _________p1 with __UNIQUE_ID(rcu), which
> generates unique variable names during compilation. Necessary
> modifications due to the changes in the RCU macros have also been
> reflected in this commit.
>
> The same idea is used for the min/max macros (commit 589a978 and commit
> e9092d0), which aims to reduce variable shadowing issues caused by hard
> coded variable names.
>
> Signed-off-by: Jim Huang <jserv@ccns.ncku.edu.tw>
> Signed-off-by: Chun-Hung Tseng <henrybear327@gmail.com>
OK, I will bite...
> ---
> include/linux/rcupdate.h | 44 +++++++++++++++++++++++-----------------
> include/linux/srcu.h | 3 ++-
> 2 files changed, 27 insertions(+), 20 deletions(-)
>
> diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> index 434d12fe2d4f..a5ab20822040 100644
> --- a/include/linux/rcupdate.h
> +++ b/include/linux/rcupdate.h
> @@ -370,39 +370,41 @@ static inline void rcu_preempt_sleep_check(void) { }
> * Converts @p from an __rcu pointer to a __kernel pointer.
> * This allows an __rcu pointer to be used with xchg() and friends.
> */
> -#define unrcu_pointer(p) \
> +#define __unrcu_pointer(p, local) \
> ({ \
> - typeof(*p) *_________p1 = (typeof(*p) *__force)(p); \
> + typeof(*p) *local = (typeof(*p) *__force)(p); \
Why not like this?
typeof(*p) *__UNIQUE_ID(rcu) = (typeof(*p) *__force)(p); \
Then we don't need the extra argument and the changes to the calls.
So what C-preprocessor subtlety am I missing?
Thanx, Paul
> rcu_check_sparse(p, __rcu); \
> - ((typeof(*p) __force __kernel *)(_________p1)); \
> + ((typeof(*p) __force __kernel *)(local)); \
> })
> +#define unrcu_pointer(p) __unrcu_pointer(p, __UNIQUE_ID(rcu))
>
> -#define __rcu_access_pointer(p, space) \
> +#define __rcu_access_pointer(p, local, space) \
> ({ \
> - typeof(*p) *_________p1 = (typeof(*p) *__force)READ_ONCE(p); \
> + typeof(*p) *local = (typeof(*p) *__force)READ_ONCE(p); \
> rcu_check_sparse(p, space); \
> - ((typeof(*p) __force __kernel *)(_________p1)); \
> + ((typeof(*p) __force __kernel *)(local)); \
> })
> -#define __rcu_dereference_check(p, c, space) \
> +#define __rcu_dereference_check(p, local, c, space) \
> ({ \
> /* Dependency order vs. p above. */ \
> - typeof(*p) *________p1 = (typeof(*p) *__force)READ_ONCE(p); \
> + typeof(*p) *local = (typeof(*p) *__force)READ_ONCE(p); \
> RCU_LOCKDEP_WARN(!(c), "suspicious rcu_dereference_check() usage"); \
> rcu_check_sparse(p, space); \
> - ((typeof(*p) __force __kernel *)(________p1)); \
> + ((typeof(*p) __force __kernel *)(local)); \
> })
> -#define __rcu_dereference_protected(p, c, space) \
> +#define __rcu_dereference_protected(p, local, c, space) \
> ({ \
> RCU_LOCKDEP_WARN(!(c), "suspicious rcu_dereference_protected() usage"); \
> rcu_check_sparse(p, space); \
> ((typeof(*p) __force __kernel *)(p)); \
> })
> -#define rcu_dereference_raw(p) \
> +#define __rcu_dereference_raw(p, local) \
> ({ \
> /* Dependency order vs. p above. */ \
> - typeof(p) ________p1 = READ_ONCE(p); \
> - ((typeof(*p) __force __kernel *)(________p1)); \
> + typeof(p) local = READ_ONCE(p); \
> + ((typeof(*p) __force __kernel *)(local)); \
> })
> +#define rcu_dereference_raw(p) __rcu_dereference_raw(p, __UNIQUE_ID(rcu))
>
> /**
> * RCU_INITIALIZER() - statically initialize an RCU-protected global variable
> @@ -489,7 +491,7 @@ do { \
> * when tearing down multi-linked structures after a grace period
> * has elapsed.
> */
> -#define rcu_access_pointer(p) __rcu_access_pointer((p), __rcu)
> +#define rcu_access_pointer(p) __rcu_access_pointer((p), __UNIQUE_ID(rcu), __rcu)
>
> /**
> * rcu_dereference_check() - rcu_dereference with debug checking
> @@ -525,7 +527,8 @@ do { \
> * annotated as __rcu.
> */
> #define rcu_dereference_check(p, c) \
> - __rcu_dereference_check((p), (c) || rcu_read_lock_held(), __rcu)
> + __rcu_dereference_check((p), __UNIQUE_ID(rcu), \
> + (c) || rcu_read_lock_held(), __rcu)
>
> /**
> * rcu_dereference_bh_check() - rcu_dereference_bh with debug checking
> @@ -540,7 +543,8 @@ do { \
> * rcu_read_lock() but also rcu_read_lock_bh() into account.
> */
> #define rcu_dereference_bh_check(p, c) \
> - __rcu_dereference_check((p), (c) || rcu_read_lock_bh_held(), __rcu)
> + __rcu_dereference_check((p), __UNIQUE_ID(rcu), \
> + (c) || rcu_read_lock_bh_held(), __rcu)
>
> /**
> * rcu_dereference_sched_check() - rcu_dereference_sched with debug checking
> @@ -555,7 +559,8 @@ do { \
> * only rcu_read_lock() but also rcu_read_lock_sched() into account.
> */
> #define rcu_dereference_sched_check(p, c) \
> - __rcu_dereference_check((p), (c) || rcu_read_lock_sched_held(), \
> + __rcu_dereference_check((p), __UNIQUE_ID(rcu), \
> + (c) || rcu_read_lock_sched_held(), \
> __rcu)
>
> /*
> @@ -565,7 +570,8 @@ do { \
> * The no-tracing version of rcu_dereference_raw() must not call
> * rcu_read_lock_held().
> */
> -#define rcu_dereference_raw_check(p) __rcu_dereference_check((p), 1, __rcu)
> +#define rcu_dereference_raw_check(p) \
> + __rcu_dereference_check((p), __UNIQUE_ID(rcu), 1, __rcu)
>
> /**
> * rcu_dereference_protected() - fetch RCU pointer when updates prevented
> @@ -584,7 +590,7 @@ do { \
> * but very ugly failures.
> */
> #define rcu_dereference_protected(p, c) \
> - __rcu_dereference_protected((p), (c), __rcu)
> + __rcu_dereference_protected((p), __UNIQUE_ID(rcu), (c), __rcu)
>
>
> /**
> diff --git a/include/linux/srcu.h b/include/linux/srcu.h
> index e6011a9975af..01226e4d960a 100644
> --- a/include/linux/srcu.h
> +++ b/include/linux/srcu.h
> @@ -117,7 +117,8 @@ static inline int srcu_read_lock_held(const struct srcu_struct *ssp)
> * lockdep_is_held() calls.
> */
> #define srcu_dereference_check(p, ssp, c) \
> - __rcu_dereference_check((p), (c) || srcu_read_lock_held(ssp), __rcu)
> + __rcu_dereference_check((p), __UNIQUE_ID(rcu), \
> + (c) || srcu_read_lock_held(ssp), __rcu)
>
> /**
> * srcu_dereference - fetch SRCU-protected pointer for later dereferencing
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] rcu: replace _________p1 with __UNIQUE_ID(rcu)
2021-09-13 23:04 ` Paul E. McKenney
@ 2021-09-13 23:17 ` Paul E. McKenney
2021-09-15 8:42 ` Henry Tseng
0 siblings, 1 reply; 5+ messages in thread
From: Paul E. McKenney @ 2021-09-13 23:17 UTC (permalink / raw)
To: Chun-Hung Tseng
Cc: josh, rostedt, mathieu.desnoyers, jiangshanlai, joel, rcu,
linux-kernel, Jim Huang
On Mon, Sep 13, 2021 at 04:04:30PM -0700, Paul E. McKenney wrote:
> On Mon, Sep 13, 2021 at 04:41:31AM +0800, Chun-Hung Tseng wrote:
> > This commit replaced _________p1 with __UNIQUE_ID(rcu), which
> > generates unique variable names during compilation. Necessary
> > modifications due to the changes in the RCU macros have also been
> > reflected in this commit.
> >
> > The same idea is used for the min/max macros (commit 589a978 and commit
> > e9092d0), which aims to reduce variable shadowing issues caused by hard
> > coded variable names.
> >
> > Signed-off-by: Jim Huang <jserv@ccns.ncku.edu.tw>
> > Signed-off-by: Chun-Hung Tseng <henrybear327@gmail.com>
>
> OK, I will bite...
>
> > ---
> > include/linux/rcupdate.h | 44 +++++++++++++++++++++++-----------------
> > include/linux/srcu.h | 3 ++-
> > 2 files changed, 27 insertions(+), 20 deletions(-)
> >
> > diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> > index 434d12fe2d4f..a5ab20822040 100644
> > --- a/include/linux/rcupdate.h
> > +++ b/include/linux/rcupdate.h
> > @@ -370,39 +370,41 @@ static inline void rcu_preempt_sleep_check(void) { }
> > * Converts @p from an __rcu pointer to a __kernel pointer.
> > * This allows an __rcu pointer to be used with xchg() and friends.
> > */
> > -#define unrcu_pointer(p) \
> > +#define __unrcu_pointer(p, local) \
> > ({ \
> > - typeof(*p) *_________p1 = (typeof(*p) *__force)(p); \
> > + typeof(*p) *local = (typeof(*p) *__force)(p); \
>
> Why not like this?
>
> typeof(*p) *__UNIQUE_ID(rcu) = (typeof(*p) *__force)(p); \
>
> Then we don't need the extra argument and the changes to the calls.
>
> So what C-preprocessor subtlety am I missing?
Never mind!!! My suggested approach would generate a unique name at
every use, except on non-gcc/non-clang compilers, which would obviously
not do what we want.
Thanx, Paul
> > rcu_check_sparse(p, __rcu); \
> > - ((typeof(*p) __force __kernel *)(_________p1)); \
> > + ((typeof(*p) __force __kernel *)(local)); \
> > })
> > +#define unrcu_pointer(p) __unrcu_pointer(p, __UNIQUE_ID(rcu))
> >
> > -#define __rcu_access_pointer(p, space) \
> > +#define __rcu_access_pointer(p, local, space) \
> > ({ \
> > - typeof(*p) *_________p1 = (typeof(*p) *__force)READ_ONCE(p); \
> > + typeof(*p) *local = (typeof(*p) *__force)READ_ONCE(p); \
> > rcu_check_sparse(p, space); \
> > - ((typeof(*p) __force __kernel *)(_________p1)); \
> > + ((typeof(*p) __force __kernel *)(local)); \
> > })
> > -#define __rcu_dereference_check(p, c, space) \
> > +#define __rcu_dereference_check(p, local, c, space) \
> > ({ \
> > /* Dependency order vs. p above. */ \
> > - typeof(*p) *________p1 = (typeof(*p) *__force)READ_ONCE(p); \
> > + typeof(*p) *local = (typeof(*p) *__force)READ_ONCE(p); \
> > RCU_LOCKDEP_WARN(!(c), "suspicious rcu_dereference_check() usage"); \
> > rcu_check_sparse(p, space); \
> > - ((typeof(*p) __force __kernel *)(________p1)); \
> > + ((typeof(*p) __force __kernel *)(local)); \
> > })
> > -#define __rcu_dereference_protected(p, c, space) \
> > +#define __rcu_dereference_protected(p, local, c, space) \
> > ({ \
> > RCU_LOCKDEP_WARN(!(c), "suspicious rcu_dereference_protected() usage"); \
> > rcu_check_sparse(p, space); \
> > ((typeof(*p) __force __kernel *)(p)); \
> > })
> > -#define rcu_dereference_raw(p) \
> > +#define __rcu_dereference_raw(p, local) \
> > ({ \
> > /* Dependency order vs. p above. */ \
> > - typeof(p) ________p1 = READ_ONCE(p); \
> > - ((typeof(*p) __force __kernel *)(________p1)); \
> > + typeof(p) local = READ_ONCE(p); \
> > + ((typeof(*p) __force __kernel *)(local)); \
> > })
> > +#define rcu_dereference_raw(p) __rcu_dereference_raw(p, __UNIQUE_ID(rcu))
> >
> > /**
> > * RCU_INITIALIZER() - statically initialize an RCU-protected global variable
> > @@ -489,7 +491,7 @@ do { \
> > * when tearing down multi-linked structures after a grace period
> > * has elapsed.
> > */
> > -#define rcu_access_pointer(p) __rcu_access_pointer((p), __rcu)
> > +#define rcu_access_pointer(p) __rcu_access_pointer((p), __UNIQUE_ID(rcu), __rcu)
> >
> > /**
> > * rcu_dereference_check() - rcu_dereference with debug checking
> > @@ -525,7 +527,8 @@ do { \
> > * annotated as __rcu.
> > */
> > #define rcu_dereference_check(p, c) \
> > - __rcu_dereference_check((p), (c) || rcu_read_lock_held(), __rcu)
> > + __rcu_dereference_check((p), __UNIQUE_ID(rcu), \
> > + (c) || rcu_read_lock_held(), __rcu)
> >
> > /**
> > * rcu_dereference_bh_check() - rcu_dereference_bh with debug checking
> > @@ -540,7 +543,8 @@ do { \
> > * rcu_read_lock() but also rcu_read_lock_bh() into account.
> > */
> > #define rcu_dereference_bh_check(p, c) \
> > - __rcu_dereference_check((p), (c) || rcu_read_lock_bh_held(), __rcu)
> > + __rcu_dereference_check((p), __UNIQUE_ID(rcu), \
> > + (c) || rcu_read_lock_bh_held(), __rcu)
> >
> > /**
> > * rcu_dereference_sched_check() - rcu_dereference_sched with debug checking
> > @@ -555,7 +559,8 @@ do { \
> > * only rcu_read_lock() but also rcu_read_lock_sched() into account.
> > */
> > #define rcu_dereference_sched_check(p, c) \
> > - __rcu_dereference_check((p), (c) || rcu_read_lock_sched_held(), \
> > + __rcu_dereference_check((p), __UNIQUE_ID(rcu), \
> > + (c) || rcu_read_lock_sched_held(), \
> > __rcu)
> >
> > /*
> > @@ -565,7 +570,8 @@ do { \
> > * The no-tracing version of rcu_dereference_raw() must not call
> > * rcu_read_lock_held().
> > */
> > -#define rcu_dereference_raw_check(p) __rcu_dereference_check((p), 1, __rcu)
> > +#define rcu_dereference_raw_check(p) \
> > + __rcu_dereference_check((p), __UNIQUE_ID(rcu), 1, __rcu)
> >
> > /**
> > * rcu_dereference_protected() - fetch RCU pointer when updates prevented
> > @@ -584,7 +590,7 @@ do { \
> > * but very ugly failures.
> > */
> > #define rcu_dereference_protected(p, c) \
> > - __rcu_dereference_protected((p), (c), __rcu)
> > + __rcu_dereference_protected((p), __UNIQUE_ID(rcu), (c), __rcu)
> >
> >
> > /**
> > diff --git a/include/linux/srcu.h b/include/linux/srcu.h
> > index e6011a9975af..01226e4d960a 100644
> > --- a/include/linux/srcu.h
> > +++ b/include/linux/srcu.h
> > @@ -117,7 +117,8 @@ static inline int srcu_read_lock_held(const struct srcu_struct *ssp)
> > * lockdep_is_held() calls.
> > */
> > #define srcu_dereference_check(p, ssp, c) \
> > - __rcu_dereference_check((p), (c) || srcu_read_lock_held(ssp), __rcu)
> > + __rcu_dereference_check((p), __UNIQUE_ID(rcu), \
> > + (c) || srcu_read_lock_held(ssp), __rcu)
> >
> > /**
> > * srcu_dereference - fetch SRCU-protected pointer for later dereferencing
> > --
> > 2.25.1
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] rcu: replace _________p1 with __UNIQUE_ID(rcu)
2021-09-13 23:17 ` Paul E. McKenney
@ 2021-09-15 8:42 ` Henry Tseng
0 siblings, 0 replies; 5+ messages in thread
From: Henry Tseng @ 2021-09-15 8:42 UTC (permalink / raw)
To: paulmck
Cc: josh, rostedt, mathieu.desnoyers, jiangshanlai, joel, rcu,
linux-kernel, Jim Huang
Thank you for the feedback!!!
I will fix the kernel test bot warning, and then make a v2 of the
patch, containing a bit more detail in the commit message.
Thanks,
Henry
On Tue, Sep 14, 2021 at 1:17 AM Paul E. McKenney <paulmck@kernel.org> wrote:
>
> On Mon, Sep 13, 2021 at 04:04:30PM -0700, Paul E. McKenney wrote:
> > On Mon, Sep 13, 2021 at 04:41:31AM +0800, Chun-Hung Tseng wrote:
> > > This commit replaced _________p1 with __UNIQUE_ID(rcu), which
> > > generates unique variable names during compilation. Necessary
> > > modifications due to the changes in the RCU macros have also been
> > > reflected in this commit.
> > >
> > > The same idea is used for the min/max macros (commit 589a978 and commit
> > > e9092d0), which aims to reduce variable shadowing issues caused by hard
> > > coded variable names.
> > >
> > > Signed-off-by: Jim Huang <jserv@ccns.ncku.edu.tw>
> > > Signed-off-by: Chun-Hung Tseng <henrybear327@gmail.com>
> >
> > OK, I will bite...
> >
> > > ---
> > > include/linux/rcupdate.h | 44 +++++++++++++++++++++++-----------------
> > > include/linux/srcu.h | 3 ++-
> > > 2 files changed, 27 insertions(+), 20 deletions(-)
> > >
> > > diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> > > index 434d12fe2d4f..a5ab20822040 100644
> > > --- a/include/linux/rcupdate.h
> > > +++ b/include/linux/rcupdate.h
> > > @@ -370,39 +370,41 @@ static inline void rcu_preempt_sleep_check(void) { }
> > > * Converts @p from an __rcu pointer to a __kernel pointer.
> > > * This allows an __rcu pointer to be used with xchg() and friends.
> > > */
> > > -#define unrcu_pointer(p) \
> > > +#define __unrcu_pointer(p, local) \
> > > ({ \
> > > - typeof(*p) *_________p1 = (typeof(*p) *__force)(p); \
> > > + typeof(*p) *local = (typeof(*p) *__force)(p); \
> >
> > Why not like this?
> >
> > typeof(*p) *__UNIQUE_ID(rcu) = (typeof(*p) *__force)(p); \
> >
> > Then we don't need the extra argument and the changes to the calls.
> >
> > So what C-preprocessor subtlety am I missing?
>
> Never mind!!! My suggested approach would generate a unique name at
> every use, except on non-gcc/non-clang compilers, which would obviously
> not do what we want.
>
> Thanx, Paul
>
> > > rcu_check_sparse(p, __rcu); \
> > > - ((typeof(*p) __force __kernel *)(_________p1)); \
> > > + ((typeof(*p) __force __kernel *)(local)); \
> > > })
> > > +#define unrcu_pointer(p) __unrcu_pointer(p, __UNIQUE_ID(rcu))
> > >
> > > -#define __rcu_access_pointer(p, space) \
> > > +#define __rcu_access_pointer(p, local, space) \
> > > ({ \
> > > - typeof(*p) *_________p1 = (typeof(*p) *__force)READ_ONCE(p); \
> > > + typeof(*p) *local = (typeof(*p) *__force)READ_ONCE(p); \
> > > rcu_check_sparse(p, space); \
> > > - ((typeof(*p) __force __kernel *)(_________p1)); \
> > > + ((typeof(*p) __force __kernel *)(local)); \
> > > })
> > > -#define __rcu_dereference_check(p, c, space) \
> > > +#define __rcu_dereference_check(p, local, c, space) \
> > > ({ \
> > > /* Dependency order vs. p above. */ \
> > > - typeof(*p) *________p1 = (typeof(*p) *__force)READ_ONCE(p); \
> > > + typeof(*p) *local = (typeof(*p) *__force)READ_ONCE(p); \
> > > RCU_LOCKDEP_WARN(!(c), "suspicious rcu_dereference_check() usage"); \
> > > rcu_check_sparse(p, space); \
> > > - ((typeof(*p) __force __kernel *)(________p1)); \
> > > + ((typeof(*p) __force __kernel *)(local)); \
> > > })
> > > -#define __rcu_dereference_protected(p, c, space) \
> > > +#define __rcu_dereference_protected(p, local, c, space) \
> > > ({ \
> > > RCU_LOCKDEP_WARN(!(c), "suspicious rcu_dereference_protected() usage"); \
> > > rcu_check_sparse(p, space); \
> > > ((typeof(*p) __force __kernel *)(p)); \
> > > })
> > > -#define rcu_dereference_raw(p) \
> > > +#define __rcu_dereference_raw(p, local) \
> > > ({ \
> > > /* Dependency order vs. p above. */ \
> > > - typeof(p) ________p1 = READ_ONCE(p); \
> > > - ((typeof(*p) __force __kernel *)(________p1)); \
> > > + typeof(p) local = READ_ONCE(p); \
> > > + ((typeof(*p) __force __kernel *)(local)); \
> > > })
> > > +#define rcu_dereference_raw(p) __rcu_dereference_raw(p, __UNIQUE_ID(rcu))
> > >
> > > /**
> > > * RCU_INITIALIZER() - statically initialize an RCU-protected global variable
> > > @@ -489,7 +491,7 @@ do { \
> > > * when tearing down multi-linked structures after a grace period
> > > * has elapsed.
> > > */
> > > -#define rcu_access_pointer(p) __rcu_access_pointer((p), __rcu)
> > > +#define rcu_access_pointer(p) __rcu_access_pointer((p), __UNIQUE_ID(rcu), __rcu)
> > >
> > > /**
> > > * rcu_dereference_check() - rcu_dereference with debug checking
> > > @@ -525,7 +527,8 @@ do { \
> > > * annotated as __rcu.
> > > */
> > > #define rcu_dereference_check(p, c) \
> > > - __rcu_dereference_check((p), (c) || rcu_read_lock_held(), __rcu)
> > > + __rcu_dereference_check((p), __UNIQUE_ID(rcu), \
> > > + (c) || rcu_read_lock_held(), __rcu)
> > >
> > > /**
> > > * rcu_dereference_bh_check() - rcu_dereference_bh with debug checking
> > > @@ -540,7 +543,8 @@ do { \
> > > * rcu_read_lock() but also rcu_read_lock_bh() into account.
> > > */
> > > #define rcu_dereference_bh_check(p, c) \
> > > - __rcu_dereference_check((p), (c) || rcu_read_lock_bh_held(), __rcu)
> > > + __rcu_dereference_check((p), __UNIQUE_ID(rcu), \
> > > + (c) || rcu_read_lock_bh_held(), __rcu)
> > >
> > > /**
> > > * rcu_dereference_sched_check() - rcu_dereference_sched with debug checking
> > > @@ -555,7 +559,8 @@ do { \
> > > * only rcu_read_lock() but also rcu_read_lock_sched() into account.
> > > */
> > > #define rcu_dereference_sched_check(p, c) \
> > > - __rcu_dereference_check((p), (c) || rcu_read_lock_sched_held(), \
> > > + __rcu_dereference_check((p), __UNIQUE_ID(rcu), \
> > > + (c) || rcu_read_lock_sched_held(), \
> > > __rcu)
> > >
> > > /*
> > > @@ -565,7 +570,8 @@ do { \
> > > * The no-tracing version of rcu_dereference_raw() must not call
> > > * rcu_read_lock_held().
> > > */
> > > -#define rcu_dereference_raw_check(p) __rcu_dereference_check((p), 1, __rcu)
> > > +#define rcu_dereference_raw_check(p) \
> > > + __rcu_dereference_check((p), __UNIQUE_ID(rcu), 1, __rcu)
> > >
> > > /**
> > > * rcu_dereference_protected() - fetch RCU pointer when updates prevented
> > > @@ -584,7 +590,7 @@ do { \
> > > * but very ugly failures.
> > > */
> > > #define rcu_dereference_protected(p, c) \
> > > - __rcu_dereference_protected((p), (c), __rcu)
> > > + __rcu_dereference_protected((p), __UNIQUE_ID(rcu), (c), __rcu)
> > >
> > >
> > > /**
> > > diff --git a/include/linux/srcu.h b/include/linux/srcu.h
> > > index e6011a9975af..01226e4d960a 100644
> > > --- a/include/linux/srcu.h
> > > +++ b/include/linux/srcu.h
> > > @@ -117,7 +117,8 @@ static inline int srcu_read_lock_held(const struct srcu_struct *ssp)
> > > * lockdep_is_held() calls.
> > > */
> > > #define srcu_dereference_check(p, ssp, c) \
> > > - __rcu_dereference_check((p), (c) || srcu_read_lock_held(ssp), __rcu)
> > > + __rcu_dereference_check((p), __UNIQUE_ID(rcu), \
> > > + (c) || srcu_read_lock_held(ssp), __rcu)
> > >
> > > /**
> > > * srcu_dereference - fetch SRCU-protected pointer for later dereferencing
> > > --
> > > 2.25.1
> > >
--
Best wishes,
Henry
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-09-15 8:42 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-12 20:41 [PATCH] rcu: replace _________p1 with __UNIQUE_ID(rcu) Chun-Hung Tseng
2021-09-13 22:57 ` kernel test robot
2021-09-13 23:04 ` Paul E. McKenney
2021-09-13 23:17 ` Paul E. McKenney
2021-09-15 8:42 ` Henry Tseng
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).