Re: [PATCH for-next v5 10/10] Subject: [PATCH for-next v4 10/10] RDMA/rxe: Disallow MR dereg and invalidate when bound

[Zhu Yanjun <zyjzyj2000@gmail.com>]

On Fri, Apr 23, 2021 at 12:13 AM Bob Pearson <rpearsonhpe@gmail.com> wrote:
>
> Check that an MR has no bound MWs before allowing a dereg or invalidate
> operation.
>
> Signed-off-by: Bob Pearson <rpearson@hpe.com>
> ---
> v5:
>   Fix typo in v4 fix.

Thanks, Bob

After removing "Subject: [PATCH for-next v4 10/10] " in the subject
line and changing the function names to rxe_xxx in this V5,
I am fine with this series.

Please wait for the comments from Jason Gunthorpe and Leon Romanovsky.

Zhu Yanjun

>
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> v4:
>   Added this patch to check mr->num_mw to disallow
>   dereg and invalidate operations when MR has MW's
>   bound.
>
> Reported-by: Zhu Yanjun <zyjzyj2000@gmail.com>
> ---
>  drivers/infiniband/sw/rxe/rxe_loc.h   |  1 +
>  drivers/infiniband/sw/rxe/rxe_mr.c    | 25 +++++++++++++++++++++++++
>  drivers/infiniband/sw/rxe/rxe_verbs.c | 11 -----------
>  3 files changed, 26 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/infiniband/sw/rxe/rxe_loc.h b/drivers/infiniband/sw/rxe/rxe_loc.h
> index 076e1460577f..93dbd81222e8 100644
> --- a/drivers/infiniband/sw/rxe/rxe_loc.h
> +++ b/drivers/infiniband/sw/rxe/rxe_loc.h
> @@ -87,6 +87,7 @@ struct rxe_mr *lookup_mr(struct rxe_pd *pd, int access, u32 key,
>  int mr_check_range(struct rxe_mr *mr, u64 iova, size_t length);
>  int advance_dma_data(struct rxe_dma_info *dma, unsigned int length);
>  int rxe_invalidate_mr(struct rxe_qp *qp, u32 rkey);
> +int rxe_dereg_mr(struct ib_mr *ibmr, struct ib_udata *udata);
>  void rxe_mr_cleanup(struct rxe_pool_entry *arg);
>
>  /* rxe_mw.c */
> diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c
> index f871879e5f80..6a2377030f52 100644
> --- a/drivers/infiniband/sw/rxe/rxe_mr.c
> +++ b/drivers/infiniband/sw/rxe/rxe_mr.c
> @@ -546,6 +546,13 @@ int rxe_invalidate_mr(struct rxe_qp *qp, u32 rkey)
>                 goto err_drop_ref;
>         }
>
> +       if (atomic_read(&mr->num_mw) > 0) {
> +               pr_warn("%s: Attempt to invalidate an MR while bound to MWs\n",
> +                       __func__);
> +               ret = -EINVAL;
> +               goto err_drop_ref;
> +       }
> +
>         mr->state = RXE_MR_STATE_FREE;
>         ret = 0;
>
> @@ -555,6 +562,24 @@ int rxe_invalidate_mr(struct rxe_qp *qp, u32 rkey)
>         return ret;
>  }
>
> +int rxe_dereg_mr(struct ib_mr *ibmr, struct ib_udata *udata)
> +{
> +       struct rxe_mr *mr = to_rmr(ibmr);
> +
> +       if (atomic_read(&mr->num_mw) > 0) {
> +               pr_warn("%s: Attempt to deregister an MR while bound to MWs\n",
> +                       __func__);
> +               return -EINVAL;
> +       }
> +
> +       mr->state = RXE_MR_STATE_ZOMBIE;
> +       rxe_drop_ref(mr_pd(mr));
> +       rxe_drop_index(mr);
> +       rxe_drop_ref(mr);
> +
> +       return 0;
> +}
> +
>  void rxe_mr_cleanup(struct rxe_pool_entry *arg)
>  {
>         struct rxe_mr *mr = container_of(arg, typeof(*mr), pelem);
> diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c
> index d22f011a20f3..89f8f00215d6 100644
> --- a/drivers/infiniband/sw/rxe/rxe_verbs.c
> +++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
> @@ -913,17 +913,6 @@ static struct ib_mr *rxe_reg_user_mr(struct ib_pd *ibpd,
>         return ERR_PTR(err);
>  }
>
> -static int rxe_dereg_mr(struct ib_mr *ibmr, struct ib_udata *udata)
> -{
> -       struct rxe_mr *mr = to_rmr(ibmr);
> -
> -       mr->state = RXE_MR_STATE_ZOMBIE;
> -       rxe_drop_ref(mr_pd(mr));
> -       rxe_drop_index(mr);
> -       rxe_drop_ref(mr);
> -       return 0;
> -}
> -
>  static struct ib_mr *rxe_alloc_mr(struct ib_pd *ibpd, enum ib_mr_type mr_type,
>                                   u32 max_num_sg)
>  {
> --
> 2.27.0
>