* [PATCH v2] rtrs-clt: silence kbuild test inconsistent intenting smatch warning
@ 2020-05-19 11:29 Danil Kipnis
2020-05-19 14:29 ` Bart Van Assche
0 siblings, 1 reply; 8+ messages in thread
From: Danil Kipnis @ 2020-05-19 11:29 UTC (permalink / raw)
To: linux-block, linux-rdma, linux-next, axboe, dledford, jgg
Cc: bvanassche, leon, danil.kipnis, jinpu.wang, kbuild test robot
Kbuild test robot reports a smatch warning:
drivers/infiniband/ulp/rtrs/rtrs-clt.c:1196 rtrs_clt_failover_req() warn: inconsistent indenting
drivers/infiniband/ulp/rtrs/rtrs-clt.c:2890 rtrs_clt_request() warn: inconsistent indenting
To get rid of the warning, move the while_each_path() macro to a newline.
Rename the macro to end_each_path() to avoid the "while should follow close
brace '}'" checkpatch error.
Fixes: 6a98d71daea1 ("RDMA/rtrs: client: main functionality")
Signed-off-by: Danil Kipnis <danil.kipnis@cloud.ionos.com>
Reported-by: kbuild test robot <lkp@intel.com>
---
v1->v2 Add fixes line
drivers/infiniband/ulp/rtrs/rtrs-clt.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
index 468fdd0d8713..0fa3a229d90e 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
@@ -734,7 +734,7 @@ struct path_it {
(it)->i < (it)->clt->paths_num; \
(it)->i++)
-#define while_each_path(it) \
+#define end_each_path(it) \
path_it_deinit(it); \
rcu_read_unlock(); \
}
@@ -1193,7 +1193,8 @@ static int rtrs_clt_failover_req(struct rtrs_clt *clt,
/* Success path */
rtrs_clt_inc_failover_cnt(alive_sess->stats);
break;
- } while_each_path(&it);
+ }
+ end_each_path(&it);
return err;
}
@@ -2887,7 +2888,8 @@ int rtrs_clt_request(int dir, struct rtrs_clt_req_ops *ops,
}
/* Success path */
break;
- } while_each_path(&it);
+ }
+ end_each_path(&it);
return err;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] rtrs-clt: silence kbuild test inconsistent intenting smatch warning
2020-05-19 11:29 [PATCH v2] rtrs-clt: silence kbuild test inconsistent intenting smatch warning Danil Kipnis
@ 2020-05-19 14:29 ` Bart Van Assche
2020-05-19 23:38 ` Jason Gunthorpe
2020-05-20 10:02 ` [PATCH v2] rtrs-clt: silence kbuild test inconsistent intenting smatch warning Danil Kipnis
0 siblings, 2 replies; 8+ messages in thread
From: Bart Van Assche @ 2020-05-19 14:29 UTC (permalink / raw)
To: Danil Kipnis, linux-block, linux-rdma, linux-next, axboe, dledford, jgg
Cc: leon, jinpu.wang, kbuild test robot
On 2020-05-19 04:29, Danil Kipnis wrote:
> Kbuild test robot reports a smatch warning:
> drivers/infiniband/ulp/rtrs/rtrs-clt.c:1196 rtrs_clt_failover_req() warn: inconsistent indenting
> drivers/infiniband/ulp/rtrs/rtrs-clt.c:2890 rtrs_clt_request() warn: inconsistent indenting
>
> To get rid of the warning, move the while_each_path() macro to a newline.
> Rename the macro to end_each_path() to avoid the "while should follow close
> brace '}'" checkpatch error.
>
> Fixes: 6a98d71daea1 ("RDMA/rtrs: client: main functionality")
>
> Signed-off-by: Danil Kipnis <danil.kipnis@cloud.ionos.com>
> Reported-by: kbuild test robot <lkp@intel.com>
> ---
> v1->v2 Add fixes line
> drivers/infiniband/ulp/rtrs/rtrs-clt.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> index 468fdd0d8713..0fa3a229d90e 100644
> --- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> +++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> @@ -734,7 +734,7 @@ struct path_it {
> (it)->i < (it)->clt->paths_num; \
> (it)->i++)
>
> -#define while_each_path(it) \
> +#define end_each_path(it) \
> path_it_deinit(it); \
> rcu_read_unlock(); \
> }
> @@ -1193,7 +1193,8 @@ static int rtrs_clt_failover_req(struct rtrs_clt *clt,
> /* Success path */
> rtrs_clt_inc_failover_cnt(alive_sess->stats);
> break;
> - } while_each_path(&it);
> + }
> + end_each_path(&it);
>
> return err;
> }
> @@ -2887,7 +2888,8 @@ int rtrs_clt_request(int dir, struct rtrs_clt_req_ops *ops,
> }
> /* Success path */
> break;
> - } while_each_path(&it);
> + }
> + end_each_path(&it);
>
> return err;
> }
I don't like the do_each_path() and end_each_path() macros because these do not
follow the pattern that is used elsewhere in the kernel to use a single macro
to iterate over a custom container. Has it been considered to combine these two
macros into a single macro, e.g. something like the following (untested) patch?
Subject: [PATCH] Combine while_each_path() and do_each_path() into
for_each_path()
---
drivers/infiniband/ulp/rtrs/rtrs-clt.c | 23 +++++++++--------------
1 file changed, 9 insertions(+), 14 deletions(-)
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
index 468fdd0d8713..8dfa56dc32bc 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
@@ -727,18 +727,13 @@ struct path_it {
struct rtrs_clt_sess *(*next_path)(struct path_it *it);
};
-#define do_each_path(path, clt, it) { \
- path_it_init(it, clt); \
- rcu_read_lock(); \
- for ((it)->i = 0; ((path) = ((it)->next_path)(it)) && \
- (it)->i < (it)->clt->paths_num; \
+#define for_each_path(path, clt, it) \
+ for (path_it_init((it), (clt)), rcu_read_lock(), (it)->i = 0; \
+ (((path) = ((it)->next_path)(it)) && \
+ (it)->i < (it)->clt->paths_num) || \
+ (path_it_deinit(it), rcu_read_unlock(), 0); \
(it)->i++)
-#define while_each_path(it) \
- path_it_deinit(it); \
- rcu_read_unlock(); \
- }
-
/**
* list_next_or_null_rr_rcu - get next list element in round-robin fashion.
* @head: the head for the list.
@@ -1177,7 +1172,7 @@ static int rtrs_clt_failover_req(struct rtrs_clt *clt,
int err = -ECONNABORTED;
struct path_it it;
- do_each_path(alive_sess, clt, &it) {
+ for_each_path(alive_sess, clt, &it) {
if (unlikely(READ_ONCE(alive_sess->state) !=
RTRS_CLT_CONNECTED))
continue;
@@ -1193,7 +1188,7 @@ static int rtrs_clt_failover_req(struct rtrs_clt *clt,
/* Success path */
rtrs_clt_inc_failover_cnt(alive_sess->stats);
break;
- } while_each_path(&it);
+ }
return err;
}
@@ -2862,7 +2857,7 @@ int rtrs_clt_request(int dir, struct rtrs_clt_req_ops *ops,
dma_dir = DMA_TO_DEVICE;
}
- do_each_path(sess, clt, &it) {
+ for_each_path(sess, clt, &it) {
if (unlikely(READ_ONCE(sess->state) != RTRS_CLT_CONNECTED))
continue;
@@ -2887,7 +2882,7 @@ int rtrs_clt_request(int dir, struct rtrs_clt_req_ops *ops,
}
/* Success path */
break;
- } while_each_path(&it);
+ }
return err;
}
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] rtrs-clt: silence kbuild test inconsistent intenting smatch warning
2020-05-19 14:29 ` Bart Van Assche
@ 2020-05-19 23:38 ` Jason Gunthorpe
2020-05-20 10:04 ` Danil Kipnis
2020-05-20 10:02 ` [PATCH v2] rtrs-clt: silence kbuild test inconsistent intenting smatch warning Danil Kipnis
1 sibling, 1 reply; 8+ messages in thread
From: Jason Gunthorpe @ 2020-05-19 23:38 UTC (permalink / raw)
To: Bart Van Assche
Cc: Danil Kipnis, linux-block, linux-rdma, linux-next, axboe,
dledford, leon, jinpu.wang, kbuild test robot
On Tue, May 19, 2020 at 07:29:15AM -0700, Bart Van Assche wrote:
> diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> index 468fdd0d8713..8dfa56dc32bc 100644
> +++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> @@ -727,18 +727,13 @@ struct path_it {
> struct rtrs_clt_sess *(*next_path)(struct path_it *it);
> };
>
> -#define do_each_path(path, clt, it) { \
> - path_it_init(it, clt); \
> - rcu_read_lock(); \
> - for ((it)->i = 0; ((path) = ((it)->next_path)(it)) && \
> - (it)->i < (it)->clt->paths_num; \
> +#define for_each_path(path, clt, it) \
> + for (path_it_init((it), (clt)), rcu_read_lock(), (it)->i = 0; \
> + (((path) = ((it)->next_path)(it)) && \
> + (it)->i < (it)->clt->paths_num) || \
> + (path_it_deinit(it), rcu_read_unlock(), 0); \
> (it)->i++)
That is nicer, even better to write it with some inlines..
Jason
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] rtrs-clt: silence kbuild test inconsistent intenting smatch warning
2020-05-19 14:29 ` Bart Van Assche
2020-05-19 23:38 ` Jason Gunthorpe
@ 2020-05-20 10:02 ` Danil Kipnis
1 sibling, 0 replies; 8+ messages in thread
From: Danil Kipnis @ 2020-05-20 10:02 UTC (permalink / raw)
To: Bart Van Assche
Cc: linux-block, linux-rdma, Linux Next Mailing List, Jens Axboe,
Doug Ledford, Jason Gunthorpe, Leon Romanovsky, Jinpu Wang,
kbuild test robot
Hi Bart,
On Tue, May 19, 2020 at 4:29 PM Bart Van Assche <bvanassche@acm.org> wrote:
>
> On 2020-05-19 04:29, Danil Kipnis wrote:
> > Kbuild test robot reports a smatch warning:
> > drivers/infiniband/ulp/rtrs/rtrs-clt.c:1196 rtrs_clt_failover_req() warn: inconsistent indenting
> > drivers/infiniband/ulp/rtrs/rtrs-clt.c:2890 rtrs_clt_request() warn: inconsistent indenting
> >
> > To get rid of the warning, move the while_each_path() macro to a newline.
> > Rename the macro to end_each_path() to avoid the "while should follow close
> > brace '}'" checkpatch error.
> >
> > Fixes: 6a98d71daea1 ("RDMA/rtrs: client: main functionality")
> >
> > Signed-off-by: Danil Kipnis <danil.kipnis@cloud.ionos.com>
> > Reported-by: kbuild test robot <lkp@intel.com>
> > ---
> > v1->v2 Add fixes line
> > drivers/infiniband/ulp/rtrs/rtrs-clt.c | 8 +++++---
> > 1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> > index 468fdd0d8713..0fa3a229d90e 100644
> > --- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> > +++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> > @@ -734,7 +734,7 @@ struct path_it {
> > (it)->i < (it)->clt->paths_num; \
> > (it)->i++)
> >
> > -#define while_each_path(it) \
> > +#define end_each_path(it) \
> > path_it_deinit(it); \
> > rcu_read_unlock(); \
> > }
> > @@ -1193,7 +1193,8 @@ static int rtrs_clt_failover_req(struct rtrs_clt *clt,
> > /* Success path */
> > rtrs_clt_inc_failover_cnt(alive_sess->stats);
> > break;
> > - } while_each_path(&it);
> > + }
> > + end_each_path(&it);
> >
> > return err;
> > }
> > @@ -2887,7 +2888,8 @@ int rtrs_clt_request(int dir, struct rtrs_clt_req_ops *ops,
> > }
> > /* Success path */
> > break;
> > - } while_each_path(&it);
> > + }
> > + end_each_path(&it);
> >
> > return err;
> > }
>
> I don't like the do_each_path() and end_each_path() macros because these do not
> follow the pattern that is used elsewhere in the kernel to use a single macro
> to iterate over a custom container. Has it been considered to combine these two
> macros into a single macro, e.g. something like the following (untested) patch?
>
>
> Subject: [PATCH] Combine while_each_path() and do_each_path() into
> for_each_path()
>
> ---
> drivers/infiniband/ulp/rtrs/rtrs-clt.c | 23 +++++++++--------------
> 1 file changed, 9 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> index 468fdd0d8713..8dfa56dc32bc 100644
> --- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> +++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> @@ -727,18 +727,13 @@ struct path_it {
> struct rtrs_clt_sess *(*next_path)(struct path_it *it);
> };
>
> -#define do_each_path(path, clt, it) { \
> - path_it_init(it, clt); \
> - rcu_read_lock(); \
> - for ((it)->i = 0; ((path) = ((it)->next_path)(it)) && \
> - (it)->i < (it)->clt->paths_num; \
> +#define for_each_path(path, clt, it) \
> + for (path_it_init((it), (clt)), rcu_read_lock(), (it)->i = 0; \
> + (((path) = ((it)->next_path)(it)) && \
> + (it)->i < (it)->clt->paths_num) || \
> + (path_it_deinit(it), rcu_read_unlock(), 0); \
> (it)->i++)
>
> -#define while_each_path(it) \
> - path_it_deinit(it); \
> - rcu_read_unlock(); \
> - }
> -
> /**
> * list_next_or_null_rr_rcu - get next list element in round-robin fashion.
> * @head: the head for the list.
> @@ -1177,7 +1172,7 @@ static int rtrs_clt_failover_req(struct rtrs_clt *clt,
> int err = -ECONNABORTED;
> struct path_it it;
>
> - do_each_path(alive_sess, clt, &it) {
> + for_each_path(alive_sess, clt, &it) {
> if (unlikely(READ_ONCE(alive_sess->state) !=
> RTRS_CLT_CONNECTED))
> continue;
> @@ -1193,7 +1188,7 @@ static int rtrs_clt_failover_req(struct rtrs_clt *clt,
> /* Success path */
> rtrs_clt_inc_failover_cnt(alive_sess->stats);
> break;
> - } while_each_path(&it);
> + }
>
> return err;
> }
> @@ -2862,7 +2857,7 @@ int rtrs_clt_request(int dir, struct rtrs_clt_req_ops *ops,
> dma_dir = DMA_TO_DEVICE;
> }
>
> - do_each_path(sess, clt, &it) {
> + for_each_path(sess, clt, &it) {
> if (unlikely(READ_ONCE(sess->state) != RTRS_CLT_CONNECTED))
> continue;
>
> @@ -2887,7 +2882,7 @@ int rtrs_clt_request(int dir, struct rtrs_clt_req_ops *ops,
> }
> /* Success path */
> break;
> - } while_each_path(&it);
> + }
>
> return err;
> }
This does look better. Will run it through tests.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] rtrs-clt: silence kbuild test inconsistent intenting smatch warning
2020-05-19 23:38 ` Jason Gunthorpe
@ 2020-05-20 10:04 ` Danil Kipnis
2020-05-20 19:11 ` Jason Gunthorpe
0 siblings, 1 reply; 8+ messages in thread
From: Danil Kipnis @ 2020-05-20 10:04 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Bart Van Assche, linux-block, linux-rdma,
Linux Next Mailing List, Jens Axboe, Doug Ledford,
Leon Romanovsky, Jinpu Wang, kbuild test robot
On Wed, May 20, 2020 at 1:38 AM Jason Gunthorpe <jgg@ziepe.ca> wrote:
>
> On Tue, May 19, 2020 at 07:29:15AM -0700, Bart Van Assche wrote:
> > diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> > index 468fdd0d8713..8dfa56dc32bc 100644
> > +++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> > @@ -727,18 +727,13 @@ struct path_it {
> > struct rtrs_clt_sess *(*next_path)(struct path_it *it);
> > };
> >
> > -#define do_each_path(path, clt, it) { \
> > - path_it_init(it, clt); \
> > - rcu_read_lock(); \
> > - for ((it)->i = 0; ((path) = ((it)->next_path)(it)) && \
> > - (it)->i < (it)->clt->paths_num; \
> > +#define for_each_path(path, clt, it) \
> > + for (path_it_init((it), (clt)), rcu_read_lock(), (it)->i = 0; \
> > + (((path) = ((it)->next_path)(it)) && \
> > + (it)->i < (it)->clt->paths_num) || \
> > + (path_it_deinit(it), rcu_read_unlock(), 0); \
> > (it)->i++)
>
> That is nicer, even better to write it with some inlines..
You mean pass a callback to an inline function that would iterate?
>
> Jason
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] rtrs-clt: silence kbuild test inconsistent intenting smatch warning
2020-05-20 10:04 ` Danil Kipnis
@ 2020-05-20 19:11 ` Jason Gunthorpe
2020-05-22 5:39 ` [PATCH] RDMA/rtrs: get rid of the do_next_path while_next_path macros Danil Kipnis
0 siblings, 1 reply; 8+ messages in thread
From: Jason Gunthorpe @ 2020-05-20 19:11 UTC (permalink / raw)
To: Danil Kipnis
Cc: Bart Van Assche, linux-block, linux-rdma,
Linux Next Mailing List, Jens Axboe, Doug Ledford,
Leon Romanovsky, Jinpu Wang, kbuild test robot
On Wed, May 20, 2020 at 12:04:28PM +0200, Danil Kipnis wrote:
> On Wed, May 20, 2020 at 1:38 AM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> >
> > On Tue, May 19, 2020 at 07:29:15AM -0700, Bart Van Assche wrote:
> > > diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> > > index 468fdd0d8713..8dfa56dc32bc 100644
> > > +++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> > > @@ -727,18 +727,13 @@ struct path_it {
> > > struct rtrs_clt_sess *(*next_path)(struct path_it *it);
> > > };
> > >
> > > -#define do_each_path(path, clt, it) { \
> > > - path_it_init(it, clt); \
> > > - rcu_read_lock(); \
> > > - for ((it)->i = 0; ((path) = ((it)->next_path)(it)) && \
> > > - (it)->i < (it)->clt->paths_num; \
> > > +#define for_each_path(path, clt, it) \
> > > + for (path_it_init((it), (clt)), rcu_read_lock(), (it)->i = 0; \
> > > + (((path) = ((it)->next_path)(it)) && \
> > > + (it)->i < (it)->clt->paths_num) || \
> > > + (path_it_deinit(it), rcu_read_unlock(), 0); \
> > > (it)->i++)
> >
> > That is nicer, even better to write it with some inlines..
>
> You mean pass a callback to an inline function that would iterate?
no, just wrap some of that logic embedded in the for statement in some
inlines, not sure
Jason
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] RDMA/rtrs: get rid of the do_next_path while_next_path macros
2020-05-20 19:11 ` Jason Gunthorpe
@ 2020-05-22 5:39 ` Danil Kipnis
2020-05-23 0:24 ` Jason Gunthorpe
0 siblings, 1 reply; 8+ messages in thread
From: Danil Kipnis @ 2020-05-22 5:39 UTC (permalink / raw)
To: linux-block, linux-rdma, linux-next, bvanassche, dledford, jgg
Cc: axboe, danil.kipnis, lkp, jinpu.wang
The macros do_each_path/while_each_path lead to a smatch warning:
drivers/infiniband/ulp/rtrs/rtrs-clt.c:1196 rtrs_clt_failover_req() warn: inconsistent indenting
drivers/infiniband/ulp/rtrs/rtrs-clt.c:2890 rtrs_clt_request() warn: inconsistent indenting
Also checkpatch complains:
ERROR: Macros with multiple statements should be enclosed in a do - while loop
The macros are used only in two places: for a normal IO path and for the
failover path triggered after errors.
Get rid of the macros and just use a for loop iterating over the list
of paths in both places. It is easier to read and also less lines of code.
Fixes: 6a98d71daea1 ("RDMA/rtrs: client: main functionality")
Reported-by: kbuild test robot <lkp@intel.com>
Signed-off-by: Danil Kipnis <danil.kipnis@cloud.ionos.com>
---
drivers/infiniband/ulp/rtrs/rtrs-clt.c | 29 ++++++++++++--------------
1 file changed, 13 insertions(+), 16 deletions(-)
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
index 468fdd0d8713..45ea5aa5f406 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
@@ -727,18 +727,6 @@ struct path_it {
struct rtrs_clt_sess *(*next_path)(struct path_it *it);
};
-#define do_each_path(path, clt, it) { \
- path_it_init(it, clt); \
- rcu_read_lock(); \
- for ((it)->i = 0; ((path) = ((it)->next_path)(it)) && \
- (it)->i < (it)->clt->paths_num; \
- (it)->i++)
-
-#define while_each_path(it) \
- path_it_deinit(it); \
- rcu_read_unlock(); \
- }
-
/**
* list_next_or_null_rr_rcu - get next list element in round-robin fashion.
* @head: the head for the list.
@@ -1177,7 +1165,10 @@ static int rtrs_clt_failover_req(struct rtrs_clt *clt,
int err = -ECONNABORTED;
struct path_it it;
- do_each_path(alive_sess, clt, &it) {
+ rcu_read_lock();
+ for (path_it_init(&it, clt);
+ (alive_sess = it.next_path(&it)) && it.i < it.clt->paths_num;
+ it.i++) {
if (unlikely(READ_ONCE(alive_sess->state) !=
RTRS_CLT_CONNECTED))
continue;
@@ -1193,7 +1184,9 @@ static int rtrs_clt_failover_req(struct rtrs_clt *clt,
/* Success path */
rtrs_clt_inc_failover_cnt(alive_sess->stats);
break;
- } while_each_path(&it);
+ }
+ path_it_deinit(&it);
+ rcu_read_unlock();
return err;
}
@@ -2862,7 +2855,9 @@ int rtrs_clt_request(int dir, struct rtrs_clt_req_ops *ops,
dma_dir = DMA_TO_DEVICE;
}
- do_each_path(sess, clt, &it) {
+ rcu_read_lock();
+ for (path_it_init(&it, clt);
+ (sess = it.next_path(&it)) && it.i < it.clt->paths_num; it.i++) {
if (unlikely(READ_ONCE(sess->state) != RTRS_CLT_CONNECTED))
continue;
@@ -2887,7 +2882,9 @@ int rtrs_clt_request(int dir, struct rtrs_clt_req_ops *ops,
}
/* Success path */
break;
- } while_each_path(&it);
+ }
+ path_it_deinit(&it);
+ rcu_read_unlock();
return err;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] RDMA/rtrs: get rid of the do_next_path while_next_path macros
2020-05-22 5:39 ` [PATCH] RDMA/rtrs: get rid of the do_next_path while_next_path macros Danil Kipnis
@ 2020-05-23 0:24 ` Jason Gunthorpe
0 siblings, 0 replies; 8+ messages in thread
From: Jason Gunthorpe @ 2020-05-23 0:24 UTC (permalink / raw)
To: Danil Kipnis
Cc: linux-block, linux-rdma, linux-next, bvanassche, dledford, axboe,
lkp, jinpu.wang
On Fri, May 22, 2020 at 07:39:24AM +0200, Danil Kipnis wrote:
> The macros do_each_path/while_each_path lead to a smatch warning:
> drivers/infiniband/ulp/rtrs/rtrs-clt.c:1196 rtrs_clt_failover_req() warn: inconsistent indenting
> drivers/infiniband/ulp/rtrs/rtrs-clt.c:2890 rtrs_clt_request() warn: inconsistent indenting
>
> Also checkpatch complains:
> ERROR: Macros with multiple statements should be enclosed in a do - while loop
>
> The macros are used only in two places: for a normal IO path and for the
> failover path triggered after errors.
>
> Get rid of the macros and just use a for loop iterating over the list
> of paths in both places. It is easier to read and also less lines of code.
>
> Fixes: 6a98d71daea1 ("RDMA/rtrs: client: main functionality")
> Reported-by: kbuild test robot <lkp@intel.com>
> Signed-off-by: Danil Kipnis <danil.kipnis@cloud.ionos.com>
> ---
> drivers/infiniband/ulp/rtrs/rtrs-clt.c | 29 ++++++++++++--------------
> 1 file changed, 13 insertions(+), 16 deletions(-)
Applied to for-next, thanks
Jason
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-05-23 0:25 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-19 11:29 [PATCH v2] rtrs-clt: silence kbuild test inconsistent intenting smatch warning Danil Kipnis
2020-05-19 14:29 ` Bart Van Assche
2020-05-19 23:38 ` Jason Gunthorpe
2020-05-20 10:04 ` Danil Kipnis
2020-05-20 19:11 ` Jason Gunthorpe
2020-05-22 5:39 ` [PATCH] RDMA/rtrs: get rid of the do_next_path while_next_path macros Danil Kipnis
2020-05-23 0:24 ` Jason Gunthorpe
2020-05-20 10:02 ` [PATCH v2] rtrs-clt: silence kbuild test inconsistent intenting smatch warning Danil Kipnis
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.