From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Michael S. Tsirkin" Subject: Re: [Qemu-devel] [PATCH] vhost: force vhost off for non-MSI guests Date: Fri, 21 Jan 2011 11:48:27 +0200 Message-ID: <20110121094827.GA26070@redhat.com> References: <20110120153521.GA24357@redhat.com> <4D38583D.9010602@codemonkey.ws> <20110120160718.GA24652@redhat.com> <4D38D208.4090403@codemonkey.ws> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvm@vger.kernel.org, Juan Quintela , Jes.Sorensen@redhat.com, jasowang@redhat.com, qemu-devel@nongnu.org, Alex Williamson To: Anthony Liguori Return-path: Received: from mx1.redhat.com ([209.132.183.28]:35511 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750710Ab1AUJsr (ORCPT ); Fri, 21 Jan 2011 04:48:47 -0500 Content-Disposition: inline In-Reply-To: <4D38D208.4090403@codemonkey.ws> Sender: kvm-owner@vger.kernel.org List-ID: On Thu, Jan 20, 2011 at 06:23:36PM -0600, Anthony Liguori wrote: > On 01/20/2011 10:07 AM, Michael S. Tsirkin wrote: > >On Thu, Jan 20, 2011 at 09:43:57AM -0600, Anthony Liguori wrote: > >>On 01/20/2011 09:35 AM, Michael S. Tsirkin wrote: > >>>When MSI is off, each interrupt needs to be bounced through the io > >>>thread when it's set/cleared, so vhost-net causes more context switches and > >>>higher CPU utilization than userspace virtio which handles networking in > >>>the same thread. > >>> > >>>We'll need to fix this by adding level irq support in kvm irqfd, > >>>for now disable vhost-net in these configurations. > >>> > >>>Signed-off-by: Michael S. Tsirkin > >>I actually think this should be a terminal error. The user asks for > >>vhost-net, if we cannot enable it, we should exit. > >> > >>Or we should warn the user that they should expect bad performance. > >>Silently doing something that the user has explicitly asked us not > >>to do is not a good behavior. > >> > >>Regards, > >> > >>Anthony Liguori > >The issue is that user has no control of the guest, and can not know > >whether the guest enables MSI. So what you ask for will just make > >some guests fail, and others fail sometimes. > >The user also has no way to know that version X of kvm does not expose a > >way to inject level interrupts with irqfd. > > > >We could have *another* flag that says "use vhost where it helps" but > >then I think this is what everyone wants to do, anyway, and libvirt > >already sets vhost=on so I prefer redefining the meaning of an existing > >flag. > > In the very least, there needs to be a vhost=force. > Having some sort of friendly default policy is fine but we need to > provide a mechanism for a user to have the final say. If you want > to redefine vhost=on to really mean, use the friendly default, > that's fine by me, but only if the vhost=force option exists. OK, I will add that, probably as a separate flag as vhost is a boolean. This will get worse performance but it will be what the user asked for. > > I actually would think libvirt would want to use vhost=force. > Debugging with vhost=on is going to be a royal pain in the ass if a > user reports bad performance. Given the libvirt XML, you can't > actually tell from the guest and the XML whether or not vhost was > actually in use or not. Yes you can: check MSI enabled in the guest, if it is - check vhost enabled in the XML. Not that bad at all, is it? > > Regards, > > Anthony Liguori We get worse performance without MSI anyway, how is this different? > >Maybe this is best handled by a documentation update? > > > >We always said: > > " use vhost=on to enable experimental in kernel accelerator\n" > > > >note 'enable' not 'require'. This is similar to how we specify > >nvectors : you can not make guest use the feature. > > > >How about this: > > > >diff --git a/qemu-options.hx b/qemu-options.hx > >index 898561d..3c937c1 100644 > >--- a/qemu-options.hx > >+++ b/qemu-options.hx > >@@ -1061,6 +1061,7 @@ DEF("net", HAS_ARG, QEMU_OPTION_net, > > " use vnet_hdr=off to avoid enabling the IFF_VNET_HDR tap flag\n" > > " use vnet_hdr=on to make the lack of IFF_VNET_HDR support an error condition\n" > > " use vhost=on to enable experimental in kernel accelerator\n" > >+ " (note: vhost=on has no effect unless guest uses MSI-X)\n" > > " use 'vhostfd=h' to connect to an already opened vhost net device\n" > > #endif > > "-net socket[,vlan=n][,name=str][,fd=h][,listen=[host]:port][,connect=host:port]\n" > > > >