Re: [Qemu-devel] [PATCH RFC] virtio: put last seen used index into ring itself

2010-05-24 Thread Avi Kivity
On 05/23/2010 07:30 PM, Michael S. Tsirkin wrote: Maybe we should use atomics on index then? This should only be helpful if you access the cacheline several times in a row. That's not the case in virtio (or here). So why does it help? We actually do access the

Re: [Qemu-devel] [PATCH RFC] virtio: put last seen used index into ring itself

2010-05-24 Thread Michael S. Tsirkin
On Mon, May 24, 2010 at 09:37:05AM +0300, Avi Kivity wrote: On 05/23/2010 07:30 PM, Michael S. Tsirkin wrote: Maybe we should use atomics on index then? This should only be helpful if you access the cacheline several times in a row. That's not the case in virtio (or here).

Re: [Qemu-devel] [PATCH RFC] virtio: put last seen used index into ring itself

2010-05-24 Thread Avi Kivity
On 05/24/2010 11:05 AM, Michael S. Tsirkin wrote: Okay, but why is lockunshare faster than unshare? No idea. -- Do not meddle in the internals of kernels, for they are subtle and quick to panic. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message

Re: [Qemu-devel] [PATCH RFC] virtio: put last seen used index into ring itself

2010-05-23 Thread Michael S. Tsirkin
On Thu, May 20, 2010 at 02:38:16PM +0930, Rusty Russell wrote: On Thu, 20 May 2010 02:31:50 pm Rusty Russell wrote: On Wed, 19 May 2010 05:36:42 pm Avi Kivity wrote: Note that this is a exclusive-shared-exclusive bounce only, too. A bounce is a bounce. I tried to measure

Re: [Qemu-devel] [PATCH RFC] virtio: put last seen used index into ring itself

2010-05-23 Thread Avi Kivity
On 05/23/2010 06:31 PM, Michael S. Tsirkin wrote: On Thu, May 20, 2010 at 02:38:16PM +0930, Rusty Russell wrote: On Thu, 20 May 2010 02:31:50 pm Rusty Russell wrote: On Wed, 19 May 2010 05:36:42 pm Avi Kivity wrote: Note that this is a exclusive-shared-exclusive bounce only,

Re: [Qemu-devel] [PATCH RFC] virtio: put last seen used index into ring itself

2010-05-23 Thread Michael S. Tsirkin
On Sun, May 23, 2010 at 06:41:33PM +0300, Avi Kivity wrote: On 05/23/2010 06:31 PM, Michael S. Tsirkin wrote: On Thu, May 20, 2010 at 02:38:16PM +0930, Rusty Russell wrote: On Thu, 20 May 2010 02:31:50 pm Rusty Russell wrote: On Wed, 19 May 2010 05:36:42 pm Avi Kivity wrote:

Re: [Qemu-devel] [PATCH RFC] virtio: put last seen used index into ring itself

2010-05-23 Thread Michael S. Tsirkin
On Thu, May 20, 2010 at 02:38:16PM +0930, Rusty Russell wrote: On Thu, 20 May 2010 02:31:50 pm Rusty Russell wrote: On Wed, 19 May 2010 05:36:42 pm Avi Kivity wrote: Note that this is a exclusive-shared-exclusive bounce only, too. A bounce is a bounce. I tried to measure

Re: [Qemu-devel] [PATCH RFC] virtio: put last seen used index into ring itself

2010-05-23 Thread Avi Kivity
On 05/23/2010 06:51 PM, Michael S. Tsirkin wrote: So locked version seems to be faster than unlocked, and share/unshare not to matter? May be due to the processor using the LOCK operation as a hint to reserve the cacheline for a bit. Maybe we should use atomics on index then?

Re: [Qemu-devel] [PATCH RFC] virtio: put last seen used index into ring itself

2010-05-23 Thread Michael S. Tsirkin
On Sun, May 23, 2010 at 07:03:10PM +0300, Avi Kivity wrote: On 05/23/2010 06:51 PM, Michael S. Tsirkin wrote: So locked version seems to be faster than unlocked, and share/unshare not to matter? May be due to the processor using the LOCK operation as a hint to reserve the cacheline

Re: [Qemu-devel] [PATCH RFC] virtio: put last seen used index into ring itself

2010-05-23 Thread Michael S. Tsirkin
On Sun, May 23, 2010 at 07:03:10PM +0300, Avi Kivity wrote: On 05/23/2010 06:51 PM, Michael S. Tsirkin wrote: So locked version seems to be faster than unlocked, and share/unshare not to matter? May be due to the processor using the LOCK operation as a hint to reserve the cacheline

Re: [Qemu-devel] [PATCH RFC] virtio: put last seen used index into ring itself

2010-05-20 Thread Avi Kivity
On 05/20/2010 01:33 AM, Michael S. Tsirkin wrote: 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

Re: [Qemu-devel] [PATCH RFC] virtio: put last seen used index into ring itself

2010-05-20 Thread Avi Kivity
On 05/20/2010 08:01 AM, Rusty Russell wrote: 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

Re: [Qemu-devel] [PATCH RFC] virtio: put last seen used index into ring itself

2010-05-20 Thread Michael S. Tsirkin
On Thu, May 20, 2010 at 02:31:50PM +0930, Rusty Russell wrote: Can we do better? The obvious idea is to try to get rid of last_used and used, and use the ring itself. We would use an invalid entry to mark the head of the ring. Any other thoughts? Rusty. We also need a way to avoid

Re: [Qemu-devel] [PATCH RFC] virtio: put last seen used index into ring itself

2010-05-20 Thread Rusty Russell
On Thu, 20 May 2010 04:30:56 pm Avi Kivity wrote: On 05/20/2010 08:01 AM, Rusty Russell wrote: 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

Re: [Qemu-devel] [PATCH RFC] virtio: put last seen used index into ring itself

2010-05-20 Thread Avi Kivity
On 05/20/2010 05:34 PM, Rusty Russell wrote: Have just one ring, no indexes. The producer places descriptors into the ring and updates the head, The consumer copies out descriptors to be processed and copies back in completed descriptors. Chaining is always linear. The descriptors contain

Re: [Qemu-devel] [PATCH RFC] virtio: put last seen used index into ring itself

2010-05-19 Thread Rusty Russell
On Wed, 12 May 2010 04:57:22 am Avi Kivity wrote: On 05/07/2010 06:23 AM, Rusty Russell wrote: On Thu, 6 May 2010 07:30:00 pm Avi Kivity wrote: On 05/05/2010 11:58 PM, Michael S. Tsirkin wrote: + /* We publish the last-seen used index at the end of the available ring. + *

Re: [Qemu-devel] [PATCH RFC] virtio: put last seen used index into ring itself

2010-05-19 Thread Avi Kivity
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

Re: [Qemu-devel] [PATCH RFC] virtio: put last seen used index into ring itself

2010-05-19 Thread Michael S. Tsirkin
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

Re: [Qemu-devel] [PATCH RFC] virtio: put last seen used index into ring itself

2010-05-19 Thread Rusty Russell
On Wed, 19 May 2010 05:36:42 pm Avi Kivity wrote: Note that this is a exclusive-shared-exclusive bounce only, too. A bounce is a bounce. I tried to measure this to show that you were wrong, but I was only able to show that you're right. How annoying. Test code below. Virtio is

Re: [Qemu-devel] [PATCH RFC] virtio: put last seen used index into ring itself

2010-05-19 Thread Rusty Russell
On Thu, 20 May 2010 02:31:50 pm Rusty Russell wrote: On Wed, 19 May 2010 05:36:42 pm Avi Kivity wrote: Note that this is a exclusive-shared-exclusive bounce only, too. A bounce is a bounce. I tried to measure this to show that you were wrong, but I was only able to show that

Re: [Qemu-devel] [PATCH RFC] virtio: put last seen used index into ring itself

2010-05-11 Thread Michael S. Tsirkin
On Tue, May 11, 2010 at 10:27:22PM +0300, Avi Kivity wrote: On 05/07/2010 06:23 AM, Rusty Russell wrote: On Thu, 6 May 2010 07:30:00 pm Avi Kivity wrote: On 05/05/2010 11:58 PM, Michael S. Tsirkin wrote: + /* We publish the last-seen used index at the end of the available ring. +

Re: [Qemu-devel] [PATCH RFC] virtio: put last seen used index into ring itself

2010-05-06 Thread Avi Kivity
On 05/05/2010 11:58 PM, Michael S. Tsirkin wrote: + /* We publish the last-seen used index at the end of the available ring. +* It is at the end for backwards compatibility. */ + vr-last_used_idx =(vr)-avail-ring[num]; + /* Verify that last used index does not spill

Re: [Qemu-devel] [PATCH RFC] virtio: put last seen used index into ring itself

2010-05-06 Thread Rusty Russell
On Thu, 6 May 2010 07:30:00 pm Avi Kivity wrote: On 05/05/2010 11:58 PM, Michael S. Tsirkin wrote: + /* We publish the last-seen used index at the end of the available ring. +* It is at the end for backwards compatibility. */ + vr-last_used_idx =(vr)-avail-ring[num]; + /* Verify