All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Avi Kivity <avi@redhat.com>
Cc: Rusty Russell <rusty@rustcorp.com.au>,
	linux-kernel@vger.kernel.org,
	virtualization@lists.linux-foundation.org, kvm@vger.kernel.org,
	qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH RFC] virtio: put last seen used index into ring itself
Date: Thu, 20 May 2010 01:33:12 +0300	[thread overview]
Message-ID: <20100519223312.GC4111@redhat.com> (raw)
In-Reply-To: <4BF39C12.7090407@redhat.com>

On Wed, May 19, 2010 at 11:06:42AM +0300, Avi Kivity wrote:
> On 05/19/2010 10:39 AM, Rusty Russell wrote:
>>
>> I think we're talking about the last 2 entries of the avail ring.  That means
>> the worst case is 1 false bounce every time around the ring.
>
> It's low, but why introduce an inefficiency when you can avoid doing it  
> for the same effort?
> 
>> I think that's
>> why we're debating it instead of measuring it :)
>>    
>
> Measure before optimize is good for code but not for protocols.   
> Protocols have to be robust against future changes.  Virtio is warty  
> enough already, we can't keep doing local optimizations.
>
>> Note that this is a exclusive->shared->exclusive bounce only, too.
>>    
>
> A bounce is a bounce.
>
> Virtio is already way too bouncy due to the indirection between the  
> avail/used rings and the descriptor pool.  A device with out of order  
> completion (like virtio-blk) will quickly randomize the unused  
> descriptor indexes, so every descriptor fetch will require a bounce.
>
> In contrast, if the rings hold the descriptors themselves instead of  
> pointers, we bounce (sizeof(descriptor)/cache_line_size) cache lines for  
> every descriptor, amortized.

On the other hand, consider that on fast path we are never using all
of the ring. With a good allocator we might be able to keep
reusing only small part of the ring, instead of wrapping around
all of it all of the time.


> -- 
> Do not meddle in the internals of kernels, for they are subtle and quick to panic.

WARNING: multiple messages have this Message-ID (diff)
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Avi Kivity <avi@redhat.com>
Cc: qemu-devel@nongnu.org, Rusty Russell <rusty@rustcorp.com.au>,
	linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
	virtualization@lists.linux-foundation.org
Subject: Re: [Qemu-devel] [PATCH RFC] virtio: put last seen used index into ring itself
Date: Thu, 20 May 2010 01:33:12 +0300	[thread overview]
Message-ID: <20100519223312.GC4111@redhat.com> (raw)
In-Reply-To: <4BF39C12.7090407@redhat.com>

On Wed, May 19, 2010 at 11:06:42AM +0300, Avi Kivity wrote:
> On 05/19/2010 10:39 AM, Rusty Russell wrote:
>>
>> I think we're talking about the last 2 entries of the avail ring.  That means
>> the worst case is 1 false bounce every time around the ring.
>
> It's low, but why introduce an inefficiency when you can avoid doing it  
> for the same effort?
> 
>> I think that's
>> why we're debating it instead of measuring it :)
>>    
>
> Measure before optimize is good for code but not for protocols.   
> Protocols have to be robust against future changes.  Virtio is warty  
> enough already, we can't keep doing local optimizations.
>
>> Note that this is a exclusive->shared->exclusive bounce only, too.
>>    
>
> A bounce is a bounce.
>
> Virtio is already way too bouncy due to the indirection between the  
> avail/used rings and the descriptor pool.  A device with out of order  
> completion (like virtio-blk) will quickly randomize the unused  
> descriptor indexes, so every descriptor fetch will require a bounce.
>
> In contrast, if the rings hold the descriptors themselves instead of  
> pointers, we bounce (sizeof(descriptor)/cache_line_size) cache lines for  
> every descriptor, amortized.

On the other hand, consider that on fast path we are never using all
of the ring. With a good allocator we might be able to keep
reusing only small part of the ring, instead of wrapping around
all of it all of the time.


> -- 
> Do not meddle in the internals of kernels, for they are subtle and quick to panic.

  parent reply	other threads:[~2010-05-19 22:37 UTC|newest]

Thread overview: 95+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-05-05 20:58 [PATCH RFC] virtio: put last seen used index into ring itself Michael S. Tsirkin
2010-05-05 20:58 ` [Qemu-devel] " Michael S. Tsirkin
2010-05-05 21:18 ` Dor Laor
2010-05-05 21:18   ` [Qemu-devel] " Dor Laor
2010-05-05 21:18 ` Dor Laor
2010-05-06  2:31 ` Rusty Russell
2010-05-06  2:31   ` [Qemu-devel] " Rusty Russell
2010-05-06  6:19   ` Michael S. Tsirkin
2010-05-06  6:19   ` Michael S. Tsirkin
2010-05-06  6:19     ` [Qemu-devel] " Michael S. Tsirkin
2010-05-07  3:33     ` Rusty Russell
2010-05-07  3:33       ` [Qemu-devel] " Rusty Russell
2010-05-07  3:33       ` Rusty Russell
2010-05-09 21:06       ` Michael S. Tsirkin
2010-05-09 21:06         ` [Qemu-devel] " Michael S. Tsirkin
2010-05-09 21:06       ` Michael S. Tsirkin
2010-05-06  2:31 ` Rusty Russell
2010-05-06 10:00 ` [Qemu-devel] " Avi Kivity
2010-05-06 10:00 ` Avi Kivity
2010-05-06 10:00   ` Avi Kivity
2010-05-07  3:23   ` Rusty Russell
2010-05-07  3:23     ` Rusty Russell
2010-05-07  3:23     ` Rusty Russell
2010-05-11 19:27     ` Avi Kivity
2010-05-11 19:27     ` Avi Kivity
2010-05-11 19:27       ` Avi Kivity
2010-05-11 19:52       ` Michael S. Tsirkin
2010-05-11 19:52         ` Michael S. Tsirkin
2010-05-11 19:52         ` Michael S. Tsirkin
2010-05-19  7:39       ` Rusty Russell
2010-05-19  7:39       ` Rusty Russell
2010-05-19  7:39         ` Rusty Russell
2010-05-19  8:06         ` Avi Kivity
2010-05-19  8:06           ` Avi Kivity
2010-05-19 22:33           ` Michael S. Tsirkin
2010-05-19 22:33           ` Michael S. Tsirkin [this message]
2010-05-19 22:33             ` Michael S. Tsirkin
2010-05-20  6:04             ` Avi Kivity
2010-05-20  6:04             ` Avi Kivity
2010-05-20  6:04               ` Avi Kivity
2010-05-20  5:01           ` Rusty Russell
2010-05-20  5:01             ` Rusty Russell
2010-05-20  5:08             ` Rusty Russell
2010-05-20  5:08               ` Rusty Russell
2010-05-23 15:31               ` Michael S. Tsirkin
2010-05-23 15:31               ` Michael S. Tsirkin
2010-05-23 15:31                 ` Michael S. Tsirkin
2010-05-23 15:41                 ` Avi Kivity
2010-05-23 15:41                   ` Avi Kivity
2010-05-23 15:51                   ` Michael S. Tsirkin
2010-05-23 15:51                   ` Michael S. Tsirkin
2010-05-23 15:51                     ` Michael S. Tsirkin
2010-05-23 16:03                     ` Avi Kivity
2010-05-23 16:03                       ` Avi Kivity
2010-05-23 16:30                       ` Michael S. Tsirkin
2010-05-23 16:30                       ` Michael S. Tsirkin
2010-05-23 16:30                         ` Michael S. Tsirkin
2010-05-24  6:37                         ` Avi Kivity
2010-05-24  6:37                         ` Avi Kivity
2010-05-24  6:37                           ` Avi Kivity
2010-05-24  8:05                           ` Michael S. Tsirkin
2010-05-24  8:05                             ` Michael S. Tsirkin
2010-05-24 11:00                             ` Avi Kivity
2010-05-24 11:00                               ` Avi Kivity
2010-05-24 11:00                             ` Avi Kivity
2010-05-24  8:05                           ` Michael S. Tsirkin
2010-05-23 17:28                       ` Michael S. Tsirkin
2010-05-23 17:28                         ` Michael S. Tsirkin
2010-05-23 17:28                       ` Michael S. Tsirkin
2010-05-23 16:03                     ` Avi Kivity
2010-05-23 15:41                 ` Avi Kivity
2010-05-23 15:56               ` Michael S. Tsirkin
2010-05-23 15:56               ` Michael S. Tsirkin
2010-05-23 15:56                 ` Michael S. Tsirkin
2010-05-20  5:08             ` Rusty Russell
2010-05-20  7:00             ` Avi Kivity
2010-05-20  7:00               ` Avi Kivity
2010-05-20 14:34               ` Rusty Russell
2010-05-20 14:34               ` Rusty Russell
2010-05-20 14:34                 ` Rusty Russell
2010-05-20 15:46                 ` Avi Kivity
2010-05-20 15:46                 ` Avi Kivity
2010-05-20 15:46                   ` Avi Kivity
2010-05-20  7:00             ` Avi Kivity
2010-05-20 10:04             ` Michael S. Tsirkin
2010-05-20 10:04               ` Michael S. Tsirkin
2010-05-20 10:04             ` Michael S. Tsirkin
2010-05-20  5:01           ` Rusty Russell
2010-05-07  3:23   ` Rusty Russell
2010-05-11 18:46 ` Ryan Harper
2010-05-11 18:46 ` Ryan Harper
2010-05-11 18:46   ` [Qemu-devel] " Ryan Harper
2010-05-11 19:48   ` Michael S. Tsirkin
2010-05-11 19:48     ` [Qemu-devel] " Michael S. Tsirkin
2010-05-11 19:48   ` 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=20100519223312.GC4111@redhat.com \
    --to=mst@redhat.com \
    --cc=avi@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=qemu-devel@nongnu.org \
    --cc=rusty@rustcorp.com.au \
    --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.