Re: [PATCH v3 9/9] dmaengine: dw-edma: Defer channel IRQ handling to workqueue

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Shared IRQs are requested before `irq->dw` is initialized, potentially causing a NULL pointer dereference on early interrupts.
--

--- Patch [9]: [PATCH v3 9/9] dmaengine: dw-edma: Defer channel IRQ handling to workqueue ---
commit 61576b5286f799082619f98fb9d5cfc6fa61833f
Author: Koichiro Den <den@valinux.co.jp>
dmaengine: dw-edma: Defer channel IRQ handling to workqueue

This patch modifies the eDMA driver to keep the hard IRQ handler minimal to
prevent performance bottlenecks under multichannel load. It defers per-channel
completion and abort processing to a workqueue, queuing a work item per
channel to allow parallel execution across multiple CPUs.

> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 2369b4c4630b9..4ee7f14c3a824 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c

[ ... ]

> @@ -1195,10 +1237,21 @@ int dw_edma_probe(struct dw_edma_chip *chip)
>  	/* Disable eDMA, only to establish the ideal initial conditions */
>  	dw_edma_core_off(dw);
>  
> +	/*
> +	 * Deferred IRQ works are queued from the hard IRQ handlers, so the
> +	 * workqueue must exist before any IRQ is requested.
> +	 */
> +	dw->wq = alloc_workqueue("dw-edma:%s", WQ_UNBOUND | WQ_HIGHPRI, 0,
> +				 dev_name(chip->dev));
> +	if (!dw->wq)
> +		return -ENOMEM;
> +
>  	/* Request IRQs */
>  	err = dw_edma_irq_request(dw, &wr_alloc, &rd_alloc);

[Severity: High]
This is a pre-existing issue, but could this lead to a NULL pointer
dereference on early interrupts?

In dw_edma_probe(), we call dw_edma_irq_request() which requests the
shared IRQ:

dw_edma_irq_request() {
    ...
    err = request_irq(irq, dw_edma_interrupt_common, IRQF_SHARED, dw->name, &dw->irq[0]);
    ...
}

If an interrupt from another device sharing this IRQ line fires immediately,
the handler will be invoked before we've fully initialized the context.

Specifically, dw_edma_interrupt_common() calls down into
dw_edma_core_handle_int(), which dereferences dw_irq->dw:

dw_edma_core_handle_int() {
    ...
    return dw_irq->dw->core->handle_int(dw_irq, dir, done, abort);
}

However, irq->dw is only initialized later in dw_edma_channel_setup():

dw_edma_channel_setup() {
    ...
    for (i = 0; i < ch_cnt; i++) {
        ...
        irq->dw = dw;
    }
}

If the shared handler fires before dw_edma_channel_setup() runs, will
dw_irq->dw be NULL here and crash the kernel?

> -	if (err)
> +	if (err) {
> +		destroy_workqueue(dw->wq);
>  		return err;
> +	}
>  
>  	/* Allocate a dedicated virtual IRQ for interrupt-emulation doorbells */
>  	err = dw_edma_emul_irq_alloc(dw);

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