[PATCH V2 0/3] misc changes for rtrs

[PATCH V2 0/3] misc changes for rtrs

[Guoqing Jiang]

Hi,

Pls review the three patches.

Thanks,
Guoqing

V2 changes:
1. fix compile warnings in the third patch
2. collect tag from Haris, thanks!

Guoqing Jiang (3):
  RDMA/rtrs: Update comments for MAX_SESS_QUEUE_DEPTH
  RDMA/rtrs-clt: Break the loop once one path is connected
  RDMA/rtrs-clt: Kill xchg_paths

 drivers/infiniband/ulp/rtrs/rtrs-clt.c | 19 ++++++-------------
 drivers/infiniband/ulp/rtrs/rtrs-pri.h |  7 +++----
 2 files changed, 9 insertions(+), 17 deletions(-)

-- 
2.31.1

[PATCH V2 1/3] RDMA/rtrs: Update comments for MAX_SESS_QUEUE_DEPTH

[Guoqing Jiang]

The maximum queue_depth should be 65535 per check_module_params,
also update other relevant comments.

Acked-by: Md Haris Iqbal <haris.iqbal@ionos.com>
Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
---
 drivers/infiniband/ulp/rtrs/rtrs-pri.h | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-pri.h b/drivers/infiniband/ulp/rtrs/rtrs-pri.h
index ac0df734eba8..a2420eecaf5a 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-pri.h
+++ b/drivers/infiniband/ulp/rtrs/rtrs-pri.h
@@ -26,11 +26,10 @@
 /*
  * Max IB immediate data size is 2^28 (MAX_IMM_PAYL_BITS)
  * and the minimum chunk size is 4096 (2^12).
- * So the maximum sess_queue_depth is 65536 (2^16) in theory.
- * But mempool_create, create_qp and ib_post_send fail with
- * "cannot allocate memory" error if sess_queue_depth is too big.
+ * So the maximum sess_queue_depth is 65535 (2^16 - 1) in theory
+ * since queue_depth in rtrs_msg_conn_rsp is defined as le16.
  * Therefore the pratical max value of sess_queue_depth is
- * somewhere between 1 and 65534 and it depends on the system.
+ * somewhere between 1 and 65535 and it depends on the system.
  */
 #define MAX_SESS_QUEUE_DEPTH 65535
 
-- 
2.31.1

[PATCH V2 2/3] RDMA/rtrs-clt: Break the loop once one path is connected

[Guoqing Jiang]

No need to iterate all paths after find one connected path.

Acked-by: Md Haris Iqbal <haris.iqbal@ionos.com>
Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
---
 drivers/infiniband/ulp/rtrs/rtrs-clt.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
index 5219bb10777a..c29eccdb4fd2 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
@@ -54,7 +54,10 @@ static inline bool rtrs_clt_is_connected(const struct rtrs_clt_sess *clt)
 
 	rcu_read_lock();
 	list_for_each_entry_rcu(clt_path, &clt->paths_list, s.entry)
-		connected |= READ_ONCE(clt_path->state) == RTRS_CLT_CONNECTED;
+		if (READ_ONCE(clt_path->state) == RTRS_CLT_CONNECTED) {
+			connected = true;
+			break;
+		}
 	rcu_read_unlock();
 
 	return connected;
-- 
2.31.1

[PATCH V2 3/3] RDMA/rtrs-clt: Kill xchg_paths

[Guoqing Jiang]

Let's call try_cmpxchg directly for the same purpose.

Acked-by: Md Haris Iqbal <haris.iqbal@ionos.com>
Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
Reported-by: kernel test robot <lkp@intel.com>
---
 drivers/infiniband/ulp/rtrs/rtrs-clt.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
index c29eccdb4fd2..2428bf5d07e3 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
@@ -2220,17 +2220,6 @@ static void rtrs_clt_stop_and_destroy_conns(struct rtrs_clt_path *clt_path)
 	}
 }
 
-static inline bool xchg_paths(struct rtrs_clt_path __rcu **rcu_ppcpu_path,
-			      struct rtrs_clt_path *clt_path,
-			      struct rtrs_clt_path *next)
-{
-	struct rtrs_clt_path **ppcpu_path;
-
-	/* Call cmpxchg() without sparse warnings */
-	ppcpu_path = (typeof(ppcpu_path))rcu_ppcpu_path;
-	return clt_path == cmpxchg(ppcpu_path, clt_path, next);
-}
-
 static void rtrs_clt_remove_path_from_arr(struct rtrs_clt_path *clt_path)
 {
 	struct rtrs_clt_sess *clt = clt_path->clt;
@@ -2305,7 +2294,8 @@ static void rtrs_clt_remove_path_from_arr(struct rtrs_clt_path *clt_path)
 		 * We race with IO code path, which also changes pointer,
 		 * thus we have to be careful not to overwrite it.
 		 */
-		if (xchg_paths(ppcpu_path, clt_path, next))
+		if (try_cmpxchg((struct rtrs_clt_path **)ppcpu_path,
+				&clt_path, next))
 			/*
 			 * @ppcpu_path was successfully replaced with @next,
 			 * that means that someone could also pick up the
-- 
2.31.1

Re: [PATCH V2 3/3] RDMA/rtrs-clt: Kill xchg_paths

[Haris Iqbal]

On Thu, Sep 8, 2022 at 11:46 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>
> Let's call try_cmpxchg directly for the same purpose.
>
> Acked-by: Md Haris Iqbal <haris.iqbal@ionos.com>
> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
> Reported-by: kernel test robot <lkp@intel.com>

I am not sure whats the correct way of using this. But technically,
this change was NOT done due to a report from the "kernel test robot".
It only pointed out the problem in the original patch. To the branch
maintainers, if its okay to keep this in this scenario, then ignore
this comment.

Thanks.

> ---
>  drivers/infiniband/ulp/rtrs/rtrs-clt.c | 14 ++------------
>  1 file changed, 2 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> index c29eccdb4fd2..2428bf5d07e3 100644
> --- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> +++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> @@ -2220,17 +2220,6 @@ static void rtrs_clt_stop_and_destroy_conns(struct rtrs_clt_path *clt_path)
>         }
>  }
>
> -static inline bool xchg_paths(struct rtrs_clt_path __rcu **rcu_ppcpu_path,
> -                             struct rtrs_clt_path *clt_path,
> -                             struct rtrs_clt_path *next)
> -{
> -       struct rtrs_clt_path **ppcpu_path;
> -
> -       /* Call cmpxchg() without sparse warnings */
> -       ppcpu_path = (typeof(ppcpu_path))rcu_ppcpu_path;
> -       return clt_path == cmpxchg(ppcpu_path, clt_path, next);
> -}
> -
>  static void rtrs_clt_remove_path_from_arr(struct rtrs_clt_path *clt_path)
>  {
>         struct rtrs_clt_sess *clt = clt_path->clt;
> @@ -2305,7 +2294,8 @@ static void rtrs_clt_remove_path_from_arr(struct rtrs_clt_path *clt_path)
>                  * We race with IO code path, which also changes pointer,
>                  * thus we have to be careful not to overwrite it.
>                  */
> -               if (xchg_paths(ppcpu_path, clt_path, next))
> +               if (try_cmpxchg((struct rtrs_clt_path **)ppcpu_path,
> +                               &clt_path, next))
>                         /*
>                          * @ppcpu_path was successfully replaced with @next,
>                          * that means that someone could also pick up the
> --
> 2.31.1
>

Re: [PATCH V2 3/3] RDMA/rtrs-clt: Kill xchg_paths

[Guoqing Jiang]

On 9/8/22 5:55 PM, Haris Iqbal wrote:
> On Thu, Sep 8, 2022 at 11:46 AM Guoqing Jiang<guoqing.jiang@linux.dev>  wrote:
>> Let's call try_cmpxchg directly for the same purpose.
>>
>> Acked-by: Md Haris Iqbal<haris.iqbal@ionos.com>
>> Signed-off-by: Guoqing Jiang<guoqing.jiang@linux.dev>
>> Reported-by: kernel test robot<lkp@intel.com>
> I am not sure whats the correct way of using this. But technically,
> this change was NOT done due to a report from the "kernel test robot".
> It only pointed out the problem in the original patch. To the branch
> maintainers, if its okay to keep this in this scenario, then ignore
> this comment.

Me either, just want to give credit to lkp for previous report, but not sure
if there is a better tag.

Thanks,
Guoqing

Re: [PATCH V2 3/3] RDMA/rtrs-clt: Kill xchg_paths

[Santosh Pradhan]

Looks good.

As clt->pcpu_path is of type "struct rtrs_clt_path __rcu
**ppcpu_path", using without typecasting with cmpxchg() would fetch
sparse warning.

I still believe we can use rcu_replace_pointer() instead of cmpxchg(),
anyway we are going to allow RCU grace period using synchronize_rcu().

Best Regards,
Santosh

On Thu, Sep 8, 2022 at 12:06 PM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>
>
>
> On 9/8/22 5:55 PM, Haris Iqbal wrote:
> > On Thu, Sep 8, 2022 at 11:46 AM Guoqing Jiang<guoqing.jiang@linux.dev>  wrote:
> >> Let's call try_cmpxchg directly for the same purpose.
> >>
> >> Acked-by: Md Haris Iqbal<haris.iqbal@ionos.com>
> >> Signed-off-by: Guoqing Jiang<guoqing.jiang@linux.dev>
> >> Reported-by: kernel test robot<lkp@intel.com>
> > I am not sure whats the correct way of using this. But technically,
> > this change was NOT done due to a report from the "kernel test robot".
> > It only pointed out the problem in the original patch. To the branch
> > maintainers, if its okay to keep this in this scenario, then ignore
> > this comment.
>
> Me either, just want to give credit to lkp for previous report, but not sure
> if there is a better tag.
>
> Thanks,
> Guoqing

Re: [PATCH V2 0/3] misc changes for rtrs

[Leon Romanovsky]

On Thu, Sep 08, 2022 at 05:45:45PM +0800, Guoqing Jiang wrote:
> Hi,
> 
> Pls review the three patches.
> 
> Thanks,
> Guoqing
> 
> V2 changes:
> 1. fix compile warnings in the third patch
> 2. collect tag from Haris, thanks!
> 
> Guoqing Jiang (3):
>   RDMA/rtrs: Update comments for MAX_SESS_QUEUE_DEPTH

This patch doesn't apply.

>   RDMA/rtrs-clt: Break the loop once one path is connected
>   RDMA/rtrs-clt: Kill xchg_paths
> 
>  drivers/infiniband/ulp/rtrs/rtrs-clt.c | 19 ++++++-------------
>  drivers/infiniband/ulp/rtrs/rtrs-pri.h |  7 +++----
>  2 files changed, 9 insertions(+), 17 deletions(-)
> 
> -- 
> 2.31.1
> 

Re: [PATCH V2 0/3] misc changes for rtrs

[Leon Romanovsky]

On Tue, Sep 20, 2022 at 03:04:41PM +0300, Leon Romanovsky wrote:
> On Thu, Sep 08, 2022 at 05:45:45PM +0800, Guoqing Jiang wrote:
> > Hi,
> > 
> > Pls review the three patches.
> > 
> > Thanks,
> > Guoqing
> > 
> > V2 changes:
> > 1. fix compile warnings in the third patch
> > 2. collect tag from Haris, thanks!
> > 
> > Guoqing Jiang (3):
> >   RDMA/rtrs: Update comments for MAX_SESS_QUEUE_DEPTH
> 
> This patch doesn't apply.

Actually, everything was already applied by me along time ago.

> 
> >   RDMA/rtrs-clt: Break the loop once one path is connected
> >   RDMA/rtrs-clt: Kill xchg_paths
> > 
> >  drivers/infiniband/ulp/rtrs/rtrs-clt.c | 19 ++++++-------------
> >  drivers/infiniband/ulp/rtrs/rtrs-pri.h |  7 +++----
> >  2 files changed, 9 insertions(+), 17 deletions(-)
> > 
> > -- 
> > 2.31.1
> >