Received-SPF: pass (google.com: domain of linux-kernel+bounces-170060-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Mon, 6 May 2024 12:11:12 -0300
From: Jason Gunthorpe <jgg@ziepe.ca>
To: Chengchang Tang <tangchengchang@huawei.com>
Cc: Junxian Huang <huangjunxian6@hisilicon.com>, leon@kernel.org,
	linux-rdma@vger.kernel.org, linuxarm@huawei.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH for-next] RDMA/hns: Support flexible WQE buffer page size
Message-ID: <20240506151112.GE901876@ziepe.ca>
References: <20240430092845.4058786-1-huangjunxian6@hisilicon.com>
 <20240430134113.GU231144@ziepe.ca>
 <fac4927b-16ed-d801-fb47-182f2aca355c@huawei.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <fac4927b-16ed-d801-fb47-182f2aca355c@huawei.com>

On Mon, May 06, 2024 at 02:47:01PM +0800, Chengchang Tang wrote:
> 
> 
> On 2024/4/30 21:41, Jason Gunthorpe wrote:
> > On Tue, Apr 30, 2024 at 05:28:45PM +0800, Junxian Huang wrote:
> >> From: Chengchang Tang <tangchengchang@huawei.com>
> >>
> >> Currently, driver fixedly allocates 4K pages for userspace WQE buffer
> >> and results in HW reading WQE with a granularity of 4K even in a 64K
> >> system. HW has to switch pages every 4K, leading to a loss of performance.
> > 
> >> In order to improve performance, add support for userspace to allocate
> >> flexible WQE buffer page size between 4K to system PAGESIZE.
> >> @@ -90,7 +90,8 @@ struct hns_roce_ib_create_qp {
> >>  	__u8    log_sq_bb_count;
> >>  	__u8    log_sq_stride;
> >>  	__u8    sq_no_prefetch;
> >> -	__u8    reserved[5];
> >> +	__u8    pageshift;
> >> +	__u8    reserved[4];
> > 
> > It doesn't make any sense to pass in a pageshift from userspace.
> > 
> > Kernel should detect whatever underlying physical contiguity userspace
> > has been able to create and configure the hardware optimally. The umem
> > already has all the tools to do this trivially.
> > 
> > Why would you need to specify anything?
> > 
> 
> For hns roce, QPs requires three wqe buffers, namely SQ wqe buffer, RQ wqe
> buffer and EXT_SGE buffer.  Due to HW constraints, they need to be configured
> with the same page size. The memory of these three buffers is allocated by
> the user-mode driver now. The user-mode driver will calculate the size of
> each region and align them to the page size. Finally, the driver will merge
> the memories of these three regions together, apply for a memory with
> continuous virtual addresses, and send the address to the kernel-mode driver
> (during this process, the user-mode driver and the kernel-mode driver only
> exchange addresses, but not the the sizes of these three areas or other
> information).

So you get a umem and the driver is slicing it up. What is the
problem? The kernel has the umem and the kernel knows the uniform page
size of that umem.


> Since the three regions share one umem, through umem's tools, such as
> ib_umem_find_best_pgsz(), they will eventually calculate the best page size
> of the entire umem, not each region. 

That is what you want, you said? Each region has to have the same page
size. So the global page size of the umem is the correct one?

> For this reason, coupled with the fact
> that currently only the address is passed when the kernel mode driver interacts
> with the user mode driver, and no other information is passed, it makes it more
> difficult to calculate the page size used by the user mode driver from the
> kernel mode driver. 

Even if it is difficult, this has to be done like this. You can't pass
a page size in from userspace, there is no good way for userspace to
do this correctly in all cases.

It sounds like you have it right, just get the page size from the
shared umem.

Jason