All of lore.kernel.org
 help / color / mirror / Atom feed
From: Zhu Yanjun <yanjun.zhu@linux.dev>
To: "Zhijian Li (Fujitsu)" <lizhijian@fujitsu.com>,
	"linux-rdma@vger.kernel.org" <linux-rdma@vger.kernel.org>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"zyjzyj2000@gmail.com" <zyjzyj2000@gmail.com>,
	"jgg@ziepe.ca" <jgg@ziepe.ca>,
	"leon@kernel.org" <leon@kernel.org>
Subject: Re: [PATCH] RDMA/rxe: Remove unused page_offset member
Date: Mon, 29 Dec 2025 08:11:18 -0800	[thread overview]
Message-ID: <6478cb24-ce3b-43c7-b33d-b27de7436d89@linux.dev> (raw)
In-Reply-To: <706b7917-28b4-4f54-8c5a-7d67729a62aa@fujitsu.com>

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


      reply	other threads:[~2025-12-29 16:11 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-26  9:41 [PATCH] RDMA/rxe: Remove unused page_offset member Li Zhijian
2025-12-27  5:30 ` Zhu Yanjun
2025-12-29  5:49   ` Zhijian Li (Fujitsu)
2025-12-29 16:11     ` Zhu Yanjun [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6478cb24-ce3b-43c7-b33d-b27de7436d89@linux.dev \
    --to=yanjun.zhu@linux.dev \
    --cc=jgg@ziepe.ca \
    --cc=leon@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    --cc=lizhijian@fujitsu.com \
    --cc=zyjzyj2000@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.