[PATCH v2] vhost: don't set vring call if guest notifiers is not enabled

lyx634449800 posted 1 patch 3 weeks, 3 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20240408073311.2049-1-yuxue.liu@jaguarmicro.com
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>
hw/virtio/vhost.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
[PATCH v2] vhost: don't set vring call if guest notifiers is not enabled
Posted by lyx634449800 3 weeks, 3 days ago
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 commit 96a3d98d2cdbd897ff5ab33427aa4cfb94077665, the author
provided a good solution. However, because the guest OS's
driver(e.g., virtio-net pmd) may not enable the msix capability, the
function k->query_guest_notifiers(qbus->parent) may return false,
resulting in the expected effect not being achieved. To address this
issue, modify the conditional statement.

Signed-off-by: Yuxue Liu <yuxue.liu@jaguarmicro.com>
---
V2: Update commit description and title

 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
Re: [PATCH v2] vhost: don't set vring call if guest notifiers is not enabled
Posted by Michael S. Tsirkin 3 weeks, 2 days ago
On Mon, Apr 08, 2024 at 03:33:11PM +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 commit 96a3d98d2cdbd897ff5ab33427aa4cfb94077665, the author
> provided a good solution. However, because the guest OS's
> driver(e.g., virtio-net pmd) may not enable the msix capability, the
> function k->query_guest_notifiers(qbus->parent) may return false,
> resulting in the expected effect not being achieved. To address this
> issue, modify the conditional statement.
> 
> Signed-off-by: Yuxue Liu <yuxue.liu@jaguarmicro.com>


I tested v1 and it fails. Sent as reply on that patch.
Since all you did is tweak description and title the
problem is probably still there in v2.

> ---
> V2: Update commit description and title
> 
>  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
Re: [PATCH v2] vhost: don't set vring call if guest notifiers is not enabled
Posted by Jason Wang 3 weeks, 2 days ago
On Mon, Apr 8, 2024 at 3:33 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 commit 96a3d98d2cdbd897ff5ab33427aa4cfb94077665, the author
> provided a good solution. However, because the guest OS's
> driver(e.g., virtio-net pmd) may not enable the msix capability, the
> function k->query_guest_notifiers(qbus->parent) may return false,
> resulting in the expected effect not being achieved. To address this
> issue, modify the conditional statement.
>
> Signed-off-by: Yuxue Liu <yuxue.liu@jaguarmicro.com>

Acked-by: Jason Wang <jasowang@redhat.com>

Thanks