Messages in this thread | | | From | cathy cai <> | Date | Wed, 3 Apr 2024 10:01:02 +0800 | Subject | Re: [RFC PATCH] net: stmmac: Fix the problem about interrupt storm |
| |
Hi Romain,
On Sun, Mar 31, 2024 at 4:35 PM Romain Gantois <romain.gantois@bootlin.com> wrote: > > 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? > Yes. It could also happen between the dev_open() and clear_bit(STMMAC_DOWN) calls. Although we did not reproduce this scenario, it should have happened if we had increased the number of test samples. In addition, I found that other people had similar problems before. The link is: https://lore.kernel.org/all/20210208140820.10410-11-Sergey.Semin@baikalelectronics.ru/
> > 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? > Clear interrupts unconditionally in the STMMAC_DOWN state directly certainly won't cause this problem. This may be too rough, maybe this design has other considerations.
> > Best Regards, > > -- > Romain Gantois, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com
Best Regards, Cathy
| |