Subject: Re: [PATCH net] atlantic: fix deadlock at aq_nic_stop

From: Jakub Kicinski <[email-masked]>
Date: Mon, 17 Oct 2022 19:44:04 -0700

On Tue, 18 Oct 2022 02:27:40 +0200 Andrew Lunn wrote:
> > > Please try to identify what is being protected. If it is driver
> > > internal state, could it be replaced with a driver mutex, rather than
> > > RTNL? Or is it network stack as a whole state, which really does
> > > require RTNL? If so, how do other drivers deal with this problem? Is
> > > it specific to MACSEC? Does MACSEC have a design problem?  
> > 
> > I already considered this possibility but discarded it because, as I
> > say above, everything else is already legitimately protected by
> > rtnl_lock.  
> 
> Did you look at other drivers using MACSEC offload? Is this driver
> unique in having stuff run in a work queue which you need to cancel?
> In fact, it is not limited to MACSEC, it could be any work queue which
> holds RTNL and needs to be cancelled.

FWIW the work APIs return a boolean to tell you if the work was
actually scheduled / canceled, and you can pair that with a reference
count of the netdev to avoid the typical _sync issues.

trigger()
	ASSERT_RTNL();
	if (schedule_work(netdev_priv->bla))
		netdev_hold();

work()
	rtnl_lock();
	if (netif_running())
		do_ya_thing();
	netdev_put();
	rtnl_unlock();

stop()
	ASSERT_RTNL();
	if (cancel_work(bla))
		netdev_put();

I think.