On Fri, Feb 02, 2024 at 01:32:39PM +0100, Hanna Czenczek wrote: > On 01.02.24 16:25, Hanna Czenczek wrote: > > On 01.02.24 15:28, Stefan Hajnoczi wrote: > > [...] > > > > Did you find a scenario where the virtio-scsi AioContext is different > > > from the scsi-hd BB's Aiocontext? > > > > Technically, that’s the reason for this thread, specifically that > > virtio_scsi_hotunplug() switches the BB back to the main context while > > scsi_device_for_each_req_async_bh() is running. Yes, we can fix that > > specific case via the in-flight counter, but I’m wondering whether > > there’s really any merit in requiring the BB to always be in > > virtio-scsi’s context, or whether it would make more sense to schedule > > everything in virtio-scsi’s context. Now that BBs/BDSs can receive > > requests from any context, that is. > > Now that I know that wouldn’t be easy, let me turn this around: As far as I > understand, scsi_device_for_each_req_async_bh() should still run in > virtio-scsi’s context, but that’s hard, so we take the BB’s context, which > we therefore require to be the same one. Further, (again AFAIU,) > virtio-scsi’s context cannot change (only set in > virtio_scsi_dataplane_setup(), which is run in > virtio_scsi_device_realize()). Therefore, why does the > scsi_device_for_each_req_async() code accommodate for BB context changes?
1. scsi_disk_reset() -> scsi_device_purge_requests() is called without in-flight requests. 2. The BH is scheduled by scsi_device_purge_requests() -> scsi_device_for_each_req_async(). 3. blk_drain() is a nop when there no in-flight requests and does not flush BHs. 3. The AioContext changes when the virtio-scsi device resets. 4. The BH executes. Kevin and I touched on the idea of flushing BHs in bdrv_drain() even when there are no requests in flight. This hasn't been implemented as of today, but would also reduce the chance of scenarios like the one I mentioned. I think it's safer to handle the case where the BH runs after an AioContext change until either everything is thread-safe or the AioContext never changes. Stefan
signature.asc
Description: PGP signature