Re: [PATCH v4 3/7] dmaengine: dw: complete cookies before releasing the channel lock

[Frank Li <Frank.li@oss.nxp.com>]

On Thu, Oct 29, 2026 at 04:29:37PM -0600, James Hilliard wrote:
> Completion detaches a descriptor from the active list under the channel
> lock, but updates its cookie later when freeing it. Termination can take
> the lock in between and complete newer descriptors. Completing the older
> cookie afterwards moves completed_cookie backwards and makes an already
> terminated transfer appear outstanding again.
>
> Complete cookies while detaching descriptors in the completion, error
> recovery and termination paths. Keep the active-before-queued ordering
> when terminating so completed_cookie advances in submission order.
> Descriptor freeing and client notification no longer update cookies.
>
> Fixes: 3bfb1d20b547 ("dmaengine: Driver for the Synopsys DesignWare DMA controller")
> Cc: stable@vger.kernel.org
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/dma/dw/core.c | 24 +++++++++++++++---------
>  1 file changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
> index 51c5460b4670..1f7afcd01565 100644
> --- a/drivers/dma/dw/core.c
> +++ b/drivers/dma/dw/core.c
> @@ -236,18 +236,17 @@ static void dwc_dostart_first_queued(struct dw_dma_chan *dwc)
>  /*----------------------------------------------------------------------*/
>
>  static void
> -dwc_descriptor_complete(struct dw_dma_chan *dwc, struct dw_desc *desc,
> -		bool callback_required)
> +dwc_descriptor_free(struct dw_dma_chan *dwc, struct dw_desc *desc,
> +		    bool callback_required)
>  {
>  	struct dma_async_tx_descriptor	*txd = &desc->txd;
>  	struct dw_desc			*child;
>  	unsigned long			flags;
>  	struct dmaengine_desc_callback	cb;
>
> -	dev_vdbg(chan2dev(&dwc->chan), "descriptor %u complete\n", txd->cookie);
> +	dev_vdbg(chan2dev(&dwc->chan), "freeing descriptor %p\n", desc);
>
>  	spin_lock_irqsave(&dwc->lock, flags);
> -	dma_cookie_complete(txd);
>  	if (callback_required)
>  		dmaengine_desc_get_callback(txd, &cb);
>  	else
> @@ -334,12 +333,14 @@ static void dwc_scan_descriptors(struct dw_dma *dw, struct dw_dma_chan *dwc)
>
>  		/* Detach the completed transfer before starting queued work. */
>  		list_splice_init(&dwc->active_list, &list);
> +		list_for_each_entry(desc, &list, desc_node)
> +			dma_cookie_complete(&desc->txd);
>  		dwc_dostart_first_queued(dwc);
>  		spin_unlock_irqrestore(&dwc->lock, flags);
>
>  		list_for_each_entry_safe(desc, _desc, &list, desc_node) {
>  			list_del_init(&desc->desc_node);
> -			dwc_descriptor_complete(dwc, desc, true);
> +			dwc_descriptor_free(dwc, desc, true);
>  		}
>  		return;
>  	}
> @@ -441,10 +442,11 @@ static void dwc_handle_error(struct dw_dma *dw, struct dw_dma_chan *dwc)
>  	list_for_each_entry(child, &bad_desc->tx_list, desc_node)
>  		dwc_dump_lli(dwc, child);
>
> +	/* Pretend the descriptor completed successfully */
> +	dma_cookie_complete(&bad_desc->txd);
>  	spin_unlock_irqrestore(&dwc->lock, flags);
>
> -	/* Pretend the descriptor completed successfully */
> -	dwc_descriptor_complete(dwc, bad_desc, true);
> +	dwc_descriptor_free(dwc, bad_desc, true);
>  	return;
>
>  unlock:
> @@ -945,12 +947,16 @@ static int dwc_terminate_all(struct dma_chan *chan)
>  	/* active_list entries will end up before queued entries */
>  	list_splice_init(&dwc->queue, &list);
>  	list_splice_init(&dwc->active_list, &list);
> +	list_for_each_entry(desc, &list, desc_node)
> +		dma_cookie_complete(&desc->txd);
>
>  	spin_unlock_irqrestore(&dwc->lock, flags);
>
>  	/* Flush all pending and queued descriptors */
> -	list_for_each_entry_safe(desc, _desc, &list, desc_node)
> -		dwc_descriptor_complete(dwc, desc, false);
> +	list_for_each_entry_safe(desc, _desc, &list, desc_node) {
> +		list_del_init(&desc->desc_node);
> +		dwc_descriptor_free(dwc, desc, false);
> +	}
>
>  	return 0;
>  }
>
> --
> 2.53.0
>