Re: [PATCH v9 08/13] nvme-pci: add SGL support for the dmabuf path

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] nvme: redundant loop in dmabuf SGL chunk calculation executes only once
--

commit 5d24f863d7ba89114279230e25c2589da96688b4
Author: Anuj Gupta <anuj20.g@samsung.com>

nvme-pci: add SGL support for the dmabuf path

Add SGL support in addition to PRP for dmabuf-backed requests,
building the descriptor list from the mapping's sg_table.

> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
[ ... ]
> +static unsigned int nvme_pci_dmabuf_sgl_nents(struct request *req)
> +{
[ ... ]
> +		do {
> +			size_t chunk = min(remaining, sg_len);
> +
> +			nents++;
> +			sg_len -= chunk;
> +			remaining -= chunk;
> +		} while (sg_len && remaining);
> +	}
> +
> +	if (unlikely(remaining))

[Severity: Low]
Since chunk is the minimum of remaining and sg_len, subtracting it from both
guarantees that at least one of them will become zero on the first iteration.

Does this mean the while (sg_len && remaining) condition is always false
immediately, making the loop execute exactly once?

Is there a missing SGL segment length cap that was intended here?

[ ... ]
> +static blk_status_t nvme_rq_setup_dmabuf_sgl(struct request *req,
> +		struct nvme_queue *nvmeq, unsigned int entries)
> +{
[ ... ]
> +		do {
> +			u32 chunk = min_t(size_t, remaining, sg_len);
> +
> +			if (WARN_ON_ONCE(mapped == entries))
> +				goto err_free;
> +			nvme_pci_sgl_set_data(&sg_list[mapped++], addr, chunk);
> +
> +			addr += chunk;
> +			sg_len -= chunk;
> +			remaining -= chunk;
> +		} while (sg_len && remaining);
> +	}

[Severity: Low]
This mapping loop shares the same logic as nvme_pci_dmabuf_sgl_nents().

Since chunk will fully consume either remaining or sg_len, the loop condition
(sg_len && remaining) will evaluate to false after the first iteration.

Could this logic be simplified, or was a chunk size limit omitted?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1791336930.git.asml.silence@gmail.com?part=8