On Mon, Jan 11, 2010 at 09:13:23AM -0600, 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.
>>>>   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.
>
> 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.
>
>>> 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.
>
> Regards,
>
> Anthony Liguori
>

BTW this is why vhost net uses queue depth in bytes.

-- 
MST


Reply via email to