On Fri, 2011-11-18 at 08:24 +0200, Sasha Levin wrote:

On Fri, 2011-11-18 at 01:08 +0000, Ben Hutchings wrote:

quoted

On Fri, 2011-11-11 at 18:34 +0530, Krishna Kumar wrote:

quoted

Changes for multiqueue virtio_net driver.

[...]

quoted

@@ -677,25 +730,35 @@ static struct rtnl_link_stats64 *virtnet
 {
 	struct virtnet_info *vi = netdev_priv(dev);
 	int cpu;
-	unsigned int start;
 
 	for_each_possible_cpu(cpu) {
-		struct virtnet_stats __percpu *stats
-			= per_cpu_ptr(vi->stats, cpu);
-		u64 tpackets, tbytes, rpackets, rbytes;
-
-		do {
-			start = u64_stats_fetch_begin(&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));
-
-		tot->rx_packets += rpackets;
-		tot->tx_packets += tpackets;
-		tot->rx_bytes   += rbytes;
-		tot->tx_bytes   += tbytes;
+		int qpair;
+
+		for (qpair = 0; qpair < vi->num_queue_pairs; qpair++) {
+			struct virtnet_send_stats __percpu *tx_stat;
+			struct virtnet_recv_stats __percpu *rx_stat;

While you're at it, you can drop the per-CPU stats and make them only
per-queue.  There is unlikely to be any benefit in maintaining them
per-CPU while receive and transmit processing is serialised per-queue.

It allows you to update stats without a lock.

But you'll already be holding a lock related to the queue.

Whats the benefit of having them per queue?

It should save some memory (and a little time when summing stats, though
that's unlikely to matter much).

The important thing is that splitting up stats per-CPU *and* per-queue
is a waste.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.