Re: [PATCH v3 23/24] dmaengine: dw-edma: Detect and recover a stalled eDMA engine

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unkillable hang in dw_edma_wait_termination() if the engine fails to recover from a stall.
--

--- Patch [23]: [PATCH v3 23/24] dmaengine: dw-edma: Detect and recover a stalled eDMA engine ---
commit 8c110ba5f33df8c06f35fa69ddd95a9b7e7cab52
Author: Koichiro Den <[email protected]>

dmaengine: dw-edma: Detect and recover a stalled eDMA engine

This commit introduces passive stall detection for write channels under
sustained multi-channel traffic. It reuses the bounded LL recheck and queues a
recovery request if a channel remains stopped with no progress for 30 ms.

> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 56032137a4bb0..262ea893174f3 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
[ ... ]
> @@ -291,6 +292,54 @@ static bool dw_edma_ll_pending(struct dw_edma_chan *chan)
>  	return chan->ll_head != chan->ll_done;
>  }
>  
[ ... ]
> +/*
> + * Called with vc.lock held for a stopped channel with LL work pending. Queue
> + * direction recovery if repeated doorbells show no progress for one recheck
> + * interval.
> + */
> +static bool dw_edma_ll_stall_check(struct dw_edma_chan *chan)
> +{
> +	struct dw_edma_engine_recovery *rec;
> +
[ ... ]
> +	rec = &chan->dw->eng_recovery[chan->dir];
> +	chan->ll_recovery_pending = true;
> +	dw_edma_engine_recovery_queue(rec);

[Severity: High]
If the hardware stalls and the recovery work queued here fails to reset it,
can this lead to an unkillable hang later when the channel is freed?

Looking at dw_edma_engine_recovery_work(), if the maximum reset attempts are
reached, the worker exits without clearing chan->ll_recovering:

drivers/dma/dw-edma/dw-edma-core.c:dw_edma_engine_recovery_work() {
    ...
		if (++rec->fails >= DW_EDMA_ENGINE_RESET_MAX_FAILS) {
			dev_err(dw->chip->dev,
				"%s engine did not drain after %u attempts; leaving channels gated\n",
				dir_name, rec->fails);
			return;
		}
    ...
}

If chan->ll_recovering remains true, the doorbell logic aborts early and does
not process the stop request when releasing the channel. 

This causes dw_edma_wait_termination() to loop indefinitely because the loop
lacks a hard timeout break and the stop request is never cleared:

drivers/dma/dw-edma/dw-edma-core.c:dw_edma_wait_termination() {
    ...
	for (;;) {
		scoped_guard(spinlock_irqsave, &chan->vc.lock)
			stopping = chan->request == EDMA_REQ_STOP;

		if (!stopping)
			return;

		if (time_after_eq(jiffies, timeout)) {
			dev_warn(chan->dw->chip->dev,
				 "timeout waiting for channel termination; still waiting\n");
			timeout = jiffies + msecs_to_jiffies(5000);
		}

		fsleep(1000);
	}
    ...
}

Could this result in an unkillable process hang when dw_edma_wait_termination()
polls indefinitely?

> +
> +	return false;
> +}

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