All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rusty Russell <rusty@rustcorp.com.au>
To: "Michael S. Tsirkin" <mst@redhat.com>,
	Thomas Lendacky <tahm@linux.vnet.ibm.com>
Cc: Sasha Levin <levinsasha928@gmail.com>,
	virtualization@lists.linux-foundation.org,
	linux-kernel@vger.kernel.org, avi@redhat.com,
	kvm@vger.kernel.org, netdev@vger.kernel.org
Subject: Re: [PATCH 2/3] virtio-net: correct capacity math on ring full
Date: Thu, 04 Oct 2012 09:54:01 +0930	[thread overview]
Message-ID: <878vbn2ify.fsf@rustcorp.com.au> (raw)
In-Reply-To: <f2f7123534aefb33a7a233d9fec56de6e9eeed73.1348824232.git.mst@redhat.com>

"Michael S. Tsirkin" <mst@redhat.com> writes:
> 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 path will reduce the actual queue use to worst-case assumptions.
With bufferbloat maybe that's a good thing, but it's true.

If we do this, the code is now wrong:

	/* This can happen with OOM and indirect buffers. */
	if (unlikely(capacity < 0)) {

Because this should now *never* happen.

But I do like the cleanup; returning capacity from add_buf() was always
hacky.  I've got an idea, we'll see what it looks like...

Cheers,
Rusty.

  reply	other threads:[~2012-10-04  1:24 UTC|newest]

Thread overview: 48+ 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 ` 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
2012-09-28  9:26 ` [PATCH 2/3] virtio-net: correct capacity math on ring full Michael S. Tsirkin
2012-09-28  9:26   ` Michael S. Tsirkin
2012-10-04  0:24   ` Rusty Russell [this message]
2012-09-28  9:26 ` [PATCH 3/3] virtio-net: put virtio net header inline with data Michael S. Tsirkin
2012-09-28  9:26   ` Michael S. Tsirkin
2012-10-03  6:44 ` [PATCH 0/3] virtio-net: inline header support Rusty Russell
2012-10-03 10:53   ` Paolo Bonzini
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
2012-10-06 12:54               ` Paolo Bonzini
2012-10-06 12:54                 ` Paolo Bonzini
2012-10-09  4:59                 ` Rusty Russell
2012-10-09  4:59                   ` Rusty Russell
2012-10-09  7:27                   ` Paolo Bonzini
2012-10-09  7:27                     ` Paolo Bonzini
2012-10-11  0:03                     ` Rusty Russell
2012-10-11  0:03                       ` Rusty Russell
2012-10-11 11:04                       ` Michael S. Tsirkin
2012-10-11 11:04                         ` Michael S. Tsirkin
2012-10-11 22:37                         ` Rusty Russell
2012-10-11 22:37                           ` Rusty Russell
2012-10-12  7:38                           ` Paolo Bonzini
2012-10-12  7:38                             ` Paolo Bonzini
2012-10-12 11:52                           ` Cornelia Huck
2012-10-12 11:52                             ` Cornelia Huck
2012-10-05  5:43             ` Rusty Russell
2012-10-03  6:44 ` 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-05  7:47             ` Paolo Bonzini
2012-10-08 21:31       ` Michael S. Tsirkin
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
2012-10-08 20:41     ` 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=878vbn2ify.fsf@rustcorp.com.au \
    --to=rusty@rustcorp.com.au \
    --cc=avi@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=levinsasha928@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mst@redhat.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.