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

From: "Troy Mitchell" <troy.mitchell@linux.spacemit.com>
To: "Andrew Lunn" <andrew@lunn.ch>, "Ali Tariq" <alitariq45892@gmail.com>
Cc: "Troy Mitchell" <troy.mitchell@linux.spacemit.com>,
	"Vadim Fedorenko" <vadim.fedorenko@linux.dev>,
	<netdev@vger.kernel.org>, "Andrew Lunn" <andrew+netdev@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	"Eric Dumazet" <edumazet@kernel.org>,
	"Jakub Kicinski" <kuba@kernel.org>,
	"Paolo Abeni" <pabeni@redhat.com>, "Yixun Lan" <dlan@kernel.org>,
	"Vivian Wang" <wangruikang@iscas.ac.cn>,
	"Chukun Pan" <amadeus@jmu.edu.cn>,
	"Siddharth Vadapalli" <s-vadapalli@ti.com>,
	"Tomas Hlavacek" <tmshlvck@gmail.com>,
	"Muhammad Bilal" <meatuni001@gmail.com>,
	"Maxime Chevallier" <maxime.chevallier@bootlin.com>,
	<linux-riscv@lists.infradead.org>, <spacemit@lists.linux.dev>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH net] net: spacemit: Hold RTNL lock in emac_resume() and emac_suspend()
Date: Sun, 08 Nov 2026 07:19:19 +0000	[thread overview]
Message-ID: <DMPNAPNEGQ5I.XYUIB7Y8HDCA@linux.spacemit.com> (raw)
In-Reply-To: <12a95b3a-9c8b-4a2d-9b81-12b3839e64a9@lunn.ch>

[-- 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 --]