Re: [PATCH v3 12/24] dmaengine: dw-edma: Keep channels stopped while ABORT is pending

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Skipping coalesced DONE and LL events during an ABORT permanently strands successfully completed or pending fully-published descriptors in the software queue, causing resource leaks, hangs, and silent data corruption.
--

--- Patch [12]: [PATCH v3 12/24] dmaengine: dw-edma: Keep channels stopped while ABORT is pending ---
commit 2cbef110fae6e22ffd20e7452128a0fd69fc4545
Author: Koichiro Den <[email protected]>

dmaengine: dw-edma: Keep channels stopped while ABORT is pending

This commit delays deferred handling of ABORT interrupts until the worker runs.
It introduces a pending flag to block channel restarts and other progress
consumers until the ABORT cleanup finishes.

> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 3970b066dca22..5bf08ff0fe464 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c

[ ... ]

> @@ -880,12 +941,14 @@ static void dw_edma_irq_work(struct work_struct *work)
>  	do {
>  		events = atomic_xchg(&chan->irq_pending, 0);
>  
> +		/* ABORT cleanup makes coalesced DONE and LL work stale. */
> +		if ((events & DW_EDMA_DEFERRED_ABORT) &&
> +		    dw_edma_abort_interrupt(chan))
> +			continue;
>  		if (events & DW_EDMA_DEFERRED_DONE)
>  			dw_edma_done_interrupt(chan);
>  		if (events & DW_EDMA_DEFERRED_LL)
>  			dw_edma_ll_interrupt(chan);
> -		if (events & DW_EDMA_DEFERRED_ABORT)
> -			dw_edma_abort_interrupt(chan);
>  	} while (atomic_read(&chan->irq_pending));
>  }

[Severity: High]
Does skipping the DONE events here strand pending and successfully completed
descriptors if the events coalesce?

If two or more descriptors are submitted and fully published to the hardware
ring, and the first completes successfully (DONE) while the second fails
(ABORT), these IRQs can coalesce.

When dw_edma_irq_work() encounters the continue statement above, it discards
the DONE event. 

dw_edma_abort_interrupt() then pops only the first descriptor, which actually
succeeded, and incorrectly completes it as DMA_TRANS_ABORTED.

The second descriptor, which actually failed, remains stuck in the desc_issued
list.

Later calls to dw_edma_start_transfer() skip it because it is fully
published:

drivers/dma/dw-edma/dw-edma-core.c:dw_edma_start_transfer() {
    ...
		/* A fully published descriptor may still be pending in hardware. */
		if (desc->start_burst == desc->nburst)
			continue;
    ...
}

Will this permanently break the driver when a subsequent valid transfer
completes and dw_edma_ll_clean_pending() hits the WARN_ON_ONCE
(desc->ll_start != chan->ll_done)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=12