From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38693) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YRBiw-00069p-Hk for qemu-devel@nongnu.org; Thu, 26 Feb 2015 22:36:23 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YRBis-0002v2-G8 for qemu-devel@nongnu.org; Thu, 26 Feb 2015 22:36:22 -0500 Received: from mx1.redhat.com ([209.132.183.28]:51686) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YRBis-0002ux-4d for qemu-devel@nongnu.org; Thu, 26 Feb 2015 22:36:18 -0500 Date: Fri, 27 Feb 2015 03:42:00 +0008 From: Jason Wang Message-Id: <1425008040.3834.4@smtp.corp.redhat.com> In-Reply-To: <20150226135717.3f5b5789.cornelia.huck@de.ibm.com> References: <1424934286-7099-1-git-send-email-jasowang@redhat.com> <1424934286-7099-4-git-send-email-jasowang@redhat.com> <20150226135717.3f5b5789.cornelia.huck@de.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Subject: Re: [Qemu-devel] [PATCH V2 03/11] virito: introduce bus specific queue limit List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cornelia Huck Cc: mst@redhat.com, Alexander Graf , qemu-devel@nongnu.org, Christian Borntraeger , Anthony Liguori , Paolo Bonzini , Richard Henderson On Thu, Feb 26, 2015 at 8:57 PM, Cornelia Huck wrote: > On Thu, 26 Feb 2015 15:04:38 +0800 > Jason Wang wrote: > >> This patch introduces a bus specific queue limitation. It will be >> useful for increasing the limit for one of the bus without >> disturbing >> other buses. >> >> Cc: Anthony Liguori >> Cc: Michael S. Tsirkin >> Cc: Alexander Graf >> Cc: Richard Henderson >> Cc: Cornelia Huck >> Cc: Christian Borntraeger >> Cc: Paolo Bonzini >> Signed-off-by: Jason Wang >> --- >> hw/net/virtio-net.c | 4 ++-- >> hw/s390x/s390-virtio-bus.c | 1 + >> hw/s390x/virtio-ccw.c | 1 + >> hw/scsi/virtio-scsi.c | 4 ++-- >> hw/virtio/virtio-mmio.c | 1 + >> hw/virtio/virtio-pci.c | 1 + >> hw/virtio/virtio.c | 52 >> +++++++++++++++++++++++++++++++----------- >> include/hw/virtio/virtio-bus.h | 1 + >> include/hw/virtio/virtio.h | 1 + >> 9 files changed, 49 insertions(+), 17 deletions(-) >> > >> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c >> index ffc22e8..5a806b5 100644 >> --- a/hw/virtio/virtio.c >> +++ b/hw/virtio/virtio.c >> @@ -541,6 +541,14 @@ void virtio_update_irq(VirtIODevice *vdev) >> virtio_notify_vector(vdev, VIRTIO_NO_VECTOR); >> } >> >> +int virtio_get_queue_max(VirtIODevice *vdev) >> +{ >> + BusState *qbus = qdev_get_parent_bus(DEVICE(vdev)); >> + VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); >> + >> + return k->queue_max; >> +} >> + > > Are all callers of this in the slow path? So we don't introduce > processing overhead. Looks not. For overhead, do you mean one introduced by VIRTIO_BUS_GET_CLASS()? Not sure how much it will affact but we've already used something like this in the datapath, e.g virtio_notify_vector(). > > >> void virtio_set_status(VirtIODevice *vdev, uint8_t val) >> { >> VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev); > > (...) > > >> @@ -777,27 +790,35 @@ void virtio_queue_notify(VirtIODevice *vdev, >> int n) >> >> uint16_t virtio_queue_vector(VirtIODevice *vdev, int n) >> { >> - return n < VIRTIO_PCI_QUEUE_MAX ? vdev->vq[n].vector : >> + BusState *qbus = qdev_get_parent_bus(DEVICE(vdev)); >> + VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); >> + >> + return n < k->queue_max ? vdev->vq[n].vector : > > Any reason you're not using the newly introduced > virtio_get_queue_max()? Yes, better use virtio_get_queue_max(). > > >> VIRTIO_NO_VECTOR; >> } > > Do we need to care about migration if the target supports a different > number of queues? See patch 9, if a target support a different number of queues, it can patch the k->queue_max during its initialization. > > > I'm also not sure whether it would be sufficient to allow transports > to > limit queues to a lower number than the core supports. We'd basically > only need to block off queue creation in the individual transports, I > think. Not sure I understand the issue correctly, with this series. There's not core limitation but only transport limitation.