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

[Zhu Yanjun]

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@xxxxxxxxx>
> >
> > 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@xxxxxxxxx>
> >
> > > ---
> > >   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.