Messages in this thread | | | Subject | Re: [V2 RFC net-next PATCH 1/2] virtio_net: convert the statistics into array | From | Eric Dumazet <> | Date | Wed, 06 Jun 2012 10:22:19 +0200 |
| |
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.
| |