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