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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing rollback in `mvneta_resume()` error path causes kernel panic and deadlock on subsequent teardown.
--

commit 3e974147b6189ed308e9434d784dbcd4c3ed5e87
Author: Yun Zhou <yun.zhou@xxxxxxxxxxxxx>

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

This commit fixes an issue on PREEMPT_RT where the mvneta IRQ handler is
left masked during suspend, causing interrupt delivery to be permanently
disabled upon resume. It addresses this by explicitly freeing the IRQ
during suspend and re-requesting it during resume.

> 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
[ ... ]
> @@ -5885,6 +5899,21 @@ static int mvneta_resume(struct device *device)
>  		mvneta_txq_hw_init(pp, txq);
>  	}
>  
> +	/* 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);
> +		netif_device_detach(dev);
> +		return err;
> +	}
> +

[Severity: High]
Does this error path leave the device in an inconsistent state that will
cause a crash during teardown?

If request_irq() fails, this returns early without updating pp->is_stopped
and without re-adding the CPU hotplug instances. However, the interface
remains logically UP.

If the interface is subsequently brought down (e.g., via ip link set down),
mvneta_stop() will be called. This leads to mvneta_stop_dev(), which
invokes napi_disable(). Since NAPI was already disabled in
mvneta_suspend() and never re-enabled here, will this cause a deadlock
waiting for NAPIF_STATE_SCHED to clear?

Additionally, for non-Armada3700 devices, mvneta_stop() will call:

mvneta_stop() {
    ...
    cpuhp_state_remove_instance_nocalls(online_hpstate,
                                        &pp->node_online);
    ...
}

Since the nodes were already removed during suspend and never re-added,
could this invoke hlist_del() on a poisoned pointer and cause an immediate
kernel panic?

Finally, since the IRQ was never acquired, will the subsequent free_irq()
or free_percpu_irq() in mvneta_stop() trigger a kernel WARNING?

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