Re: [PATCH 1/2] kvm tools: Respect ISR status in virtio header

[Asias He <asias.hejun@gmail.com>]

On 05/07/2011 06:34 PM, Sasha Levin wrote:
> On Sat, 2011-05-07 at 11:30 +0200, Ingo Molnar wrote:
>> * Asias He <asias.hejun@gmail.com> wrote:
>>
>>> Inject IRQ to guest only when ISR status is low which means
>>> guest has read ISR status and device has cleared this bit as
>>> the side effect of this reading.
>>>
>>> This reduces a lot of unnecessary IRQ inject from device to
>>> guest.
>>>
>>> Netpef test shows this patch changes:
>>>
>>> the host to guest bandwidth
>>> from 2866.27 Mbps (cpu 33.96%) to 5548.87 Mbps (cpu 53.87%),
>>>
>>> the guest to host bandwitdth
>>> form 1408.86 Mbps (cpu 99.9%) to 1301.29 Mbps (cpu 99.9%).
>>>
>>> The bottleneck of the guest to host bandwidth is guest cpu power.
>>>
>>> Signed-off-by: Asias He <asias.hejun@gmail.com>
>>> ---
>> Hm, the ISR flag seems to be an explicit IRQ-ack mechanism, not just an 
>> optimization.
>>
>> Perhaps if the guest kernel side virtio driver expects us to do honor these 
>> acks and not inject double irqs when the virtio driver does not expect them?
>>
>> There's this code in drivers/virtio/virtio_pci.c:
>>
>>         /* reading the ISR has the effect of also clearing it so it's very
>>          * important to save off the value. */
>>         isr = ioread8(vp_dev->ioaddr + VIRTIO_PCI_ISR);
>>
>> Which seems to suggest that this ISR flag is more important than just a 
>> performance hint.
>>
>> Pekka: was this the patch perhaps that fixed the ping latency problem for you?
>>
>> Could any virtio gents on Cc: please confirm/deny this theory? :-)
>>
>> The original problem was that the virtio-net driver in tools/kvm/virtio/net.c 
>> was producing unexplained latencies (long ping latencies) under certain 
>> circumstances. Sometimes it triggered spontaneously, sometimes it needed a ping 
>> -f flood to trigger. The root cause of that race is still not understood.
> 
> Looks like it solved the ping -f issue here.
> 
> Why was this change only implemented in virtio-net? shouldn't it go to
> the other virtio drivers as well?

I will send follow up patchs to virtio-* ;-)


-- 
Best Regards,
Asias He