Re: [PATCH v4 15/24] dmaengine: dw-edma: Recheck stopped LL channels before restart

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Double read of volatile `jiffies` variable without a snapshot leads to a race condition where the timer delay can underflow, causing indefinite stalling.
--

--- Patch [15]: [PATCH v4 15/24] dmaengine: dw-edma: Recheck stopped LL channels before restart ---
commit 9769569e17606605660b53b027f9a3d71abb5a3d
Author: Koichiro Den <den@valinux.co.jp>

dmaengine: dw-edma: Recheck stopped LL channels before restart

This commit introduces a delayed recheck mechanism for stopped LL channels.
It samples DMA_LLP again before restarting a stopped channel to reclaim
newly visible progress that might have been missed due to an interrupt race.

> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 16d3d2a8d1c26..0d1e6f48cac1d 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c

[ ... ]

> +static void dw_edma_ll_recheck_work(struct work_struct *work)
> +{
> +	struct dw_edma_chan *chan =
> +		container_of(to_delayed_work(work), struct dw_edma_chan,
> +			     ll_recheck_work);
> +	unsigned long delay;
> +
> +	guard(spinlock_irqsave)(&chan->vc.lock);
> +
> +	if (unlikely(READ_ONCE(chan->dw->teardown))) {
> +		chan->ll_recheck_at = 0;
> +		return;
> +	}
> +
> +	if (!chan->ll_recheck_at)
> +		return;
> +
> +	if (time_before(jiffies, chan->ll_recheck_at)) {
> +		delay = chan->ll_recheck_at - jiffies;

[Severity: High]
Could this code suffer from a race condition due to reading jiffies twice?

Since jiffies is volatile, the compiler will emit two separate memory loads.
If jiffies advances past chan->ll_recheck_at between the time_before()
check and the subtraction, delay will underflow to an extremely large
unsigned value. This would schedule the recheck work indefinitely far into
the future, potentially hanging the DMA channel.

Should jiffies be cached in a local variable (e.g., unsigned long now =
jiffies;) before the check to avoid this TOCTOU race?

> +		mod_delayed_work(chan->dw->wq, &chan->ll_recheck_work, delay);
> +		return;
> +	}
> +
> +	chan->ll_recheck_at = 0;
> +	if (chan->request == EDMA_REQ_NONE)
> +		dw_edma_core_ch_maybe_doorbell(chan);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729143036.3087722-1-den@valinux.co.jp?part=15