[v1] RDMA/rxe: Remove unused page_offset member

[PATCH] RDMA/rxe: Remove unused page_offset member

Posted by Li Zhijian 1 month, 1 week ago

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,
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@fujitsu.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;
 
-- 
2.41.0

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

Posted by Zhu Yanjun 1 month, 1 week ago

在 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?

But removing page_offset seems correct.

Thanks,
Reviewed-by: Zhu Yanjun <yanjun.zhu@linux.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@fujitsu.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;
>

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

Posted by Zhijian Li (Fujitsu) 1 month, 1 week ago


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.


Thanks
Zhijian


> 
> But removing page_offset seems correct.
> 
> Thanks,
> Reviewed-by: Zhu Yanjun <yanjun.zhu@linux.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@fujitsu.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;
>

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

Posted by Zhu Yanjun 1 month, 1 week ago

在 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@linux.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@fujitsu.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;
>>