Am 13.09.2018 um 17:17 hat Paolo Bonzini geschrieben: > On 13/09/2018 14:52, Kevin Wolf wrote: > > bdrv_do_drained_begin/end() assume that they are called with the > > AioContext lock of bs held. If we call drain functions from a coroutine > > with the AioContext lock held, we yield and schedule a BH to move out of > > coroutine context. This means that the lock for the home context of the > > coroutine is released and must be re-acquired in the bottom half. > > What exactly needs the lock, is it bdrv_drain_invoke? > > Would it make sense to always do release/acquire in bdrv_drain, and > always do acquire/release in bdrv_drain_invoke? (Conditional locking is > tricky...).
The thing that made it obvious was an aio_poll() call around which we want to release the lock temporarily, and if you don't hold it, you get a crash. This aio_poll() has actually disappeared in v2, and I'm not sure if AIO_WAIT_WHILE() can hit it, but I think locking is still right. I'm not sure what data structures are actually protected by it, but the simple rule as documented for bdrv_co_drain() has always been to hold the AioContext lock of bs when you call bdrv_drain(bs), so this patch just obeys it. Kevin