diff mbox series

[for-next,2/5] RDMA/rtrs-clt: fix CHECK type warnings

Message ID 20220114154753.983568-3-haris.iqbal@ionos.com (mailing list archive)
State Changes Requested
Delegated to: Jason Gunthorpe
Headers show
Series Misc update for RTRS | expand

Commit Message

Md Haris Iqbal Jan. 14, 2022, 3:47 p.m. UTC 
From: Gioh Kim <gi-oh.kim@ionos.com>

First this patch removes list_next_or_null_rr_rcu macro to
fix below warnings. That macro is used only twice.
CHECK:MACRO_ARG_REUSE: Macro argument reuse 'head' - possible side-effects?
CHECK:MACRO_ARG_REUSE: Macro argument reuse 'ptr' - possible side-effects?
CHECK:MACRO_ARG_REUSE: Macro argument reuse 'memb' - possible side-effects?

This patch also fixes below warning:
CHECK:SPACING: No space is necessary after a cast

Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
---
 drivers/infiniband/ulp/rtrs/rtrs-clt.c | 49 ++++++++++++--------------
 1 file changed, 22 insertions(+), 27 deletions(-)

Comments

Jason Gunthorpe Jan. 28, 2022, 2:55 p.m. UTC | #1 
On Fri, Jan 14, 2022 at 04:47:50PM +0100, Md Haris Iqbal wrote:

> -/**
> - * list_next_or_null_rr_rcu - get next list element in round-robin fashion.
> - * @head:	the head for the list.
> - * @ptr:        the list head to take the next element from.
> - * @type:       the type of the struct this is embedded in.
> - * @memb:       the name of the list_head within the struct.
> - *
> - * Next element returned in round-robin fashion, i.e. head will be skipped,
> - * but if list is observed as empty, NULL will be returned.
> - *
> - * This primitive may safely run concurrently with the _rcu list-mutation
> - * primitives such as list_add_rcu() as long as it's guarded by rcu_read_lock().
> - */
> -#define list_next_or_null_rr_rcu(head, ptr, type, memb) \
> -({ \
> -	list_next_or_null_rcu(head, ptr, type, memb) ?: \
> -		list_next_or_null_rcu(head, READ_ONCE((ptr)->next), \
> -				      type, memb); \
> -})

Why not put this in a static inline instead of open coding it? Type is
always the same for both usages, right?

Jason
Md Haris Iqbal Feb. 7, 2022, 4:07 p.m. UTC | #2 
On Fri, Jan 28, 2022 at 3:55 PM Jason Gunthorpe <jgg@nvidia.com> wrote:
>
> On Fri, Jan 14, 2022 at 04:47:50PM +0100, Md Haris Iqbal wrote:
>
> > -/**
> > - * list_next_or_null_rr_rcu - get next list element in round-robin fashion.
> > - * @head:    the head for the list.
> > - * @ptr:        the list head to take the next element from.
> > - * @type:       the type of the struct this is embedded in.
> > - * @memb:       the name of the list_head within the struct.
> > - *
> > - * Next element returned in round-robin fashion, i.e. head will be skipped,
> > - * but if list is observed as empty, NULL will be returned.
> > - *
> > - * This primitive may safely run concurrently with the _rcu list-mutation
> > - * primitives such as list_add_rcu() as long as it's guarded by rcu_read_lock().
> > - */
> > -#define list_next_or_null_rr_rcu(head, ptr, type, memb) \
> > -({ \
> > -     list_next_or_null_rcu(head, ptr, type, memb) ?: \
> > -             list_next_or_null_rcu(head, READ_ONCE((ptr)->next), \
> > -                                   type, memb); \
> > -})
>
> Why not put this in a static inline instead of open coding it? Type is
> always the same for both usages, right?

Yes. Makes sense. I will send this change along with the next patchset.

Thanks for the review and comment.

>
> Jason
diff mbox series

Patch

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
index 7c3f98e57889..62ba0f17ac9d 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
@@ -748,26 +748,6 @@  struct path_it {
 	struct rtrs_clt_path *(*next_path)(struct path_it *it);
 };
 
-/**
- * list_next_or_null_rr_rcu - get next list element in round-robin fashion.
- * @head:	the head for the list.
- * @ptr:        the list head to take the next element from.
- * @type:       the type of the struct this is embedded in.
- * @memb:       the name of the list_head within the struct.
- *
- * Next element returned in round-robin fashion, i.e. head will be skipped,
- * but if list is observed as empty, NULL will be returned.
- *
- * This primitive may safely run concurrently with the _rcu list-mutation
- * primitives such as list_add_rcu() as long as it's guarded by rcu_read_lock().
- */
-#define list_next_or_null_rr_rcu(head, ptr, type, memb) \
-({ \
-	list_next_or_null_rcu(head, ptr, type, memb) ?: \
-		list_next_or_null_rcu(head, READ_ONCE((ptr)->next), \
-				      type, memb); \
-})
-
 /**
  * get_next_path_rr() - Returns path in round-robin fashion.
  * @it:	the path pointer
@@ -797,10 +777,20 @@  static struct rtrs_clt_path *get_next_path_rr(struct path_it *it)
 		path = list_first_or_null_rcu(&clt->paths_list,
 					      typeof(*path), s.entry);
 	else
-		path = list_next_or_null_rr_rcu(&clt->paths_list,
-						&path->s.entry,
-						typeof(*path),
-						s.entry);
+		/*
+		 * Next element returned in round-robin fashion, i.e. head will be skipped,
+		 * but if list is observed as empty, NULL will be returned.
+		 *
+		 * This primitive may safely run concurrently with the _rcu list-mutation
+		 * primitives such as list_add_rcu() as long as it's guarded by rcu_read_lock().
+		 */
+		path = list_next_or_null_rcu(&clt->paths_list,
+					     &path->s.entry,
+					     typeof(*path),
+					     s.entry) ?:
+			list_next_or_null_rcu(&clt->paths_list,
+					      READ_ONCE((&path->s.entry)->next),
+					      typeof(*path), s.entry);
 	rcu_assign_pointer(*ppcpu_path, path);
 
 	return path;
@@ -1863,7 +1853,7 @@  static int rtrs_rdma_conn_established(struct rtrs_clt_con *con,
 		}
 		clt_path->queue_depth = queue_depth;
 		clt_path->s.signal_interval = min_not_zero(queue_depth,
-						(unsigned short) SERVICE_CON_QUEUE_DEPTH);
+						(unsigned short)SERVICE_CON_QUEUE_DEPTH);
 		clt_path->max_hdr_size = le32_to_cpu(msg->max_hdr_size);
 		clt_path->max_io_size = le32_to_cpu(msg->max_io_size);
 		clt_path->flags = le32_to_cpu(msg->flags);
@@ -2268,8 +2258,13 @@  static void rtrs_clt_remove_path_from_arr(struct rtrs_clt_path *clt_path)
 	 * removed.  If @sess is the last element, then @next is NULL.
 	 */
 	rcu_read_lock();
-	next = list_next_or_null_rr_rcu(&clt->paths_list, &clt_path->s.entry,
-					typeof(*next), s.entry);
+	next = list_next_or_null_rcu(&clt->paths_list,
+				     &clt_path->s.entry,
+				     typeof(*next),
+				     s.entry) ?:
+		list_next_or_null_rcu(&clt->paths_list,
+				      READ_ONCE((&clt_path->s.entry)->next),
+				      typeof(*next), s.entry);
 	rcu_read_unlock();
 
 	/*