Re: [PATCH] udmabuf: respect the device's maximum segment size

[Christian König]

On 9/26/26 09:12, Karl Mehltretter wrote:
> get_sg_table() merges physically contiguous pages without accounting
> for the mapping device's maximum segment size. This affects both
> importer mappings and the udmabuf misc device mapping used for CPU
> access.
> 
> With DMA_API_DEBUG enabled, DMA_BUF_IOCTL_SYNC on a 64 MiB udmabuf
> reports:
> 
>   DMA-API: misc udmabuf: mapping sg segment longer than device claims to support [len=65884160] [max=65536]
> 
> Use sg_alloc_table_from_pages_segment() with the mapping device's
> maximum segment size. Keep a PAGE_SIZE minimum because the allocator
> warns and returns -EINVAL for smaller limits.
> 
> Before commit 5bf888673e0d ("udmabuf: Do not create malformed
> scatterlists"), each entry covered one page.
> 
> Fixes: 5bf888673e0d ("udmabuf: Do not create malformed scatterlists")
> Assisted-by: LLM
> Signed-off-by: Karl Mehltretter <kmehltretter@xxxxxxxxx>
> ---
> 
> Notes:
>     Tested on v7.3-rc4-70-gfe2ec83746e5 in QEMU (x86_64, TCG) with
>     DMA_API_DEBUG (all_errors=1) and DMABUF_DEBUG, A/B against the same
>     base:
>     
>                                             before        after
>       DMA_BUF_IOCTL_SYNC, 64 MiB udmabuf    1 report      0
>       vivid import, 4 MiB udmabuf           2 reports     0
>       vivid import, 2 MiB hugetlb udmabuf   2 reports     0
>         frames captured                     5/5           5/5
>     
>     vb2-dma-contig rejected the non-contiguous import in both runs.
> 
>  drivers/dma-buf/udmabuf.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
> index df6dd00462423..09f1eb8432f19 100644
> --- a/drivers/dma-buf/udmabuf.c
> +++ b/drivers/dma-buf/udmabuf.c
> @@ -139,9 +139,13 @@ static struct sg_table *get_sg_table(struct device *dev, struct dma_buf *buf,
>  	if (!sg)
>  		return ERR_PTR(-ENOMEM);
>  
> -	ret = sg_alloc_table_from_pages(sg, ubuf->pages, ubuf->pagecount, 0,
> -					ubuf->pagecount << PAGE_SHIFT,
> -					GFP_KERNEL);
> +	/* The SG allocator requires a segment limit of at least PAGE_SIZE. */
> +	ret = sg_alloc_table_from_pages_segment(sg, ubuf->pages, ubuf->pagecount,
> +						0, ubuf->pagecount << PAGE_SHIFT,
> +						max_t(unsigned int,
> +						      dma_get_max_seg_size(dev),
> +						      PAGE_SIZE),

Please return -EINVAL instead when dma_get_max_seg_size() returns that the segment size is smaller than a page.

In general I think that the sg_alloc_table_from_pages_segment() approach is because of the broken design of the old DMA API. Stuff like that should be handled by the iterator going over the DMA segments instead. But yeah that is not something you can fix in one patch.

So apart from the error handling the patch looks good to me.

Regards,
Christian.

> +						GFP_KERNEL);
>  	if (ret < 0)
>  		goto err_alloc;
>