| Message ID | 20230611193924.2444914-1-stefanha@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | virtio-scsi: avoid dangling host notifier in ->ioeventfd_stop() | expand |
Buglink: https://gitlab.com/qemu-project/qemu/-/issues/1680 On Sun, Jun 11, 2023, 15:39 Stefan Hajnoczi <stefanha@redhat.com> wrote: > virtio_scsi_dataplane_stop() calls blk_drain_all(), which invokes > ->drained_begin()/->drained_end() after we've already detached the host > notifier. virtio_scsi_drained_end() currently attaches the host notifier > again and leaves it dangling after dataplane has stopped. > > This results in the following assertion failure because > virtio_scsi_defer_to_dataplane() is called from the IOThread instead of > the main loop thread: > > qemu-system-x86_64: ../softmmu/memory.c:1111: > memory_region_transaction_commit: Assertion `qemu_mutex_iothread_locked()' > failed. > > Reported-by: Jean-Louis Dupond <jean-louis@dupond.be> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > --- > hw/scsi/virtio-scsi.c | 20 ++++++++++++++++++-- > 1 file changed, 18 insertions(+), 2 deletions(-) > > diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c > index 9c8ef0aaa6..45b95ea070 100644 > --- a/hw/scsi/virtio-scsi.c > +++ b/hw/scsi/virtio-scsi.c > @@ -1125,7 +1125,16 @@ static void virtio_scsi_drained_begin(SCSIBus *bus) > uint32_t total_queues = VIRTIO_SCSI_VQ_NUM_FIXED + > s->parent_obj.conf.num_queues; > > - if (!s->dataplane_started) { > + /* > + * Drain is called when stopping dataplane but the host notifier has > + * already been detached. Detaching multiple times is a no-op if > nothing > + * else is using the monitoring same file descriptor, but avoid it > just in > + * case. > + * > + * Also, don't detach if dataplane has not even been started yet > because > + * the host notifier isn't attached. > + */ > + if (s->dataplane_stopping || !s->dataplane_started) { > return; > } > > @@ -1143,7 +1152,14 @@ static void virtio_scsi_drained_end(SCSIBus *bus) > uint32_t total_queues = VIRTIO_SCSI_VQ_NUM_FIXED + > s->parent_obj.conf.num_queues; > > - if (!s->dataplane_started) { > + /* > + * Drain is called when stopping dataplane. Keep the host notifier > detached > + * so it's not left dangling after dataplane is stopped. > + * > + * Also, don't attach if dataplane has not even been started yet. > We're not > + * ready. > + */ > + if (s->dataplane_stopping || !s->dataplane_started) { > return; > } > > -- > 2.40.1 > > >
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index 9c8ef0aaa6..45b95ea070 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -1125,7 +1125,16 @@ static void virtio_scsi_drained_begin(SCSIBus *bus) uint32_t total_queues = VIRTIO_SCSI_VQ_NUM_FIXED + s->parent_obj.conf.num_queues; - if (!s->dataplane_started) { + /* + * Drain is called when stopping dataplane but the host notifier has + * already been detached. Detaching multiple times is a no-op if nothing + * else is using the monitoring same file descriptor, but avoid it just in + * case. + * + * Also, don't detach if dataplane has not even been started yet because + * the host notifier isn't attached. + */ + if (s->dataplane_stopping || !s->dataplane_started) { return; } @@ -1143,7 +1152,14 @@ static void virtio_scsi_drained_end(SCSIBus *bus) uint32_t total_queues = VIRTIO_SCSI_VQ_NUM_FIXED + s->parent_obj.conf.num_queues; - if (!s->dataplane_started) { + /* + * Drain is called when stopping dataplane. Keep the host notifier detached + * so it's not left dangling after dataplane is stopped. + * + * Also, don't attach if dataplane has not even been started yet. We're not + * ready. + */ + if (s->dataplane_stopping || !s->dataplane_started) { return; }
virtio_scsi_dataplane_stop() calls blk_drain_all(), which invokes ->drained_begin()/->drained_end() after we've already detached the host notifier. virtio_scsi_drained_end() currently attaches the host notifier again and leaves it dangling after dataplane has stopped. This results in the following assertion failure because virtio_scsi_defer_to_dataplane() is called from the IOThread instead of the main loop thread: qemu-system-x86_64: ../softmmu/memory.c:1111: memory_region_transaction_commit: Assertion `qemu_mutex_iothread_locked()' failed. Reported-by: Jean-Louis Dupond <jean-louis@dupond.be> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- hw/scsi/virtio-scsi.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-)