On Wed, 06 Jun 2012 10:45:41 +0200, Eric Dumazet eric.duma...@gmail.com wrote:
On Wed, 2012-06-06 at 10:35 +0200, Eric Dumazet wrote:
From: Eric Dumazet eduma...@google.com
commit 3fa2a1df909 (virtio-net: per cpu 64 bit stats (v2)) added a race
on 32bit arches.
We must use separate
On Sun, Jun 10, 2012 at 04:06:34PM +0930, Rusty Russell wrote:
On Wed, 06 Jun 2012 10:45:41 +0200, Eric Dumazet eric.duma...@gmail.com
wrote:
On Wed, 2012-06-06 at 10:35 +0200, Eric Dumazet wrote:
From: Eric Dumazet eduma...@google.com
commit 3fa2a1df909 (virtio-net: per cpu 64 bit
On Sun, 2012-06-10 at 10:03 +0300, Michael S. Tsirkin wrote:
One question though: do we want to lay the structure
out so that the rx sync structure precedes the rx counters?
I am not sure its worth having holes in the structure, since its percpu
data.
That would be 8 bytes lost per cpu and
On Sun, Jun 10, 2012 at 12:21:14PM +0200, Eric Dumazet wrote:
On Sun, 2012-06-10 at 10:03 +0300, Michael S. Tsirkin wrote:
One question though: do we want to lay the structure
out so that the rx sync structure precedes the rx counters?
I am not sure its worth having holes in the
On Wed, Jun 06, 2012 at 10:35:24AM +0200, Eric Dumazet wrote:
From: Eric Dumazet eduma...@google.com
commit 3fa2a1df909 (virtio-net: per cpu 64 bit stats (v2)) added a race
on 32bit arches.
We must use separate syncp for rx and tx path as they can be run at the
same time on different
From: Michael S. Tsirkin m...@redhat.com
Date: Sun, 10 Jun 2012 13:25:12 +0300
On Wed, Jun 06, 2012 at 10:35:24AM +0200, Eric Dumazet wrote:
From: Eric Dumazet eduma...@google.com
commit 3fa2a1df909 (virtio-net: per cpu 64 bit stats (v2)) added a race
on 32bit arches.
We must use
From: Eric Dumazet eduma...@google.com
commit 3fa2a1df909 (virtio-net: per cpu 64 bit stats (v2)) added a race
on 32bit arches.
We must use separate syncp for rx and tx path as they can be run at the
same time on different cpus. Thus one sequence increment can be lost and
readers spin forever.
On Wed, 2012-06-06 at 10:35 +0200, Eric Dumazet wrote:
From: Eric Dumazet eduma...@google.com
commit 3fa2a1df909 (virtio-net: per cpu 64 bit stats (v2)) added a race
on 32bit arches.
We must use separate syncp for rx and tx path as they can be run at the
same time on different cpus. Thus
On 06/06/2012 04:45 PM, Eric Dumazet wrote:
On Wed, 2012-06-06 at 10:35 +0200, Eric Dumazet wrote:
From: Eric Dumazeteduma...@google.com
commit 3fa2a1df909 (virtio-net: per cpu 64 bit stats (v2)) added a race
on 32bit arches.
We must use separate syncp for rx and tx path as they can be run at
On Wed, Jun 06, 2012 at 10:45:41AM +0200, Eric Dumazet wrote:
On Wed, 2012-06-06 at 10:35 +0200, Eric Dumazet wrote:
From: Eric Dumazet eduma...@google.com
commit 3fa2a1df909 (virtio-net: per cpu 64 bit stats (v2)) added a race
on 32bit arches.
We must use separate syncp for rx and
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
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
On Wed, 6 Jun 2012 17:49:42 +0300
Michael S. Tsirkin m...@redhat.com wrote:
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
On Wed, 2012-06-06 at 17:49 +0300, Michael S. Tsirkin wrote:
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
On Wed, Jun 06, 2012 at 05:19:04PM +0200, Eric Dumazet wrote:
On Wed, 2012-06-06 at 17:49 +0300, Michael S. Tsirkin wrote:
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
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
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 Wed, Jun 06, 2012 at 08:14:32AM -0700, Stephen Hemminger wrote:
On Wed, 6 Jun 2012 17:49:42 +0300
Michael S. Tsirkin m...@redhat.com wrote:
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
On Wed, Jun 06, 2012 at 05:19:04PM +0200, Eric Dumazet wrote:
On Wed, 2012-06-06 at 17:49 +0300, Michael S. Tsirkin wrote:
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
On Wed, 2012-06-06 at 21:51 +0300, Michael S. Tsirkin wrote:
BTW for cards that do implement the counters in software,
under xmit lock, is anything wrong with simply taking the xmit lock
when we get the stats instead of the per-cpu trick + seqlock?
I still dont understand why you would do
On Wed, Jun 06, 2012 at 09:54:01PM +0200, Eric Dumazet wrote:
On Wed, 2012-06-06 at 21:51 +0300, Michael S. Tsirkin wrote:
BTW for cards that do implement the counters in software,
under xmit lock, is anything wrong with simply taking the xmit lock
when we get the stats instead of the
On Wed, 2012-06-06 at 19:57 +0300, Michael S. Tsirkin wrote:
So for virtio since all counters get incremented from bh we can
ensure they are read atomically, simply but reading them
from the correct CPU with bh disabled.
And then we don't need u64_stats_sync at all.
Really ? How are you
On Wed, 2012-06-06 at 21:43 +0300, Michael S. Tsirkin wrote:
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
On Wed, 2012-06-06 at 22:58 +0300, Michael S. Tsirkin wrote:
Absolutely, I am talking about virtio here. I'm not kicking
u64_stats_sync idea I am just saying that simple locking
would work for virtio and might be better as it
gives us a way to get counters atomically.
Which lock do you own
On Wed, Jun 06, 2012 at 10:08:09PM +0200, Eric Dumazet wrote:
On Wed, 2012-06-06 at 22:58 +0300, Michael S. Tsirkin wrote:
Absolutely, I am talking about virtio here. I'm not kicking
u64_stats_sync idea I am just saying that simple locking
would work for virtio and might be better as it
On Wed, Jun 06, 2012 at 10:06:11PM +0200, Eric Dumazet wrote:
On Wed, 2012-06-06 at 21:43 +0300, Michael S. Tsirkin wrote:
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
On Wed, 2012-06-06 at 23:16 +0300, Michael S. Tsirkin wrote:
On Wed, Jun 06, 2012 at 10:08:09PM +0200, Eric Dumazet wrote:
On Wed, 2012-06-06 at 22:58 +0300, Michael S. Tsirkin wrote:
Absolutely, I am talking about virtio here. I'm not kicking
u64_stats_sync idea I am just saying that
On Wed, 2012-06-06 at 22:08 +0200, Eric Dumazet wrote:
On Wed, 2012-06-06 at 22:58 +0300, Michael S. Tsirkin wrote:
Absolutely, I am talking about virtio here. I'm not kicking
u64_stats_sync idea I am just saying that simple locking
would work for virtio and might be better as it
gives
On Wed, 2012-06-06 at 21:19 +0100, Ben Hutchings wrote:
On Wed, 2012-06-06 at 22:08 +0200, Eric Dumazet wrote:
On Wed, 2012-06-06 at 22:58 +0300, Michael S. Tsirkin wrote:
Absolutely, I am talking about virtio here. I'm not kicking
u64_stats_sync idea I am just saying that simple
On Wed, 2012-06-06 at 23:16 +0300, Michael S. Tsirkin wrote:
On Wed, Jun 06, 2012 at 10:08:09PM +0200, Eric Dumazet wrote:
On Wed, 2012-06-06 at 22:58 +0300, Michael S. Tsirkin wrote:
Absolutely, I am talking about virtio here. I'm not kicking
u64_stats_sync idea I am just saying that
On Wed, 2012-06-06 at 22:24 +0200, Eric Dumazet wrote:
(ndo_get_stats64() is not allowed to sleep, and I cant see how you are
going to disable napi without sleeping)
In case you wonder, take a look at bond_get_stats() in
drivers/net/bonding/bond_main.c
On Wed, Jun 06, 2012 at 09:35:04PM +0100, Ben Hutchings wrote:
On Wed, 2012-06-06 at 23:16 +0300, Michael S. Tsirkin wrote:
On Wed, Jun 06, 2012 at 10:08:09PM +0200, Eric Dumazet wrote:
On Wed, 2012-06-06 at 22:58 +0300, Michael S. Tsirkin wrote:
Absolutely, I am talking about virtio
32 matches
Mail list logo
