Re: [PATCH v3 2/4] dmaengine: dw: serialize error recovery with channel termination

[Frank Li]

On Fri, Oct 23, 2026 at 10:27:23AM -0600, James Hilliard wrote:
> The tasklet samples RAW.ERROR before taking the channel lock. A
> concurrent termination can empty the active list before dwc_handle_error()
> uses that cached error to select a descriptor. The list head is then
> interpreted as a descriptor. Even with an active descriptor, moving
> queue.next unconditionally can put an empty queue's head on the active
> list and pass it to dwc_dostart().
>
> Recheck the error status under the channel lock and stop the channel
> before selecting its failed descriptor. Only one transaction is issued
> at a time, so take that descriptor directly instead of scanning for
> completions first. The scan can otherwise complete it and start queued
> work before the error handler has identified the failed transaction.
>
> Detach the failed descriptor under the same lock and use the queued-start
> helper, which leaves an empty queue alone. Clear the software-LLP flag
> before restarting so a failed multiblock transfer does not prevent the
> next transaction from starting. Keep the completion callback outside the
> lock, with the detached descriptor owned by the tasklet.
>
> Fixes: 3bfb1d20b547 ("dmaengine: Driver for the Synopsys DesignWare DMA controller")
> Fixes: f336e42f73d9 ("dw_dmac: Move single descriptor from dwc->queue to dwc->active_list in dwc_complete_all")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: James Hilliard <james.hilliard1@xxxxxxxxx>
> ---

Reviewed-by: Frank Li <Frank.Li@xxxxxxx>

>  drivers/dma/dw/core.c | 22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
> index 07a5f5105787..51c5460b4670 100644
> --- a/drivers/dma/dw/core.c
> +++ b/drivers/dma/dw/core.c
> @@ -408,10 +408,17 @@ static void dwc_handle_error(struct dw_dma *dw, struct dw_dma_chan *dwc)
>  	struct dw_desc *child;
>  	unsigned long flags;
>
> -	dwc_scan_descriptors(dw, dwc);
> -
>  	spin_lock_irqsave(&dwc->lock, flags);
>
> +	/* Termination may have cleared the error since the tasklet sampled it. */
> +	if (!(dma_readl(dw, RAW.ERROR) & dwc->mask))
> +		goto unlock;
> +
> +	dwc_chan_disable(dw, dwc);
> +	clear_bit(DW_DMA_IS_SOFT_LLP, &dwc->flags);
> +	if (list_empty(&dwc->active_list))
> +		goto unlock;
> +
>  	/*
>  	 * The descriptor currently at the head of the active list is
>  	 * borked. Since we don't have any way to report errors, we'll
> @@ -419,12 +426,7 @@ static void dwc_handle_error(struct dw_dma *dw, struct dw_dma_chan *dwc)
>  	 */
>  	bad_desc = dwc_first_active(dwc);
>  	list_del_init(&bad_desc->desc_node);
> -	list_move(dwc->queue.next, dwc->active_list.prev);
> -
> -	/* Clear the error flag and try to restart the controller */
> -	dma_writel(dw, CLEAR.ERROR, dwc->mask);
> -	if (!list_empty(&dwc->active_list))
> -		dwc_dostart(dwc, dwc_first_active(dwc));
> +	dwc_dostart_first_queued(dwc);
>
>  	/*
>  	 * WARN may seem harsh, but since this only happens
> @@ -443,6 +445,10 @@ static void dwc_handle_error(struct dw_dma *dw, struct dw_dma_chan *dwc)
>
>  	/* Pretend the descriptor completed successfully */
>  	dwc_descriptor_complete(dwc, bad_desc, true);
> +	return;
> +
> +unlock:
> +	spin_unlock_irqrestore(&dwc->lock, flags);
>  }
>
>  static void dw_dma_tasklet(struct tasklet_struct *t)
>
> --
> 2.53.0
>