| Message ID | 20230403183004.347205-3-stefanha@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | block: remove aio_disable_external() API | expand |
On 4/3/23 20:29, Stefan Hajnoczi wrote: > This patch is part of an effort to remove the aio_disable_external() > API because it does not fit in a multi-queue block layer world where > many AioContexts may be submitting requests to the same disk. > > The SCSI emulation code is already in good shape to stop using > aio_disable_external(). It was only used by commit 9c5aad84da1c > ("virtio-scsi: fixed virtio_scsi_ctx_check failed when detaching scsi > disk") to ensure that virtio_scsi_hotunplug() works while the guest > driver is submitting I/O. > > Ensure virtio_scsi_hotunplug() is safe as follows: > > 1. qdev_simple_device_unplug_cb() -> qdev_unrealize() -> > device_set_realized() calls qatomic_set(&dev->realized, false) so > that future scsi_device_get() calls return NULL because they exclude > SCSIDevices with realized=false. > > That means virtio-scsi will reject new I/O requests to this > SCSIDevice with VIRTIO_SCSI_S_BAD_TARGET even while > virtio_scsi_hotunplug() is still executing. We are protected against > new requests! > > 2. Add a call to scsi_device_purge_requests() from scsi_unrealize() so > that in-flight requests are cancelled synchronously. This ensures > that no in-flight requests remain once qdev_simple_device_unplug_cb() > returns. > > Thanks to these two conditions we don't need aio_disable_external() > anymore. > > Cc: Zhengui Li <lizhengui@huawei.com> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > --- > hw/scsi/scsi-disk.c | 1 + > hw/scsi/virtio-scsi.c | 3 --- > 2 files changed, 1 insertion(+), 3 deletions(-) > > diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c > index 97c9b1c8cd..e01bd84541 100644 > --- a/hw/scsi/scsi-disk.c > +++ b/hw/scsi/scsi-disk.c > @@ -2522,6 +2522,7 @@ static void scsi_realize(SCSIDevice *dev, Error **errp) > > static void scsi_unrealize(SCSIDevice *dev) > { > + scsi_device_purge_requests(dev, SENSE_CODE(RESET)); > del_boot_device_lchs(&dev->qdev, NULL); > } > > diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c > index 000961446c..a02f9233ec 100644 > --- a/hw/scsi/virtio-scsi.c > +++ b/hw/scsi/virtio-scsi.c > @@ -1061,11 +1061,8 @@ static void virtio_scsi_hotunplug(HotplugHandler *hotplug_dev, DeviceState *dev, > VirtIODevice *vdev = VIRTIO_DEVICE(hotplug_dev); > VirtIOSCSI *s = VIRTIO_SCSI(vdev); > SCSIDevice *sd = SCSI_DEVICE(dev); > - AioContext *ctx = s->ctx ?: qemu_get_aio_context(); > > - aio_disable_external(ctx); > qdev_simple_device_unplug_cb(hotplug_dev, dev, errp); > - aio_enable_external(ctx); > > if (s->ctx) { > virtio_scsi_acquire(s); Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index 97c9b1c8cd..e01bd84541 100644 --- a/hw/scsi/scsi-disk.c +++ b/hw/scsi/scsi-disk.c @@ -2522,6 +2522,7 @@ static void scsi_realize(SCSIDevice *dev, Error **errp) static void scsi_unrealize(SCSIDevice *dev) { + scsi_device_purge_requests(dev, SENSE_CODE(RESET)); del_boot_device_lchs(&dev->qdev, NULL); } diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index 000961446c..a02f9233ec 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -1061,11 +1061,8 @@ static void virtio_scsi_hotunplug(HotplugHandler *hotplug_dev, DeviceState *dev, VirtIODevice *vdev = VIRTIO_DEVICE(hotplug_dev); VirtIOSCSI *s = VIRTIO_SCSI(vdev); SCSIDevice *sd = SCSI_DEVICE(dev); - AioContext *ctx = s->ctx ?: qemu_get_aio_context(); - aio_disable_external(ctx); qdev_simple_device_unplug_cb(hotplug_dev, dev, errp); - aio_enable_external(ctx); if (s->ctx) { virtio_scsi_acquire(s);
This patch is part of an effort to remove the aio_disable_external() API because it does not fit in a multi-queue block layer world where many AioContexts may be submitting requests to the same disk. The SCSI emulation code is already in good shape to stop using aio_disable_external(). It was only used by commit 9c5aad84da1c ("virtio-scsi: fixed virtio_scsi_ctx_check failed when detaching scsi disk") to ensure that virtio_scsi_hotunplug() works while the guest driver is submitting I/O. Ensure virtio_scsi_hotunplug() is safe as follows: 1. qdev_simple_device_unplug_cb() -> qdev_unrealize() -> device_set_realized() calls qatomic_set(&dev->realized, false) so that future scsi_device_get() calls return NULL because they exclude SCSIDevices with realized=false. That means virtio-scsi will reject new I/O requests to this SCSIDevice with VIRTIO_SCSI_S_BAD_TARGET even while virtio_scsi_hotunplug() is still executing. We are protected against new requests! 2. Add a call to scsi_device_purge_requests() from scsi_unrealize() so that in-flight requests are cancelled synchronously. This ensures that no in-flight requests remain once qdev_simple_device_unplug_cb() returns. Thanks to these two conditions we don't need aio_disable_external() anymore. Cc: Zhengui Li <lizhengui@huawei.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- hw/scsi/scsi-disk.c | 1 + hw/scsi/virtio-scsi.c | 3 --- 2 files changed, 1 insertion(+), 3 deletions(-)