From mboxrd@z Thu Jan 1 00:00:00 1970 From: Asias He Subject: Re: [PATCH 1/2] kvm tools: Respect ISR status in virtio header Date: Sat, 07 May 2011 18:39:25 +0800 Message-ID: <4DC5215D.9020207@gmail.com> References: <1304735660-10844-1-git-send-email-asias.hejun@gmail.com> <20110507093027.GD27657@elte.hu> <1304764440.10534.22.camel@lappy> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: Ingo Molnar , "Michael S. Tsirkin" , Rusty Russell , Mark McLoughlin , Anthony Liguori , Pekka Enberg , Cyrill Gorcunov , Prasad Joshi , kvm@vger.kernel.org To: Sasha Levin Return-path: Received: from mail-px0-f173.google.com ([209.85.212.173]:47205 "EHLO mail-px0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751636Ab1EGKkk (ORCPT ); Sat, 7 May 2011 06:40:40 -0400 Received: by pxi16 with SMTP id 16so2741122pxi.4 for ; Sat, 07 May 2011 03:40:39 -0700 (PDT) In-Reply-To: <1304764440.10534.22.camel@lappy> Sender: kvm-owner@vger.kernel.org List-ID: On 05/07/2011 06:34 PM, Sasha Levin wrote: > 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? I will send follow up patchs to virtio-* ;-) -- Best Regards, Asias He