diff mbox series

[RFC,04/12] RDMA/rtrs-srv: refactor the handling of failure case in map_cont_bufs

Message ID 20221113010823.6436-5-guoqing.jiang@linux.dev (mailing list archive)
State Changes Requested
Headers show
Series Misc changes for rtrs | expand

Commit Message

Guoqing Jiang Nov. 13, 2022, 1:08 a.m. UTC
Let's call unmap_cont_bufs when failure happens, and also only update
mrs_num after everything is settled which means we can remove 'mri'.

Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
---
 drivers/infiniband/ulp/rtrs/rtrs-srv.c | 47 +++++++++++---------------
 1 file changed, 20 insertions(+), 27 deletions(-)

Comments

Haris Iqbal Nov. 14, 2022, 4:21 p.m. UTC | #1
On Sun, Nov 13, 2022 at 2:08 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>
> Let's call unmap_cont_bufs when failure happens, and also only update
> mrs_num after everything is settled which means we can remove 'mri'.
>
> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
Acked-by: Md Haris Iqbal <haris.iqbal@ionos.com>

Thanks for this. It looks much better now without the weird while loop
and the gotos

> ---
>  drivers/infiniband/ulp/rtrs/rtrs-srv.c | 47 +++++++++++---------------
>  1 file changed, 20 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> index 063082d29fc6..88eae0dcf87f 100644
> --- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> +++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> @@ -561,9 +561,11 @@ static int map_cont_bufs(struct rtrs_srv_path *srv_path)
>  {
>         struct rtrs_srv_sess *srv = srv_path->srv;
>         struct rtrs_path *ss = &srv_path->s;
> -       int i, mri, err, mrs_num;
> +       int i, err, mrs_num;
>         unsigned int chunk_bits;
>         int chunks_per_mr = 1;
> +       struct ib_mr *mr;
> +       struct sg_table *sgt;
>
>         /*
>          * Here we map queue_depth chunks to MR.  Firstly we have to
> @@ -586,16 +588,14 @@ static int map_cont_bufs(struct rtrs_srv_path *srv_path)
>         if (!srv_path->mrs)
>                 return -ENOMEM;
>
> -       srv_path->mrs_num = mrs_num;
> -
> -       for (mri = 0; mri < mrs_num; mri++) {
> -               struct rtrs_srv_mr *srv_mr = &srv_path->mrs[mri];
> -               struct sg_table *sgt = &srv_mr->sgt;
> +       for (srv_path->mrs_num = 0; srv_path->mrs_num < mrs_num;
> +            srv_path->mrs_num++) {
> +               struct rtrs_srv_mr *srv_mr = &srv_path->mrs[srv_path->mrs_num];
>                 struct scatterlist *s;
> -               struct ib_mr *mr;
>                 int nr, nr_sgt, chunks;
>
> -               chunks = chunks_per_mr * mri;
> +               sgt = &srv_mr->sgt;
> +               chunks = chunks_per_mr * srv_path->mrs_num;
>                 if (!always_invalidate)
>                         chunks_per_mr = min_t(int, chunks_per_mr,
>                                               srv->queue_depth - chunks);
> @@ -644,31 +644,24 @@ static int map_cont_bufs(struct rtrs_srv_path *srv_path)
>
>                 ib_update_fast_reg_key(mr, ib_inc_rkey(mr->rkey));
>                 srv_mr->mr = mr;
> -
> -               continue;
> -err:
> -               while (mri--) {
> -                       srv_mr = &srv_path->mrs[mri];
> -                       sgt = &srv_mr->sgt;
> -                       mr = srv_mr->mr;
> -                       rtrs_iu_free(srv_mr->iu, srv_path->s.dev->ib_dev, 1);
> -dereg_mr:
> -                       ib_dereg_mr(mr);
> -unmap_sg:
> -                       ib_dma_unmap_sg(srv_path->s.dev->ib_dev, sgt->sgl,
> -                                       sgt->nents, DMA_BIDIRECTIONAL);
> -free_sg:
> -                       sg_free_table(sgt);
> -               }
> -               kfree(srv_path->mrs);
> -
> -               return err;
>         }
>
>         chunk_bits = ilog2(srv->queue_depth - 1) + 1;
>         srv_path->mem_bits = (MAX_IMM_PAYL_BITS - chunk_bits);
>
>         return 0;
> +
> +dereg_mr:
> +       ib_dereg_mr(mr);
> +unmap_sg:
> +       ib_dma_unmap_sg(srv_path->s.dev->ib_dev, sgt->sgl,
> +                       sgt->nents, DMA_BIDIRECTIONAL);
> +free_sg:
> +       sg_free_table(sgt);
> +err:
> +       unmap_cont_bufs(srv_path);
> +
> +       return err;
>  }
>
>  static void rtrs_srv_hb_err_handler(struct rtrs_con *c)
> --
> 2.31.1
>
diff mbox series

Patch

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
index 063082d29fc6..88eae0dcf87f 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
@@ -561,9 +561,11 @@  static int map_cont_bufs(struct rtrs_srv_path *srv_path)
 {
 	struct rtrs_srv_sess *srv = srv_path->srv;
 	struct rtrs_path *ss = &srv_path->s;
-	int i, mri, err, mrs_num;
+	int i, err, mrs_num;
 	unsigned int chunk_bits;
 	int chunks_per_mr = 1;
+	struct ib_mr *mr;
+	struct sg_table *sgt;
 
 	/*
 	 * Here we map queue_depth chunks to MR.  Firstly we have to
@@ -586,16 +588,14 @@  static int map_cont_bufs(struct rtrs_srv_path *srv_path)
 	if (!srv_path->mrs)
 		return -ENOMEM;
 
-	srv_path->mrs_num = mrs_num;
-
-	for (mri = 0; mri < mrs_num; mri++) {
-		struct rtrs_srv_mr *srv_mr = &srv_path->mrs[mri];
-		struct sg_table *sgt = &srv_mr->sgt;
+	for (srv_path->mrs_num = 0; srv_path->mrs_num < mrs_num;
+	     srv_path->mrs_num++) {
+		struct rtrs_srv_mr *srv_mr = &srv_path->mrs[srv_path->mrs_num];
 		struct scatterlist *s;
-		struct ib_mr *mr;
 		int nr, nr_sgt, chunks;
 
-		chunks = chunks_per_mr * mri;
+		sgt = &srv_mr->sgt;
+		chunks = chunks_per_mr * srv_path->mrs_num;
 		if (!always_invalidate)
 			chunks_per_mr = min_t(int, chunks_per_mr,
 					      srv->queue_depth - chunks);
@@ -644,31 +644,24 @@  static int map_cont_bufs(struct rtrs_srv_path *srv_path)
 
 		ib_update_fast_reg_key(mr, ib_inc_rkey(mr->rkey));
 		srv_mr->mr = mr;
-
-		continue;
-err:
-		while (mri--) {
-			srv_mr = &srv_path->mrs[mri];
-			sgt = &srv_mr->sgt;
-			mr = srv_mr->mr;
-			rtrs_iu_free(srv_mr->iu, srv_path->s.dev->ib_dev, 1);
-dereg_mr:
-			ib_dereg_mr(mr);
-unmap_sg:
-			ib_dma_unmap_sg(srv_path->s.dev->ib_dev, sgt->sgl,
-					sgt->nents, DMA_BIDIRECTIONAL);
-free_sg:
-			sg_free_table(sgt);
-		}
-		kfree(srv_path->mrs);
-
-		return err;
 	}
 
 	chunk_bits = ilog2(srv->queue_depth - 1) + 1;
 	srv_path->mem_bits = (MAX_IMM_PAYL_BITS - chunk_bits);
 
 	return 0;
+
+dereg_mr:
+	ib_dereg_mr(mr);
+unmap_sg:
+	ib_dma_unmap_sg(srv_path->s.dev->ib_dev, sgt->sgl,
+			sgt->nents, DMA_BIDIRECTIONAL);
+free_sg:
+	sg_free_table(sgt);
+err:
+	unmap_cont_bufs(srv_path);
+
+	return err;
 }
 
 static void rtrs_srv_hb_err_handler(struct rtrs_con *c)