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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.