[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-block] [PATCH v2 06/20] block: Avoid unnecessary aio_poll() in AIO
From: |
Kevin Wolf |
Subject: |
[Qemu-block] [PATCH v2 06/20] block: Avoid unnecessary aio_poll() in AIO_WAIT_WHILE() |
Date: |
Tue, 29 May 2018 19:21:42 +0200 |
Commit 91af091f923 added an additional aio_poll() to BDRV_POLL_WHILE()
in order to make sure that all pending BHs are executed on drain. This
was the wrong place to make the fix, as it is useless overhead for all
other users of the macro and unnecessarily complicates the mechanism.
This patch effectively reverts said commit (the context has changed a
bit and the code has moved to AIO_WAIT_WHILE()) and instead polls in the
loop condition for drain.
The effect is probably hard to measure in any real-world use case
because actual I/O will dominate, but if I run only the initialisation
part of 'qemu-img convert' where it calls bdrv_block_status() for the
whole image to find out how much data there is copy, this phase actually
needs only roughly half the time after this patch.
Signed-off-by: Kevin Wolf <address@hidden>
Reviewed-by: Stefan Hajnoczi <address@hidden>
---
include/block/aio-wait.h | 22 ++++++++--------------
block/io.c | 11 ++++++++++-
2 files changed, 18 insertions(+), 15 deletions(-)
diff --git a/include/block/aio-wait.h b/include/block/aio-wait.h
index 8c90a2e66e..783d3678dd 100644
--- a/include/block/aio-wait.h
+++ b/include/block/aio-wait.h
@@ -73,29 +73,23 @@ typedef struct {
*/
#define AIO_WAIT_WHILE(wait, ctx, cond) ({ \
bool waited_ = false; \
- bool busy_ = true; \
AioWait *wait_ = (wait); \
AioContext *ctx_ = (ctx); \
if (in_aio_context_home_thread(ctx_)) { \
- while ((cond) || busy_) { \
- busy_ = aio_poll(ctx_, (cond)); \
- waited_ |= !!(cond) | busy_; \
+ while ((cond)) { \
+ aio_poll(ctx_, true); \
+ waited_ = true; \
} \
} else { \
assert(qemu_get_current_aio_context() == \
qemu_get_aio_context()); \
/* Increment wait_->num_waiters before evaluating cond. */ \
atomic_inc(&wait_->num_waiters); \
- while (busy_) { \
- if ((cond)) { \
- waited_ = busy_ = true; \
- aio_context_release(ctx_); \
- aio_poll(qemu_get_aio_context(), true); \
- aio_context_acquire(ctx_); \
- } else { \
- busy_ = aio_poll(ctx_, false); \
- waited_ |= busy_; \
- } \
+ while ((cond)) { \
+ aio_context_release(ctx_); \
+ aio_poll(qemu_get_aio_context(), true); \
+ aio_context_acquire(ctx_); \
+ waited_ = true; \
} \
atomic_dec(&wait_->num_waiters); \
} \
diff --git a/block/io.c b/block/io.c
index 58f50ba253..aa973f1d4a 100644
--- a/block/io.c
+++ b/block/io.c
@@ -182,13 +182,22 @@ static void bdrv_drain_invoke(BlockDriverState *bs, bool
begin)
BDRV_POLL_WHILE(bs, !data.done);
}
+/* Returns true if BDRV_POLL_WHILE() should go into a blocking aio_poll() */
+static bool bdrv_drain_poll(BlockDriverState *bs)
+{
+ /* Execute pending BHs first and check everything else only after the BHs
+ * have executed. */
+ while (aio_poll(bs->aio_context, false));
+ return atomic_read(&bs->in_flight);
+}
+
static bool bdrv_drain_recurse(BlockDriverState *bs)
{
BdrvChild *child, *tmp;
bool waited;
/* Wait for drained requests to finish */
- waited = BDRV_POLL_WHILE(bs, atomic_read(&bs->in_flight) > 0);
+ waited = BDRV_POLL_WHILE(bs, bdrv_drain_poll(bs));
QLIST_FOREACH_SAFE(child, &bs->children, next, tmp) {
BlockDriverState *bs = child->bs;
--
2.13.6
- [Qemu-block] [PATCH v2 00/20] Drain fixes and cleanups, part 3, Kevin Wolf, 2018/05/29
- [Qemu-block] [PATCH v2 01/20] test-bdrv-drain: bdrv_drain() works with cross-AioContext events, Kevin Wolf, 2018/05/29
- [Qemu-block] [PATCH v2 02/20] block: Use bdrv_do_drain_begin/end in bdrv_drain_all(), Kevin Wolf, 2018/05/29
- [Qemu-block] [PATCH v2 03/20] block: Remove 'recursive' parameter from bdrv_drain_invoke(), Kevin Wolf, 2018/05/29
- [Qemu-block] [PATCH v2 04/20] block: Don't manually poll in bdrv_drain_all(), Kevin Wolf, 2018/05/29
- [Qemu-block] [PATCH v2 05/20] tests/test-bdrv-drain: bdrv_drain_all() works in coroutines now, Kevin Wolf, 2018/05/29
- [Qemu-block] [PATCH v2 06/20] block: Avoid unnecessary aio_poll() in AIO_WAIT_WHILE(),
Kevin Wolf <=
- [Qemu-block] [PATCH v2 07/20] block: Really pause block jobs on drain, Kevin Wolf, 2018/05/29
- [Qemu-block] [PATCH v2 08/20] block: Remove bdrv_drain_recurse(), Kevin Wolf, 2018/05/29
- [Qemu-block] [PATCH v2 09/20] test-bdrv-drain: Add test for node deletion, Kevin Wolf, 2018/05/29
- [Qemu-block] [PATCH v2 11/20] test-bdrv-drain: Test node deletion in subtree recursion, Kevin Wolf, 2018/05/29
- [Qemu-block] [PATCH v2 12/20] block: Don't poll in parent drain callbacks, Kevin Wolf, 2018/05/29
- [Qemu-block] [PATCH v2 10/20] block: Drain recursively with a single BDRV_POLL_WHILE(), Kevin Wolf, 2018/05/29
- [Qemu-block] [PATCH v2 13/20] test-bdrv-drain: Graph change through parent callback, Kevin Wolf, 2018/05/29
- [Qemu-block] [PATCH v2 14/20] block: Defer .bdrv_drain_begin callback to polling phase, Kevin Wolf, 2018/05/29
- [Qemu-block] [PATCH v2 16/20] block: Allow AIO_WAIT_WHILE with NULL ctx, Kevin Wolf, 2018/05/29
- [Qemu-block] [PATCH v2 15/20] test-bdrv-drain: Test that bdrv_drain_invoke() doesn't poll, Kevin Wolf, 2018/05/29