netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Thomas Lendacky <tahm@linux.vnet.ibm.com>
Cc: kvm@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	virtualization@lists.linux-foundation.org, avi@redhat.com,
	Sasha Levin <levinsasha928@gmail.com>
Subject: [PATCH 2/3] virtio-net: correct capacity math on ring full
Date: Fri, 28 Sep 2012 11:26:09 +0200	[thread overview]
Message-ID: <f2f7123534aefb33a7a233d9fec56de6e9eeed73.1348824232.git.mst@redhat.com> (raw)
In-Reply-To: <cover.1348824232.git.mst@redhat.com>

Capacity math on ring full is wrong: we are
looking at num_sg but that might be optimistic
because of indirect buffer use.

The implementation also penalizes fast path
with extra memory accesses for the benefit of
ring full condition handling which is slow path.

It's easy to query ring capacity so let's do just that.

This change also makes it easier to move vnet header
for tx around as follow-up patch does.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/net/virtio_net.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 83d2b0c..316f1be 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -95,7 +95,6 @@ struct skb_vnet_hdr {
 		struct virtio_net_hdr hdr;
 		struct virtio_net_hdr_mrg_rxbuf mhdr;
 	};
-	unsigned int num_sg;
 };
 
 struct padded_vnet_hdr {
@@ -557,10 +556,10 @@ again:
 	return received;
 }
 
-static unsigned int free_old_xmit_skbs(struct virtnet_info *vi)
+static void free_old_xmit_skbs(struct virtnet_info *vi)
 {
 	struct sk_buff *skb;
-	unsigned int len, tot_sgs = 0;
+	unsigned int len;
 	struct virtnet_stats *stats = this_cpu_ptr(vi->stats);
 
 	while ((skb = virtqueue_get_buf(vi->svq, &len)) != NULL) {
@@ -571,16 +570,15 @@ static unsigned int free_old_xmit_skbs(struct virtnet_info *vi)
 		stats->tx_packets++;
 		u64_stats_update_end(&stats->tx_syncp);
 
-		tot_sgs += skb_vnet_hdr(skb)->num_sg;
 		dev_kfree_skb_any(skb);
 	}
-	return tot_sgs;
 }
 
 static int xmit_skb(struct virtnet_info *vi, struct sk_buff *skb)
 {
 	struct skb_vnet_hdr *hdr = skb_vnet_hdr(skb);
 	const unsigned char *dest = ((struct ethhdr *)skb->data)->h_dest;
+	unsigned num_sg;
 
 	pr_debug("%s: xmit %p %pM\n", vi->dev->name, skb, dest);
 
@@ -619,8 +617,8 @@ static int xmit_skb(struct virtnet_info *vi, struct sk_buff *skb)
 	else
 		sg_set_buf(vi->tx_sg, &hdr->hdr, sizeof hdr->hdr);
 
-	hdr->num_sg = skb_to_sgvec(skb, vi->tx_sg + 1, 0, skb->len) + 1;
-	return virtqueue_add_buf(vi->svq, vi->tx_sg, hdr->num_sg,
+	num_sg = skb_to_sgvec(skb, vi->tx_sg + 1, 0, skb->len) + 1;
+	return virtqueue_add_buf(vi->svq, vi->tx_sg, num_sg,
 				 0, skb, GFP_ATOMIC);
 }
 
@@ -664,7 +662,8 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
 		netif_stop_queue(dev);
 		if (unlikely(!virtqueue_enable_cb_delayed(vi->svq))) {
 			/* More just got used, free them then recheck. */
-			capacity += free_old_xmit_skbs(vi);
+			free_old_xmit_skbs(vi);
+			capacity = virtqueue_get_capacity(vi->svq);
 			if (capacity >= 2+MAX_SKB_FRAGS) {
 				netif_start_queue(dev);
 				virtqueue_disable_cb(vi->svq);
-- 
MST

  parent reply	other threads:[~2012-09-28  9:26 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-28  9:26 [PATCH 0/3] virtio-net: inline header support Michael S. Tsirkin
2012-09-28  9:26 ` [PATCH 1/3] virtio: add API to query ring capacity Michael S. Tsirkin
2012-09-28  9:26 ` Michael S. Tsirkin [this message]
2012-10-04  0:24   ` [PATCH 2/3] virtio-net: correct capacity math on ring full Rusty Russell
2012-09-28  9:26 ` [PATCH 3/3] virtio-net: put virtio net header inline with data Michael S. Tsirkin
2012-10-03  6:44 ` [PATCH 0/3] virtio-net: inline header support Rusty Russell
2012-10-03  7:10   ` Rusty Russell
2012-10-04  1:24   ` Anthony Liguori
2012-10-04  3:34     ` Rusty Russell
2012-10-04  4:29       ` Anthony Liguori
2012-10-04  7:44         ` Rusty Russell
2012-10-05  7:47           ` Paolo Bonzini
2012-10-08 21:31       ` Michael S. Tsirkin
2012-10-04  1:35   ` Anthony Liguori
2012-10-04  5:17     ` Rusty Russell
2012-10-08 20:41   ` Michael S. Tsirkin
     [not found] ` <87vces2gxq.fsf__45058.6618776017$1349247807$gmane$org@rustcorp.com.au>
2012-10-03 10:53   ` Paolo Bonzini
2012-10-04  0:11     ` Rusty Russell
2012-10-04  7:09       ` Paolo Bonzini
2012-10-04 12:51         ` Rusty Russell
2012-10-04 13:23           ` Paolo Bonzini
2012-10-05  5:43             ` Rusty Russell
     [not found]             ` <87391t1nkq.fsf__40391.6521034718$1349505001$gmane$org@rustcorp.com.au>
2012-10-06 12:54               ` Paolo Bonzini
2012-10-09  4:59                 ` Rusty Russell
2012-10-09  7:27                   ` Paolo Bonzini
2012-10-11  0:03                     ` Rusty Russell
2012-10-11 11:04                       ` Michael S. Tsirkin
2012-10-11 22:37                         ` Rusty Russell
2012-10-12  7:38                           ` Paolo Bonzini
2012-10-12 11:52                           ` Cornelia Huck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f2f7123534aefb33a7a233d9fec56de6e9eeed73.1348824232.git.mst@redhat.com \
    --to=mst@redhat.com \
    --cc=avi@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=levinsasha928@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=tahm@linux.vnet.ibm.com \
    --cc=virtualization@lists.linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).