Re: [PATCH net] net: spacemit: Hold RTNL lock in emac_resume() and emac_suspend()

["Troy Mitchell" <troy.mitchell@linux.spacemit.com>]

[-- Attachment #1: Type: text/plain, Size: 1200 bytes --]

On Sat Nov 7, 2026 at 5:32 PM GMT, Andrew Lunn wrote:
> On Fri, Nov 06, 2026 at 03:44:50PM +0500, Ali Tariq wrote:
>> 
>> 
>> On 11/6/26 2:43 PM, Troy Mitchell wrote:
>> > On Thu,  5 Nov 2026 17:34:37 +0500, Ali Tariq wrote:
>> > > @@ -2056,7 +2059,9 @@ static int emac_suspend(struct device *dev)
>> > >   		return 0;
>> > >   	}
>> > > +	rtnl_lock();
>> > >   	emac_stop(ndev);
>> > > +	rtnl_unlock();
>> > >   	clk_disable_unprepare(priv->bus_clk);
>> > >   	netif_device_detach(ndev);
>> > This can deadlock. emac_stop() -> emac_down() calls
>> > cancel_work_sync(&priv->tx_timeout_task) with RTNL now held, while
>> > emac_tx_timeout_task() starts with rtnl_lock().
>> > 
>> 
>> Thanks Troy, that makes sense.
>> 
>> For the deadlock, I think the correct function to call is rtnl_trylock()
>> inside the emac_tx_timeout_task(), which would return if the lock is already
>> held and will hold the lock if it is not.
>
> rtnl_trylock() is generally a workaround needed for badly written
> code. Please try to avoid it.
Agreed. If the reset is done in place, the work does not need RTNL at all,
so .ndo_stop can keep cancel_work_sync() as is.


-- 
Troy Mitchell


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]