From: "Paul E. McKenney" <paulmck@kernel.org>
To: Chun-Hung Tseng <henrybear327@gmail.com>
Cc: josh@joshtriplett.org, rostedt@goodmis.org,
mathieu.desnoyers@efficios.com, jiangshanlai@gmail.com,
joel@joelfernandes.org, rcu@vger.kernel.org,
linux-kernel@vger.kernel.org, Jim Huang <jserv@ccns.ncku.edu.tw>
Subject: Re: [PATCH] rcu: replace _________p1 with __UNIQUE_ID(rcu)
Date: Mon, 13 Sep 2021 16:17:51 -0700 [thread overview]
Message-ID: <20210913231751.GA2495405@paulmck-ThinkPad-P17-Gen-1> (raw)
In-Reply-To: <20210913230430.GM4156@paulmck-ThinkPad-P17-Gen-1>
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
> >
next prev parent reply other threads:[~2021-09-13 23:19 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2021-09-15 8:42 ` Henry Tseng
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210913231751.GA2495405@paulmck-ThinkPad-P17-Gen-1 \
--to=paulmck@kernel.org \
--cc=henrybear327@gmail.com \
--cc=jiangshanlai@gmail.com \
--cc=joel@joelfernandes.org \
--cc=josh@joshtriplett.org \
--cc=jserv@ccns.ncku.edu.tw \
--cc=linux-kernel@vger.kernel.org \
--cc=mathieu.desnoyers@efficios.com \
--cc=rcu@vger.kernel.org \
--cc=rostedt@goodmis.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).