[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH v2 13/16] block: only call aio_poll from iothread
From: |
Paolo Bonzini |
Subject: |
[Qemu-devel] [PATCH v2 13/16] block: only call aio_poll from iothread |
Date: |
Wed, 16 Mar 2016 15:16:54 +0100 |
aio_poll is not thread safe; for example bdrv_drain can hang if
the last in-flight I/O operation is completed in the I/O thread after
the main thread has checked bs->in_flight.
The bug remains latent as long as all of it is called within
aio_context_acquire/aio_context_release, but this will change soon.
To fix this, if bdrv_drain is called from outside the I/O thread handle
it internally in the BDS, without involving AioContext and aio_poll.
Reviewed-by: Fam Zheng <address@hidden>
Signed-off-by: Paolo Bonzini <address@hidden>
---
v1->v2: Remove misleading comment [Fam]
block.c | 2 ++
block/io.c | 18 +++++++++++++++---
include/block/block_int.h | 5 ++++-
3 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/block.c b/block.c
index 47f2367..2f13bfd 100644
--- a/block.c
+++ b/block.c
@@ -247,6 +247,7 @@ BlockDriverState *bdrv_new(void)
qemu_co_queue_init(&bs->throttled_reqs[1]);
bs->refcnt = 1;
bs->aio_context = qemu_get_aio_context();
+ qemu_event_init(&bs->in_flight_event, true);
QTAILQ_INSERT_TAIL(&all_bdrv_states, bs, bs_list);
@@ -2385,6 +2386,7 @@ static void bdrv_delete(BlockDriverState *bs)
bdrv_make_anon(bs);
QTAILQ_REMOVE(&all_bdrv_states, bs, bs_list);
+ qemu_event_destroy(&bs->in_flight_event);
g_free(bs);
}
diff --git a/block/io.c b/block/io.c
index 0a99131..c66b037 100644
--- a/block/io.c
+++ b/block/io.c
@@ -239,11 +239,21 @@ bool bdrv_requests_pending(BlockDriverState *bs)
static bool bdrv_drain_io_recurse(BlockDriverState *bs)
{
- BdrvChild *child;
+ AioContext *ctx = bdrv_get_aio_context(bs);
bool waited = false;
+ BdrvChild *child;
while (atomic_read(&bs->in_flight) > 0) {
- aio_poll(bdrv_get_aio_context(bs), true);
+ if (aio_context_in_iothread(ctx)) {
+ aio_poll(ctx, true);
+ } else {
+ qemu_event_reset(&bs->in_flight_event);
+ if (atomic_read(&bs->in_flight) > 0) {
+ aio_context_release(ctx);
+ qemu_event_wait(&bs->in_flight_event);
+ aio_context_acquire(ctx);
+ }
+ }
waited = true;
}
@@ -455,7 +465,9 @@ void bdrv_inc_in_flight(BlockDriverState *bs)
void bdrv_dec_in_flight(BlockDriverState *bs)
{
- atomic_dec(&bs->in_flight);
+ if (atomic_fetch_dec(&bs->in_flight) == 1) {
+ qemu_event_set(&bs->in_flight_event);
+ }
}
static bool coroutine_fn wait_serialising_requests(BdrvTrackedRequest *self)
diff --git a/include/block/block_int.h b/include/block/block_int.h
index b0311ac..489aa3c 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -404,9 +404,12 @@ struct BlockDriverState {
/* Callback before write request is processed */
NotifierWithReturnList before_write_notifiers;
- /* number of in-flight requests; overall and serialising */
+ /* number of in-flight requests; overall and serialising.
+ * in_flight_event is set when in_flight becomes 0.
+ */
unsigned int in_flight;
unsigned int serialising_in_flight;
+ QemuEvent in_flight_event;
/* I/O throttling.
* throttle_state tells us if this BDS has I/O limits configured.
--
1.8.3.1
- [Qemu-devel] [PATCH v2 05/16] mirror: use bottom half to re-enter coroutine, (continued)
- [Qemu-devel] [PATCH v2 05/16] mirror: use bottom half to re-enter coroutine, Paolo Bonzini, 2016/03/16
- [Qemu-devel] [PATCH v2 07/16] block: change drain to look only at one child at a time, Paolo Bonzini, 2016/03/16
- [Qemu-devel] [PATCH v2 09/16] block: wait for all pending I/O when doing synchronous requests, Paolo Bonzini, 2016/03/16
- [Qemu-devel] [PATCH v2 12/16] aio: introduce aio_context_in_iothread, Paolo Bonzini, 2016/03/16
- [Qemu-devel] [PATCH v2 11/16] sheepdog: disable dataplane, Paolo Bonzini, 2016/03/16
- [Qemu-devel] [PATCH v2 08/16] blockjob: introduce .drain callback for jobs, Paolo Bonzini, 2016/03/16
- [Qemu-devel] [PATCH v2 10/16] nfs: replace aio_poll with bdrv_drain, Paolo Bonzini, 2016/03/16
- [Qemu-devel] [PATCH v2 13/16] block: only call aio_poll from iothread,
Paolo Bonzini <=
- [Qemu-devel] [PATCH v2 16/16] aio: convert from RFifoLock to QemuRecMutex, Paolo Bonzini, 2016/03/16
- [Qemu-devel] [PATCH v2 14/16] iothread: release AioContext around aio_poll, Paolo Bonzini, 2016/03/16
- [Qemu-devel] [PATCH v2 15/16] qemu-thread: introduce QemuRecMutex, Paolo Bonzini, 2016/03/16
- Re: [Qemu-devel] [PATCH v2 00/16] AioContext fine-grained locking, part 1 of 3, including bdrv_drain rewrite, Stefan Hajnoczi, 2016/03/18