diff mbox series

vhost: don't set vring call if no enabled msix

Message ID 20240408060842.2012-1-yuxue.liu@jaguarmicro.com (mailing list archive)
State New
Headers show
Series vhost: don't set vring call if no enabled msix | expand

Commit Message

Yuxue Liu yuxue.liu@jaguarmicro.com April 8, 2024, 6:08 a.m. UTC
When conducting performance testing using testpmd in the guest os,
it was observed that the performance was lower compared to the
scenario of direct vfio-pci usage.

In the virtual machine operating system, even if the virtio device
does not use msix interrupts, vhost still sets vring call fd. This
leads to unnecessary performance overhead. If the guest driver does
not enable msix capability (e.g virtio-net pmd), we should also
check and clear the vring call fd.

Signed-off-by: Yuxue Liu <yuxue.liu@jaguarmicro.com>
---
 hw/virtio/vhost.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

Comments

Jason Wang April 8, 2024, 6:17 a.m. UTC | #1
On Mon, Apr 8, 2024 at 2:09 PM lyx634449800 <yuxue.liu@jaguarmicro.com> wrote:
>
> When conducting performance testing using testpmd in the guest os,
> it was observed that the performance was lower compared to the
> scenario of direct vfio-pci usage.
>
> In the virtual machine operating system, even if the virtio device
> does not use msix interrupts, vhost still sets vring call fd. This
> leads to unnecessary performance overhead. If the guest driver does
> not enable msix capability (e.g virtio-net pmd), we should also
> check and clear the vring call fd.
>
> Signed-off-by: Yuxue Liu <yuxue.liu@jaguarmicro.com>

Patch looks good, I would like to do the following tweaks:

1) explain what is not enough since commit:

commit 96a3d98d2cdbd897ff5ab33427aa4cfb94077665
Author: Jason Wang <jasowang@redhat.com>
Date:   Mon Aug 1 16:07:58 2016 +0800

    vhost: don't set vring call if no vector

2) tweak the title to "vhost: don't set vring call if guest notifiers
is not enabled" as it's not necessarily pci but also ccw.

Thanks

> ---
>  hw/virtio/vhost.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index f50180e60e..b972c84e67 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -1266,13 +1266,15 @@ 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) == VIRTIO_NO_VECTOR) {
> -        file.fd = -1;
> -        r = dev->vhost_ops->vhost_set_vring_call(dev, &file);
> -        if (r) {
> -            goto fail_vector;
> +    if (k->query_guest_notifiers) {
> +        if (!k->query_guest_notifiers(qbus->parent) ||
> +            (k->query_guest_notifiers(qbus->parent) &&
> +            virtio_queue_vector(vdev, idx) == VIRTIO_NO_VECTOR)) {
> +            file.fd = -1;
> +            r = dev->vhost_ops->vhost_set_vring_call(dev, &file);
> +            if (r) {
> +                goto fail_vector;
> +            }
>          }
>      }
>
> --
> 2.43.0
>
Michael S. Tsirkin April 9, 2024, 9:31 a.m. UTC | #2
On Mon, Apr 08, 2024 at 02:08:42PM +0800, lyx634449800 wrote:
> When conducting performance testing using testpmd in the guest os,
> it was observed that the performance was lower compared to the
> scenario of direct vfio-pci usage.
> 
> In the virtual machine operating system, even if the virtio device
> does not use msix interrupts, vhost still sets vring call fd. This
> leads to unnecessary performance overhead. If the guest driver does
> not enable msix capability (e.g virtio-net pmd), we should also
> check and clear the vring call fd.
> 
> Signed-off-by: Yuxue Liu <yuxue.liu@jaguarmicro.com>


Fails testing under cross-i686-tci:

https://gitlab.com/mstredhat/qemu/-/jobs/6578881990

36/258 qemu:qtest+qtest-i386 / qtest-i386/ioh3420-test                    OK               0.15s   1 subtests passed
▶  37/258 ERROR:../tests/qtest/qos-test.c:191:subprocess_run_one_test: child process (/aarch64/virt/generic-pcihost/pci-bus-generic/pci-bus/virtio-net-pci/virtio-net/virtio-net-tests/vhost-user/migrate/subprocess [22197]) failed unexpectedly ERROR         
 38/258 qemu:qtest+qtest-i386 / qtest-i386/lpc-ich9-test                   OK               0.16s   1 subtests passed
 37/258 qemu:qtest+qtest-aarch64 / qtest-aarch64/qos-test                  ERROR           13.20s   killed by signal 6 SIGABRT
>>> G_TEST_DBUS_DAEMON=/builds/mstredhat/qemu/tests/dbus-vmstate-daemon.sh PYTHON=/builds/mstredhat/qemu/build/pyvenv/bin/python3 MALLOC_PERTURB_=144 QTEST_QEMU_BINARY=./qemu-system-aarch64 /builds/mstredhat/qemu/build/tests/qtest/qos-test --tap -k
――――――――――――――――――――――――――――――――――――― ✀  ―――――――――――――――――――――――――――――――――――――
stderr:
**
ERROR:../tests/qtest/vhost-user-test.c:468:chr_read: assertion failed (err == NULL): Bad file descriptor (g-unix-error-quark, 0)
**
ERROR:../tests/qtest/qos-test.c:191:subprocess_run_one_test: child process (/aarch64/virt/generic-pcihost/pci-bus-generic/pci-bus/virtio-net-pci/virtio-net/virtio-net-tests/vhost-user/migrate/subprocess [22197]) failed unexpectedly
(test program exited with status code -6)
――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――


> ---
>  hw/virtio/vhost.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index f50180e60e..b972c84e67 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -1266,13 +1266,15 @@ 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) == VIRTIO_NO_VECTOR) {
> -        file.fd = -1;
> -        r = dev->vhost_ops->vhost_set_vring_call(dev, &file);
> -        if (r) {
> -            goto fail_vector;
> +    if (k->query_guest_notifiers) {
> +        if (!k->query_guest_notifiers(qbus->parent) ||
> +            (k->query_guest_notifiers(qbus->parent) &&
> +            virtio_queue_vector(vdev, idx) == VIRTIO_NO_VECTOR)) {
> +            file.fd = -1;
> +            r = dev->vhost_ops->vhost_set_vring_call(dev, &file);
> +            if (r) {
> +                goto fail_vector;
> +            }
>          }
>      }
>  
> -- 
> 2.43.0
diff mbox series

Patch

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index f50180e60e..b972c84e67 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1266,13 +1266,15 @@  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) == VIRTIO_NO_VECTOR) {
-        file.fd = -1;
-        r = dev->vhost_ops->vhost_set_vring_call(dev, &file);
-        if (r) {
-            goto fail_vector;
+    if (k->query_guest_notifiers) {
+        if (!k->query_guest_notifiers(qbus->parent) ||
+            (k->query_guest_notifiers(qbus->parent) &&
+            virtio_queue_vector(vdev, idx) == VIRTIO_NO_VECTOR)) {
+            file.fd = -1;
+            r = dev->vhost_ops->vhost_set_vring_call(dev, &file);
+            if (r) {
+                goto fail_vector;
+            }
         }
     }