[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 08/17] block: Add missing locking in bdrv_co_
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH v2 08/17] block: Add missing locking in bdrv_co_drain_bh_cb() |
Date: |
Thu, 13 Sep 2018 19:36:43 +0200 |
User-agent: |
Mutt/1.9.1 (2017-09-22) |
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
- [Qemu-devel] [PATCH v2 05/17] test-blockjob: Acquire AioContext around job_cancel_sync(), (continued)
- [Qemu-devel] [PATCH v2 05/17] test-blockjob: Acquire AioContext around job_cancel_sync(), Kevin Wolf, 2018/09/13
- [Qemu-devel] [PATCH v2 06/17] job: Use AIO_WAIT_WHILE() in job_finish_sync(), Kevin Wolf, 2018/09/13
- [Qemu-devel] [PATCH v2 09/17] block-backend: Add .drained_poll callback, Kevin Wolf, 2018/09/13
- [Qemu-devel] [PATCH v2 08/17] block: Add missing locking in bdrv_co_drain_bh_cb(), Kevin Wolf, 2018/09/13
- [Qemu-devel] [PATCH v2 07/17] test-bdrv-drain: Test AIO_WAIT_WHILE() in completion callback, Kevin Wolf, 2018/09/13
- [Qemu-devel] [PATCH v2 10/17] block-backend: Fix potential double blk_delete(), Kevin Wolf, 2018/09/13
- [Qemu-devel] [PATCH v2 11/17] block-backend: Decrease in_flight only after callback, Kevin Wolf, 2018/09/13