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

[Sasha Levin <levinsasha928@gmail.com>]

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?

-- 

Sasha.