From 6fe050124f9433521227b5da8f560c7a3c248513 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 10 Oct 2018 20:08:38 +0100 Subject: [PATCH 07/49] block: Don't manually poll in bdrv_drain_all() RH-Author: Kevin Wolf Message-id: <20181010200843.6710-5-kwolf@redhat.com> Patchwork-id: 82584 O-Subject: [RHEL-8 qemu-kvm PATCH 04/44] block: Don't manually poll in bdrv_drain_all() Bugzilla: 1637976 RH-Acked-by: Max Reitz RH-Acked-by: John Snow RH-Acked-by: Thomas Huth All involved nodes are already idle, we called bdrv_do_drain_begin() on them. The comment in the code suggested that this was not correct because the completion of a request on one node could spawn a new request on a different node (which might have been drained before, so we wouldn't drain the new request). In reality, new requests to different nodes aren't spawned out of nothing, but only in the context of a parent request, and they aren't submitted to random nodes, but only to child nodes. As long as we still poll for the completion of the parent request (which we do), draining each root node separately is good enough. Remove the additional polling code from bdrv_drain_all_begin() and replace it with an assertion that all nodes are already idle after we drained them separately. Signed-off-by: Kevin Wolf Reviewed-by: Stefan Hajnoczi (cherry picked from commit c13ad59f012cbbccb866a10477458e69bc868dbb) Signed-off-by: Kevin Wolf Signed-off-by: Danilo C. L. de Paula --- block/io.c | 41 ++++++++++++----------------------------- 1 file changed, 12 insertions(+), 29 deletions(-) diff --git a/block/io.c b/block/io.c index aa41f1e..e5fc42c 100644 --- a/block/io.c +++ b/block/io.c @@ -376,6 +376,16 @@ void bdrv_drain(BlockDriverState *bs) bdrv_drained_end(bs); } +static void bdrv_drain_assert_idle(BlockDriverState *bs) +{ + BdrvChild *child, *next; + + assert(atomic_read(&bs->in_flight) == 0); + QLIST_FOREACH_SAFE(child, &bs->children, next, next) { + bdrv_drain_assert_idle(child->bs); + } +} + /* * Wait for pending requests to complete across all BlockDriverStates * @@ -390,11 +400,8 @@ void bdrv_drain(BlockDriverState *bs) */ void bdrv_drain_all_begin(void) { - /* Always run first iteration so any pending completion BHs run */ - bool waited = true; BlockDriverState *bs; BdrvNextIterator it; - GSList *aio_ctxs = NULL, *ctx; /* BDRV_POLL_WHILE() for a node can only be called from its own I/O thread * or the main loop AioContext. We potentially use BDRV_POLL_WHILE() on @@ -408,35 +415,11 @@ void bdrv_drain_all_begin(void) aio_context_acquire(aio_context); bdrv_do_drained_begin(bs, true, NULL); aio_context_release(aio_context); - - if (!g_slist_find(aio_ctxs, aio_context)) { - aio_ctxs = g_slist_prepend(aio_ctxs, aio_context); - } } - /* Note that completion of an asynchronous I/O operation can trigger any - * number of other I/O operations on other devices---for example a - * coroutine can submit an I/O request to another device in response to - * request completion. Therefore we must keep looping until there was no - * more activity rather than simply draining each device independently. - */ - while (waited) { - waited = false; - - for (ctx = aio_ctxs; ctx != NULL; ctx = ctx->next) { - AioContext *aio_context = ctx->data; - - aio_context_acquire(aio_context); - for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) { - if (aio_context == bdrv_get_aio_context(bs)) { - waited |= bdrv_drain_recurse(bs); - } - } - aio_context_release(aio_context); - } + for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) { + bdrv_drain_assert_idle(bs); } - - g_slist_free(aio_ctxs); } void bdrv_drain_all_end(void) -- 1.8.3.1