question on virtio

question on virtio

[Michael S. Tsirkin]

Hi!
I see this in virtio_ring.c:

        /* Put entry in available array (but don't update avail->idx *
	   until they do sync). */

Why is it done this way?
It seems that updating the index straight away would be simpler, while
this might allow the host to specilatively look up the buffer and handle
it, without waiting for the kick.

-- 
MST

[Qemu-devel] question on virtio

[Michael S. Tsirkin]

Hi!
I see this in virtio_ring.c:

        /* Put entry in available array (but don't update avail->idx *
	   until they do sync). */

Why is it done this way?
It seems that updating the index straight away would be simpler, while
this might allow the host to specilatively look up the buffer and handle
it, without waiting for the kick.

-- 
MST

question on virtio

[Michael S. Tsirkin]

Hi!
I see this in virtio_ring.c:

        /* Put entry in available array (but don't update avail->idx *
	   until they do sync). */

Why is it done this way?
It seems that updating the index straight away would be simpler, while
this might allow the host to specilatively look up the buffer and handle
it, without waiting for the kick.

-- 
MST

Re: question on virtio

[Anthony Liguori]

On 05/05/2010 06:09 AM, Michael S. Tsirkin wrote:
> Hi!
> I see this in virtio_ring.c:
>
>          /* Put entry in available array (but don't update avail->idx *
> 	   until they do sync). */
>
> Why is it done this way?
> It seems that updating the index straight away would be simpler, while
> this might allow the host to specilatively look up the buffer and handle
> it, without waiting for the kick.
>    

It should be okay as long as you don't update idx for partial vectors.

Regards,

Anthony Liguori

Re: question on virtio

[Anthony Liguori]

On 05/05/2010 06:09 AM, Michael S. Tsirkin wrote:
> Hi!
> I see this in virtio_ring.c:
>
>          /* Put entry in available array (but don't update avail->idx *
> 	   until they do sync). */
>
> Why is it done this way?
> It seems that updating the index straight away would be simpler, while
> this might allow the host to specilatively look up the buffer and handle
> it, without waiting for the kick.
>    

It should be okay as long as you don't update idx for partial vectors.

Regards,

Anthony Liguori

[Qemu-devel] Re: question on virtio

[Anthony Liguori]

On 05/05/2010 06:09 AM, Michael S. Tsirkin wrote:
> Hi!
> I see this in virtio_ring.c:
>
>          /* Put entry in available array (but don't update avail->idx *
> 	   until they do sync). */
>
> Why is it done this way?
> It seems that updating the index straight away would be simpler, while
> this might allow the host to specilatively look up the buffer and handle
> it, without waiting for the kick.
>    

It should be okay as long as you don't update idx for partial vectors.

Regards,

Anthony Liguori

Re: question on virtio

[Michael S. Tsirkin]

On Wed, May 05, 2010 at 02:40:15PM -0500, Anthony Liguori wrote:
> On 05/05/2010 06:09 AM, Michael S. Tsirkin wrote:
>> Hi!
>> I see this in virtio_ring.c:
>>
>>          /* Put entry in available array (but don't update avail->idx *
>> 	   until they do sync). */
>>
>> Why is it done this way?
>> It seems that updating the index straight away would be simpler, while
>> this might allow the host to specilatively look up the buffer and handle
>> it, without waiting for the kick.
>>    
>
> It should be okay as long as you don't update idx for partial vectors.
>
> Regards,
>
> Anthony Liguori

Sorry, what do you mean by partial vectors here?

Re: question on virtio

[Michael S. Tsirkin]

On Wed, May 05, 2010 at 02:40:15PM -0500, Anthony Liguori wrote:
> On 05/05/2010 06:09 AM, Michael S. Tsirkin wrote:
>> Hi!
>> I see this in virtio_ring.c:
>>
>>          /* Put entry in available array (but don't update avail->idx *
>> 	   until they do sync). */
>>
>> Why is it done this way?
>> It seems that updating the index straight away would be simpler, while
>> this might allow the host to specilatively look up the buffer and handle
>> it, without waiting for the kick.
>>    
>
> It should be okay as long as you don't update idx for partial vectors.
>
> Regards,
>
> Anthony Liguori

Sorry, what do you mean by partial vectors here?

[Qemu-devel] Re: question on virtio

[Michael S. Tsirkin]

On Wed, May 05, 2010 at 02:40:15PM -0500, Anthony Liguori wrote:
> On 05/05/2010 06:09 AM, Michael S. Tsirkin wrote:
>> Hi!
>> I see this in virtio_ring.c:
>>
>>          /* Put entry in available array (but don't update avail->idx *
>> 	   until they do sync). */
>>
>> Why is it done this way?
>> It seems that updating the index straight away would be simpler, while
>> this might allow the host to specilatively look up the buffer and handle
>> it, without waiting for the kick.
>>    
>
> It should be okay as long as you don't update idx for partial vectors.
>
> Regards,
>
> Anthony Liguori

Sorry, what do you mean by partial vectors here?

Re: question on virtio

[Rusty Russell]

On Wed, 5 May 2010 08:39:47 pm Michael S. Tsirkin wrote:
> Hi!
> I see this in virtio_ring.c:
> 
>         /* Put entry in available array (but don't update avail->idx *
> 	   until they do sync). */
> 
> Why is it done this way?
> It seems that updating the index straight away would be simpler, while
> this might allow the host to specilatively look up the buffer and handle
> it, without waiting for the kick.

I agree.  From my TODO:
        what if we actually expose in ->add_buf?

I don't *think* anyone adds buffers without being ready for them to be used,
so changing this should be safe.

Want to give it a try and report back?

Thanks!
Rusty.

Re: question on virtio

[Rusty Russell]

On Wed, 5 May 2010 08:39:47 pm Michael S. Tsirkin wrote:
> Hi!
> I see this in virtio_ring.c:
> 
>         /* Put entry in available array (but don't update avail->idx *
> 	   until they do sync). */
> 
> Why is it done this way?
> It seems that updating the index straight away would be simpler, while
> this might allow the host to specilatively look up the buffer and handle
> it, without waiting for the kick.

I agree.  From my TODO:
        what if we actually expose in ->add_buf?

I don't *think* anyone adds buffers without being ready for them to be used,
so changing this should be safe.

Want to give it a try and report back?

Thanks!
Rusty.

[Qemu-devel] Re: question on virtio

[Rusty Russell]

On Wed, 5 May 2010 08:39:47 pm Michael S. Tsirkin wrote:
> Hi!
> I see this in virtio_ring.c:
> 
>         /* Put entry in available array (but don't update avail->idx *
> 	   until they do sync). */
> 
> Why is it done this way?
> It seems that updating the index straight away would be simpler, while
> this might allow the host to specilatively look up the buffer and handle
> it, without waiting for the kick.

I agree.  From my TODO:
        what if we actually expose in ->add_buf?

I don't *think* anyone adds buffers without being ready for them to be used,
so changing this should be safe.

Want to give it a try and report back?

Thanks!
Rusty.

Re: [Qemu-devel] question on virtio

[Jamie Lokier]

Michael S. Tsirkin wrote:
> Hi!
> I see this in virtio_ring.c:
> 
>         /* Put entry in available array (but don't update avail->idx *
> 	   until they do sync). */
> 
> Why is it done this way?
> It seems that updating the index straight away would be simpler, while
> this might allow the host to specilatively look up the buffer and handle
> it, without waiting for the kick.

Even better, if the host updates a location containing which index it
has seen recently, you can avoid the kick entirely during sustained
flows - just like your recent patch to avoid sending irqs to the
guest.

-- Jamie

Re: [Qemu-devel] question on virtio

[Jamie Lokier]

Michael S. Tsirkin wrote:
> Hi!
> I see this in virtio_ring.c:
> 
>         /* Put entry in available array (but don't update avail->idx *
> 	   until they do sync). */
> 
> Why is it done this way?
> It seems that updating the index straight away would be simpler, while
> this might allow the host to specilatively look up the buffer and handle
> it, without waiting for the kick.

Even better, if the host updates a location containing which index it
has seen recently, you can avoid the kick entirely during sustained
flows - just like your recent patch to avoid sending irqs to the
guest.

-- Jamie

Re: [Qemu-devel] question on virtio

[Jamie Lokier]

Michael S. Tsirkin wrote:
> Hi!
> I see this in virtio_ring.c:
> 
>         /* Put entry in available array (but don't update avail->idx *
> 	   until they do sync). */
> 
> Why is it done this way?
> It seems that updating the index straight away would be simpler, while
> this might allow the host to specilatively look up the buffer and handle
> it, without waiting for the kick.

Even better, if the host updates a location containing which index it
has seen recently, you can avoid the kick entirely during sustained
flows - just like your recent patch to avoid sending irqs to the
guest.

-- Jamie