Hi Sergey,
> From: Sergey Shtylyov, Sent: Tuesday, November 28, 2023 1:28 AM
>
> On 11/27/23 3:24 PM, 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() should calls rtnl_trylock(). Otherwise, a deadlock
> > may happen in ravb_tx_timeout_work() like below:
> >
> > CPU0 CPU1
> > ravb_tx_timeout()
> > schedule_work()
> > ...
> > __dev_close_many()
> > // Under rtnl lock
> > ravb_close()
> > cancel_work_sync()
> > // Waiting
> > ravb_tx_timeout_work()
> > rtnl_lock()
> > // This is possible to cause a deadlock
> >
> > If rtnl_trylock() fails, rescheduling the work with sleep for 1 msec.
> >
> > Fixes: c156633f1353 ("Renesas Ethernet AVB driver proper")
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>
>
> Reviewed-by: Sergey Shtylyov <s.shtylyov@xxxxxx>
Thank you very much for your review!
> [...]
>
> > diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> > index c70cff80cc99..7c007ecd3ff6 100644
> > --- a/drivers/net/ethernet/renesas/ravb_main.c
> > +++ b/drivers/net/ethernet/renesas/ravb_main.c
> > @@ -1874,6 +1874,12 @@ static void ravb_tx_timeout_work(struct work_struct *work)
> > struct net_device *ndev = priv->ndev;
> > int error;
> >
> > + if (!rtnl_trylock()) {
> > + usleep_range(1000, 2000);
>
> Why not msleep(1) though?
It's for the following guideline:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/timers/timers-howto.rst?h=v6.7-rc3
-----
SLEEPING FOR ~USECS OR SMALL MSECS ( 10us - 20ms):
* Use usleep_range
-----
Best regards,
Yoshihiro Shimoda
> > + schedule_work(&priv->work);
> > + return;
> > + }
> > +
> > netif_tx_stop_all_queues(ndev);
> >
> > /* Stop PTP Clock driver */
> [...]
>
> MBR, Sergey
