public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	"Paul E. McKenney" <paulmck@kernel.org>,
	Joel Fernandes <joel@joelfernandes.org>,
	Josh Triplett <josh@joshtriplett.org>,
	Boqun Feng <boqun.feng@gmail.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Lai Jiangshan <jiangshanlai@gmail.com>,
	Zqiang <qiang1.zhang@intel.com>
Subject: [RFC PATCH 01/13] rcu: rcupdate.h: Fix parentheses around macro parameter use
Date: Thu,  4 May 2023 16:05:15 -0400	[thread overview]
Message-ID: <20230504200527.1935944-2-mathieu.desnoyers@efficios.com> (raw)
In-Reply-To: <20230504200527.1935944-1-mathieu.desnoyers@efficios.com>

linux/rcupdate.h macros use the *p parameter without parentheses, e.g.:

  typeof(*p)

rather than

  typeof(*(p))

The following test-case shows how it can generate confusion due to C
operator precedence being reversed compared to the expectations:

    #define m(p) \
    do { \
            __typeof__(*p) v = 0; \
    } while (0)

    void fct(unsigned long long *p1)
    {
            m(p1 + 1);      /* works */
            m(1 + p1);      /* broken */
    }

Add missing parentheses around macro parameter use in the following
pattern to ensure operator precedence behaves as expected:

- m(&p) is changed for m(&(p)).

Remove useless parentheses in the following macro argument usage
patterns:

- m((x), y) is changed for m(x, y), because comma has the lowest
  operator precedence, and thus the extra parentheses are useless.
- "(_r_a_p__v) where _r_a_p__v is a variable and not a macro parameter,
  is changed for "_r_a_p__v",

An extra whitespace is also removed in kvfree_rcu_arg_2() for
consistency.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: "Paul E. McKenney" <paulmck@kernel.org>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Josh Triplett <josh@joshtriplett.org>
Cc: Boqun Feng <boqun.feng@gmail.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Lai Jiangshan <jiangshanlai@gmail.com>
Cc: Zqiang <qiang1.zhang@intel.com>
---
 include/linux/rcupdate.h | 46 ++++++++++++++++++++--------------------
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index dcd2cf1e8326..4f26c3080953 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -215,7 +215,7 @@ void rcu_tasks_trace_qs_blkd(struct task_struct *t);
 										\
 		if (likely(!READ_ONCE((t)->trc_reader_special.b.need_qs)) &&	\
 		    likely(!___rttq_nesting)) {					\
-			rcu_trc_cmpxchg_need_qs((t), 0,	TRC_NEED_QS_CHECKED);	\
+			rcu_trc_cmpxchg_need_qs(t, 0, TRC_NEED_QS_CHECKED);	\
 		} else if (___rttq_nesting && ___rttq_nesting != INT_MIN &&	\
 			   !READ_ONCE((t)->trc_reader_special.b.blocked)) {	\
 			rcu_tasks_trace_qs_blkd(t);				\
@@ -227,7 +227,7 @@ void rcu_tasks_trace_qs_blkd(struct task_struct *t);
 
 #define rcu_tasks_qs(t, preempt)					\
 do {									\
-	rcu_tasks_classic_qs((t), (preempt));				\
+	rcu_tasks_classic_qs(t, preempt);				\
 	rcu_tasks_trace_qs(t);						\
 } while (0)
 
@@ -430,16 +430,16 @@ static inline void rcu_preempt_sleep_check(void) { }
 
 #ifdef __CHECKER__
 #define rcu_check_sparse(p, space) \
-	((void)(((typeof(*p) space *)p) == p))
+	((void)(((typeof(*(p)) space *)(p)) == (p)))
 #else /* #ifdef __CHECKER__ */
 #define rcu_check_sparse(p, space)
 #endif /* #else #ifdef __CHECKER__ */
 
 #define __unrcu_pointer(p, local)					\
 ({									\
-	typeof(*p) *local = (typeof(*p) *__force)(p);			\
+	typeof(*(p)) *local = (typeof(*(p)) *__force)(p);		\
 	rcu_check_sparse(p, __rcu);					\
-	((typeof(*p) __force __kernel *)(local)); 			\
+	((typeof(*(p)) __force __kernel *)(local));			\
 })
 /**
  * unrcu_pointer - mark a pointer as not being RCU protected
@@ -452,29 +452,29 @@ static inline void rcu_preempt_sleep_check(void) { }
 
 #define __rcu_access_pointer(p, local, space) \
 ({ \
-	typeof(*p) *local = (typeof(*p) *__force)READ_ONCE(p); \
+	typeof(*(p)) *local = (typeof(*(p)) *__force)READ_ONCE(p); \
 	rcu_check_sparse(p, space); \
-	((typeof(*p) __force __kernel *)(local)); \
+	((typeof(*(p)) __force __kernel *)(local)); \
 })
 #define __rcu_dereference_check(p, local, c, space) \
 ({ \
 	/* Dependency order vs. p above. */ \
-	typeof(*p) *local = (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 *)(local)); \
+	((typeof(*(p)) __force __kernel *)(local)); \
 })
 #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)); \
+	((typeof(*(p)) __force __kernel *)(p)); \
 })
 #define __rcu_dereference_raw(p, local) \
 ({ \
 	/* Dependency order vs. p above. */ \
 	typeof(p) local = READ_ONCE(p); \
-	((typeof(*p) __force __kernel *)(local)); \
+	((typeof(*(p)) __force __kernel *)(local)); \
 })
 #define rcu_dereference_raw(p) __rcu_dereference_raw(p, __UNIQUE_ID(rcu))
 
@@ -520,10 +520,10 @@ do {									      \
 	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));		      \
+	if (__builtin_constant_p(v) && _r_a_p__v == (uintptr_t)NULL)	      \
+		WRITE_ONCE(p, (typeof(p))_r_a_p__v);			      \
 	else								      \
-		smp_store_release(&p, RCU_INITIALIZER((typeof(p))_r_a_p__v)); \
+		smp_store_release(&(p), RCU_INITIALIZER((typeof(p))_r_a_p__v));\
 } while (0)
 
 /**
@@ -539,8 +539,8 @@ do {									      \
  */
 #define rcu_replace_pointer(rcu_ptr, ptr, c)				\
 ({									\
-	typeof(ptr) __tmp = rcu_dereference_protected((rcu_ptr), (c));	\
-	rcu_assign_pointer((rcu_ptr), (ptr));				\
+	typeof(ptr) __tmp = rcu_dereference_protected(rcu_ptr, c);	\
+	rcu_assign_pointer(rcu_ptr, ptr);				\
 	__tmp;								\
 })
 
@@ -571,7 +571,7 @@ do {									      \
  * down multi-linked structures after a grace period has elapsed.  However,
  * rcu_dereference_protected() is normally preferred for this use case.
  */
-#define rcu_access_pointer(p) __rcu_access_pointer((p), __UNIQUE_ID(rcu), __rcu)
+#define rcu_access_pointer(p) __rcu_access_pointer(p, __UNIQUE_ID(rcu), __rcu)
 
 /**
  * rcu_dereference_check() - rcu_dereference with debug checking
@@ -607,7 +607,7 @@ do {									      \
  * annotated as __rcu.
  */
 #define rcu_dereference_check(p, c) \
-	__rcu_dereference_check((p), __UNIQUE_ID(rcu), \
+	__rcu_dereference_check(p, __UNIQUE_ID(rcu), \
 				(c) || rcu_read_lock_held(), __rcu)
 
 /**
@@ -623,7 +623,7 @@ do {									      \
  * rcu_read_lock() but also rcu_read_lock_bh() into account.
  */
 #define rcu_dereference_bh_check(p, c) \
-	__rcu_dereference_check((p), __UNIQUE_ID(rcu), \
+	__rcu_dereference_check(p, __UNIQUE_ID(rcu), \
 				(c) || rcu_read_lock_bh_held(), __rcu)
 
 /**
@@ -639,7 +639,7 @@ do {									      \
  * only rcu_read_lock() but also rcu_read_lock_sched() into account.
  */
 #define rcu_dereference_sched_check(p, c) \
-	__rcu_dereference_check((p), __UNIQUE_ID(rcu), \
+	__rcu_dereference_check(p, __UNIQUE_ID(rcu), \
 				(c) || rcu_read_lock_sched_held(), \
 				__rcu)
 
@@ -651,7 +651,7 @@ do {									      \
  * rcu_read_lock_held().
  */
 #define rcu_dereference_raw_check(p) \
-	__rcu_dereference_check((p), __UNIQUE_ID(rcu), 1, __rcu)
+	__rcu_dereference_check(p, __UNIQUE_ID(rcu), 1, __rcu)
 
 /**
  * rcu_dereference_protected() - fetch RCU pointer when updates prevented
@@ -670,7 +670,7 @@ do {									      \
  * but very ugly failures.
  */
 #define rcu_dereference_protected(p, c) \
-	__rcu_dereference_protected((p), __UNIQUE_ID(rcu), (c), __rcu)
+	__rcu_dereference_protected(p, __UNIQUE_ID(rcu), c, __rcu)
 
 
 /**
@@ -1021,7 +1021,7 @@ static inline notrace void rcu_read_unlock_sched_notrace(void)
 #define KVFREE_GET_MACRO(_1, _2, NAME, ...) NAME
 #define kvfree_rcu_arg_2(ptr, rhf)					\
 do {									\
-	typeof (ptr) ___p = (ptr);					\
+	typeof(ptr) ___p = (ptr);					\
 									\
 	if (___p) {									\
 		BUILD_BUG_ON(!__is_kvfree_rcu_offset(offsetof(typeof(*(ptr)), rhf)));	\
-- 
2.25.1


  reply	other threads:[~2023-05-04 21:02 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-04 20:05 [RFC PATCH 00/13] Fix parentheses around macro parameter use in headers Mathieu Desnoyers
2023-05-04 20:05 ` Mathieu Desnoyers [this message]
2023-05-04 20:05 ` [RFC PATCH 02/13] rculist.h: Fix parentheses around macro pointer parameter use Mathieu Desnoyers
2023-05-04 20:05 ` [RFC PATCH 03/13] rculist_nulls.h: Add " Mathieu Desnoyers
2023-05-04 20:05 ` [RFC PATCH 04/13] rculist_bl.h: Fix " Mathieu Desnoyers
2023-05-04 20:05 ` [RFC PATCH 05/13] list.h: " Mathieu Desnoyers
2023-05-08 12:16   ` Andy Shevchenko
2023-05-08 13:46     ` Mathieu Desnoyers
2023-05-12 11:02       ` Andy Shevchenko
2023-05-12 14:32         ` Mathieu Desnoyers
2023-05-04 20:05 ` [RFC PATCH 06/13] list_nulls.h: " Mathieu Desnoyers
2023-05-04 20:05 ` [RFC PATCH 07/13] list_bl.h: " Mathieu Desnoyers
2023-05-04 20:05 ` [RFC PATCH 08/13] llist.h: " Mathieu Desnoyers
2023-05-04 20:05 ` [RFC PATCH 09/13] klist.h: Fix parentheses around macro " Mathieu Desnoyers
2023-05-04 20:05 ` [RFC PATCH 10/13] resource_ext.h: Remove useless parentheses around macro parameters Mathieu Desnoyers
2023-05-04 20:05 ` [RFC PATCH 11/13] netdevice.h: Fix parentheses around macro parameter use Mathieu Desnoyers
2023-05-05 18:44   ` Jakub Kicinski
2023-05-05 18:54     ` Mathieu Desnoyers
2023-05-04 20:05 ` [RFC PATCH 12/13] blk-mq.h: " Mathieu Desnoyers
2023-05-05 13:56   ` Mathieu Desnoyers
2023-05-05 18:40     ` Linus Torvalds
2023-05-05 18:49       ` Mathieu Desnoyers
2023-05-05 19:54         ` Linus Torvalds
2023-05-05 20:08           ` Mathieu Desnoyers
2023-05-05 20:22             ` Linus Torvalds
2023-05-05 20:28               ` Mathieu Desnoyers
2023-05-08 14:28                 ` Mathieu Desnoyers
2023-05-06 15:45           ` David Laight
2023-05-04 20:05 ` [RFC PATCH 13/13] bio.h: " Mathieu Desnoyers

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=20230504200527.1935944-2-mathieu.desnoyers@efficios.com \
    --to=mathieu.desnoyers@efficios.com \
    --cc=akpm@linux-foundation.org \
    --cc=boqun.feng@gmail.com \
    --cc=jiangshanlai@gmail.com \
    --cc=joel@joelfernandes.org \
    --cc=josh@joshtriplett.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=paulmck@kernel.org \
    --cc=qiang1.zhang@intel.com \
    --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