Hello! On 10/17/23 11:53 AM, Yoshihiro Shimoda wrote: > Fix races between ravb_tx_timeout_work() and functions of net_device_ops > and ethtool_ops by using rtnl_trylock() and rtnl_unlock(). Note that > since ravb_close() is under the rtnl lock and calls cancel_work_sync(), > ravb_tx_timeout_work() calls rtnl_trylock() to avoid a deadlock. I don't quite follow... how calling cancel_work_sync() is a problem? I thought the problem was that unregister_netdev() can be called with the TX timeout work still pending? And, more generally, shouldn't we protect against the TX timeout work being executed on a different CPU than the {net_device|ethtool}_ops methods are being executed (is that possible?)? I also had a suspicion that we still miss the spinlock calls in ravb_tx_timeout_work() but nothing in particular jumped at me... mind looking into that? :-) > Fixes: c156633f1353 ("Renesas Ethernet AVB driver proper") > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx> > --- > drivers/net/ethernet/renesas/ravb_main.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c > index 0ef0b88b7145..b53533ab4599 100644 > --- a/drivers/net/ethernet/renesas/ravb_main.c > +++ b/drivers/net/ethernet/renesas/ravb_main.c [...] > @@ -1907,6 +1910,7 @@ static void ravb_tx_timeout_work(struct work_struct *work) > */ > netdev_err(ndev, "%s: ravb_dmac_init() failed, error %d\n", > __func__, error); > + rtnl_unlock(); > return; Perhaps *goto* instead here? > } > ravb_emac_init(ndev); > @@ -1917,6 +1921,7 @@ static void ravb_tx_timeout_work(struct work_struct *work) > ravb_ptp_init(ndev, priv->pdev); > > netif_tx_start_all_queues(ndev); ... and add label here? > + rtnl_unlock(); > } MBR, Sergey