public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Rusty Russell <rusty@rustcorp.com.au>
To: Avi Kivity <avi@qumranet.com>
Cc: Mark McLoughlin <markmc@redhat.com>,
	Anthony Liguori <aliguori@us.ibm.com>,
	linux-kernel@vger.kernel.org,
	virtualization@lists.linux-foundation.org
Subject: Re: [PATCH] virtio_net: free transmit skbs in a timer
Date: Tue, 20 May 2008 11:37:59 +1000	[thread overview]
Message-ID: <200805201138.00435.rusty@rustcorp.com.au> (raw)
In-Reply-To: <48317FF7.1020707@qumranet.com>

On Monday 19 May 2008 23:26:15 Avi Kivity wrote:
> Rusty Russell wrote:
> >>> Well, we do have such a thing, in the ring suppression flags.
> >>
> >> Can you point me at this?
> >
> > Ah, sorry, I misunderstood.  No, we don't have a threshold like this, we
> > have an all-or-nothing flag in each direction.
>
> Please point me anyway?

Sure, linux/virtio_ring.h:

/* The Host uses this in used->flags to advise the Guest: don't kick me when
 * you add a buffer.  It's unreliable, so it's simply an optimization.  Guest
 * will still kick if it's out of buffers. */
#define VRING_USED_F_NO_NOTIFY  1
/* The Guest uses this in avail->flags to advise the Host: don't interrupt me
 * when you consume a buffer.  It's unreliable, so it's simply an
 * optimization.  */
#define VRING_AVAIL_F_NO_INTERRUPT      1

> > We have the ability to add new fields to the rings.  I've put it on my
> > TODO to benchmark what this does.  It may or may not help.  In this case,
> > notification when there are no more packets in xmit ring would be
> > sufficient.  We already kick the host when we fill a ring even if
> > it says it doesn't need it, perhaps this would be symmetry.
>
> Notification on ring full is too late with smp.  You need to warn the
> other side in advance.

No, you misunderstand; this is not a performance issue.  On xmit, the driver 
cleans up any old used packets before trying to send anyway.  So it doesn't 
care when xmit packets are used, and suppresses the 'used' interrupt on the 
xmit virtqueue.  Only if the xmit ring is full does it enable the xmit-used 
notification.  This is optimal.

The issue is that we *do* actually care when xmit packets are used: we're 
supposed to free them in a timely manner and if the packet flow stops, we 
don't.  By always sending a used interrupt when *all* packets are used, we 
would cover this case quite nicely without impacting the normal case of 
packet flow.

> The reason I'm interested in adjustable thresholds is that you can then
> to tx mitigation without (usually) suffering the worst-case latency when
> you aren't streaming or streaming slower than what you tuned for.

Right, this would be a threshold that the host would set, approx. "when you've 
put this many packets in the xmit ring, tell me" (the opposite direction of 
the discussion above).  Currently we will kick the host on the first packet, 
and qemu will suppress the notifications based on some timer and we'll notify 
it anyway if the ring fills (which is suboptimal).  With this technique the 
host could double the threshold up to some maximum percentage of the ring.

While I like the Xen scheme, we can do the same thing from within the guest 
with the existing scheme using an internal threshold.  We are always allowed 
to send "spurious" notifications to the host, so it can't break anything.

Added to TODO.

Thanks,
Rusty.

      reply	other threads:[~2008-05-20  4:36 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-04-30 14:31 [PATCH] virtio_net: free transmit skbs in a timer Mark McLoughlin
2008-05-02 10:55 ` Rusty Russell
2008-05-12 20:37   ` Mark McLoughlin
2008-05-13  7:47     ` Avi Kivity
2008-05-14  6:07       ` Rusty Russell
2008-05-14  8:59         ` Avi Kivity
2008-05-15 15:29           ` Mark McLoughlin
2008-05-15 15:32             ` Avi Kivity
2008-05-15 23:25               ` Rusty Russell
2008-05-18  6:40                 ` Avi Kivity
2008-05-18 14:16                   ` Rusty Russell
2008-05-18 14:27                     ` Avi Kivity
2008-05-19  1:52                       ` Rusty Russell
2008-05-19 10:26                         ` Avi Kivity
2008-05-19 12:21                           ` Rusty Russell
2008-05-19 13:26                             ` Avi Kivity
2008-05-20  1:37                               ` Rusty Russell [this message]

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=200805201138.00435.rusty@rustcorp.com.au \
    --to=rusty@rustcorp.com.au \
    --cc=aliguori@us.ibm.com \
    --cc=avi@qumranet.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=markmc@redhat.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