From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37866) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bUPcC-0006ps-Hz for qemu-devel@nongnu.org; Mon, 01 Aug 2016 22:39:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bUPc7-0000P1-IK for qemu-devel@nongnu.org; Mon, 01 Aug 2016 22:39:32 -0400 Received: from mx1.redhat.com ([209.132.183.28]:41584) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bUPc7-0000Ok-9Z for qemu-devel@nongnu.org; Mon, 01 Aug 2016 22:39:27 -0400 References: <1470038878-5599-1-git-send-email-jasowang@redhat.com> <20160801200029.30da2f30.cornelia.huck@de.ibm.com> From: Jason Wang Message-ID: <633c8858-ea7d-0ed1-538f-e6c4eaf160c6@redhat.com> Date: Tue, 2 Aug 2016 10:39:22 +0800 MIME-Version: 1.0 In-Reply-To: <20160801200029.30da2f30.cornelia.huck@de.ibm.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] vhost: don't set vring call if no vector List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cornelia Huck Cc: mst@redhat.com, qemu-devel@nongnu.org On 2016=E5=B9=B408=E6=9C=8802=E6=97=A5 02:00, Cornelia Huck wrote: > On Mon, 1 Aug 2016 16:07:58 +0800 > Jason Wang wrote: > >> We used to set vring call fd unconditionally even if guest driver does >> not use MSIX for this vritqueue at all. This will cause lots of >> unnecessary userspace access and other checks for drivers does not use >> interrupt at all (e.g virtio-net pmd). So check and clean vring call >> fd if guest does not use any vector for this virtqueue at >> all. > So the basic idea is: don't setup signalling via eventfd if the guest > did not enable interrupts for this queue, right? Right. >> Perf diffs (on rx) shows lots of cpus wasted on vhost_signal() were sa= ved: >> >> # >> 28.12% -27.82% [vhost] [k] vhost_signal >> 14.44% -1.69% [kernel.vmlinux] [k] copy_user_generic_string >> 7.05% +1.53% [kernel.vmlinux] [k] __free_page_frag >> 6.51% +5.53% [vhost] [k] vhost_get_vq_desc >> ... >> >> Pktgen tests shows 15.8% improvement on rx pps and 6.5% on tx pps. >> >> Before: RX 2.08Mpps TX 1.35Mpps >> After: RX 2.41Mpps TX 1.44Mpps >> >> Signed-off-by: Jason Wang >> --- >> hw/virtio/vhost.c | 14 ++++++++++++++ >> 1 file changed, 14 insertions(+) >> >> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c >> index 3d0c807..bd051ab 100644 >> --- a/hw/virtio/vhost.c >> +++ b/hw/virtio/vhost.c >> @@ -822,6 +822,9 @@ static int vhost_virtqueue_start(struct vhost_dev = *dev, >> struct vhost_virtqueue *vq, >> unsigned idx) >> { >> + BusState *qbus =3D BUS(qdev_get_parent_bus(DEVICE(vdev))); >> + VirtioBusState *vbus =3D VIRTIO_BUS(qbus); >> + VirtioBusClass *k =3D VIRTIO_BUS_GET_CLASS(vbus); >> hwaddr s, l, a; >> int r; >> int vhost_vq_index =3D dev->vhost_ops->vhost_get_vq_index(dev, i= dx); >> @@ -912,8 +915,19 @@ static int vhost_virtqueue_start(struct vhost_dev= *dev, >> vhost_virtqueue_mask(dev, vdev, idx, false); >> } >> >> + if (k->query_guest_notifiers && >> + k->query_guest_notifiers(qbus->parent) && >> + virtio_queue_vector(vdev, idx) =3D=3D VIRTIO_NO_VECTOR) { > I'm trying to imagine what this means for virtio-ccw. Keep in mind that > we don't have the concept of setting a 'vector' by the OS (the vector > is setup internally to the queue index and the OS does not see it.) > > ->query_guest_notifiers() is true if the OS has enabled the subchannel > of the proxy device (i.e., if it is enabled for doing *anything* with > the subchannel, regardless whether the OS wants to be notified or is > planning to poll.) The second condition will never hold true for any > valid queue once the OS has setup the queues. I see, so If I understand correctly, there's no way to detect whether or=20 not guest will use a specific virtqueue interrupt? > > So this won't break anything for virtio-ccw AFAICS, but I don't think > we gain anything. Yes, but if we could infer whether or not polling is used in the driver,=20 this is probably all we can do for ccw. Thanks > >> + file.fd =3D -1; >> + r =3D dev->vhost_ops->vhost_set_vring_call(dev, &file); >> + if (r) { >> + goto fail_vector; >> + } >> + } >> + >> return 0; >> >> +fail_vector: >> fail_kick: >> fail_alloc: >> cpu_physical_memory_unmap(vq->ring, virtio_queue_get_ring_size(v= dev, idx),