Re: [PATCH] vhost: don't set vring call if no enabled msix

[Jason Wang]

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
>