[v1] vhost: Don't set vring call if guest notifier is disabled

[PATCH] vhost: Don't set vring call if guest notifier is disabled

Posted by oenhan@gmail.com 1 month, 2 weeks ago

From: Huaitong Han <hanht2@chinatelecom.cn>

The vring call fd is set even when the guest does not use MSIX (e.g., virtio
PMD). This results in unnecessary CPU overhead for handling virtio interrupts.
The previous patch only optimized the condition when query_queue_notifier was
enabled and the vector was unset. However, if query_queue_notifier is disabled,
the vring call FD should also be unset to avoid this inefficiency.

Reported-by: Zhiyuan Yuan <yuanzhiyuan@chinatelecom.cn>
Signed-off-by: Huaitong Han <hanht2@chinatelecom.cn>
---
 hw/virtio/vhost.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 6aa72fd434..d17e7cc6fe 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1342,8 +1342,8 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
     }
 
     if (k->query_guest_notifiers &&
-        k->query_guest_notifiers(qbus->parent) &&
-        virtio_queue_vector(vdev, idx) == VIRTIO_NO_VECTOR) {
+        (!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) {
-- 
2.43.5

Re: [PATCH] vhost: Don't set vring call if guest notifier is disabled

Posted by Stefano Garzarella 1 month, 1 week ago

On Tue, Feb 18, 2025 at 04:32:08PM +0800, oenhan@gmail.com wrote:
>From: Huaitong Han <hanht2@chinatelecom.cn>
>
>The vring call fd is set even when the guest does not use MSIX (e.g., virtio
>PMD). This results in unnecessary CPU overhead for handling virtio interrupts.
>The previous patch only optimized the condition when query_queue_notifier was

Which patch/commit are you referring to?

The last commit touching that check is commit 96a3d98d2c ("vhost: don't
set vring call if no vector"), but I'm not sure if you are referring to
it. If it's the case, please add it in that form in the description.

It would also be better to add the Fixes tag as well and Cc stable if we 
would like to have this fix in the stable branch.

Thanks,
Stefano

>enabled and the vector was unset. However, if query_queue_notifier is disabled,
>the vring call FD should also be unset to avoid this inefficiency.
>
>Reported-by: Zhiyuan Yuan <yuanzhiyuan@chinatelecom.cn>
>Signed-off-by: Huaitong Han <hanht2@chinatelecom.cn>
>---
> hw/virtio/vhost.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
>index 6aa72fd434..d17e7cc6fe 100644
>--- a/hw/virtio/vhost.c
>+++ b/hw/virtio/vhost.c
>@@ -1342,8 +1342,8 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
>     }
>
>     if (k->query_guest_notifiers &&
>-        k->query_guest_notifiers(qbus->parent) &&
>-        virtio_queue_vector(vdev, idx) == VIRTIO_NO_VECTOR) {
>+        (!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) {
>-- 
>2.43.5
>

Re: [PATCH] vhost: Don't set vring call if guest notifier is disabled

Posted by Lei Yang 1 month ago

I tested this patch with virtio-net regression tests, everything works fine.

Tested-by: Lei Yang <leiyang@redhat.com>

On Tue, Feb 18, 2025 at 8:04 PM Stefano Garzarella <sgarzare@redhat.com> wrote:
>
> On Tue, Feb 18, 2025 at 04:32:08PM +0800, oenhan@gmail.com wrote:
> >From: Huaitong Han <hanht2@chinatelecom.cn>
> >
> >The vring call fd is set even when the guest does not use MSIX (e.g., virtio
> >PMD). This results in unnecessary CPU overhead for handling virtio interrupts.
> >The previous patch only optimized the condition when query_queue_notifier was
>
> Which patch/commit are you referring to?
>
> The last commit touching that check is commit 96a3d98d2c ("vhost: don't
> set vring call if no vector"), but I'm not sure if you are referring to
> it. If it's the case, please add it in that form in the description.
>
> It would also be better to add the Fixes tag as well and Cc stable if we
> would like to have this fix in the stable branch.
>
> Thanks,
> Stefano
>
> >enabled and the vector was unset. However, if query_queue_notifier is disabled,
> >the vring call FD should also be unset to avoid this inefficiency.
> >
> >Reported-by: Zhiyuan Yuan <yuanzhiyuan@chinatelecom.cn>
> >Signed-off-by: Huaitong Han <hanht2@chinatelecom.cn>
> >---
> > hw/virtio/vhost.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> >diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> >index 6aa72fd434..d17e7cc6fe 100644
> >--- a/hw/virtio/vhost.c
> >+++ b/hw/virtio/vhost.c
> >@@ -1342,8 +1342,8 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
> >     }
> >
> >     if (k->query_guest_notifiers &&
> >-        k->query_guest_notifiers(qbus->parent) &&
> >-        virtio_queue_vector(vdev, idx) == VIRTIO_NO_VECTOR) {
> >+        (!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) {
> >--
> >2.43.5
> >
>
>