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
2021-04-26T09:20:11Z

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
>