[PATCH] net: mvneta: free/request IRQ across suspend/resume

[PATCH] net: mvneta: free/request IRQ across suspend/resume

[Yun Zhou]

On PREEMPT_RT, the mvneta IRQ handler is force-threaded. Under high
network traffic, the IRQ can enter suspend with desc->depth == 1
(masked by the oneshot mechanism between handler invocations).

During suspend, the kernel increments depth to 2 and masks the
interrupt at the MPIC level (clearing the SRC_CTL CPU routing bit,
due to IRQCHIP_MASK_ON_SUSPEND). On resume, depth is decremented
back to 1, but since it does not reach 0, the unmask is never
called. The MPIC CPU routing remains cleared, permanently disabling
interrupt delivery.

Fix by freeing the IRQ in suspend and re-requesting it in resume.
This ensures a clean IRQ state (depth=0, proper hardware routing)
on every resume cycle, regardless of the pre-suspend depth. This
follows the approach used by other drivers (e.g. igb).

Fixes: 9768b45ceb0b ("net: mvneta: support suspend and resume")
Signed-off-by: Yun Zhou <yun.zhou@windriver.com>
---
 drivers/net/ethernet/marvell/mvneta.c | 28 +++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index 0c061fb0ed07..e7e9a58dbe55 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -5819,6 +5819,20 @@ static int mvneta_suspend(struct device *device)
 	mvneta_stop_dev(pp);
 	rtnl_unlock();
 
+	/* Release IRQ to avoid stale MPIC mask state on resume.
+	 * On PREEMPT_RT, forced-threaded oneshot IRQs may leave the
+	 * interrupt masked (depth>0) at suspend time. This prevents
+	 * resume_device_irqs() from restoring the MPIC CPU routing,
+	 * permanently disabling the interrupt. Re-requesting the IRQ
+	 * on resume guarantees a clean state.
+	 */
+	if (pp->neta_armada3700)
+		free_irq(dev->irq, pp);
+	else {
+		on_each_cpu(mvneta_percpu_disable, pp, true);
+		free_percpu_irq(dev->irq, pp->ports);
+	}
+
 	for (queue = 0; queue < rxq_number; queue++) {
 		struct mvneta_rx_queue *rxq = &pp->rxqs[queue];
 
@@ -5895,6 +5909,20 @@ static int mvneta_resume(struct device *device)
 						 &pp->node_dead);
 	}
 
+	/* Re-request IRQ (see comment in mvneta_suspend) */
+	if (pp->neta_armada3700) {
+		err = request_irq(dev->irq, mvneta_isr, 0, dev->name, pp);
+	} else {
+		err = request_percpu_irq(dev->irq, mvneta_percpu_isr,
+					dev->name, pp->ports);
+		if (!err)
+			on_each_cpu(mvneta_percpu_enable, pp, true);
+	}
+	if (err) {
+		netdev_err(dev, "cannot request irq %d\n", dev->irq);
+		return err;
+	}
+
 	rtnl_lock();
 	mvneta_start_dev(pp);
 	rtnl_unlock();
-- 
2.43.0

Re: [PATCH] net: mvneta: free/request IRQ across suspend/resume

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing cleanup in `mvneta_resume()` error path causes CPU hotplug deadlocks.
- [High] Inconsistent state on resume failure leads to hard deadlock and invalid IRQ free during device stop.
--

commit 888983e0bf18d7a13bd3110a9d378dd82a1883c0
Author: Yun Zhou <yun.zhou@windriver.com>

net: mvneta: free/request IRQ across suspend/resume

This commit frees the IRQ during suspend and requests it again during resume.
This fixes an issue on PREEMPT_RT where forced-threaded IRQs could remain
masked on the MPIC after a suspend/resume cycle.

> diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> --- a/drivers/net/ethernet/marvell/mvneta.c
> +++ b/drivers/net/ethernet/marvell/mvneta.c
[ ... ]
> @@ -5895,6 +5909,20 @@ static int mvneta_resume(struct device *device)
>  						 &pp->node_dead);
>  	}
>  
> +	/* Re-request IRQ (see comment in mvneta_suspend) */
> +	if (pp->neta_armada3700) {
> +		err = request_irq(dev->irq, mvneta_isr, 0, dev->name, pp);
> +	} else {
> +		err = request_percpu_irq(dev->irq, mvneta_percpu_isr,
> +					dev->name, pp->ports);
> +		if (!err)
> +			on_each_cpu(mvneta_percpu_enable, pp, true);
> +	}
> +	if (err) {
> +		netdev_err(dev, "cannot request irq %d\n", dev->irq);
> +		return err;

[Severity: High]
Does this error path need to revert the earlier resume operations like the CPU
hotplug registrations and pp->is_stopped state?

If this returns early without reverting cpuhp_state_add_instance_nocalls() and
setting pp->is_stopped back to true, the device remains on the CPU hotplug
lists in an inconsistent state.

If a CPU is later taken offline, mvneta_cpu_dead() will run and unconditionally
call napi_disable(). Since mvneta_resume() failed before calling
mvneta_start_dev(), the NAPI context is already disabled, which causes a hard
deadlock in napi_disable().

Additionally, does this leave the device logically up but with unrequested
IRQs?

If the interface is later brought down, mvneta_stop() will run and call
mvneta_stop_dev() followed by free_irq() or free_percpu_irq(). The
mvneta_stop_dev() call will also deadlock in napi_disable() because the NAPI
context was never re-enabled. The subsequent free_irq() will then trigger a
warning since the IRQ was never successfully requested during resume.

> +	}
> +
>  	rtnl_lock();
>  	mvneta_start_dev(pp);
>  	rtnl_unlock();

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616072649.2514602-1-yun.zhou@windriver.com?part=1