| Message ID | 20230419172817.272758-17-stefanha@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | block: remove aio_disable_external() API | expand |
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?
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
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:-)
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); } }
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(+)