From: "Michael S. Tsirkin" <mst@redhat.com>
To: Shirley Ma <mashirle@us.ibm.com>
Cc: Herbert Xu <herbert@gondor.hengli.com.au>,
rusty@rustcorp.com.au, davem@davemloft.net, kvm@vger.kernel.org,
netdev@vger.kernel.org
Subject: Re: [PATCH 2/2] virtio_net: remove send completion interrupts and avoid TX queue overrun through packet drop
Date: Tue, 22 Mar 2011 13:36:50 +0200 [thread overview]
Message-ID: <20110322113649.GA17071@redhat.com> (raw)
In-Reply-To: <1300730587.3441.24.camel@localhost.localdomain>
On Mon, Mar 21, 2011 at 11:03:07AM -0700, Shirley Ma wrote:
> On Fri, 2011-03-18 at 18:41 -0700, Shirley Ma wrote:
> > > > + /* Drop packet instead of stop queue for better
> > performance
> > > */
> > >
> > > I would like to see some justification as to why this is the right
> > > way to go and not just papering over the real problem.
> >
> > Fair. KVM guest virtio_net TX queue stop/restart is pretty expensive,
> > which involves:
> >
> > 1. Guest enable callback: one memory barrier, interrupt flag set
>
> Missed this cost: for history reason, it also involves a guest exit from
> I/O write (PCI_QUEUE_NOTIFY).
OK, after some research, it looks like the reason was the tx timer that
qemu used to use. So the hack of avoiding the add_buf call will
avoid this kick and so break these hosts.
I guess we can add a feature bit to detect a new host
and so avoid the kick. We are running low on feature bits
unfortunately, but just fo testing, could you quantify the difference
that this makes using the following patch:
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index cc2f73e..6106017 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -185,11 +185,6 @@ int virtqueue_add_buf_gfp(struct virtqueue *_vq,
if (vq->num_free < out + in) {
pr_debug("Can't add buf len %i - avail = %i\n",
out + in, vq->num_free);
- /* FIXME: for historical reasons, we force a notify here if
- * there are outgoing parts to the buffer. Presumably the
- * host should service the ring ASAP. */
- if (out)
- vq->notify(&vq->vq);
END_USE(vq);
return -ENOSPC;
}
--
MST
next prev parent reply other threads:[~2011-03-22 11:36 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-03-17 0:12 [PATCH 2/2] virtio_net: remove send completion interrupts and avoid TX queue overrun through packet drop Shirley Ma
2011-03-17 5:02 ` Michael S. Tsirkin
2011-03-17 15:18 ` Shirley Ma
2011-03-18 3:28 ` Shirley Ma
2011-03-18 13:15 ` Michael S. Tsirkin
2011-03-18 16:54 ` Shirley Ma
2011-03-17 5:10 ` Rusty Russell
2011-03-17 15:10 ` Shirley Ma
2011-03-18 13:33 ` Herbert Xu
2011-03-19 1:41 ` Shirley Ma
2011-03-21 18:03 ` Shirley Ma
2011-03-22 11:36 ` Michael S. Tsirkin [this message]
2011-03-23 2:26 ` Shirley Ma
2011-03-24 0:30 ` Rusty Russell
2011-03-24 4:14 ` Shirley Ma
2011-03-24 14:28 ` Michael S. Tsirkin
2011-03-24 17:46 ` Shirley Ma
2011-03-24 18:10 ` Michael S. Tsirkin
2011-03-25 4:51 ` Rusty Russell
2011-03-25 4:50 ` Rusty Russell
2011-03-27 7:52 ` Michael S. Tsirkin
2011-04-04 6:13 ` Rusty Russell
2011-03-24 0:16 ` Rusty Russell
2011-03-24 6:39 ` Michael S. Tsirkin
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=20110322113649.GA17071@redhat.com \
--to=mst@redhat.com \
--cc=davem@davemloft.net \
--cc=herbert@gondor.hengli.com.au \
--cc=kvm@vger.kernel.org \
--cc=mashirle@us.ibm.com \
--cc=netdev@vger.kernel.org \
--cc=rusty@rustcorp.com.au \
/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).