Re: [PATCH for-next v2 2/2] RDMA/rxe: Enable asynchronous prefetch for ODP MRs

[Zhu Yanjun <yanjun.zhu@linux.dev>]

On 09.05.25 14:19, Daisuke Matsuda wrote:
> 
> 
> On 2025/05/06 0:25, Zhu Yanjun wrote:
>> On 03.05.25 15:42, Daisuke Matsuda wrote:
>>> Calling ibv_advise_mr(3) with flags other than IBV_ADVISE_MR_FLAG_FLUSH
>>> invokes asynchronous requests. It is best-effort, and thus can safely be
>>> deferred to the system-wide workqueue.
>>>
>>> Signed-off-by: Daisuke Matsuda <dskmtsd@gmail.com>
>>
>> I have made tests with rdma-core after applying this patch series. It 
>> seems that it can work well.
>> I read through this commit. Other than the following minor problems, I 
>> am fine with this commit.
>>
>> Reviewed-by: Zhu Yanjun <yanjun.zhu@linux.dev>
>>
>>> ---
>>>   drivers/infiniband/sw/rxe/rxe_odp.c | 81 ++++++++++++++++++++++++++++-
>>>   1 file changed, 80 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/infiniband/sw/rxe/rxe_odp.c 
>>> b/drivers/infiniband/sw/rxe/rxe_odp.c
>>> index e5c60b061d7e..d98b385a18ce 100644
>>> --- a/drivers/infiniband/sw/rxe/rxe_odp.c
>>> +++ b/drivers/infiniband/sw/rxe/rxe_odp.c
>>> @@ -425,6 +425,73 @@ enum resp_states rxe_odp_do_atomic_write(struct 
>>> rxe_mr *mr, u64 iova, u64 value)
>>>       return RESPST_NONE;
>>>   }
>>> +struct prefetch_mr_work {
>>> +    struct work_struct work;
>>> +    u32 pf_flags;
>>> +    u32 num_sge;
>>> +    struct {
>>> +        u64 io_virt;
>>> +        struct rxe_mr *mr;
>>> +        size_t length;
>>> +    } frags[];
>>> +};
>>
>> The struct prefetch_mr_work should be moved into header file? IMO, it 
>> is better to move this struct to rxe_loc.h?
> 
> This struct is not likely to be used in other files.

Normally a struct should be in a header file. Not mandatory.

Zhu Yanjun

> I think leaving it here would be easier for other developers to 
> understand because relevant codes are gathered.
> If there is any specific reason to move, I will do so.