On Wed, Jun 06, 2012 at 01:02:19PM +0800, Jason Wang wrote:
On 06/05/2012 06:10 PM, Michael S. Tsirkin wrote:
On Tue, Jun 05, 2012 at 04:38:41PM +0800, Jason Wang wrote:
Satistics counters is useful for debugging and performance optimization, so
this
patch lets virtio_net driver collect
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 jasow...@redhat.com
---
Satistics counters is useful for debugging and performance optimization, so this
patch lets virtio_net driver collect following and export them to userspace
through ethtool -S:
- number of packets sent/received
- number of bytes sent/received
- number of callbacks for tx/rx
- number of kick for
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
On Wed, Jun 06, 2012 at 03:52:17PM +0800, Jason Wang wrote:
Satistics counters is useful for debugging and performance optimization, so
this
patch lets virtio_net driver collect following and export them to userspace
through ethtool -S:
- number of packets sent/received
- number of bytes
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 Mon, Jun 4, 2012 at 12:15 PM, Michael S. Tsirkin m...@redhat.com wrote:
On Fri, Jun 01, 2012 at 10:13:06AM +0100, Stefan Hajnoczi wrote:
Other block drivers (cciss, rbd, nbd) use spin_unlock_irq() so I followed
that.
To me this seems wrong: blk_run_queue() uses spin_lock_irqsave() but we
On Wed, Jun 06, 2012 at 03:52:17PM +0800, Jason Wang wrote:
Satistics counters is useful for debugging and performance optimization, so
this
patch lets virtio_net driver collect following and export them to userspace
through ethtool -S:
- number of packets sent/received
- number of bytes
On 06/06/2012 04:27 PM, Michael S. Tsirkin wrote:
On Wed, Jun 06, 2012 at 03:52:17PM +0800, Jason Wang wrote:
Satistics counters is useful for debugging and performance optimization, so this
patch lets virtio_net driver collect following and export them to userspace
through ethtool -S:
-
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 Mon, Jun 4, 2012 at 12:11 PM, Michael S. Tsirkin m...@redhat.com wrote:
On Fri, Jun 01, 2012 at 10:13:06AM +0100, Stefan Hajnoczi wrote:
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 774c31d..d674977 100644
--- a/drivers/block/virtio_blk.c
+++
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
35 matches
Mail list logo
