On Sun, Nov 13, 2022 at 2:08 AM Guoqing Jiang <guoqing.jiang@xxxxxxxxx> 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@xxxxxxxxx>
Acked-by: Md Haris Iqbal <haris.iqbal@xxxxxxxxx>
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
>
