Re: [RFC PATCH] net: stmmac: Fix the problem about interrupt storm

[Romain Gantois <romain.gantois@bootlin.com>]

Hello Cathy,

On Wed, 27 Mar 2024, Cathy Cai wrote:

> Tx queue time out then reset adapter. When reset the adapter, stmmac driver
> sets the state to STMMAC_DOWN and calls dev_close() function. If an interrupt
> is triggered at this instant after setting state to STMMAC_DOWN, before the
> dev_close() call.
> 
...
> -	set_bit(STMMAC_DOWN, &priv->state);
>  	dev_close(priv->dev);
> +	set_bit(STMMAC_DOWN, &priv->state);
>  	dev_open(priv->dev, NULL);
>  	clear_bit(STMMAC_DOWN, &priv->state);
>  	clear_bit(STMMAC_RESETING, &priv->state);

If this IRQ issue can happen whenever STMMAC_DOWN is set while the net device is 
open, then it could also happen between the dev_open() and 
clear_bit(STMMAC_DOWN) calls right? So you'd have to clear STMMAC_DOWN before 
calling dev_open() but then I don't see the usefulness of setting STMMAC_DOWN 
and clearing it immediately. Maybe closing and opening the net device should be 
enough?

Moreover, it seems strange to me that stmmac_interrupt() unconditionnally 
ignores interrupts when the driver is in STMMAC_DOWN state. This seems like 
dangerous behaviour, since it could cause IRQ storm issues whenever something 
in the driver sets this state. I'm not too familiar with the interrupt handling 
in this driver, but maybe stmmac_interrupt() could clear interrupts 
unconditionnally in the STMMAC_DOWN state?

Best Regards,

-- 
Romain Gantois, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com