On 7/19/23 00:38, Zhu Yanjun wrote:
> On Wed, Jul 19, 2023 at 2:00 AM Bob Pearson <rpearsonhpe@xxxxxxxxx> 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@xxxxxxxxx>
>> ---
>> 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),
>
> In your commits, you mentioned that ah, qp, mw and mr will be
> protected. But here, only ah, qp and mw are set.
> Not sure if mr is also protected and set in other place.
> Except that, I am fine with it.
>
> Acked-by: Zhu Yanjun <zyjzyj2000@xxxxxxxxx>
>
> Zhu Yanjun
>
>> };
>>
>> int rxe_register_device(struct rxe_dev *rxe, const char *ibdev_name)
>> --
>> 2.39.2
>>
All of the verbs objects except MR are allocated and freed by rdma-core. MR is allocated
and freed by the drivers. So for MR the kfree_rcu call is made in rxe_dereg_mr().
I changed it from kfree_rcu_mightsleep to the kfree_rcu variant to match the three others.
Bob
