Re: [PATCH 04/17] dmaengine: dw-edma: Clean up vchan descriptors on termination

[Frank Li]

On Tue, Jun 16, 2026 at 12:40:58AM +0900, Koichiro Den wrote:
> dw-edma resets channel state from terminate_all() paths, but pending
> virt-dma descriptors can remain on the submitted and issued lists. A
> later issue_pending() may then restart work that the client already
> terminated, possibly into buffers that were already reused. Descriptors
> that are never restarted leak instead.
>
> Move issued and submitted descriptors to the terminated list whenever a
> termination request completes. Also release virt-dma resources from
> free_chan_resources().
>
> If termination was deferred because the channel was still running, wait
> until the STOP path deconfigures the channel before synchronizing or
> freeing virt-dma resources. Otherwise dmaengine_terminate_sync() can
> return before the deferred STOP cleanup has moved issued descriptors to
> the terminated list and before the channel is known to have stopped.
>
> The old free_chan_resources() loop usually broke as soon as
> terminate_all() returned zero, so it did not effectively spin until the
> timeout. This wait can now last until the existing timeout, so use
> cond_resched() instead of busy-polling with cpu_relax(), and warn if the
> timeout expires.
>
> Fixes: e63d79d1ffcd ("dmaengine: Add Synopsys eDMA IP core driver")
> Signed-off-by: Koichiro Den <den@xxxxxxxxxxxxx>
> ---
>  drivers/dma/dw-edma/dw-edma-core.c | 78 ++++++++++++++++++++++++------
>  1 file changed, 64 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index bedaee6d30ab..2777dc0b2aed 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -15,6 +15,7 @@
>  #include <linux/irq.h>
>  #include <linux/dma/edma.h>
>  #include <linux/dma-mapping.h>
> +#include <linux/sched.h>
>  #include <linux/string_choices.h>
>
>  #include "dw-edma-core.h"
> @@ -113,6 +114,28 @@ static void dw_edma_terminate_vdesc(struct virt_dma_desc *vd)
>  	vchan_terminate_vdesc(vd);
>  }
>
> +static void dw_edma_terminate_vdesc_list(struct list_head *head)
> +{
> +	struct virt_dma_desc *vd, *_vd;
> +
> +	list_for_each_entry_safe(vd, _vd, head, node)
> +		dw_edma_terminate_vdesc(vd);
> +}
> +
> +/* Must be called with vc.lock held. */
> +static void dw_edma_terminate_all_descs(struct dw_edma_chan *chan)
> +{
> +	/*
> +	 * This order must not be reversed. Cookies are assigned when
> +	 * descriptors are submitted, so desc_issued contains older cookies
> +	 * than desc_submitted. Completing desc_submitted first could move
> +	 * chan->vc.chan.completed_cookie backwards when desc_issued is
> +	 * terminated afterwards.
> +	 */
> +	dw_edma_terminate_vdesc_list(&chan->vc.desc_issued);
> +	dw_edma_terminate_vdesc_list(&chan->vc.desc_submitted);
> +}
> +

Is it possible move every thing to temp termniate queue by hold lock, then
call dw_edma_terminate_vdesc(vd) outside lock.

Frank
>  static void dw_edma_device_caps(struct dma_chan *dchan,
>  				struct dma_slave_caps *caps)
>  {
> @@ -190,20 +213,25 @@ static int dw_edma_device_resume(struct dma_chan *dchan)
>  static int dw_edma_device_terminate_all(struct dma_chan *dchan)
>  {
>  	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
> +	unsigned long flags;
>  	int err = 0;
>
> +	spin_lock_irqsave(&chan->vc.lock, flags);
>  	if (!chan->configured) {
> -		/* Do nothing */
> +		dw_edma_terminate_all_descs(chan);
>  	} else if (chan->status == EDMA_ST_PAUSE) {
> +		dw_edma_terminate_all_descs(chan);
>  		chan->status = EDMA_ST_IDLE;
>  		chan->configured = false;
>  	} else if (chan->status == EDMA_ST_IDLE) {
> +		dw_edma_terminate_all_descs(chan);
>  		chan->configured = false;
>  	} else if (dw_edma_core_ch_status(chan) == DMA_COMPLETE) {
>  		/*
>  		 * The channel is in a false BUSY state, probably didn't
>  		 * receive or lost an interrupt
>  		 */
> +		dw_edma_terminate_all_descs(chan);
>  		chan->status = EDMA_ST_IDLE;
>  		chan->configured = false;
>  	} else if (chan->request > EDMA_REQ_PAUSE) {
> @@ -211,6 +239,7 @@ static int dw_edma_device_terminate_all(struct dma_chan *dchan)
>  	} else {
>  		chan->request = EDMA_REQ_STOP;
>  	}
> +	spin_unlock_irqrestore(&chan->vc.lock, flags);
>
>  	return err;
>  }
> @@ -544,7 +573,7 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
>  			break;
>
>  		case EDMA_REQ_STOP:
> -			dw_edma_terminate_vdesc(vd);
> +			dw_edma_terminate_all_descs(chan);
>  			chan->request = EDMA_REQ_NONE;
>  			chan->status = EDMA_ST_IDLE;
>  			break;
> @@ -616,28 +645,49 @@ static int dw_edma_alloc_chan_resources(struct dma_chan *dchan)
>  	return 0;
>  }
>
> +static void dw_edma_wait_termination(struct dma_chan *dchan)
> +{
> +	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
> +	unsigned long timeout = jiffies + msecs_to_jiffies(5000);
> +	unsigned long flags;
> +	bool configured = true;
> +
> +	/*
> +	 * dw_edma_device_terminate_all() may defer cleanup to a later interrupt
> +	 * while the channel is still running. Retry until the channel is
> +	 * deconfigured, which marks that termination completed.
> +	 */
> +	while (time_before(jiffies, timeout)) {
> +		dw_edma_device_terminate_all(dchan);
> +
> +		spin_lock_irqsave(&chan->vc.lock, flags);
> +		configured = chan->configured;
> +		spin_unlock_irqrestore(&chan->vc.lock, flags);
> +		if (!configured)
> +			return;
> +
> +		cond_resched();
> +	}
> +
> +	dev_warn(chan->dw->chip->dev,
> +		 "timeout waiting for channel termination\n");
> +}
> +
>  static void dw_edma_device_synchronize(struct dma_chan *dchan)
>  {
>  	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
>
> +	dw_edma_wait_termination(dchan);
>  	vchan_synchronize(&chan->vc);
>  }
>
>  static void dw_edma_free_chan_resources(struct dma_chan *dchan)
>  {
> -	unsigned long timeout = jiffies + msecs_to_jiffies(5000);
> -	int ret;
> -
> -	while (time_before(jiffies, timeout)) {
> -		ret = dw_edma_device_terminate_all(dchan);
> -		if (!ret)
> -			break;
> -
> -		if (time_after_eq(jiffies, timeout))
> -			return;
> +	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
>
> -		cpu_relax();
> -	}
> +	dw_edma_wait_termination(dchan);
> +	vchan_synchronize(&chan->vc);
> +	vchan_free_chan_resources(&chan->vc);
>  }
>
>  static int dw_edma_channel_setup(struct dw_edma *dw, u32 wr_alloc, u32 rd_alloc)
> --
> 2.51.0
>