Re: [PATCH v2 04/19] 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 | <xdx3g3rcw4vc7xvu36hh3ypkazclp2v2z7kp4oc2jxgcku4baf@yw4slias6qst> |
On Thu, Jul 23, 2026 at 11:43:29AM -0500, Frank Li wrote: > On Thu, Jul 23, 2026 at 09:07:05AM +0000, [email protected] wrote: > > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > > - [High] Ring buffer capacity is permanently leaked on error and termination paths because `ll_end` is not advanced. > > -- > > > > --- Patch [4]: [PATCH v2 04/19] dmaengine: dw-edma: Make DMA link list work as a circular buffer --- > > commit 2d5b6554b58830487f70a13eab5e1ac3de304658 > > Author: Frank Li <[email protected]> > > > > dmaengine: dw-edma: Make DMA link list work as a circular buffer > > > > This commit updates the driver to use the DMA linked list as a circular > > ring buffer. It appends entries at ll_head with the current cycle bit and > > reserves the final entry to link back to the start. > > > > > diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c > > > --- a/drivers/dma/dw-edma/dw-edma-core.c > > > +++ b/drivers/dma/dw-edma/dw-edma-core.c > > > > [ ... ] > > > > > @@ -645,6 +687,8 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan) > > > DMA_TRANS_NOERROR); > > > list_del(&vd->node); > > > vchan_cookie_complete(vd); > > > + if (!chan->non_ll) > > > + chan->ll_end = chan->ll_head; > > > } > > > > > > if (chan->request == EDMA_REQ_PAUSE) { > > > > [Severity: High] > > Does this leak ring capacity when descriptors are aborted or terminated? > > > > This block successfully reclaims ring capacity on successful completion by > > advancing chan->ll_end. However, if a client driver queues transfers and > > then aborts them (for example, via dmaengine_terminate_all() or during an > > abort interrupt), the descriptors are removed from the list but chan->ll_end > > does not appear to be updated to reflect the aborted descriptors. > > > > Since chan->ll_valid remains true after a termination, subsequent transfers > > will bypass the dw_edma_core_reset_ll() ring reset logic and continue > > appending to the un-reclaimed ring. > > Make sense. Need change at these abnormal case. Indeed. This is a bisectability issue. The final tree handles it in patch 14, but patches 4 through 13 still have this issue. I'll sort it out so each intermediate patch remains valid. Best regards, Koichiro > > Frank > > > > Will this eventually cause dw_edma_core_get_free_num() to permanently return > > 0 and deadlock the channel once the un-reclaimed slots exhaust the ring? > > > > -- > > Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4