Re: [PATCH for-next 2/2] RDMA/rxe: Enable rcu locking of indexed objects

[Jason Gunthorpe <jgg@nvidia.com>]

On Tue, Jul 18, 2023 at 12:59:44PM -0500, Bob Pearson wrote:
> Make rcu_read locking of critical sections with the indexed
> verbs objects be protected from early freeing of those objects.
> The AH, QP, MR and MW objects are looked up from their indices
> contained in received packets or WQEs during I/O processing.
> Make these objects be freed using kfree_rcu to avoid races.
> 
> Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
> ---
>  drivers/infiniband/sw/rxe/rxe_pool.h  | 1 +
>  drivers/infiniband/sw/rxe/rxe_verbs.c | 6 +++++-
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_pool.h b/drivers/infiniband/sw/rxe/rxe_pool.h
> index b42e26427a70..ef2f6f88e254 100644
> --- a/drivers/infiniband/sw/rxe/rxe_pool.h
> +++ b/drivers/infiniband/sw/rxe/rxe_pool.h
> @@ -25,6 +25,7 @@ struct rxe_pool_elem {
>  	struct kref		ref_cnt;
>  	struct list_head	list;
>  	struct completion	complete;
> +	struct rcu_head		rcu;
>  	u32			index;
>  };
>  
> diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c
> index 903f0b71447e..b899924b81eb 100644
> --- a/drivers/infiniband/sw/rxe/rxe_verbs.c
> +++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
> @@ -1395,7 +1395,7 @@ static int rxe_dereg_mr(struct ib_mr *ibmr, struct ib_udata *udata)
>  	if (cleanup_err)
>  		rxe_err_mr(mr, "cleanup failed, err = %d", cleanup_err);
>  
> -	kfree_rcu_mightsleep(mr);
> +	kfree_rcu(mr, elem.rcu);
>  	return 0;
>  
>  err_out:
> @@ -1494,6 +1494,10 @@ static const struct ib_device_ops rxe_dev_ops = {
>  	INIT_RDMA_OBJ_SIZE(ib_srq, rxe_srq, ibsrq),
>  	INIT_RDMA_OBJ_SIZE(ib_ucontext, rxe_ucontext, ibuc),
>  	INIT_RDMA_OBJ_SIZE(ib_mw, rxe_mw, ibmw),
> +
> +	.rcu_offset_ah = offsetof(struct rxe_ah, elem.rcu),
> +	.rcu_offset_qp = offsetof(struct rxe_qp, elem.rcu),
> +	.rcu_offset_mw = offsetof(struct rxe_mw, elem.rcu),
>  };

This isn't quite the right calculation.. It works because the ib
struct is at the top of these structs

Do it like this:

/*
 * When used the core will RCU free the object using the driver provided
 * rcu member.
 */
#define INIT_RDMA_OBJ_SIZE_RCU(ib_struct, drv_struct, member, rcu_member)    \
	.rcu_offset_##ib_struct =                                            \
		(offsetof(struct drv_struct, rcu_member) -                   \
		 offsetof(struct drv_struct, member) +                       \
		 BUILD_BUG_ON_ZERO(offsetof(struct drv_struct, rcu_member) > \
				   offsetof(struct drv_struct, member))),    \
	INIT_RDMA_OBJ_SIZE(ib_struct, drv_struct, member),

(and maybe the prior patch could stick with the ssize_t and we could
remove the BUILD_BUG_ON_ZERO?)

Jason