| Message ID | 4da4117c-a24a-4512-9b79-fbed126e03bc@exht1.ad.emulex.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
Le lundi 27 juin 2011 à 12:10 +0530, Sathya Perla a écrit : > Problem initially reproted and fixed by Eric Dumazet <eric.dumazet@gmail.com> > > netdev_stats_update() resets netdev->stats and then accumulates stats from > various rings. This is wrong as stats readers can sometimes catch zero values. > Use temporary variables instead for accumulating per-ring values. > > Signed-off-by: Sathya Perla <sathya.perla@emulex.com> > --- Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> Thanks -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Eric Dumazet <eric.dumazet@gmail.com> Date: Mon, 27 Jun 2011 08:56:29 +0200 > Le lundi 27 juin 2011 à 12:10 +0530, Sathya Perla a écrit : >> Problem initially reproted and fixed by Eric Dumazet <eric.dumazet@gmail.com> >> >> netdev_stats_update() resets netdev->stats and then accumulates stats from >> various rings. This is wrong as stats readers can sometimes catch zero values. >> Use temporary variables instead for accumulating per-ring values. >> >> Signed-off-by: Sathya Perla <sathya.perla@emulex.com> >> --- > > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> Applied. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 27 Jun 2011 12:10:48 +0530 Sathya Perla <sathya.perla@emulex.com> wrote: > Problem initially reproted and fixed by Eric Dumazet <eric.dumazet@gmail.com> > > netdev_stats_update() resets netdev->stats and then accumulates stats from > various rings. This is wrong as stats readers can sometimes catch zero values. > Use temporary variables instead for accumulating per-ring values. > > Signed-off-by: Sathya Perla <sathya.perla@emulex.com> Should also use u64_stats_sync to ensure correct rollover or 32 bit SMP platform. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Le lundi 27 juin 2011 à 09:43 -0700, Stephen Hemminger a écrit : > On Mon, 27 Jun 2011 12:10:48 +0530 > Sathya Perla <sathya.perla@emulex.com> wrote: > > > Problem initially reproted and fixed by Eric Dumazet <eric.dumazet@gmail.com> > > > > netdev_stats_update() resets netdev->stats and then accumulates stats from > > various rings. This is wrong as stats readers can sometimes catch zero values. > > Use temporary variables instead for accumulating per-ring values. > > > > Signed-off-by: Sathya Perla <sathya.perla@emulex.com> > > Should also use u64_stats_sync to ensure correct rollover or 32 bit SMP > platform. These are "unsigned long" fields, you dont need u64_stats_sync. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 27 Jun 2011 19:20:41 +0200 Eric Dumazet <eric.dumazet@gmail.com> wrote: > Le lundi 27 juin 2011 à 09:43 -0700, Stephen Hemminger a écrit : > > On Mon, 27 Jun 2011 12:10:48 +0530 > > Sathya Perla <sathya.perla@emulex.com> wrote: > > > > > Problem initially reproted and fixed by Eric Dumazet <eric.dumazet@gmail.com> > > > > > > netdev_stats_update() resets netdev->stats and then accumulates stats from > > > various rings. This is wrong as stats readers can sometimes catch zero values. > > > Use temporary variables instead for accumulating per-ring values. > > > > > > Signed-off-by: Sathya Perla <sathya.perla@emulex.com> > > > > Should also use u64_stats_sync to ensure correct rollover or 32 bit SMP > > platform. > > These are "unsigned long" fields, you dont need u64_stats_sync. The source fields (in be.h) are u64, but since destination is unsigned long that works. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/benet/be_main.c b/drivers/net/benet/be_main.c index c4f564c..5ca06b0 100644 --- a/drivers/net/benet/be_main.c +++ b/drivers/net/benet/be_main.c @@ -428,33 +428,38 @@ void netdev_stats_update(struct be_adapter *adapter) struct net_device_stats *dev_stats = &adapter->netdev->stats; struct be_rx_obj *rxo; struct be_tx_obj *txo; + unsigned long pkts = 0, bytes = 0, mcast = 0, drops = 0; int i; - memset(dev_stats, 0, sizeof(*dev_stats)); for_all_rx_queues(adapter, rxo, i) { - dev_stats->rx_packets += rx_stats(rxo)->rx_pkts; - dev_stats->rx_bytes += rx_stats(rxo)->rx_bytes; - dev_stats->multicast += rx_stats(rxo)->rx_mcast_pkts; + pkts += rx_stats(rxo)->rx_pkts; + bytes += rx_stats(rxo)->rx_bytes; + mcast += rx_stats(rxo)->rx_mcast_pkts; /* no space in linux buffers: best possible approximation */ if (adapter->generation == BE_GEN3) { if (!(lancer_chip(adapter))) { - struct be_erx_stats_v1 *erx_stats = + struct be_erx_stats_v1 *erx = be_erx_stats_from_cmd(adapter); - dev_stats->rx_dropped += - erx_stats->rx_drops_no_fragments[rxo->q.id]; + drops += erx->rx_drops_no_fragments[rxo->q.id]; } } else { - struct be_erx_stats_v0 *erx_stats = + struct be_erx_stats_v0 *erx = be_erx_stats_from_cmd(adapter); - dev_stats->rx_dropped += - erx_stats->rx_drops_no_fragments[rxo->q.id]; + drops += erx->rx_drops_no_fragments[rxo->q.id]; } } + dev_stats->rx_packets = pkts; + dev_stats->rx_bytes = bytes; + dev_stats->multicast = mcast; + dev_stats->rx_dropped = drops; + pkts = bytes = 0; for_all_tx_queues(adapter, txo, i) { - dev_stats->tx_packets += tx_stats(txo)->be_tx_pkts; - dev_stats->tx_bytes += tx_stats(txo)->be_tx_bytes; + pkts += tx_stats(txo)->be_tx_pkts; + bytes += tx_stats(txo)->be_tx_bytes; } + dev_stats->tx_packets = pkts; + dev_stats->tx_bytes = bytes; /* bad pkts received */ dev_stats->rx_errors = drvs->rx_crc_errors +
Problem initially reproted and fixed by Eric Dumazet <eric.dumazet@gmail.com> netdev_stats_update() resets netdev->stats and then accumulates stats from various rings. This is wrong as stats readers can sometimes catch zero values. Use temporary variables instead for accumulating per-ring values. Signed-off-by: Sathya Perla <sathya.perla@emulex.com> --- drivers/net/benet/be_main.c | 29 +++++++++++++++++------------ 1 files changed, 17 insertions(+), 12 deletions(-)