Re: [PATCH v4 4/9] dmaengine: dw-edma: Complete descriptors before pausing

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but remains unfixed: dw_edma_device_pause() and dw_edma_device_resume() modify shared state and access virt-dma lists without holding the required chan->vc.lock.
--

--- Patch [4]: [PATCH v4 4/9] dmaengine: dw-edma: Complete descriptors before pausing ---
commit ae2fd62162735ab1efcac8f4a660ab9c1e877d64
Author: Koichiro Den <den@valinux.co.jp>

dmaengine: dw-edma: Complete descriptors before pausing

This commit ensures that the DMA engine checks for descriptor completion
before acknowledging a pause request. If there is no work to start on
resume, it leaves the channel idle. It also ignores done interrupts while
the channel is paused so a stale or repeated interrupt cannot change its
state or start queued work.

> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index daf70df59e077..ac40a832a9c76 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -327,7 +327,8 @@ static int dw_edma_device_resume(struct dma_chan *dchan)
>  		err = -EPERM;
>  	} else {
>  		chan->status = EDMA_ST_BUSY;
> -		dw_edma_start_transfer(chan);
> +		if (!dw_edma_start_transfer(chan))
> +			chan->status = EDMA_ST_IDLE;

[Severity: High]
This is a pre-existing issue, but does dw_edma_device_resume() (and
dw_edma_device_pause()) modify shared state and access virt-dma lists
without holding the required chan->vc.lock?

For dw_edma_device_resume(), calling dw_edma_start_transfer() locklessly
iterates over chan->vc.desc_issued via vchan_next_desc() and mutates chunk
structures.

Could this cause list corruption or lockdep splats if run concurrently with
the interrupt handler or dmaengine_terminate_all()?

Similarly, in dw_edma_device_pause(), can a time-of-check to time-of-use
race occur if it evaluates chan->status == EDMA_ST_BUSY locklessly?

If dw_edma_done_interrupt() concurrently completes the last descriptor and
transitions the state to EDMA_ST_IDLE, the pause function could then
overwrite chan->request = EDMA_REQ_PAUSE. 

Could this leave the channel in an invalid state (status == EDMA_ST_IDLE,
request == EDMA_REQ_PAUSE) where it cannot be resumed (since resume expects
EDMA_ST_PAUSE) and cannot process new descriptors?

>  	}
>  
>  	return err;

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