Re: [PATCH] net: fec: ethtool: fix unbalanced IRQ wake disable

[Sascha Hauer <s.hauer@pengutronix.de>]

On Wed, Feb 23, 2022 at 10:28:34AM +0000, Joakim Zhang wrote:
> 
> Hi Sascha,
> 
> > -----Original Message-----
> > From: Sascha Hauer <s.hauer@pengutronix.de>
> > Sent: 2022年2月23日 16:09
> > To: netdev@vger.kernel.org
> > Cc: Joakim Zhang <qiangqing.zhang@nxp.com>; David S . Miller
> > <davem@davemloft.net>; kernel@pengutronix.de; Ahmad Fatoum
> > <a.fatoum@pengutronix.de>; Sascha Hauer <s.hauer@pengutronix.de>
> > Subject: [PATCH] net: fec: ethtool: fix unbalanced IRQ wake disable
> > 
> > From: Ahmad Fatoum <a.fatoum@pengutronix.de>
> > 
> > Userspace can trigger a kernel warning by using the ethtool ioctls to disable
> > WoL, when it was not enabled before:
> > 
> >   $ ethtool -s eth0 wol d ; ethtool -s eth0 wol d
> >   Unbalanced IRQ 54 wake disable
> >   WARNING: CPU: 2 PID: 17532 at kernel/irq/manage.c:900
> > irq_set_irq_wake+0x108/0x148
> > 
> > This is because fec_enet_set_wol happily calls disable_irq_wake, even if the
> > wake IRQ is already disabled.
> 
> I have not found disable_irq_wake in fec_enet_set_wol.
> https://elixir.bootlin.com/linux/v5.17-rc5/source/drivers/net/ethernet/freescale/fec_main.c#L2882
> 
> > Looking at other drivers, like lpc_eth, suggests the way to go is to do wake
> > IRQ enabling/disabling in the suspend/resume callbacks.
> > Doing so avoids the warning at no loss of functionality.
> 
> FEC done as this way:
> https://elixir.bootlin.com/linux/v5.17-rc5/source/drivers/net/ethernet/freescale/fec_main.c#L4075
> https://elixir.bootlin.com/linux/v5.17-rc5/source/drivers/net/ethernet/freescale/fec_main.c#L4119
> 
> > This only affects userspace with older ethtool versions. Newer ones use
> > netlink and disabling before enabling will be refused before reaching the
> > driver.
> 
> Ahh, what I misunderstand? All the description makes me confusion. Please use the latest kernel.

This patch was forward ported from v5.16. I should have had a closer
look before posting, then I maybe would have realized that 0b6f65c707e5
("net: fec: fix system hang during suspend/resume") already fixes the
issue.

Sorry for the noise.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |