Re: [PATCH v5 04/24] dmaengine: dw-edma: Make DMA link list work as a circular buffer
Koichiro Den <[email protected]>
| Newsgroups | org.kernel.vger.dmaengine,dev.linux.lists.sashiko-reviews |
|---|---|
| Message-ID | <a62w2vkektwuksvz4oqkozqu27knzsqdroj7xprfm47twz5cs7@rd42oqapg5z2> |
On Wed, Aug 12, 2026 at 04:14:08PM +0000, [email protected] wrote: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > - [High] The driver now outright rejects DMA descriptors that exceed the hardware linked-list (LL) ring capacity, breaking support for large scatter-gather (SG) lists. > -- > > --- Patch [4]: [PATCH v5 04/24] dmaengine: dw-edma: Make DMA link list work as a circular buffer --- > commit 4de65eea41cbb28142107d984aea6044ff700244 > Author: Frank Li <[email protected]> > > dmaengine: dw-edma: Make DMA link list work as a circular buffer > > This commit converts the DMA linked list to operate as a circular ring buffer > instead of rebuilding the entire list for every transfer. It also adds logic > to clear control words before first use and rejects descriptor requests that > exceed the usable ring capacity. > > > diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c > > index a6bb68ffdfe26..1af262b6e881e 100644 > > --- a/drivers/dma/dw-edma/dw-edma-core.c > > +++ b/drivers/dma/dw-edma/dw-edma-core.c > > @@ -51,13 +51,19 @@ dw_edma_alloc_desc(struct dw_edma_chan *chan, size_t nburst) > > { > > struct dw_edma_desc *desc; > > > > + /* > > + * For now, a descriptor that does not fit would stall the channel > > + * forever: reject it up front. > > + */ > > + if (!chan->non_ll && nburst > chan->ll_max - 1) > > + return NULL; > > [Severity: High] > Does this cause a functional regression for DMA clients submitting large > scatter-gather lists? This is intentional and only an intermediate restriction. At patch 4, the circular ring cannot reclaim entries before descriptor completion. Accepting a descriptor larger than the usable ring would therefore leave it stuck one the ring fills, so explicitly rejecting it at prep time is safer. Patch 13 adds ll_done-based reclamation and removes this check. So the final tree still supports descriptors larger than the ring. Patches 4 through 12 temporarily narrow what can be prepared. I would prefer to keep this guard so the circular-ring conversion and progress reclamation stay separate. ... However, this might become a problem if I follow Frank's suggestion to post the first 10 patches separately and the follow-up takes time to land. (see https://lore.kernel.org/r/anzgNmxSV2vf1w1X@lizhi-Precision-Tower-5810/) Frank, what do you think? AFAICT, on the integrations I tested, the LL region is 4KiB, which holds roughly 170 LLEs, and it seems unlikely for one descriptor to be that large in practice. The in-tree nvmet_pci_epf consumer, for example, submits each segment separately with dmaengine_prep_config_single_safe(). Do you think this temporary restriction is acceptable in the first series, or should I preserve existing large-descriptor support before splitting it? I think it is acceptable, but would like your view before spliting and sending the series. Best regards, Koichiro > > Previously, large transfers were handled by chunking them into batches and > restarting the channel on each completion interrupt. By transitioning to a > circular buffer and returning NULL for descriptors that exceed the usable ring > capacity outright, dmaengine_prep_slave_sg() will now fail for those clients. > > Since the commit message notes this rejection is in place "until reclaim > support lands," will this break existing users who depend on handling large SG > lists in the meantime? > > -- > Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4