lkml.org 
[lkml]   [2012]   [Jun]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] virtio-net: fix a race on 32bit arches
On Wed, Jun 06, 2012 at 03:10:10PM +0200, Eric Dumazet wrote:
> On Wed, 2012-06-06 at 14:13 +0300, Michael S. Tsirkin wrote:
>
> > We currently do all stats either on napi callback or from
> > start_xmit callback.
> > This makes them safe, yes?
>
> Hmm, then _bh() variant is needed in virtnet_stats(), as explained in
> include/linux/u64_stats_sync.h section 6)
>
> * 6) If counter might be written by an interrupt, readers should block interrupts.
> * (On UP, there is no seqcount_t protection, a reader allowing interrupts could
> * read partial values)
>
> Yes, its tricky...

Sounds good, but I have a question: this realies on counters
being atomic on 64 bit.
Would not it be better to always use a seqlock even on 64 bit?
This way counters would actually be correct and in sync.
As it is if we want e.g. average packet size,
we can not rely e.g. on it being bytes/packets.

> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 5214b1e..705aaa7 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -703,12 +703,12 @@ static struct rtnl_link_stats64 *virtnet_stats(struct net_device *dev,
> u64 tpackets, tbytes, rpackets, rbytes;
>
> do {
> - start = u64_stats_fetch_begin(&stats->syncp);
> + start = u64_stats_fetch_begin_bh(&stats->syncp);
> tpackets = stats->tx_packets;
> tbytes = stats->tx_bytes;
> rpackets = stats->rx_packets;
> rbytes = stats->rx_bytes;
> - } while (u64_stats_fetch_retry(&stats->syncp, start));
> + } while (u64_stats_fetch_retry_bh(&stats->syncp, start));
>
> tot->rx_packets += rpackets;
> tot->tx_packets += tpackets;
>


\
 
 \ /
  Last update: 2012-06-06 17:41    [W:0.102 / U:0.268 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site