netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Shirley Ma <mashirle@us.ibm.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Rusty Russell <rusty@rustcorp.com.au>,
	Herbert Xu <herbert@gondor.hengli.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: Thu, 24 Mar 2011 10:46:49 -0700	[thread overview]
Message-ID: <1300988809.3441.48.camel@localhost.localdomain> (raw)
In-Reply-To: <20110324142822.GD12958@redhat.com>

On Thu, 2011-03-24 at 16:28 +0200, Michael S. Tsirkin wrote:
> On Thu, Mar 24, 2011 at 11:00:53AM +1030, Rusty Russell wrote:
> > > With simply removing the notify here, it does help the case when TX
> > > overrun hits too often, for example for 1K message size, the single
> > > TCP_STREAM performance improved from 2.xGb/s to 4.xGb/s.
> > 
> > OK, we'll be getting rid of the "kick on full", so please delete that on
> > all benchmarks.
> > 
> > Now, does the capacity check before add_buf() still win anything?  I
> > can't see how unless we have some weird bug.
> > 
> > Once we've sorted that out, we should look at the more radical change
> > of publishing last_used and using that to intuit whether interrupts
> > should be sent.  If we're not careful with ordering and barriers that
> > could introduce more bugs.
> 
> Right. I am working on this, and trying to be careful.
> One thing I'm in doubt about: sometimes we just want to
> disable interrupts. Should still use flags in that case?
> I thought that if we make the published index 0 to vq->num - 1,
> then a special value in the index field could disable
> interrupts completely. We could even reuse the space
> for the flags field to stick the index in. Too complex?
> > Anything else on the optimization agenda I've missed?
> > 
> > Thanks,
> > Rusty.
> 
> Several other things I am looking at, wellcome cooperation:
> 1. It's probably a good idea to update avail index
>    immediately instead of upon kick: for RX
>    this might help parallelism with the host.
Is that possible to use the same idea for publishing last used idx to
publish avail idx? Then we can save guest iowrite/exits.

> 2. Adding an API to add a single buffer instead of s/g,
>    seems to help a bit.
> 
> 3. For TX sometimes we free a single buffer, sometimes
>    a ton of them, which might make the transmit latency
>    vary. It's probably a good idea to limit this,
>    maybe free the minimal number possible to keep the device
>    going without stops, maybe free up to MAX_SKB_FRAGS.

I am playing with it now, to collect more perf data to see what's the
best value to free number of used buffers.

> 4. If the ring is full, we now notify right after
>    the first entry is consumed. For TX this is suboptimal,
>    we should try delaying the interrupt on host.

> More ideas, would be nice if someone can try them out:
> 1. We are allocating/freeing buffers for indirect descriptors.
>    Use some kind of pool instead?
>    And we could preformat part of the descriptor.
> 2. I didn't have time to work on virtio2 ideas presented
>    at the kvm forum yet, any takers?
If I have time, I will look at this.

Thanks
Shirley



  reply	other threads:[~2011-03-24 17:46 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
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 [this message]
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=1300988809.3441.48.camel@localhost.localdomain \
    --to=mashirle@us.ibm.com \
    --cc=davem@davemloft.net \
    --cc=herbert@gondor.hengli.com.au \
    --cc=kvm@vger.kernel.org \
    --cc=mst@redhat.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).