Re: [PATCH net-next] r8169: Add 64bit statistics

Le lundi 05 mars 2012 à 00:24 +0100, Francois Romieu a écrit :

Eric Dumazet [off-list ref] :
[...]

quoted

small point : Using this means you have a 32bit hole here (on 32bit
build). Its minor, you dont need to change.

Ok.

[...]

quoted

	You could try to put these somewhere else, to try to keep this portion
as read only memory, to be more SMP friendly.
(Some loaded server could have one CPU serving RX stuff, and other cpus
doing TX stuff) 

Point taken. It could make sense to rework the rtl8169_private struct a bit
more.

[...]

quoted

quoted

@@ -6070,20 +6084,49 @@ static void rtl_set_rx_mode(struct net_device *dev)
 }
 
 /**
- *  rtl8169_get_stats - Get rtl8169 read/write statistics
+ *  rtl8169_get_stats64 - Get rtl8169 read/write statistics
  *  @dev: The Ethernet Device to get statistics for

missing @stats

This documentation is almost useless anyway. I removed it.

[...]

quoted

You dont need _bytes and _packets temp variables, as @stats points to a
private memory, we can use it as working storage, just do :

       do {
               start = u64_stats_fetch_begin_bh(&tp->rx_stats.syncp);
               stats->rx_packets = tp->rx_stats.packets;
               stats->rx_bytes = tp->rx_stats.bytes;
       } while (u64_stats_fetch_retry_bh(&tp->rx_stats.syncp, start));

It should give something like the patch below.

If I understand correctly we do not care much about the error counters,
right ?

From: Junchang Wang <redacted>
Date: Sun, 4 Mar 2012 23:30:32 +0100
Subject: [PATCH 1/2] r8169: add 64bit statistics.

Switch to use ndo_get_stats64 to get 64bit statistics.
Two sync entries are used (one for Rx and one for Tx).

Signed-off-by: Junchang Wang <redacted>
Reviewed-by: Eric Dumazet <redacted>
Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
---
 drivers/net/ethernet/realtek/r8169.c |   59 ++++++++++++++++++++++++++--------
 1 files changed, 45 insertions(+), 14 deletions(-)

Seems Good To Me, thanks !