Re: [PATCH] virtio-net: fix a race on 32bit arches

["Michael S. Tsirkin" <mst@redhat.com>]

On Wed, Jun 06, 2012 at 07:13:02PM +0200, Eric Dumazet wrote:
> On Wed, 2012-06-06 at 19:17 +0300, Michael S. Tsirkin wrote:
> 
> > But why do you say at most 1 packet?
> > 
> > Consider get_stats doing:
> >                u64_stats_update_begin(&stats->syncp);
> >                stats->tx_bytes += skb->len;
> > 
> > on 64 bit at this point
> > tx_packets might get incremented any number of times, no?
> > 
> >                 stats->tx_packets++;
> >                 u64_stats_update_end(&stats->syncp);
> > 
> > now tx_bytes and tx_packets are out of sync by more than 1.
> 
> You lost me there.
> 
> No idea of what you are thinking about.

Sorry about that. This is not a bug. I am saying two things:

1. We are trying to look at counters for purposes of tuning the device.
E.g. if ethtool reports packets and bytes, we'd like to calculate
average packet size by bytes/packets.

If both counters are read atomically the metric becomes more exact.
Not a must but nice to have.

2. 32 bit systems have some overhead because of the seqlock.
virtio could instead simply keep tx counters in the queue structure, and
get the tx lock when they are read.


-- 
MST