From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sasha Levin Subject: Re: [PATCH 1/2] kvm tools: Respect ISR status in virtio header Date: Sat, 07 May 2011 13:34:00 +0300 Message-ID: <1304764440.10534.22.camel@lappy> References: <1304735660-10844-1-git-send-email-asias.hejun@gmail.com> <20110507093027.GD27657@elte.hu> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: Asias He , "Michael S. Tsirkin" , Rusty Russell , Mark McLoughlin , Anthony Liguori , Pekka Enberg , Cyrill Gorcunov , Prasad Joshi , kvm@vger.kernel.org To: Ingo Molnar Return-path: Received: from mail-ww0-f44.google.com ([74.125.82.44]:42047 "EHLO mail-ww0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754486Ab1EGKeM (ORCPT ); Sat, 7 May 2011 06:34:12 -0400 Received: by wwa36 with SMTP id 36so4281590wwa.1 for ; Sat, 07 May 2011 03:34:10 -0700 (PDT) In-Reply-To: <20110507093027.GD27657@elte.hu> Sender: kvm-owner@vger.kernel.org List-ID: On Sat, 2011-05-07 at 11:30 +0200, Ingo Molnar wrote: > * Asias He 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 > > --- > 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.