--- /dev/null
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Hanna Czenczek <hreitz@redhat.com>
+Date: Fri, 2 Feb 2024 16:31:56 +0100
+Subject: [PATCH] virtio-scsi: Attach event vq notifier with no_poll
+
+As of commit 38738f7dbbda90fbc161757b7f4be35b52205552 ("virtio-scsi:
+don't waste CPU polling the event virtqueue"), we only attach an io_read
+notifier for the virtio-scsi event virtqueue instead, and no polling
+notifiers. During operation, the event virtqueue is typically
+non-empty, but none of the buffers are intended to be used immediately.
+Instead, they only get used when certain events occur. Therefore, it
+makes no sense to continuously poll it when non-empty, because it is
+supposed to be and stay non-empty.
+
+We do this by using virtio_queue_aio_attach_host_notifier_no_poll()
+instead of virtio_queue_aio_attach_host_notifier() for the event
+virtqueue.
+
+Commit 766aa2de0f29b657148e04599320d771c36fd126 ("virtio-scsi: implement
+BlockDevOps->drained_begin()") however has virtio_scsi_drained_end() use
+virtio_queue_aio_attach_host_notifier() for all virtqueues, including
+the event virtqueue. This can lead to it being polled again, undoing
+the benefit of commit 38738f7dbbda90fbc161757b7f4be35b52205552.
+
+Fix it by using virtio_queue_aio_attach_host_notifier_no_poll() for the
+event virtqueue.
+
+ ("virtio-scsi: implement BlockDevOps->drained_begin()")
+
+Reported-by: Fiona Ebner <f.ebner@proxmox.com>
+Fixes: 766aa2de0f29b657148e04599320d771c36fd126
+Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
+Tested-by: Fiona Ebner <f.ebner@proxmox.com>
+Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
+Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
+Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
+---
+ hw/scsi/virtio-scsi.c | 7 ++++++-
+ 1 file changed, 6 insertions(+), 1 deletion(-)
+
+diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
+index 45b95ea070..ad24a882fd 100644
+--- a/hw/scsi/virtio-scsi.c
++++ b/hw/scsi/virtio-scsi.c
+@@ -1148,6 +1148,7 @@ static void virtio_scsi_drained_begin(SCSIBus *bus)
+ static void virtio_scsi_drained_end(SCSIBus *bus)
+ {
+ VirtIOSCSI *s = container_of(bus, VirtIOSCSI, bus);
++ VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s);
+ VirtIODevice *vdev = VIRTIO_DEVICE(s);
+ uint32_t total_queues = VIRTIO_SCSI_VQ_NUM_FIXED +
+ s->parent_obj.conf.num_queues;
+@@ -1165,7 +1166,11 @@ static void virtio_scsi_drained_end(SCSIBus *bus)
+
+ for (uint32_t i = 0; i < total_queues; i++) {
+ VirtQueue *vq = virtio_get_queue(vdev, i);
+- virtio_queue_aio_attach_host_notifier(vq, s->ctx);
++ if (vq == vs->event_vq) {
++ virtio_queue_aio_attach_host_notifier_no_poll(vq, s->ctx);
++ } else {
++ virtio_queue_aio_attach_host_notifier(vq, s->ctx);
++ }
+ }
+ }
+
--- /dev/null
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Hanna Czenczek <hreitz@redhat.com>
+Date: Fri, 2 Feb 2024 16:31:57 +0100
+Subject: [PATCH] virtio: Re-enable notifications after drain
+
+During drain, we do not care about virtqueue notifications, which is why
+we remove the handlers on it. When removing those handlers, whether vq
+notifications are enabled or not depends on whether we were in polling
+mode or not; if not, they are enabled (by default); if so, they have
+been disabled by the io_poll_start callback.
+
+Because we do not care about those notifications after removing the
+handlers, this is fine. However, we have to explicitly ensure they are
+enabled when re-attaching the handlers, so we will resume receiving
+notifications. We do this in virtio_queue_aio_attach_host_notifier*().
+If such a function is called while we are in a polling section,
+attaching the notifiers will then invoke the io_poll_start callback,
+re-disabling notifications.
+
+Because we will always miss virtqueue updates in the drained section, we
+also need to poll the virtqueue once after attaching the notifiers.
+
+Buglink: https://issues.redhat.com/browse/RHEL-3934
+Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
+Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
+---
+ hw/virtio/virtio.c | 42 ++++++++++++++++++++++++++++++++++++++++++
+ include/block/aio.h | 7 ++++++-
+ 2 files changed, 48 insertions(+), 1 deletion(-)
+
+diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
+index 969c25f4cf..02cce83111 100644
+--- a/hw/virtio/virtio.c
++++ b/hw/virtio/virtio.c
+@@ -3526,6 +3526,17 @@ static void virtio_queue_host_notifier_aio_poll_end(EventNotifier *n)
+
+ void virtio_queue_aio_attach_host_notifier(VirtQueue *vq, AioContext *ctx)
+ {
++ /*
++ * virtio_queue_aio_detach_host_notifier() can leave notifications disabled.
++ * Re-enable them. (And if detach has not been used before, notifications
++ * being enabled is still the default state while a notifier is attached;
++ * see virtio_queue_host_notifier_aio_poll_end(), which will always leave
++ * notifications enabled once the polling section is left.)
++ */
++ if (!virtio_queue_get_notification(vq)) {
++ virtio_queue_set_notification(vq, 1);
++ }
++
+ aio_set_event_notifier(ctx, &vq->host_notifier,
+ virtio_queue_host_notifier_read,
+ virtio_queue_host_notifier_aio_poll,
+@@ -3533,6 +3544,13 @@ void virtio_queue_aio_attach_host_notifier(VirtQueue *vq, AioContext *ctx)
+ aio_set_event_notifier_poll(ctx, &vq->host_notifier,
+ virtio_queue_host_notifier_aio_poll_begin,
+ virtio_queue_host_notifier_aio_poll_end);
++
++ /*
++ * We will have ignored notifications about new requests from the guest
++ * while no notifiers were attached, so "kick" the virt queue to process
++ * those requests now.
++ */
++ event_notifier_set(&vq->host_notifier);
+ }
+
+ /*
+@@ -3543,14 +3561,38 @@ void virtio_queue_aio_attach_host_notifier(VirtQueue *vq, AioContext *ctx)
+ */
+ void virtio_queue_aio_attach_host_notifier_no_poll(VirtQueue *vq, AioContext *ctx)
+ {
++ /* See virtio_queue_aio_attach_host_notifier() */
++ if (!virtio_queue_get_notification(vq)) {
++ virtio_queue_set_notification(vq, 1);
++ }
++
+ aio_set_event_notifier(ctx, &vq->host_notifier,
+ virtio_queue_host_notifier_read,
+ NULL, NULL);
++
++ /*
++ * See virtio_queue_aio_attach_host_notifier().
++ * Note that this may be unnecessary for the type of virtqueues this
++ * function is used for. Still, it will not hurt to have a quick look into
++ * whether we can/should process any of the virtqueue elements.
++ */
++ event_notifier_set(&vq->host_notifier);
+ }
+
+ void virtio_queue_aio_detach_host_notifier(VirtQueue *vq, AioContext *ctx)
+ {
+ aio_set_event_notifier(ctx, &vq->host_notifier, NULL, NULL, NULL);
++
++ /*
++ * aio_set_event_notifier_poll() does not guarantee whether io_poll_end()
++ * will run after io_poll_begin(), so by removing the notifier, we do not
++ * know whether virtio_queue_host_notifier_aio_poll_end() has run after a
++ * previous virtio_queue_host_notifier_aio_poll_begin(), i.e. whether
++ * notifications are enabled or disabled. It does not really matter anyway;
++ * we just removed the notifier, so we do not care about notifications until
++ * we potentially re-attach it. The attach_host_notifier functions will
++ * ensure that notifications are enabled again when they are needed.
++ */
+ }
+
+ void virtio_queue_host_notifier_read(EventNotifier *n)
+diff --git a/include/block/aio.h b/include/block/aio.h
+index 32042e8905..79efadfa48 100644
+--- a/include/block/aio.h
++++ b/include/block/aio.h
+@@ -498,9 +498,14 @@ void aio_set_event_notifier(AioContext *ctx,
+ AioPollFn *io_poll,
+ EventNotifierHandler *io_poll_ready);
+
+-/* Set polling begin/end callbacks for an event notifier that has already been
++/*
++ * Set polling begin/end callbacks for an event notifier that has already been
+ * registered with aio_set_event_notifier. Do nothing if the event notifier is
+ * not registered.
++ *
++ * Note that if the io_poll_end() callback (or the entire notifier) is removed
++ * during polling, it will not be called, so an io_poll_begin() is not
++ * necessarily always followed by an io_poll_end().
+ */
+ void aio_set_event_notifier_poll(AioContext *ctx,
+ EventNotifier *notifier,
projects / pve-qemu.git / commitdiff