Re: [PATCH 6/8] dmaengine: plx_dma: unmap descriptor before completing cookie

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] dmaengine: plx_dma: deadlock in plx_dma_process_desc due to missing _bh
- [High] dmaengine: plx_dma: recursive deadlock when client submits from callback
--

commit ad3cf51e19996d2f8cb3cdd66ceba6e9d034f3e7
Author: Wolfram Sang <wsa+renesas@sang-engineering.com>

dmaengine: plx_dma: unmap descriptor before completing cookie

To ensure that buffers are flushed, make sure that the descriptor gets
unmapped before completing the cookie.

Link: https://lore.kernel.org/r/20261009093745.539924-1-vkoul@kernel.org

> diff --git a/drivers/dma/plx_dma.c b/drivers/dma/plx_dma.c
> index 409898e92c320..7b0fd9843b6a1 100644
> --- a/drivers/dma/plx_dma.c
> +++ b/drivers/dma/plx_dma.c
> @@ -156,8 +156,8 @@ static void plx_dma_process_desc(struct plx_dma_dev *plxdev)
>  		else
>  			res.result = DMA_TRANS_READ_FAILED;
>  
> -		dma_cookie_complete(&desc->txd);
>  		dma_descriptor_unmap(&desc->txd);
> +		dma_cookie_complete(&desc->txd);
>  		dmaengine_desc_get_callback_invoke(&desc->txd, &res);

[Severity: High]
Is it possible for a deadlock to occur here due to how ring_lock is acquired?

In plx_dma_process_desc(), ring_lock is acquired using spin_lock() without
disabling bottom halves. If it is called from process context via
plx_dma_tx_status() or plx_dma_abort_desc() and is interrupted by the
plx_dma_desc_task() tasklet, the tasklet will attempt to acquire ring_lock
on the same CPU.

[Severity: High]
Does this code risk a recursive deadlock if a client submits a new
transaction from its completion callback?

plx_dma_process_desc() invokes the client callback synchronously while
holding ring_lock. If a client callback queues the next transaction via
dmaengine_prep_dma_memcpy(), it will enter plx_dma_prep_memcpy() which
attempts to acquire ring_lock again, deadlocking on the same CPU:

drivers/dma/plx_dma.c:plx_dma_prep_memcpy() {
    ...
    spin_lock_bh(&plxdev->ring_lock);
    ...
}

Standard DMA engine patterns require drivers to drop their primary locks
before invoking client callbacks.

>  		desc->txd.callback = NULL;
>  		desc->txd.callback_result = NULL;
> @@ -183,8 +183,8 @@ static void plx_dma_abort_desc(struct plx_dma_dev *plxdev)
>  		res.residue = desc->orig_size;
>  		res.result = DMA_TRANS_ABORTED;
>  
> -		dma_cookie_complete(&desc->txd);
>  		dma_descriptor_unmap(&desc->txd);
> +		dma_cookie_complete(&desc->txd);
>  		dmaengine_desc_get_callback_invoke(&desc->txd, &res);

[Severity: High]
Can a recursive deadlock also happen during abort operations?

Similar to plx_dma_process_desc(), plx_dma_abort_desc() invokes the client
callback synchronously while holding ring_lock (acquired via
spin_lock_bh()). A client queuing a new transaction from this callback
would attempt to acquire the same lock.

>  		desc->txd.callback = NULL;
>  		desc->txd.callback_result = NULL;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261010124348.19055-1-wsa+renesas@sang-engineering.com?part=6