Re: [PATCH for-next 1/3] RDMA/erdma: Use dma_map_page to map scatter MTT buffer

[Cheng Xu <chengyou@linux.alibaba.com>]

On 7/28/25 3:08 PM, Leon Romanovsky wrote:
> On Mon, Jul 28, 2025 at 11:08:46AM +0800, Boshi Yu wrote:
>>
>>
>> On 2025/7/27 19:27, Leon Romanovsky wrote:
>>> On Fri, Jul 25, 2025 at 01:53:54PM +0800, Boshi Yu wrote:
>>>> Each high-level indirect MTT entry is assumed to point to exactly one page
>>>> of the low-level MTT buffer, but dma_map_sg may merge contiguous physical
>>>> pages when mapping. To avoid extra overhead from splitting merged regions,
>>>> use dma_map_page to map the scatter MTT buffer page by page.
>>>>
>>>> Reviewed-by: Cheng Xu <chengyou@linux.alibaba.com>
>>>> Signed-off-by: Boshi Yu <boshiyu@linux.alibaba.com>
>>>> ---
>>>>   drivers/infiniband/hw/erdma/erdma_verbs.c | 110 ++++++++++++++--------
>>>>   drivers/infiniband/hw/erdma/erdma_verbs.h |   4 +-
>>>>   2 files changed, 71 insertions(+), 43 deletions(-)
>>>
>>> <...>
>>>
>>>> +	pg_dma = vzalloc(npages * sizeof(dma_addr_t));
>>>> +	if (!pg_dma)
>>>> +		return 0;
>>>> -	sg_init_table(sglist, npages);
>>>> +	addr = buf;
>>>>   	for (i = 0; i < npages; i++) {
>>>> -		pg = vmalloc_to_page(buf);
>>>> +		pg = vmalloc_to_page(addr);
>>>
>>> <...>
>>>> +
>>>> +		pg_dma[i] = dma_map_page(&dev->pdev->dev, pg, 0, PAGE_SIZE,
>>>> +					 DMA_TO_DEVICE);
>>>
>>> Does it work?
>>
>> Hi Leon,
>>
>> I would like to confirm which part you think is not working properly. I
>> guess that you might be concerned that if the buffer is not page-aligned, it
>> could cause problems with dma_map_page.
>>
>> In fact, when allocating the MTT buffer, we ensure that it is always
>> page-aligned and that its length is a multiple of PAGE_SIZE. We have also
>> tested the new code in our production environment, and it works well.
>>
>> Look forward to your further reply if I have misunderstood your concerns.
> 
> DMA API expects Kmalloc addresses and not Vmalloc ones.
> 

Hi Leon,

Thanks for your reply. Could you provide some references for this point?
We cannot find the constraint in the Kernel Documentation.

To our best knowledge, vzalloc allocates enough pages from the page level
allocator, and vmalloc_to_page converts the buffer to 'struct page *', then
dma_map_page can accept 'struct page *' as an input parameter to generate
the DMA address.

We can find many similar uses in the kernel. 

For example, pds_vfio_dirty_seq_ack in drivers/vfio/pci/pds/dirty.c:
<...>
	for (unsigned long long i = 0; i < npages; i++) {
		struct page *page = vmalloc_to_page(bmp);

		if (!page) {
			err = -EFAULT;
			goto out_free_pages;
		}

		pages[i] = page;
		bmp += PAGE_SIZE;
	}

	err = sg_alloc_table_from_pages(&sg_table, pages, npages, page_offset,
					bmp_bytes, GFP_KERNEL);
	if (err)
		goto out_free_pages;

	err = dma_map_sgtable(pdsc_dev, &sg_table, dma_dir, 0);
	if (err)
		goto out_free_sg_table;
<...>

Another example, irdma_map_vm_page_list in drivers/infiniband/hw/irdma/utils.c:
<...>
	for (i = 0; i < pg_cnt; i++) {
		vm_page = vmalloc_to_page(addr);
		if (!vm_page)
			goto err;

		pg_dma[i] = dma_map_page(hw->device, vm_page, 0, PAGE_SIZE,
					 DMA_BIDIRECTIONAL);
		if (dma_mapping_error(hw->device, pg_dma[i]))
			goto err;

		addr += PAGE_SIZE;
	}

<...>

If we have misunderstood something, please point it out and we would appreciate it.

Thanks,
Cheng Xu

> Thanks