[PATCH for-next 0/9] Misc patches for RTRS

[PATCH for-next 0/9] Misc patches for RTRS

[Md Haris Iqbal]

Hi Jason, hi Leon,

Please consider to include following changes to the next merge window.

Jack Wang (4):
  RDMA/rtrs-srv: Do not unconditionally enable irq
  RDMA/rtrs-clt: Start hb after path_up
  RDMA/rtrs-clt: Fix the max_send_wr setting
  RDMA/rtrs-clt: Remove the warnings for req in_use check

Md Haris Iqbal (4):
  RDMA/rtrs-clt: Add warning logs for RDMA events
  RDMA/rtrs-srv: Check return values while processing info request
  RDMA/rtrs-srv: Free srv_mr iu only when always_invalidate is true
  RDMA/rtrs-srv: Destroy path files after making sure no IOs in-flight

Supriti Singh (1):
  RDMA/rtrs: use %pe to print errors

 drivers/infiniband/ulp/rtrs/rtrs-clt.c | 15 ++++++-----
 drivers/infiniband/ulp/rtrs/rtrs-srv.c | 37 +++++++++++++++++++-------
 drivers/infiniband/ulp/rtrs/rtrs.c     |  4 +--
 3 files changed, 37 insertions(+), 19 deletions(-)

-- 
2.25.1

[PATCH for-next 1/9] RDMA/rtrs-clt: Add warning logs for RDMA events

[Md Haris Iqbal]

Some RDMA CM events can trigger connection close or recovery for a
certain rtrs_clt_path. Such close/recovery triggers should happen after
an appropriate log message, since they can lead to IO failures.

Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
Signed-off-by: Grzegorz Prajsner <grzegorz.prajsner@ionos.com>
---
 drivers/infiniband/ulp/rtrs/rtrs-clt.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
index 07261523c554..984a4a1db3c8 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
@@ -2025,6 +2025,8 @@ static int rtrs_clt_rdma_cm_handler(struct rdma_cm_id *cm_id,
 		/*
 		 * Device removal is a special case.  Queue close and return 0.
 		 */
+		rtrs_wrn_rl(s, "CM event: %s, status: %d\n", rdma_event_msg(ev->event),
+			    ev->status);
 		rtrs_clt_close_conns(clt_path, false);
 		return 0;
 	default:
-- 
2.25.1

[PATCH for-next 2/9] RDMA/rtrs-srv: Do not unconditionally enable irq

[Md Haris Iqbal]

From: Jack Wang <jinpu.wang@ionos.com>

When IO is completed, rtrs can be called in softirq context,
unconditionally enabling irq could cause panic.

To be on safe side, use spin_lock_irqsave and spin_unlock_irqrestore
instread.

Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
Signed-off-by: Florian-Ewald Mueller <florian-ewald.mueller@ionos.com>
Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
Signed-off-by: Grzegorz Prajsner <grzegorz.prajsner@ionos.com>
---
 drivers/infiniband/ulp/rtrs/rtrs-srv.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
index 75e56604e462..ab4200041fd3 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
@@ -65,8 +65,9 @@ static bool rtrs_srv_change_state(struct rtrs_srv_path *srv_path,
 {
 	enum rtrs_srv_state old_state;
 	bool changed = false;
+	unsigned long flags;
 
-	spin_lock_irq(&srv_path->state_lock);
+	spin_lock_irqsave(&srv_path->state_lock, flags);
 	old_state = srv_path->state;
 	switch (new_state) {
 	case RTRS_SRV_CONNECTED:
@@ -87,7 +88,7 @@ static bool rtrs_srv_change_state(struct rtrs_srv_path *srv_path,
 	}
 	if (changed)
 		srv_path->state = new_state;
-	spin_unlock_irq(&srv_path->state_lock);
+	spin_unlock_irqrestore(&srv_path->state_lock, flags);
 
 	return changed;
 }
-- 
2.25.1

[PATCH for-next 3/9] RDMA/rtrs-clt: Start hb after path_up

[Md Haris Iqbal]

From: Jack Wang <jinpu.wang@ionos.com>

If we start hb too early, it will confuse server side to close
the session.

Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
Reviewed-by: Md Haris Iqbal <haris.iqbal@ionos.com>
Signed-off-by: Grzegorz Prajsner <grzegorz.prajsner@ionos.com>
---
 drivers/infiniband/ulp/rtrs/rtrs-clt.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
index 984a4a1db3c8..83ebd9be760e 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
@@ -2352,8 +2352,6 @@ static int init_conns(struct rtrs_clt_path *clt_path)
 	if (err)
 		goto destroy;
 
-	rtrs_start_hb(&clt_path->s);
-
 	return 0;
 
 destroy:
@@ -2627,6 +2625,7 @@ static int init_path(struct rtrs_clt_path *clt_path)
 		goto out;
 	}
 	rtrs_clt_path_up(clt_path);
+	rtrs_start_hb(&clt_path->s);
 out:
 	mutex_unlock(&clt_path->init_mutex);
 
-- 
2.25.1

[PATCH for-next 4/9] RDMA/rtrs-srv: Check return values while processing info request

[Md Haris Iqbal]

While processing info request, it could so happen that the srv_path goes
to CLOSING state, cause of any of the error events from RDMA. That state
change should be picked up while trying to change the state in
process_info_req, by checking the return value. In case the state change
call in process_info_req fails, we fail the processing.

We should also check the return value for rtrs_srv_path_up, since it
sends a link event to the client above, and the client can fail for any
reason.

Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
Signed-off-by: Grzegorz Prajsner <grzegorz.prajsner@ionos.com>
---
 drivers/infiniband/ulp/rtrs/rtrs-srv.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
index ab4200041fd3..4be0e5b132d4 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
@@ -710,20 +710,23 @@ static void rtrs_srv_info_rsp_done(struct ib_cq *cq, struct ib_wc *wc)
 	WARN_ON(wc->opcode != IB_WC_SEND);
 }
 
-static void rtrs_srv_path_up(struct rtrs_srv_path *srv_path)
+static int rtrs_srv_path_up(struct rtrs_srv_path *srv_path)
 {
 	struct rtrs_srv_sess *srv = srv_path->srv;
 	struct rtrs_srv_ctx *ctx = srv->ctx;
-	int up;
+	int up, ret = 0;
 
 	mutex_lock(&srv->paths_ev_mutex);
 	up = ++srv->paths_up;
 	if (up == 1)
-		ctx->ops.link_ev(srv, RTRS_SRV_LINK_EV_CONNECTED, NULL);
+		ret = ctx->ops.link_ev(srv, RTRS_SRV_LINK_EV_CONNECTED, NULL);
 	mutex_unlock(&srv->paths_ev_mutex);
 
 	/* Mark session as established */
-	srv_path->established = true;
+	if (!ret)
+		srv_path->established = true;
+
+	return ret;
 }
 
 static void rtrs_srv_path_down(struct rtrs_srv_path *srv_path)
@@ -852,7 +855,12 @@ static int process_info_req(struct rtrs_srv_con *con,
 		goto iu_free;
 	kobject_get(&srv_path->kobj);
 	get_device(&srv_path->srv->dev);
-	rtrs_srv_change_state(srv_path, RTRS_SRV_CONNECTED);
+	err = rtrs_srv_change_state(srv_path, RTRS_SRV_CONNECTED);
+	if (!err) {
+		rtrs_err(s, "rtrs_srv_change_state(), err: %d\n", err);
+		goto iu_free;
+	}
+
 	rtrs_srv_start_hb(srv_path);
 
 	/*
@@ -861,7 +869,11 @@ static int process_info_req(struct rtrs_srv_con *con,
 	 * all connections are successfully established.  Thus, simply notify
 	 * listener with a proper event if we are the first path.
 	 */
-	rtrs_srv_path_up(srv_path);
+	err = rtrs_srv_path_up(srv_path);
+	if (err) {
+		rtrs_err(s, "rtrs_srv_path_up(), err: %d\n", err);
+		goto iu_free;
+	}
 
 	ib_dma_sync_single_for_device(srv_path->s.dev->ib_dev,
 				      tx_iu->dma_addr,
-- 
2.25.1

[PATCH for-next 5/9] RDMA/rtrs-srv: Free srv_mr iu only when always_invalidate is true

[Md Haris Iqbal]

Since srv_mr->iu is allocated and used only when always_invalidate is
true, free it only when always_invalidate is true.

Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
Signed-off-by: Grzegorz Prajsner <grzegorz.prajsner@ionos.com>
---
 drivers/infiniband/ulp/rtrs/rtrs-srv.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
index 4be0e5b132d4..925b71481c62 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
@@ -551,7 +551,10 @@ static void unmap_cont_bufs(struct rtrs_srv_path *srv_path)
 		struct rtrs_srv_mr *srv_mr;
 
 		srv_mr = &srv_path->mrs[i];
-		rtrs_iu_free(srv_mr->iu, srv_path->s.dev->ib_dev, 1);
+
+		if (always_invalidate)
+			rtrs_iu_free(srv_mr->iu, srv_path->s.dev->ib_dev, 1);
+
 		ib_dereg_mr(srv_mr->mr);
 		ib_dma_unmap_sg(srv_path->s.dev->ib_dev, srv_mr->sgt.sgl,
 				srv_mr->sgt.nents, DMA_BIDIRECTIONAL);
-- 
2.25.1

[PATCH for-next 6/9] RDMA/rtrs-srv: Destroy path files after making sure no IOs in-flight

[Md Haris Iqbal]

Destroying path files may lead to the freeing of rdma_stats. This creates
the following race.

An IO is in-flight, or has just passed the session state check in
process_read/process_write. The close_work gets triggered and the function
rtrs_srv_close_work() starts and does destroy path which frees the
rdma_stats. After this the function process_read/process_write resumes and
tries to update the stats through the function rtrs_srv_update_rdma_stats

This commit solves the problem by moving the destroy path function to a
later point. This point makes sure any inflights are completed. This is
done by qp drain, and waiting for all in-flights through ops_id.

Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
Signed-off-by: Santosh Kumar Pradhan <santosh.pradhan@ionos.com>
Signed-off-by: Grzegorz Prajsner <grzegorz.prajsner@ionos.com>
---
 drivers/infiniband/ulp/rtrs/rtrs-srv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
index 925b71481c62..1d33efb8fb03 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
@@ -1532,7 +1532,6 @@ static void rtrs_srv_close_work(struct work_struct *work)
 
 	srv_path = container_of(work, typeof(*srv_path), close_work);
 
-	rtrs_srv_destroy_path_files(srv_path);
 	rtrs_srv_stop_hb(srv_path);
 
 	for (i = 0; i < srv_path->s.con_num; i++) {
@@ -1552,6 +1551,8 @@ static void rtrs_srv_close_work(struct work_struct *work)
 	/* Wait for all completion */
 	wait_for_completion(&srv_path->complete_done);
 
+	rtrs_srv_destroy_path_files(srv_path);
+
 	/* Notify upper layer if we are the last path */
 	rtrs_srv_path_down(srv_path);
 
-- 
2.25.1

[PATCH for-next 7/9] RDMA/rtrs-clt: Fix the max_send_wr setting

[Md Haris Iqbal]

From: Jack Wang <jinpu.wang@ionos.com>

For each write request, we need Request, Response Memory Registration,
Local Invalidate.

Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
Reviewed-by: Md Haris Iqbal <haris.iqbal@ionos.com>
Signed-off-by: Grzegorz Prajsner <grzegorz.prajsner@ionos.com>
---
 drivers/infiniband/ulp/rtrs/rtrs-clt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
index 83ebd9be760e..df10d29c3fe9 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
@@ -1699,7 +1699,7 @@ static int create_con_cq_qp(struct rtrs_clt_con *con)
 		clt_path->s.dev_ref++;
 		max_send_wr = min_t(int, wr_limit,
 			      /* QD * (REQ + RSP + FR REGS or INVS) + drain */
-			      clt_path->queue_depth * 3 + 1);
+			      clt_path->queue_depth * 4 + 1);
 		max_recv_wr = min_t(int, wr_limit,
 			      clt_path->queue_depth * 3 + 1);
 		max_send_sge = 2;
-- 
2.25.1

[PATCH for-next 8/9] RDMA/rtrs-clt: Remove the warnings for req in_use check

[Md Haris Iqbal]

From: Jack Wang <jinpu.wang@ionos.com>

As we chain the WR during write request: memory registration,
rdma write, local invalidate, if only the last WR fail to send due
to send queue overrun, the server can send back the reply, while
client mark the req->in_use to false in case of error in rtrs_clt_req
when error out from rtrs_post_rdma_write_sg.

Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
Reviewed-by: Md Haris Iqbal <haris.iqbal@ionos.com>
Signed-off-by: Grzegorz Prajsner <grzegorz.prajsner@ionos.com>
---
 drivers/infiniband/ulp/rtrs/rtrs-clt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
index df10d29c3fe9..8c5054d18402 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
@@ -384,7 +384,7 @@ static void complete_rdma_req(struct rtrs_clt_io_req *req, int errno,
 	struct rtrs_clt_path *clt_path;
 	int err;
 
-	if (WARN_ON(!req->in_use))
+	if (!req->in_use)
 		return;
 	if (WARN_ON(!req->con))
 		return;
-- 
2.25.1

[PATCH for-next 9/9] RDMA/rtrs: use %pe to print errors

[Md Haris Iqbal]

From: Supriti Singh <supriti.singh@ionos.com>

While printing error, replace %ld by %pe. %pe prints a string
whereas %ld would print an error code.

Signed-off-by: Supriti Singh <supriti.singh@ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
Signed-off-by: Grzegorz Prajsner <grzegorz.prajsner@ionos.com>
Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
---
 drivers/infiniband/ulp/rtrs/rtrs-clt.c | 6 +++---
 drivers/infiniband/ulp/rtrs/rtrs.c     | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
index 8c5054d18402..493efbba2fe3 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
@@ -1391,9 +1391,9 @@ static int alloc_path_reqs(struct rtrs_clt_path *clt_path)
 				      clt_path->max_pages_per_mr);
 		if (IS_ERR(req->mr)) {
 			err = PTR_ERR(req->mr);
+			pr_err("Failed to alloc clt_path->max_pages_per_mr %d: %pe\n",
+			       clt_path->max_pages_per_mr, req->mr);
 			req->mr = NULL;
-			pr_err("Failed to alloc clt_path->max_pages_per_mr %d\n",
-			       clt_path->max_pages_per_mr);
 			goto out;
 		}
 
@@ -2061,7 +2061,7 @@ static int create_cm(struct rtrs_clt_con *con)
 			       RDMA_PS_IB : RDMA_PS_TCP, IB_QPT_RC);
 	if (IS_ERR(cm_id)) {
 		err = PTR_ERR(cm_id);
-		rtrs_err(s, "Failed to create CM ID, err: %d\n", err);
+		rtrs_err(s, "Failed to create CM ID, err: %pe\n", cm_id);
 
 		return err;
 	}
diff --git a/drivers/infiniband/ulp/rtrs/rtrs.c b/drivers/infiniband/ulp/rtrs/rtrs.c
index d80edfffd2e4..4e17d546d4cc 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs.c
@@ -242,8 +242,8 @@ static int create_cq(struct rtrs_con *con, int cq_vector, int nr_cqe,
 		cq = ib_cq_pool_get(cm_id->device, nr_cqe, cq_vector, poll_ctx);
 
 	if (IS_ERR(cq)) {
-		rtrs_err(con->path, "Creating completion queue failed, errno: %ld\n",
-			  PTR_ERR(cq));
+		rtrs_err(con->path, "Creating completion queue failed, errno: %pe\n",
+			  cq);
 		return PTR_ERR(cq);
 	}
 	con->cq = cq;
-- 
2.25.1

Re: [PATCH for-next 0/9] Misc patches for RTRS

[Leon Romanovsky]

On Wed, Nov 15, 2023 at 04:27:40PM +0100, Md Haris Iqbal wrote:
> Hi Jason, hi Leon,
> 
> Please consider to include following changes to the next merge window.
> 
> Jack Wang (4):
>   RDMA/rtrs-srv: Do not unconditionally enable irq
>   RDMA/rtrs-clt: Start hb after path_up
>   RDMA/rtrs-clt: Fix the max_send_wr setting
>   RDMA/rtrs-clt: Remove the warnings for req in_use check
> 
> Md Haris Iqbal (4):
>   RDMA/rtrs-clt: Add warning logs for RDMA events

All these patches, except this one, fix errors and should have Fixes line.
Please add and resubmit.

I took this warning patch to -next.

Thanks

>   RDMA/rtrs-srv: Check return values while processing info request
>   RDMA/rtrs-srv: Free srv_mr iu only when always_invalidate is true
>   RDMA/rtrs-srv: Destroy path files after making sure no IOs in-flight
> 
> Supriti Singh (1):
>   RDMA/rtrs: use %pe to print errors
> 
>  drivers/infiniband/ulp/rtrs/rtrs-clt.c | 15 ++++++-----
>  drivers/infiniband/ulp/rtrs/rtrs-srv.c | 37 +++++++++++++++++++-------
>  drivers/infiniband/ulp/rtrs/rtrs.c     |  4 +--
>  3 files changed, 37 insertions(+), 19 deletions(-)
> 
> -- 
> 2.25.1
> 

Re: [PATCH for-next 0/9] Misc patches for RTRS

[Haris Iqbal]

On Sun, Nov 19, 2023 at 2:00 PM Leon Romanovsky <leon@kernel.org> wrote:
>
> On Wed, Nov 15, 2023 at 04:27:40PM +0100, Md Haris Iqbal wrote:
> > Hi Jason, hi Leon,
> >
> > Please consider to include following changes to the next merge window.
> >
> > Jack Wang (4):
> >   RDMA/rtrs-srv: Do not unconditionally enable irq
> >   RDMA/rtrs-clt: Start hb after path_up
> >   RDMA/rtrs-clt: Fix the max_send_wr setting
> >   RDMA/rtrs-clt: Remove the warnings for req in_use check
> >
> > Md Haris Iqbal (4):
> >   RDMA/rtrs-clt: Add warning logs for RDMA events
>
> All these patches, except this one, fix errors and should have Fixes line.
> Please add and resubmit.

Sure. Will do. Thanks

>
> I took this warning patch to -next.
>
> Thanks
>
> >   RDMA/rtrs-srv: Check return values while processing info request
> >   RDMA/rtrs-srv: Free srv_mr iu only when always_invalidate is true
> >   RDMA/rtrs-srv: Destroy path files after making sure no IOs in-flight
> >
> > Supriti Singh (1):
> >   RDMA/rtrs: use %pe to print errors
> >
> >  drivers/infiniband/ulp/rtrs/rtrs-clt.c | 15 ++++++-----
> >  drivers/infiniband/ulp/rtrs/rtrs-srv.c | 37 +++++++++++++++++++-------
> >  drivers/infiniband/ulp/rtrs/rtrs.c     |  4 +--
> >  3 files changed, 37 insertions(+), 19 deletions(-)
> >
> > --
> > 2.25.1
> >

Re: (subset) [PATCH for-next 0/9] Misc patches for RTRS

[Leon Romanovsky]

On Wed, 15 Nov 2023 16:27:40 +0100, Md Haris Iqbal wrote:
> Please consider to include following changes to the next merge window.
> 
> Jack Wang (4):
>   RDMA/rtrs-srv: Do not unconditionally enable irq
>   RDMA/rtrs-clt: Start hb after path_up
>   RDMA/rtrs-clt: Fix the max_send_wr setting
>   RDMA/rtrs-clt: Remove the warnings for req in_use check
> 
> [...]

Applied, thanks!

[1/9] RDMA/rtrs-clt: Add warning logs for RDMA events
      https://git.kernel.org/rdma/rdma/c/0529e26d8b7b51

Best regards,
-- 
Leon Romanovsky <leon@kernel.org>