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 <[email protected]> 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/[email protected]?part=8
