Messages in this thread | | | Subject | Re: [PATCH net-next v6 7/9] forcedeth: implement ndo_get_stats64() API | From | Eric Dumazet <> | Date | Thu, 17 Nov 2011 07:34:38 +0100 |
| |
Le mercredi 16 novembre 2011 à 14:15 -0800, David Decotigny a écrit : > This commit implements the ndo_get_stats64() API for forcedeth. Since > hardware stats are being updated from different contexts (process and > timer), this commit adds synchronization. For software stats, it > relies on the u64_stats_sync.h API. > > Tested: > - 16-way SMP x86_64 -> > RX bytes:7244556582 (7.2 GB) TX bytes:181904254 (181.9 MB) > - pktgen + loopback: identical rx_bytes/tx_bytes and rx_packets/tx_packets > > > > Signed-off-by: David Decotigny <david.decotigny@google.com> > --- > drivers/net/ethernet/nvidia/forcedeth.c | 197 +++++++++++++++++++++++-------- > 1 files changed, 146 insertions(+), 51 deletions(-) >
> +static struct rtnl_link_stats64* > +nv_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *storage) > + __acquires(&netdev_priv(dev)->hwstats_lock) > + __releases(&netdev_priv(dev)->hwstats_lock) > { > struct fe_priv *np = netdev_priv(dev); > + unsigned int syncp_start; > + > + /* > + * Note: because HW stats are not always available and for > + * consistency reasons, the following ifconfig stats are > + * managed by software: rx_bytes, tx_bytes, rx_packets and > + * tx_packets. The related hardware stats reported by ethtool > + * should be equivalent to these ifconfig stats, with 4 > + * additional bytes per packet (Ethernet FCS CRC), except for > + * tx_packets when TSO kicks in. > + */ > + > + /* software stats */ > + do { > + syncp_start = u64_stats_fetch_begin(&np->swstats_rx_syncp); > + storage->rx_packets = np->stat_rx_packets; > + storage->rx_bytes = np->stat_rx_bytes; > + storage->rx_missed_errors = np->stat_rx_missed_errors; > + } while (u64_stats_fetch_retry(&np->swstats_rx_syncp, syncp_start)); > + > + do { > + syncp_start = u64_stats_fetch_begin(&np->swstats_tx_syncp); > + storage->tx_packets = np->stat_tx_packets; > + storage->tx_bytes = np->stat_tx_bytes; > + storage->tx_dropped = np->stat_tx_dropped; > + } while (u64_stats_fetch_retry(&np->swstats_tx_syncp, syncp_start)); >
I have no idea why you think u64_stats_fetch_begin() is safe on 32bit arch here.
Hint : On CONFIG_SMP=n build, only preemption is disabled in u64_stats_fetch_begin()
So an interrupt could come and change your counters while you were reading them.
Its very unlikely, but its possible.
You should use the _bh variants.
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |