Re: [Qemu-devel] [PATCH] vhost: force vhost off for non-MSI guests

[Michael S. Tsirkin]

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<address@hidden>
> >>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"
> >
> >