From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [RFC][PATCH 28/45] qemu-kvm: msix: Drop tracking of used vectors Date: Tue, 18 Oct 2011 14:38:36 +0200 Message-ID: <4E9D734C.2060504@siemens.com> References: <20111017154852.GC7876@redhat.com> <4E9C81CC.5040900@web.de> <20111018115813.GF28776@redhat.com> <4E9D6C5B.8050204@siemens.com> <20111018123305.GK28776@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: Avi Kivity , Marcelo Tosatti , "kvm@vger.kernel.org" , Alex Williamson , "qemu-devel@nongnu.org" To: "Michael S. Tsirkin" Return-path: Received: from david.siemens.de ([192.35.17.14]:15674 "EHLO david.siemens.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757544Ab1JRMim (ORCPT ); Tue, 18 Oct 2011 08:38:42 -0400 In-Reply-To: <20111018123305.GK28776@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 2011-10-18 14:33, Michael S. Tsirkin wrote: > On Tue, Oct 18, 2011 at 02:08:59PM +0200, Jan Kiszka wrote: >> On 2011-10-18 13:58, Michael S. Tsirkin wrote: >>> On Mon, Oct 17, 2011 at 09:28:12PM +0200, Jan Kiszka wrote: >>>> On 2011-10-17 17:48, Michael S. Tsirkin wrote: >>>>> On Mon, Oct 17, 2011 at 11:28:02AM +0200, Jan Kiszka wrote: >>>>>> This optimization was only required to keep KVM route usage low. Now >>>>>> that we solve that problem via lazy updates, we can drop the field. We >>>>>> still need interfaces to clear pending vectors, though (and we have to >>>>>> make use of them more broadly - but that's unrelated to this patch). >>>>>> >>>>>> Signed-off-by: Jan Kiszka >>>>> >>>>> Lazy updates should be an implementation detail. >>>>> IMO resource tracking of vectors makes sense >>>>> as an API. Making devices deal with pending >>>>> vectors as a concept, IMO, does not. >>>> >>>> There is really no use for tracking the vector lifecycle once we have >>>> lazy updates (except for static routes). It's a way too invasive >>>> concept, and it's not needed for anything but KVM. >>> >>> I think it's needed. The PCI spec states that when the device >>> does not need an interrupt anymore, it should clear the pending >>> bit. >> >> That should be done explicitly if it is required outside existing >> clearing points. We already have that service, it's called >> msix_clear_vector. > > We do? I don't seem to see it upstream... True. From the device's POV, MSI-X (and also MSI!) vectors are actually level-triggered. So we should communicate the level to the MSI core and not just the edge. Needs more fixing > >> That alone does not justify msix_vector_use and all >> the state and logic behind it IMHO. > > To me it looks like an abstraction that solves both > this problem and the resource allocation problem. > Resources are actually limited BTW, this is not just > a KVM thing. qemu.git currently lets guests decide > what to do with them, but it might turn out to > be benefitial to warn the management application > that it is shooting itself in the foot. > >>> The use/unuse is IMO a decent API for this, >>> because it uses a familiar resource tracking concept. >>> Exposing this knowledge of msix to devices seems >>> like a worse API. >>> >>>> >>>> If you want an example, check >>>> http://permalink.gmane.org/gmane.comp.emulators.kvm.devel/70915 and >>>> compare it to the changes done to hpet in this series. >>>> >>>> Jan >>>> >>> >>> This seems to be a general argument that lazy updates are good? >>> I have no real problem with them, besides the fact that >>> we need an API to reserve space in the routing >>> table so that device setup can fail upfront. >> >> That's not possible, even with used vectors, as devices change their >> vector usage depending on how the guest configures the devices. If you >> (pre-)allocate all possible vectors, you may run out of resources >> earlier than needed actually. > > This really depends, but please do look at how with virtio > we report resource shortage to guest and let it fall back to > level interrups. You seem to remove that capability. To my understanding, virtio will be the exception as no other device will have a chance to react on resource shortage while sending(!) an MSI message. > > I actually would not mind preallocating everything upfront which is much > easier. But with your patch we get a silent failure or a drastic > slowdown which is much more painful IMO. Again: did we already saw that limit? And where does it come from if not from KVM? > >> That's also why we do those data == 0 >> checks to skip used but unconfigured vectors. >> >> Jan > > These checks work more or less by luck BTW. It's > a hack which I hope lazy allocation will replace. The check is still valid (for x86) when we have to use static routes (device assignment, vhost). For lazy updates, it's obsolete, that's true. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:59413) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RG8wK-0002X0-Gr for qemu-devel@nongnu.org; Tue, 18 Oct 2011 08:38:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RG8wI-0005pD-T1 for qemu-devel@nongnu.org; Tue, 18 Oct 2011 08:38:40 -0400 Received: from david.siemens.de ([192.35.17.14]:26559) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RG8wI-0005p7-Be for qemu-devel@nongnu.org; Tue, 18 Oct 2011 08:38:38 -0400 Message-ID: <4E9D734C.2060504@siemens.com> Date: Tue, 18 Oct 2011 14:38:36 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <20111017154852.GC7876@redhat.com> <4E9C81CC.5040900@web.de> <20111018115813.GF28776@redhat.com> <4E9D6C5B.8050204@siemens.com> <20111018123305.GK28776@redhat.com> In-Reply-To: <20111018123305.GK28776@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC][PATCH 28/45] qemu-kvm: msix: Drop tracking of used vectors List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: Alex Williamson , Marcelo Tosatti , Avi Kivity , "kvm@vger.kernel.org" , "qemu-devel@nongnu.org" On 2011-10-18 14:33, Michael S. Tsirkin wrote: > On Tue, Oct 18, 2011 at 02:08:59PM +0200, Jan Kiszka wrote: >> On 2011-10-18 13:58, Michael S. Tsirkin wrote: >>> On Mon, Oct 17, 2011 at 09:28:12PM +0200, Jan Kiszka wrote: >>>> On 2011-10-17 17:48, Michael S. Tsirkin wrote: >>>>> On Mon, Oct 17, 2011 at 11:28:02AM +0200, Jan Kiszka wrote: >>>>>> This optimization was only required to keep KVM route usage low. Now >>>>>> that we solve that problem via lazy updates, we can drop the field. We >>>>>> still need interfaces to clear pending vectors, though (and we have to >>>>>> make use of them more broadly - but that's unrelated to this patch). >>>>>> >>>>>> Signed-off-by: Jan Kiszka >>>>> >>>>> Lazy updates should be an implementation detail. >>>>> IMO resource tracking of vectors makes sense >>>>> as an API. Making devices deal with pending >>>>> vectors as a concept, IMO, does not. >>>> >>>> There is really no use for tracking the vector lifecycle once we have >>>> lazy updates (except for static routes). It's a way too invasive >>>> concept, and it's not needed for anything but KVM. >>> >>> I think it's needed. The PCI spec states that when the device >>> does not need an interrupt anymore, it should clear the pending >>> bit. >> >> That should be done explicitly if it is required outside existing >> clearing points. We already have that service, it's called >> msix_clear_vector. > > We do? I don't seem to see it upstream... True. From the device's POV, MSI-X (and also MSI!) vectors are actually level-triggered. So we should communicate the level to the MSI core and not just the edge. Needs more fixing > >> That alone does not justify msix_vector_use and all >> the state and logic behind it IMHO. > > To me it looks like an abstraction that solves both > this problem and the resource allocation problem. > Resources are actually limited BTW, this is not just > a KVM thing. qemu.git currently lets guests decide > what to do with them, but it might turn out to > be benefitial to warn the management application > that it is shooting itself in the foot. > >>> The use/unuse is IMO a decent API for this, >>> because it uses a familiar resource tracking concept. >>> Exposing this knowledge of msix to devices seems >>> like a worse API. >>> >>>> >>>> If you want an example, check >>>> http://permalink.gmane.org/gmane.comp.emulators.kvm.devel/70915 and >>>> compare it to the changes done to hpet in this series. >>>> >>>> Jan >>>> >>> >>> This seems to be a general argument that lazy updates are good? >>> I have no real problem with them, besides the fact that >>> we need an API to reserve space in the routing >>> table so that device setup can fail upfront. >> >> That's not possible, even with used vectors, as devices change their >> vector usage depending on how the guest configures the devices. If you >> (pre-)allocate all possible vectors, you may run out of resources >> earlier than needed actually. > > This really depends, but please do look at how with virtio > we report resource shortage to guest and let it fall back to > level interrups. You seem to remove that capability. To my understanding, virtio will be the exception as no other device will have a chance to react on resource shortage while sending(!) an MSI message. > > I actually would not mind preallocating everything upfront which is much > easier. But with your patch we get a silent failure or a drastic > slowdown which is much more painful IMO. Again: did we already saw that limit? And where does it come from if not from KVM? > >> That's also why we do those data == 0 >> checks to skip used but unconfigured vectors. >> >> Jan > > These checks work more or less by luck BTW. It's > a hack which I hope lazy allocation will replace. The check is still valid (for x86) when we have to use static routes (device assignment, vhost). For lazy updates, it's obsolete, that's true. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux