Re: [PATCH] RDMA/rxe: Remove unused page_offset member

[Zhu Yanjun]

在 2025/12/28 21:49, Zhijian Li (Fujitsu) 写道:
> On 27/12/2025 13:30, Zhu Yanjun wrote:
> > 在 2025/12/26 1:41, Li Zhijian 写道:
> > > The `page_offset` member of the `rxe_mr` struct was initialized based on
> > > `ibmr.iova`, which at the initialization point hadn't been properly set.
> > >
> > > Consequently, the value assigned to `page_offset` was incorrect. However,
> >
> > Hi, Zhijian
> >
> > Why page_offset was incorrect? Can you explain it and add the explainations into commit log?
>
>
> > > The `page_offset` member of the `rxe_mr` struct was initialized based on
> > > `ibmr.iova`, which at the initialization point hadn't been properly set.
>
> The reason is stated in the line above.
>
> Are you suggesting that I should add more details about when/where
> `ibmr.iova` is correctly initialized? If so, I can clarify that it
> is assigned its value in the ib_sg_to_pages() function.

Yes, that’s exactly what I meant.

It would be helpful to explicitly document when and where ibmr.iova is 
initialized, since it’s not obvious to the reader that it is always 
valid at the point of use. Clarifying that ibmr.iova is assigned in 
ib_sg_to_pages() should address the concern and make the data flow 
easier to follow.

Thanks for adding that clarification.

Zhu Yanjun

> Thanks
> Zhijian
>
>
> > But removing page_offset seems correct.
> >
> > Thanks,
> > Reviewed-by: Zhu Yanjun <yanjun.zhu@xxxxxxxxx>
> >
> > Zhu Yanjun
> >
> > > since `page_offset` was never utilized throughout the code, it can be safely
> > > removed to clean up the codebase and avoid future confusion.
> > >
> > > Signed-off-by: Li Zhijian <lizhijian@xxxxxxxxxxx>
> > > ---
> > >    drivers/infiniband/sw/rxe/rxe_mr.c    | 1 -
> > >    drivers/infiniband/sw/rxe/rxe_odp.c   | 1 -
> > >    drivers/infiniband/sw/rxe/rxe_verbs.h | 1 -
> > >    3 files changed, 3 deletions(-)
> > >
> > > diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c
> > > index bcb97b3ea58a..b28b56db725a 100644
> > > --- a/drivers/infiniband/sw/rxe/rxe_mr.c
> > > +++ b/drivers/infiniband/sw/rxe/rxe_mr.c
> > > @@ -237,7 +237,6 @@ int rxe_map_mr_sg(struct ib_mr *ibmr, struct scatterlist *sgl,
> > >        mr->nbuf = 0;
> > >        mr->page_shift = ilog2(page_size);
> > >        mr->page_mask = ~((u64)page_size - 1);
> > > -    mr->page_offset = mr->ibmr.iova & (page_size - 1);
> > >        return ib_sg_to_pages(ibmr, sgl, sg_nents, sg_offset, rxe_set_page);
> > >    }
> > > diff --git a/drivers/infiniband/sw/rxe/rxe_odp.c b/drivers/infiniband/sw/rxe/rxe_odp.c
> > > index f58e3ec6252f..8b6a8b064d3c 100644
> > > --- a/drivers/infiniband/sw/rxe/rxe_odp.c
> > > +++ b/drivers/infiniband/sw/rxe/rxe_odp.c
> > > @@ -110,7 +110,6 @@ int rxe_odp_mr_init_user(struct rxe_dev *rxe, u64 start, u64 length,
> > >        mr->access = access_flags;
> > >        mr->ibmr.length = length;
> > >        mr->ibmr.iova = iova;
> > > -    mr->page_offset = ib_umem_offset(&umem_odp->umem);
> > >        err = rxe_odp_init_pages(mr);
> > >        if (err) {
> > > diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.h b/drivers/infiniband/sw/rxe/rxe_verbs.h
> > > index fd48075810dd..f94ce85eb807 100644
> > > --- a/drivers/infiniband/sw/rxe/rxe_verbs.h
> > > +++ b/drivers/infiniband/sw/rxe/rxe_verbs.h
> > > @@ -347,7 +347,6 @@ struct rxe_mr {
> > >        int            access;
> > >        atomic_t        num_mw;
> > > -    unsigned int        page_offset;
> > >        unsigned int        page_shift;
> > >        u64            page_mask;