Re: [PATCH] vhost-net: utilize PUBLISH_USED_IDX feature

2010-05-20 Thread Avi Kivity

On 05/20/2010 01:27 AM, Michael S. Tsirkin wrote:

On Wed, May 19, 2010 at 08:04:51PM +0300, Avi Kivity wrote:
   

On 05/18/2010 04:19 AM, Michael S. Tsirkin wrote:
 

With PUBLISH_USED_IDX, guest tells us which used entries
it has consumed. This can be used to reduce the number
of interrupts: after we write a used entry, if the guest has not yet
consumed the previous entry, or if the guest has already consumed the
new entry, we do not need to interrupt.
This imporves bandwidth by 30% under some workflows.

Signed-off-by: Michael S. Tsirkinm...@redhat.com
---

Rusty, Dave, this patch depends on the patch
virtio: put last seen used index into ring itself
which is currently destined at Rusty's tree.
Rusty, if you are taking that one for 2.6.35, please
take this one as well.
Dave, any objections?

   

I object: I think the index should have its own cacheline,
 

The issue here is that host/guest do not know each
other's cache line size. I guess we could just put it
at offset 128 or something like that ... Rusty?
   


Not so pretty, but ok.

   

and that it should be documented before merging.
 

I think you meant to object to the virtio patch, not this one.  This
patch does not introduce new layout, just implements host support.
virtio spec patch will follow: it is not part of linux tree so
there is no patch dependency.
   


There is a reviewer dependency.

--
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 to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] vhost-net: utilize PUBLISH_USED_IDX feature

2010-05-19 Thread Avi Kivity

On 05/18/2010 04:19 AM, Michael S. Tsirkin wrote:

With PUBLISH_USED_IDX, guest tells us which used entries
it has consumed. This can be used to reduce the number
of interrupts: after we write a used entry, if the guest has not yet
consumed the previous entry, or if the guest has already consumed the
new entry, we do not need to interrupt.
This imporves bandwidth by 30% under some workflows.

Signed-off-by: Michael S. Tsirkinm...@redhat.com
---

Rusty, Dave, this patch depends on the patch
virtio: put last seen used index into ring itself
which is currently destined at Rusty's tree.
Rusty, if you are taking that one for 2.6.35, please
take this one as well.
Dave, any objections?
   


I object: I think the index should have its own cacheline, and that it 
should be documented before merging.


--
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 to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] vhost-net: utilize PUBLISH_USED_IDX feature

2010-05-19 Thread Michael S. Tsirkin
On Wed, May 19, 2010 at 08:04:51PM +0300, Avi Kivity wrote:
 On 05/18/2010 04:19 AM, Michael S. Tsirkin wrote:
 With PUBLISH_USED_IDX, guest tells us which used entries
 it has consumed. This can be used to reduce the number
 of interrupts: after we write a used entry, if the guest has not yet
 consumed the previous entry, or if the guest has already consumed the
 new entry, we do not need to interrupt.
 This imporves bandwidth by 30% under some workflows.

 Signed-off-by: Michael S. Tsirkinm...@redhat.com
 ---

 Rusty, Dave, this patch depends on the patch
 virtio: put last seen used index into ring itself
 which is currently destined at Rusty's tree.
 Rusty, if you are taking that one for 2.6.35, please
 take this one as well.
 Dave, any objections?


 I object: I think the index should have its own cacheline,

The issue here is that host/guest do not know each
other's cache line size. I guess we could just put it
at offset 128 or something like that ... Rusty?

 and that it should be documented before merging.

I think you meant to object to the virtio patch, not this one.  This
patch does not introduce new layout, just implements host support.
virtio spec patch will follow: it is not part of linux tree so
there is no patch dependency.

 -- 
 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 to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] vhost-net: utilize PUBLISH_USED_IDX feature

2010-05-19 Thread Rusty Russell
On Thu, 20 May 2010 07:57:18 am Michael S. Tsirkin wrote:
 On Wed, May 19, 2010 at 08:04:51PM +0300, Avi Kivity wrote:
  On 05/18/2010 04:19 AM, Michael S. Tsirkin wrote:
  With PUBLISH_USED_IDX, guest tells us which used entries
  it has consumed. This can be used to reduce the number
  of interrupts: after we write a used entry, if the guest has not yet
  consumed the previous entry, or if the guest has already consumed the
  new entry, we do not need to interrupt.
  This imporves bandwidth by 30% under some workflows.
 
  Signed-off-by: Michael S. Tsirkinm...@redhat.com
  ---
 
  Rusty, Dave, this patch depends on the patch
  virtio: put last seen used index into ring itself
  which is currently destined at Rusty's tree.
  Rusty, if you are taking that one for 2.6.35, please
  take this one as well.
  Dave, any objections?
 
 
  I object: I think the index should have its own cacheline,
 
 The issue here is that host/guest do not know each
 other's cache line size. I guess we could just put it
 at offset 128 or something like that ... Rusty?

I was assuming you'd put it at the end of the padding.

I think it's a silly optimization, but Avi obviously feels strongly about
it and I respect his opinion.

Please resubmit...
Thanks,
Rusty.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] vhost-net: utilize PUBLISH_USED_IDX feature

2010-05-17 Thread David Miller
From: Michael S. Tsirkin m...@redhat.com
Date: Tue, 18 May 2010 04:19:31 +0300

 With PUBLISH_USED_IDX, guest tells us which used entries
 it has consumed. This can be used to reduce the number
 of interrupts: after we write a used entry, if the guest has not yet
 consumed the previous entry, or if the guest has already consumed the
 new entry, we do not need to interrupt.
 This imporves bandwidth by 30% under some workflows.
 
 Signed-off-by: Michael S. Tsirkin m...@redhat.com
 ---
 
 Rusty, Dave, this patch depends on the patch
 virtio: put last seen used index into ring itself
 which is currently destined at Rusty's tree.
 Rusty, if you are taking that one for 2.6.35, please
 take this one as well.
 Dave, any objections?

None:

Acked-by: David S. Miller da...@davemloft.net
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html