On Tue, 21 Oct 2008, Stephen Hemminger wrote: > The network statistics were being updated by multiple CPU's causing cache > line bounces. Since this driver already keeps statistics per ring, use > those to compute the bytes/packet statistics. > > Signed-off-by: Stephen Hemminger <[EMAIL PROTECTED]>
Yeah, this is code we developed for e1000(e) and that was for only one queue, it should be a problem, and we should fix it one way or another. Thanks Stephen, everything seems good after a quick review, except for one note: see below. > Compile tested only, evaluation in progress okay > --- a/drivers/net/igb/igb_main.c 2008-10-21 08:57:11.000000000 -0700 > +++ b/drivers/net/igb/igb_main.c 2008-10-21 09:05:43.000000000 -0700 > @@ -3029,9 +3029,27 @@ static struct net_device_stats * > igb_get_stats(struct net_device *netdev) > { > struct igb_adapter *adapter = netdev_priv(netdev); > + struct net_device_stats *stats = &adapter->net_stats; > + int i; > + > + stats->tx_bytes = 0; > + stats->tx_packets = 0; > + for (i = 0; i < adapter->num_tx_queues; i++) { > + struct igb_ring *tx_ring = &adapter->tx_ring[i]; > + stats->tx_bytes += tx_ring->tx_stats.bytes; > + stats->tx_packets += tx_ring->tx_stats.packets; > + } > + stats->rx_bytes = 0; > + stats->rx_packets = 0; > + for (i = 0; i < adapter->num_rx_queues; i++) { > + struct igb_ring *rx_ring = &adapter->rx_ring[i]; > + stats->rx_bytes += rx_ring->rx_stats.bytes; > + stats->rx_packets += rx_ring->rx_stats.packets; > + } > + > + igb_update_stats(adapter); I don't think you want to put this in the code that can be called directly from IOCTL from userspace. This function can take a lot of cycles and some silly applications like gkrellm call it quite frequently. The update_stats function will be called as part of the watchdog anyway, and you already got the interesting stats for realtime with the tx and rx bytes/packets. > - /* only return the current stats */ > - return &adapter->net_stats; > + return stats; > } > > /** > @@ -3711,8 +3729,6 @@ done_cleaning: > tx_ring->total_packets += total_packets; > tx_ring->tx_stats.bytes += total_bytes; > tx_ring->tx_stats.packets += total_packets; > - adapter->net_stats.tx_bytes += total_bytes; > - adapter->net_stats.tx_packets += total_packets; > return retval; > } > > @@ -3953,8 +3969,6 @@ next_desc: > rx_ring->total_bytes += total_bytes; > rx_ring->rx_stats.packets += total_packets; > rx_ring->rx_stats.bytes += total_bytes; > - adapter->net_stats.rx_bytes += total_bytes; > - adapter->net_stats.rx_packets += total_packets; > return cleaned; > } The same thing should be done to ixgbe too, but I'm just mentioning it for posterity, not because I'd expect you to do so. ------------------------------------------------------------------------- This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world http://moblin-contest.org/redirect.php?banner_id=100&url=/ _______________________________________________ E1000-devel mailing list E1000-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/e1000-devel