Re: [Qemu-devel] Re: [RFC][PATCH] performance improvement for windows guests, running on top of virtio block device

[Avi Kivity <avi@redhat.com>]

On 01/11/2010 05:13 PM, Anthony Liguori wrote:
> On 01/11/2010 08:46 AM, Avi Kivity wrote:
>> On 01/11/2010 04:37 PM, Anthony Liguori wrote:
>>>> That has the downside of bouncing a cache line on unrelated exits.
>>>
>>>
>>> The read and write sides of the ring are widely separated in 
>>> physical memory specifically to avoid cache line bouncing.
>>
>> I meant, exits on random vcpus will cause the cacheline containing 
>> the notification disable flag to bounce around.  As it is, we read it 
>> on the vcpu that owns the queue and write it on that vcpu or the I/O 
>> thread.
>
> Bottom halves are always run from the IO thread.

Okay, so that won't be an issue.

>>>>   It probably doesn't matter with qemu as it is now, since it will 
>>>> bounce qemu_mutex, but it will hurt with large guests (especially 
>>>> if they have many rings).
>>>>
>>>> IMO we should get things to work well without riding on unrelated 
>>>> exits, especially as we're trying to reduce those exits.
>>>
>>> A block I/O request can potentially be very, very long lived.  By 
>>> serializing requests like this, there's a high likelihood that it's 
>>> going to kill performance with anything capable of processing 
>>> multiple requests.
>>
>> Right, that's why I suggested having a queue depth at which disabling 
>> notification kicks in.  The patch hardcodes this depth to 1, 
>> unpatched qemu is infinite, a good value is probably spindle count + 
>> VAT.
>
> That means we would need a user visible option which is quite 
> unfortunate.

We could guess it, perhaps.

> Also, that logic only really makes sense with cache=off.  With 
> cache=writethrough, you can get pathological cases whereas you have an 
> uncached access followed by cached accesses.  In fact, with 
> read-ahead, this is probably not an uncommon scenario.

So you'd increase the disable depths when cache!=none.

>>> OTOH, if we aggressively poll the ring when we have an opportunity 
>>> to, there's very little down side to that and it addresses the 
>>> serialization problem.
>>
>> But we can't guarantee that we'll get those opportunities, so it 
>> doesn't address the problem in a general way.  A guest that doesn't 
>> use hpet and only has a single virtio-blk device will not have any 
>> reason to exit to qemu.
>
> We can mitigate this with a timer but honestly, we need to do perf 
> measurements to see.  My feeling is that we will need some more 
> aggressive form of polling than just waiting for IO completion.  I 
> don't think queue depth is enough because it assumes that all requests 
> are equal.  When dealing with cache=off or even just storage with it's 
> own cache, that's simply not the case.

Maybe we can adapt behaviour dynamically based on how fast results come in.

-- 
error compiling committee.c: too many arguments to function