Re: [PATCH for-next v2 1/4] RDMA/erdma: Support non-contiguous kernel QP buffers

[Leon Romanovsky <leon@kernel.org>]

On Thu, Aug 27, 2026 at 04:25:20PM +0800, Cheng Xu wrote:
> A single coherent allocation for kernel QP queues can fail for large
> queues when memory is fragmented.
> 
> Allocate page-sized coherent buffers and describe them with the existing
> MTT. Keep the userspace QP path unchanged.
> 
> Signed-off-by: Cheng Xu <chengyou@linux.alibaba.com>
> ---
>  drivers/infiniband/hw/erdma/erdma_cq.c    |   4 +-
>  drivers/infiniband/hw/erdma/erdma_qp.c    |  38 +++--
>  drivers/infiniband/hw/erdma/erdma_verbs.c | 195 +++++++++++++---------
>  drivers/infiniband/hw/erdma/erdma_verbs.h |  44 ++++-
>  4 files changed, 179 insertions(+), 102 deletions(-)

<...>

> +static void erdma_free_kmem(struct erdma_dev *dev, struct erdma_mem *mem)
> +{
> +	struct erdma_buf_list *buf_list;
> +	u32 i;
> +
> +	if (!mem->kmem)
> +		return;

Can it be null?

> +
> +	buf_list = mem->kmem->buf_list;
> +	if (buf_list) {

Write the unwinding section of erdma_alloc_kmem() to don't call to
erdma_free_kmem().

> +		for (i = 0; i < mem->page_cnt; i++)
> +			if (buf_list[i].buf)
> +				dma_free_coherent(&dev->pdev->dev, PAGE_SIZE,
> +						  buf_list[i].buf,
> +						  buf_list[i].dma_addr);
> +		kfree(buf_list);
> +	}
> +
> +	kfree(mem->kmem);
> +	mem->kmem = NULL;
> +}
> +
> +static int erdma_alloc_kmem(struct erdma_dev *dev, struct erdma_mem *mem,
> +			    size_t size)
> +{
> +	struct erdma_buf_list *buf_list;
> +	u32 i;
> +
> +	mem->type = ERDMA_KMEM;
> +	mem->page_size = PAGE_SIZE;
> +	mem->page_offset = 0;
> +	mem->page_cnt = DIV_ROUND_UP(size, PAGE_SIZE);
> +	mem->mtt_nents = mem->page_cnt;
> +	mem->len = size;
> +
> +	mem->kmem = kzalloc_obj(*mem->kmem);
> +	if (!mem->kmem)
> +		goto err_free_mem;

There is no point to call to erdma_free_kmem() here and you can return
immediately.

> +
> +	buf_list = kcalloc(mem->page_cnt, sizeof(*buf_list), GFP_KERNEL);
> +	if (!buf_list)
> +		goto err_free_mem;

Please write error unwinding explicitly.

> +	mem->kmem->buf_list = buf_list;
> +
> +	for (i = 0; i < mem->page_cnt; i++) {
> +		buf_list[i].buf = dma_alloc_coherent(&dev->pdev->dev, PAGE_SIZE,
> +						     &buf_list[i].dma_addr,
> +						     GFP_KERNEL);
> +		if (!buf_list[i].buf)
> +			goto err_free_mem;
> +	}
> +
> +	mem->mtt = erdma_create_mtt(dev, MTT_SIZE(mem->page_cnt), true);
> +	if (IS_ERR(mem->mtt)) {
> +		mem->mtt = NULL;

Why? You will release mem anyway.

> +		goto err_free_mem;
> +	}
> +	for (i = 0; i < mem->page_cnt; i++)
> +		mem->mtt->buf[i] = buf_list[i].dma_addr;
> +
> +	return 0;
> +
> +err_free_mem:
> +	erdma_free_kmem(dev, mem);
> +	return -ENOMEM;
> +}
> +

<...>

> +struct erdma_buf_list {
> +	void *buf;
> +	dma_addr_t dma_addr;
> +};

This struct is very similar to scatter-gather list, why don't you use it
directly?

Thanks