Thread (17 messages) 17 messages, 6 authors, 2012-06-08

Re: [V2 RFC net-next PATCH 1/2] virtio_net: convert the statistics into array

From: Eric Dumazet <hidden>
Date: 2012-06-06 08:22:27
Also in: lkml, netdev

On Wed, 2012-06-06 at 15:52 +0800, Jason Wang wrote:
Currently, we store the statistics in the independent fields of virtnet_stats,
this is not scalable when we want to add more counters. As suggested by Michael,
this patch convert it to an array and use the enum as the index to access them.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/net/virtio_net.c |   30 +++++++++++++++++-------------
 1 files changed, 17 insertions(+), 13 deletions(-)
 struct virtnet_stats {
 	struct u64_stats_sync syncp;
-	u64 tx_bytes;
-	u64 tx_packets;
-
-	u64 rx_bytes;
-	u64 rx_packets;
+	u64 data[VIRTNET_NUM_STATS];
 };
 
Interesting, but I fear you'll have a lot of problems.

Current code is buggy, and you are adding more possible races.

We could have one cpu doing the :

       u64_stats_update_begin(&stats->syncp);
       stats->rx_bytes += skb->len;
       stats->rx_packets++;
       u64_stats_update_end(&stats->syncp);

And another one doing :

       u64_stats_update_begin(&stats->syncp);
       stats->tx_bytes += skb->len;
       stats->tx_packets++;
       u64_stats_update_end(&stats->syncp);
 
And one syncp sequence increment can be lost, since both cpus are
basically doing this at the same time :

    write_seqcount_begin(&syncp->seq);

I'll send a fix in a separate thread.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help