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

[sashiko-bot@kernel.org]

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