On 8/3/22 18:28, Stefan Hajnoczi wrote:
diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c
index 8bb6e6acfc..a575c3f0cd 100644
--- a/hw/scsi/virtio-scsi-dataplane.c
+++ b/hw/scsi/virtio-scsi-dataplane.c
@@ -66,6 +66,21 @@ static int virtio_scsi_set_host_notifier(VirtIOSCSI *s,
VirtQueue *vq, int n)
return 0;
}
+/* Context: BH in IOThread */
+static void virtio_scsi_dataplane_start_bh(void *opaque)
+{
+ VirtIOSCSI *s = opaque;
+ VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s);
+ int i;
+
+ virtio_queue_aio_attach_host_notifier(vs->ctrl_vq, s->ctx);
+ virtio_queue_aio_attach_host_notifier_no_poll(vs->event_vq, s->ctx);
+
+ for (i = 0; i < vs->conf.num_queues; i++) {
+ virtio_queue_aio_attach_host_notifier(vs->cmd_vqs[i], s->ctx);
+ }
+}
+
/* Context: BH in IOThread */
static void virtio_scsi_dataplane_stop_bh(void *opaque)
{
@@ -136,16 +151,18 @@ int virtio_scsi_dataplane_start(VirtIODevice *vdev)
memory_region_transaction_commit();
- aio_context_acquire(s->ctx);
- virtio_queue_aio_attach_host_notifier(vs->ctrl_vq, s->ctx);
- virtio_queue_aio_attach_host_notifier_no_poll(vs->event_vq, s->ctx);
-
- for (i = 0; i < vs->conf.num_queues; i++) {
- virtio_queue_aio_attach_host_notifier(vs->cmd_vqs[i], s->ctx);
- }
-
s->dataplane_starting = false;
s->dataplane_started = true;
+
+ /*
+ * Attach notifiers from within the IOThread. It's possible to attach
+ * notifiers from our thread directly but this approach has the advantages
+ * that virtio_scsi_dataplane_start_bh() is symmetric with
+ * virtio_scsi_dataplane_stop_bh() and the s->dataplane_started assignment
+ * above doesn't require explicit synchronization.
+ */
+ aio_context_acquire(s->ctx);
+ aio_wait_bh_oneshot(s->ctx, virtio_scsi_dataplane_start_bh, s);
aio_context_release(s->ctx);
Taking the AioContext lock for code that is running in another thread
seems wrong. But really there is no need to take the lock: I think it
wanted to protect against the handler running before
s->dataplane_starting/started were set, but it's not needed now because
the iothread is busy running the bottom half.
Also, please do the same in virtio-blk as well.
Thanks,
Paolo