[PATCH v2 16/16] virtio: make it possible to detach host notifier from any thread

Stefan Hajnoczi posted 16 patches 2 years, 9 months ago
There is a newer version of this series
[PATCH v2 16/16] virtio: make it possible to detach host notifier from any thread
Posted by Stefan Hajnoczi 2 years, 9 months ago
virtio_queue_aio_detach_host_notifier() does two things:
1. It removes the fd handler from the event loop.
2. It processes the virtqueue one last time.

The first step can be peformed by any thread and without taking the
AioContext lock.

The second step may need the AioContext lock (depending on the device
implementation) and runs in the thread where request processing takes
place. virtio-blk and virtio-scsi therefore call
virtio_queue_aio_detach_host_notifier() from a BH that is scheduled in
AioContext

Scheduling a BH is undesirable for .drained_begin() functions. The next
patch will introduce a .drained_begin() function that needs to call
virtio_queue_aio_detach_host_notifier().

Move the virtqueue processing out to the callers of
virtio_queue_aio_detach_host_notifier() so that the function can be
called from any thread. This is in preparation for the next patch.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/block/dataplane/virtio-blk.c | 2 ++
 hw/scsi/virtio-scsi-dataplane.c | 9 +++++++++
 2 files changed, 11 insertions(+)

diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index b28d81737e..bd7cc6e76b 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -286,8 +286,10 @@ static void virtio_blk_data_plane_stop_bh(void *opaque)
 
     for (i = 0; i < s->conf->num_queues; i++) {
         VirtQueue *vq = virtio_get_queue(s->vdev, i);
+        EventNotifier *host_notifier = virtio_queue_get_host_notifier(vq);
 
         virtio_queue_aio_detach_host_notifier(vq, s->ctx);
+        virtio_queue_host_notifier_read(host_notifier);
     }
 }
 
diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c
index 20bb91766e..81643445ed 100644
--- a/hw/scsi/virtio-scsi-dataplane.c
+++ b/hw/scsi/virtio-scsi-dataplane.c
@@ -71,12 +71,21 @@ static void virtio_scsi_dataplane_stop_bh(void *opaque)
 {
     VirtIOSCSI *s = opaque;
     VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s);
+    EventNotifier *host_notifier;
     int i;
 
     virtio_queue_aio_detach_host_notifier(vs->ctrl_vq, s->ctx);
+    host_notifier = virtio_queue_get_host_notifier(vs->ctrl_vq);
+    virtio_queue_host_notifier_read(host_notifier);
+
     virtio_queue_aio_detach_host_notifier(vs->event_vq, s->ctx);
+    host_notifier = virtio_queue_get_host_notifier(vs->event_vq);
+    virtio_queue_host_notifier_read(host_notifier);
+
     for (i = 0; i < vs->conf.num_queues; i++) {
         virtio_queue_aio_detach_host_notifier(vs->cmd_vqs[i], s->ctx);
+        host_notifier = virtio_queue_get_host_notifier(vs->cmd_vqs[i]);
+        virtio_queue_host_notifier_read(host_notifier);
     }
 }
 
-- 
2.39.2
Re: [PATCH v2 16/16] virtio: make it possible to detach host notifier from any thread
Posted by Eric Blake 2 years, 9 months ago
On Wed, Apr 19, 2023 at 01:28:17PM -0400, Stefan Hajnoczi wrote:
> virtio_queue_aio_detach_host_notifier() does two things:
> 1. It removes the fd handler from the event loop.
> 2. It processes the virtqueue one last time.
> 
> The first step can be peformed by any thread and without taking the
> AioContext lock.
> 
> The second step may need the AioContext lock (depending on the device
> implementation) and runs in the thread where request processing takes
> place. virtio-blk and virtio-scsi therefore call
> virtio_queue_aio_detach_host_notifier() from a BH that is scheduled in
> AioContext
> 
> Scheduling a BH is undesirable for .drained_begin() functions. The next
> patch will introduce a .drained_begin() function that needs to call
> virtio_queue_aio_detach_host_notifier().
> 
> Move the virtqueue processing out to the callers of
> virtio_queue_aio_detach_host_notifier() so that the function can be
> called from any thread. This is in preparation for the next patch.
>

This mentions a next patch, but is 16/16 in the series.  Am I missing
something?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org
Re: [PATCH v2 16/16] virtio: make it possible to detach host notifier from any thread
Posted by Stefan Hajnoczi 2 years, 9 months ago
On Wed, 19 Apr 2023 at 14:52, Eric Blake <eblake@redhat.com> wrote:
>
> On Wed, Apr 19, 2023 at 01:28:17PM -0400, Stefan Hajnoczi wrote:
> > virtio_queue_aio_detach_host_notifier() does two things:
> > 1. It removes the fd handler from the event loop.
> > 2. It processes the virtqueue one last time.
> >
> > The first step can be peformed by any thread and without taking the
> > AioContext lock.
> >
> > The second step may need the AioContext lock (depending on the device
> > implementation) and runs in the thread where request processing takes
> > place. virtio-blk and virtio-scsi therefore call
> > virtio_queue_aio_detach_host_notifier() from a BH that is scheduled in
> > AioContext
> >
> > Scheduling a BH is undesirable for .drained_begin() functions. The next
> > patch will introduce a .drained_begin() function that needs to call
> > virtio_queue_aio_detach_host_notifier().
> >
> > Move the virtqueue processing out to the callers of
> > virtio_queue_aio_detach_host_notifier() so that the function can be
> > called from any thread. This is in preparation for the next patch.
> >
>
> This mentions a next patch, but is 16/16 in the series.  Am I missing
> something?

Good thing you caught this. The patch series was truncated because I
was in the middle of git rebase -i :(.

I will send a v3 with the remaining patches.

Thanks,
Stefan
Re: [PATCH v2 16/16] virtio: make it possible to detach host notifier from any thread
Posted by Juan Quintela 2 years, 9 months ago
Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Wed, 19 Apr 2023 at 14:52, Eric Blake <eblake@redhat.com> wrote:
>>
>> On Wed, Apr 19, 2023 at 01:28:17PM -0400, Stefan Hajnoczi wrote:
>> > virtio_queue_aio_detach_host_notifier() does two things:
>> > 1. It removes the fd handler from the event loop.
>> > 2. It processes the virtqueue one last time.
>> >
>> > The first step can be peformed by any thread and without taking the
>> > AioContext lock.
>> >
>> > The second step may need the AioContext lock (depending on the device
>> > implementation) and runs in the thread where request processing takes
>> > place. virtio-blk and virtio-scsi therefore call
>> > virtio_queue_aio_detach_host_notifier() from a BH that is scheduled in
>> > AioContext
>> >
>> > Scheduling a BH is undesirable for .drained_begin() functions. The next
>> > patch will introduce a .drained_begin() function that needs to call
>> > virtio_queue_aio_detach_host_notifier().
>> >
>> > Move the virtqueue processing out to the callers of
>> > virtio_queue_aio_detach_host_notifier() so that the function can be
>> > called from any thread. This is in preparation for the next patch.
>> >
>>
>> This mentions a next patch, but is 16/16 in the series.  Am I missing
>> something?
>
> Good thing you caught this. The patch series was truncated because I
> was in the middle of git rebase -i :(.
>
> I will send a v3 with the remaining patches.

I saw that it was not migration/* stuff and though that I was done O:-)