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 15:43:06 +0200 Message-ID: <20110121134306.GA6980@redhat.com> References: <20110120153521.GA24357@redhat.com> <4D38583D.9010602@codemonkey.ws> <20110120160718.GA24652@redhat.com> <4D38D208.4090403@codemonkey.ws> <1295573746.2931.59.camel@x201> <20110121095503.GD26070@redhat.com> <1295615953.2931.65.camel@x201> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Anthony Liguori , kvm@vger.kernel.org, Juan Quintela , Jes.Sorensen@redhat.com, jasowang@redhat.com, qemu-devel@nongnu.org To: Alex Williamson Return-path: Received: from mx1.redhat.com ([209.132.183.28]:60444 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750971Ab1AUNnX (ORCPT ); Fri, 21 Jan 2011 08:43:23 -0500 Content-Disposition: inline In-Reply-To: <1295615953.2931.65.camel@x201> Sender: kvm-owner@vger.kernel.org List-ID: On Fri, Jan 21, 2011 at 06:19:13AM -0700, Alex Williamson wrote: > On Fri, 2011-01-21 at 11:55 +0200, Michael S. Tsirkin wrote: > > On Thu, Jan 20, 2011 at 06:35:46PM -0700, Alex Williamson wrote: > > > On Thu, 2011-01-20 at 18:23 -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. > > > > > > > > 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. > > > > > > If we add a force option, let's please distinguish hotplug from VM > > > creation time. The latter can abort. Hotplug should print an error and > > > fail the initfn. > > > > It can't abort at init - MSI is disabled at init, it needs to be enabled > > by the guest later. And aborting the guest in the middle of the run > > is a very bad idea. > > Yeah, I was thinking about the ordering of device being added vs guest > enabling MSI this morning. Waiting until the guest decides to try to > start using the device to NAK it with an abort is very undesirable. > What if when we have vhost=on,force, the device doesn't advertise an > INTx (PCI_INTERRUPT_PIN = 0)? > > Alex Then we break backward compatibility with old guests. I don't see what the issue is really: It is trivial to check that the guest uses MSIX. -- MST From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=60035 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PgHGu-0008CE-QP for qemu-devel@nongnu.org; Fri, 21 Jan 2011 08:43:26 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PgHGt-0000oR-If for qemu-devel@nongnu.org; Fri, 21 Jan 2011 08:43:24 -0500 Received: from mx1.redhat.com ([209.132.183.28]:64870) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PgHGt-0000oF-Ae for qemu-devel@nongnu.org; Fri, 21 Jan 2011 08:43:23 -0500 Date: Fri, 21 Jan 2011 15:43:06 +0200 From: "Michael S. Tsirkin" Subject: Re: [Qemu-devel] [PATCH] vhost: force vhost off for non-MSI guests Message-ID: <20110121134306.GA6980@redhat.com> References: <20110120153521.GA24357@redhat.com> <4D38583D.9010602@codemonkey.ws> <20110120160718.GA24652@redhat.com> <4D38D208.4090403@codemonkey.ws> <1295573746.2931.59.camel@x201> <20110121095503.GD26070@redhat.com> <1295615953.2931.65.camel@x201> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1295615953.2931.65.camel@x201> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Williamson Cc: kvm@vger.kernel.org, Juan Quintela , Jes.Sorensen@redhat.com, jasowang@redhat.com, qemu-devel@nongnu.org On Fri, Jan 21, 2011 at 06:19:13AM -0700, Alex Williamson wrote: > On Fri, 2011-01-21 at 11:55 +0200, Michael S. Tsirkin wrote: > > On Thu, Jan 20, 2011 at 06:35:46PM -0700, Alex Williamson wrote: > > > On Thu, 2011-01-20 at 18:23 -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. > > > > > > > > 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. > > > > > > If we add a force option, let's please distinguish hotplug from VM > > > creation time. The latter can abort. Hotplug should print an error and > > > fail the initfn. > > > > It can't abort at init - MSI is disabled at init, it needs to be enabled > > by the guest later. And aborting the guest in the middle of the run > > is a very bad idea. > > Yeah, I was thinking about the ordering of device being added vs guest > enabling MSI this morning. Waiting until the guest decides to try to > start using the device to NAK it with an abort is very undesirable. > What if when we have vhost=on,force, the device doesn't advertise an > INTx (PCI_INTERRUPT_PIN = 0)? > > Alex Then we break backward compatibility with old guests. I don't see what the issue is really: It is trivial to check that the guest uses MSIX. -- MST