[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 04/20] virtio-scsi: stop using aio_disable_external() duri
From: |
Kevin Wolf |
Subject: |
Re: [PATCH v4 04/20] virtio-scsi: stop using aio_disable_external() during unplug |
Date: |
Fri, 28 Apr 2023 16:22:55 +0200 |
Am 25.04.2023 um 19:27 hat Stefan Hajnoczi geschrieben:
> 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>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
qemu-iotests 040 starts failing for me after this patch, with what looks
like a use-after-free error of some kind.
(gdb) bt
#0 0x000055b6e3e1f31c in job_type (job=0xe3e3e3e3e3e3e3e3) at ../job.c:238
#1 0x000055b6e3e1cee5 in is_block_job (job=0xe3e3e3e3e3e3e3e3) at
../blockjob.c:41
#2 0x000055b6e3e1ce7d in block_job_next_locked (bjob=0x55b6e72b7570) at
../blockjob.c:54
#3 0x000055b6e3df6370 in blockdev_mark_auto_del (blk=0x55b6e74af0a0) at
../blockdev.c:157
#4 0x000055b6e393e23b in scsi_qdev_unrealize (qdev=0x55b6e7c04d40) at
../hw/scsi/scsi-bus.c:303
#5 0x000055b6e3db0d0e in device_set_realized (obj=0x55b6e7c04d40, value=false,
errp=0x55b6e497c918 <error_abort>) at ../hw/core/qdev.c:599
#6 0x000055b6e3dba36e in property_set_bool (obj=0x55b6e7c04d40,
v=0x55b6e7d7f290, name=0x55b6e41bd6d8 "realized", opaque=0x55b6e7246d20,
errp=0x55b6e497c918 <error_abort>)
at ../qom/object.c:2285
#7 0x000055b6e3db7e65 in object_property_set (obj=0x55b6e7c04d40,
name=0x55b6e41bd6d8 "realized", v=0x55b6e7d7f290, errp=0x55b6e497c918
<error_abort>) at ../qom/object.c:1420
#8 0x000055b6e3dbd84a in object_property_set_qobject (obj=0x55b6e7c04d40,
name=0x55b6e41bd6d8 "realized", value=0x55b6e74c1890, errp=0x55b6e497c918
<error_abort>)
at ../qom/qom-qobject.c:28
#9 0x000055b6e3db8570 in object_property_set_bool (obj=0x55b6e7c04d40,
name=0x55b6e41bd6d8 "realized", value=false, errp=0x55b6e497c918 <error_abort>)
at ../qom/object.c:1489
#10 0x000055b6e3daf2b5 in qdev_unrealize (dev=0x55b6e7c04d40) at
../hw/core/qdev.c:306
#11 0x000055b6e3db509d in qdev_simple_device_unplug_cb
(hotplug_dev=0x55b6e81c3630, dev=0x55b6e7c04d40, errp=0x7ffec5519200) at
../hw/core/qdev-hotplug.c:72
#12 0x000055b6e3c520f9 in virtio_scsi_hotunplug (hotplug_dev=0x55b6e81c3630,
dev=0x55b6e7c04d40, errp=0x7ffec5519200) at ../hw/scsi/virtio-scsi.c:1065
#13 0x000055b6e3db4dec in hotplug_handler_unplug (plug_handler=0x55b6e81c3630,
plugged_dev=0x55b6e7c04d40, errp=0x7ffec5519200) at ../hw/core/hotplug.c:56
#14 0x000055b6e3a28f84 in qdev_unplug (dev=0x55b6e7c04d40, errp=0x7ffec55192e0)
at ../softmmu/qdev-monitor.c:935
#15 0x000055b6e3a290fa in qmp_device_del (id=0x55b6e74c1760 "scsi0",
errp=0x7ffec55192e0) at ../softmmu/qdev-monitor.c:955
#16 0x000055b6e3fb0a5f in qmp_marshal_device_del (args=0x7f61cc005eb0,
ret=0x7f61d5a8ae38, errp=0x7f61d5a8ae40) at qapi/qapi-commands-qdev.c:114
#17 0x000055b6e3fd52e1 in do_qmp_dispatch_bh (opaque=0x7f61d5a8ae08) at
../qapi/qmp-dispatch.c:128
#18 0x000055b6e4007b9e in aio_bh_call (bh=0x55b6e7dea730) at ../util/async.c:155
#19 0x000055b6e4007d2e in aio_bh_poll (ctx=0x55b6e72447c0) at
../util/async.c:184
#20 0x000055b6e3fe3b45 in aio_dispatch (ctx=0x55b6e72447c0) at
../util/aio-posix.c:421
#21 0x000055b6e4009544 in aio_ctx_dispatch (source=0x55b6e72447c0,
callback=0x0, user_data=0x0) at ../util/async.c:326
#22 0x00007f61ddc14c7f in g_main_dispatch (context=0x55b6e7244b20) at
../glib/gmain.c:3454
#23 g_main_context_dispatch (context=0x55b6e7244b20) at ../glib/gmain.c:4172
#24 0x000055b6e400a7e8 in glib_pollfds_poll () at ../util/main-loop.c:290
#25 0x000055b6e400a0c2 in os_host_main_loop_wait (timeout=0) at
../util/main-loop.c:313
#26 0x000055b6e4009fa2 in main_loop_wait (nonblocking=0) at
../util/main-loop.c:592
#27 0x000055b6e3a3047b in qemu_main_loop () at ../softmmu/runstate.c:731
#28 0x000055b6e3dab27d in qemu_default_main () at ../softmmu/main.c:37
#29 0x000055b6e3dab2b8 in main (argc=24, argv=0x7ffec55196a8) at
../softmmu/main.c:48
(gdb) p jobs
$4 = {lh_first = 0x0}
Kevin
- [PATCH v4 00/20] block: remove aio_disable_external() API, Stefan Hajnoczi, 2023/04/25
- [PATCH v4 03/20] virtio-scsi: avoid race between unplug and transport event, Stefan Hajnoczi, 2023/04/25
- [PATCH v4 04/20] virtio-scsi: stop using aio_disable_external() during unplug, Stefan Hajnoczi, 2023/04/25
- Re: [PATCH v4 04/20] virtio-scsi: stop using aio_disable_external() during unplug,
Kevin Wolf <=
- [PATCH v4 01/20] block-backend: split blk_do_set_aio_context(), Stefan Hajnoczi, 2023/04/25
- [PATCH v4 02/20] hw/qdev: introduce qdev_is_realized() helper, Stefan Hajnoczi, 2023/04/25
- [PATCH v4 05/20] util/vhost-user-server: rename refcount to in_flight counter, Stefan Hajnoczi, 2023/04/25
- [PATCH v4 06/20] block/export: wait for vhost-user-blk requests when draining, Stefan Hajnoczi, 2023/04/25
- [PATCH v4 07/20] block/export: stop using is_external in vhost-user-blk server, Stefan Hajnoczi, 2023/04/25
- [PATCH v4 08/20] hw/xen: do not use aio_set_fd_handler(is_external=true) in xen_xenstore, Stefan Hajnoczi, 2023/04/25
- [PATCH v4 09/20] block: add blk_in_drain() API, Stefan Hajnoczi, 2023/04/25
- [PATCH v4 10/20] block: drain from main loop thread in bdrv_co_yield_to_drain(), Stefan Hajnoczi, 2023/04/25
- [PATCH v4 11/20] xen-block: implement BlockDevOps->drained_begin(), Stefan Hajnoczi, 2023/04/25
- [PATCH v4 12/20] hw/xen: do not set is_external=true on evtchn fds, Stefan Hajnoczi, 2023/04/25