[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <6478cb24-ce3b-43c7-b33d-b27de7436d89@linux.dev>
Date: Mon, 29 Dec 2025 08:11:18 -0800
From: Zhu Yanjun <yanjun.zhu@...ux.dev>
To: "Zhijian Li (Fujitsu)" <lizhijian@...itsu.com>,
"linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>
Cc: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"zyjzyj2000@...il.com" <zyjzyj2000@...il.com>, "jgg@...pe.ca"
<jgg@...pe.ca>, "leon@...nel.org" <leon@...nel.org>
Subject: Re: [PATCH] RDMA/rxe: Remove unused page_offset member
在 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@...ux.dev>
>>
>> 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@...itsu.com>
>>> ---
>>> 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;
>>
Powered by blists - more mailing lists