Re: [PATCH for-next 04/13] RDMA/rtrs-clt: remove unnecessary dev_ref of rtrs_sess

[Jinpu Wang <jinpu.wang@cloud.ionos.com>]

On Mon, Oct 12, 2020 at 3:18 PM Jack Wang <jinpu.wang@cloud.ionos.com> wrote:
>
> From: Gioh Kim <gi-oh.kim@cloud.ionos.com>
>
> dev_ref of rtrs_sess is used for counting connections sharing
> the rtrs_ib_dev object. But rtrs_ib_dev has its own reference counter.
> We can use rtrs_ib_dev_get/put.
>
> Signed-off-by: Gioh Kim <gi-oh.kim@cloud.ionos.com>
> Signed-off-by: Jack Wang <jinpu.wang@cloud.ionos.com>

Please ignore this one, we found problem during tests, sorry.
> ---
>  drivers/infiniband/ulp/rtrs/rtrs-clt.c | 35 +++++++++++---------------
>  drivers/infiniband/ulp/rtrs/rtrs-pri.h |  1 -
>  2 files changed, 14 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> index 4677e8ed29ae..b1c0c1400c8a 100644
> --- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> +++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> @@ -1523,6 +1523,19 @@ static int create_con_cq_qp(struct rtrs_clt_con *con)
>         struct rtrs_msg_rkey_rsp *rsp;
>
>         lockdep_assert_held(&con->con_mutex);
> +       /*
> +        * The whole session uses device from user connection.
> +        * Be careful not to close user connection before ib dev
> +        * is gracefully put.
> +        */
> +       sess->s.dev = rtrs_ib_dev_find_or_add(con->c.cm_id->device,
> +                                             &dev_pd);
> +       if (!sess->s.dev) {
> +               rtrs_wrn(sess->clt,
> +                        "rtrs_ib_dev_find_get_or_add(): no memory\n");
> +               return -ENOMEM;
> +       }
> +
>         if (con->c.cid == 0) {
>                 /*
>                  * One completion for each receive and two for each send
> @@ -1531,23 +1544,7 @@ static int create_con_cq_qp(struct rtrs_clt_con *con)
>                  * in case qp gets into error state
>                  */
>                 wr_queue_size = SERVICE_CON_QUEUE_DEPTH * 3 + 2;
> -               /* We must be the first here */
> -               if (WARN_ON(sess->s.dev))
> -                       return -EINVAL;
>
> -               /*
> -                * The whole session uses device from user connection.
> -                * Be careful not to close user connection before ib dev
> -                * is gracefully put.
> -                */
> -               sess->s.dev = rtrs_ib_dev_find_or_add(con->c.cm_id->device,
> -                                                      &dev_pd);
> -               if (!sess->s.dev) {
> -                       rtrs_wrn(sess->clt,
> -                                 "rtrs_ib_dev_find_get_or_add(): no memory\n");
> -                       return -ENOMEM;
> -               }
> -               sess->s.dev_ref = 1;
>                 query_fast_reg_mode(sess);
>         } else {
>                 /*
> @@ -1560,8 +1557,6 @@ static int create_con_cq_qp(struct rtrs_clt_con *con)
>                 if (WARN_ON(!sess->queue_depth))
>                         return -EINVAL;
>
> -               /* Shared between connections */
> -               sess->s.dev_ref++;
>                 wr_queue_size =
>                         min_t(int, sess->s.dev->ib_dev->attrs.max_qp_wr,
>                               /* QD * (REQ + RSP + FR REGS or INVS) + drain */
> @@ -1604,10 +1599,8 @@ static void destroy_con_cq_qp(struct rtrs_clt_con *con)
>                 con->rsp_ius = NULL;
>                 con->queue_size = 0;
>         }
> -       if (sess->s.dev_ref && !--sess->s.dev_ref) {
> -               rtrs_ib_dev_put(sess->s.dev);
> +       if (rtrs_ib_dev_put(sess->s.dev))
>                 sess->s.dev = NULL;
> -       }
>  }
>
>  static void stop_cm(struct rtrs_clt_con *con)
> diff --git a/drivers/infiniband/ulp/rtrs/rtrs-pri.h b/drivers/infiniband/ulp/rtrs/rtrs-pri.h
> index 0a93c87ef92b..287ff78921c7 100644
> --- a/drivers/infiniband/ulp/rtrs/rtrs-pri.h
> +++ b/drivers/infiniband/ulp/rtrs/rtrs-pri.h
> @@ -103,7 +103,6 @@ struct rtrs_sess {
>         unsigned int            con_num;
>         unsigned int            recon_cnt;
>         struct rtrs_ib_dev      *dev;
> -       int                     dev_ref;
>         struct ib_cqe           *hb_cqe;
>         void                    (*hb_err_handler)(struct rtrs_con *con);
>         struct workqueue_struct *hb_wq;
> --
> 2.25.1
>