On 08.01.2024 22:22, Sergey Shtylyov wrote: > On 1/5/24 11:23 AM, Claudiu wrote: > >> From: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx> >> >> Return the cached statistics in case the interface is down. There should be >> no drawback to this, as cached statistics are updated in ravb_close(). >> >> The commit prepares the code for the addition of runtime PM support. >> >> Suggested-by: Sergey Shtylyov <s.shtylyov@xxxxxx> >> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx> > [...] > >> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c >> index 76035afd4054..168b6208db37 100644 >> --- a/drivers/net/ethernet/renesas/ravb_main.c >> +++ b/drivers/net/ethernet/renesas/ravb_main.c >> @@ -2117,6 +2117,9 @@ static struct net_device_stats *ravb_get_stats(struct net_device *ndev) >> const struct ravb_hw_info *info = priv->info; >> struct net_device_stats *nstats, *stats0, *stats1; >> >> + if (!(ndev->flags & IFF_UP)) > > Well, I guess it's OK to read the counters in the reset mode... BUT > won't this race with pm_runtime_put_autosuspend() when its call gets added > to ravb_close()? I re-checked it and, yes, this is true. A sync runtime suspend would be better here. But, as of my current investigation, even with this ravb_get_stats() can still race with ravb_open()/ravb_close() as they are called though different locking scheme (ravb_open()/ravb_close() is called with rtnl locked while ravb_get_stats() can be called only with dev_base_lock rwlock locked for reading). A mutex in the driver should to help with this. Thank you, Claudiu Beznea > >> + return &ndev->stats; >> + >> nstats = &ndev->stats; >> stats0 = &priv->stats[RAVB_BE]; >> > [...] > > MBR, Sergey >
