On 1/22/25 9:33 PM, Sergey Shtylyov wrote: [...] >> Fix the suspend path by ensuring the rtnl lock is held where required. > > Maybe suspend/resume path (the same w/the subject)? > >> Calls to ravb_open, ravb_close and wol operations must be performed under >> the rtnl lock to prevent conflicts with ongoing ndo operations. > > [...] > >> Reported-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx> >> Closes: https://lore.kernel.org/netdev/4c6419d8-c06b-495c-b987-d66c2e1ff848@xxxxxxxxx/ >> Fixes: 0184165b2f42 ("ravb: add sleep PM suspend/resume support") >> Signed-off-by: Kory Maincent <kory.maincent@xxxxxxxxxxx> > > FWIW: > > Reviewed-by: Sergey Shtylyov <s.shtylyov@xxxxxx> > > [...] >> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c >> index bc395294a32d..2c6d8e4966c3 100644 >> --- a/drivers/net/ethernet/renesas/ravb_main.c >> +++ b/drivers/net/ethernet/renesas/ravb_main.c > [...] >> @@ -3245,19 +3250,25 @@ static int ravb_resume(struct device *dev) >> if (!netif_running(ndev)) >> return 0; >> >> + rtnl_lock(); >> /* If WoL is enabled restore the interface. */ >> if (priv->wol_enabled) { >> ret = ravb_wol_restore(ndev); >> - if (ret) >> + if (ret) { >> + rtnl_unlock(); >> return ret; >> + } >> } else { >> ret = pm_runtime_force_resume(dev); >> - if (ret) >> + if (ret) { >> + rtnl_unlock(); >> return ret; > > Hm, are you sure we need to have rtnl_lock around pm_runtime_force_resume() too? Anyway, the above *if* statements are needlessly duplicated, I think it's time that we do away with this! :-) [...] MBR, Sergey
