1. Patchew
  2. linux
  3. [PATCH net-next v2 00/21] net: ravb: Add suspend to RAM and runtime PM support for RZ/G3S
  4. View patch

[PATCH net-next v2 18/21] net: ravb: Return cached statistics if the interface is down

Claudiu posted 21 patches 2 years ago
There is a newer version of this series
[PATCH net-next v2 18/21] net: ravb: Return cached statistics if the interface is down
Posted by Claudiu 2 years ago 
From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Return the cached statistics in case the interface is down. There should be
no drawback to this, as most of the statistics are updated on the data path
and if runtime PM is enabled and the interface is down, the registers that
are explicitly read on ravb_get_stats() are zero anyway on most of the IP
variants.

The commit prepares the code for the addition of runtime PM support.

Suggested-by: Sergey Shtylyov <s.shtylyov@omp.ru>
Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v2:
- none; this patch is new

 drivers/net/ethernet/renesas/ravb_main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index a2a64c22ec41..1995cf7ff084 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -2110,6 +2110,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 (!netif_running(ndev))
+		return &ndev->stats;
+
 	nstats = &ndev->stats;
 	stats0 = &priv->stats[RAVB_BE];
 
-- 
2.39.2
Re: [PATCH net-next v2 18/21] net: ravb: Return cached statistics if the interface is down
Posted by Sergey Shtylyov 2 years ago 
On 12/14/23 2:45 PM, Claudiu wrote:

> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> Return the cached statistics in case the interface is down. There should be
> no drawback to this, as most of the statistics are updated on the data path
> and if runtime PM is enabled and the interface is down, the registers that
> are explicitly read on ravb_get_stats() are zero anyway on most of the IP
> variants.
> 
> The commit prepares the code for the addition of runtime PM support.
> 
> Suggested-by: Sergey Shtylyov <s.shtylyov@omp.ru>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
> 
> Changes in v2:
> - none; this patch is new
> 
>  drivers/net/ethernet/renesas/ravb_main.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index a2a64c22ec41..1995cf7ff084 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -2110,6 +2110,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 (!netif_running(ndev))

   I'm afraid this is racy as __LINK_STATE_START bit gets set
by __dev_open() before calling the ndo_open() method... :-(

> +		return &ndev->stats;
> +

   The sh_eth driver calls sh_eth_get_stats() from sh_eth_dev_exit();
perhaps it's worth doing something similar?

MBR, Sergey
Re: [PATCH net-next v2 18/21] net: ravb: Return cached statistics if the interface is down
Posted by claudiu beznea 2 years ago 

On 16.12.2023 22:02, Sergey Shtylyov wrote:
> On 12/14/23 2:45 PM, Claudiu wrote:
> 
>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>
>> Return the cached statistics in case the interface is down. There should be
>> no drawback to this, as most of the statistics are updated on the data path
>> and if runtime PM is enabled and the interface is down, the registers that
>> are explicitly read on ravb_get_stats() are zero anyway on most of the IP
>> variants.
>>
>> The commit prepares the code for the addition of runtime PM support.
>>
>> Suggested-by: Sergey Shtylyov <s.shtylyov@omp.ru>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>> ---
>>
>> Changes in v2:
>> - none; this patch is new
>>
>>  drivers/net/ethernet/renesas/ravb_main.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
>> index a2a64c22ec41..1995cf7ff084 100644
>> --- a/drivers/net/ethernet/renesas/ravb_main.c
>> +++ b/drivers/net/ethernet/renesas/ravb_main.c
>> @@ -2110,6 +2110,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 (!netif_running(ndev))
> 
>    I'm afraid this is racy as __LINK_STATE_START bit gets set
> by __dev_open() before calling the ndo_open() method... :-(

But (at least on my setup), both ndo_get_stats and ndo_open are called with
rtnl_mutex locked.

> 
>> +		return &ndev->stats;
>> +
> 
>    The sh_eth driver calls sh_eth_get_stats() from sh_eth_dev_exit();
> perhaps it's worth doing something similar?

Indeed, it might help to keep updated those few registers that gets updated
only in ndo_get_stats.


> 
> MBR, Sergey
Re: [PATCH net-next v2 18/21] net: ravb: Return cached statistics if the interface is down
Posted by Sergey Shtylyov 2 years ago 
On 12/17/23 4:54 PM, claudiu beznea wrote:

[...]
>>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>>
>>> Return the cached statistics in case the interface is down. There should be
>>> no drawback to this, as most of the statistics are updated on the data path
>>> and if runtime PM is enabled and the interface is down, the registers that
>>> are explicitly read on ravb_get_stats() are zero anyway on most of the IP
>>> variants.
>>>
>>> The commit prepares the code for the addition of runtime PM support.
>>>
>>> Suggested-by: Sergey Shtylyov <s.shtylyov@omp.ru>
>>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>> ---
>>>
>>> Changes in v2:
>>> - none; this patch is new
>>>
>>>  drivers/net/ethernet/renesas/ravb_main.c | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
>>> index a2a64c22ec41..1995cf7ff084 100644
>>> --- a/drivers/net/ethernet/renesas/ravb_main.c
>>> +++ b/drivers/net/ethernet/renesas/ravb_main.c
>>> @@ -2110,6 +2110,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 (!netif_running(ndev))
>>
>>    I'm afraid this is racy as __LINK_STATE_START bit gets set
>> by __dev_open() before calling the ndo_open() method... :-(
> 
> But (at least on my setup), both ndo_get_stats and ndo_open are called with
> rtnl_mutex locked.

   Unfortunately, it's not always so -- see e.g. netstat_show() in net/core/net-sysfs.c...
 
[...]

MBR, Sergey
Re: [PATCH net-next v2 18/21] net: ravb: Return cached statistics if the interface is down
Posted by Sergey Shtylyov 2 years ago 
On 12/14/23 2:45 PM, Claudiu wrote:

> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> Return the cached statistics in case the interface is down. There should be
> no drawback to this, as most of the statistics are updated on the data path
> and if runtime PM is enabled and the interface is down, the registers that
> are explicitly read on ravb_get_stats() are zero anyway on most of the IP
> variants.
> 
> The commit prepares the code for the addition of runtime PM support.
> 
> Suggested-by: Sergey Shtylyov <s.shtylyov@omp.ru>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
> 
> Changes in v2:
> - none; this patch is new
> 
>  drivers/net/ethernet/renesas/ravb_main.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index a2a64c22ec41..1995cf7ff084 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -2110,6 +2110,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 (!netif_running(ndev))

   I'm afraid this is racy as __LINK_STATE_START bit gets set
by __dev_open() before calling the ndo_open() method... :-(

> +		return &ndev->stats;
> +

   The sh_eth driver calls sh_eth_get_stats() from sh_eth_dev_exit();
perhaps it's worth doing something similar?

MBR, Sergey

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.