|
 |
26ba25 |
From 27fd652033779a16561160bef8aeda7f8f9c04be Mon Sep 17 00:00:00 2001
|
|
|
26ba25 |
From: Kevin Wolf <kwolf@redhat.com>
|
|
|
26ba25 |
Date: Wed, 10 Oct 2018 20:22:05 +0100
|
|
|
26ba25 |
Subject: [PATCH 39/49] block-backend: Decrease in_flight only after callback
|
|
|
26ba25 |
|
|
|
26ba25 |
RH-Author: Kevin Wolf <kwolf@redhat.com>
|
|
|
26ba25 |
Message-id: <20181010202213.7372-27-kwolf@redhat.com>
|
|
|
26ba25 |
Patchwork-id: 82617
|
|
|
26ba25 |
O-Subject: [RHEL-8 qemu-kvm PATCH 36/44] block-backend: Decrease in_flight only after callback
|
|
|
26ba25 |
Bugzilla: 1637976
|
|
|
26ba25 |
RH-Acked-by: Max Reitz <mreitz@redhat.com>
|
|
|
26ba25 |
RH-Acked-by: John Snow <jsnow@redhat.com>
|
|
|
26ba25 |
RH-Acked-by: Thomas Huth <thuth@redhat.com>
|
|
|
26ba25 |
|
|
|
26ba25 |
Request callbacks can do pretty much anything, including operations that
|
|
|
26ba25 |
will yield from the coroutine (such as draining the backend). In that
|
|
|
26ba25 |
case, a decreased in_flight would be visible to other code and could
|
|
|
26ba25 |
lead to a drain completing while the callback hasn't actually completed
|
|
|
26ba25 |
yet.
|
|
|
26ba25 |
|
|
|
26ba25 |
Note that reordering these operations forbids calling drain directly
|
|
|
26ba25 |
inside an AIO callback. As Paolo explains, indirectly calling it is
|
|
|
26ba25 |
okay:
|
|
|
26ba25 |
|
|
|
26ba25 |
- Calling it through a coroutine is okay, because then
|
|
|
26ba25 |
bdrv_drained_begin() goes through bdrv_co_yield_to_drain() and you
|
|
|
26ba25 |
have in_flight=2 when bdrv_co_yield_to_drain() yields, then soon
|
|
|
26ba25 |
in_flight=1 when the aio_co_wake() in the AIO callback completes, then
|
|
|
26ba25 |
in_flight=0 after the bottom half starts.
|
|
|
26ba25 |
|
|
|
26ba25 |
- Calling it through a bottom half would be okay too, as long as the AIO
|
|
|
26ba25 |
callback remembers to do inc_in_flight/dec_in_flight just like
|
|
|
26ba25 |
bdrv_co_yield_to_drain() and bdrv_co_drain_bh_cb() do
|
|
|
26ba25 |
|
|
|
26ba25 |
A few more important cases that come to mind:
|
|
|
26ba25 |
|
|
|
26ba25 |
- A coroutine that yields because of I/O is okay, with a sequence
|
|
|
26ba25 |
similar to bdrv_co_yield_to_drain().
|
|
|
26ba25 |
|
|
|
26ba25 |
- A coroutine that yields with no I/O pending will correctly decrease
|
|
|
26ba25 |
in_flight to zero before yielding.
|
|
|
26ba25 |
|
|
|
26ba25 |
- Calling more AIO from the callback won't overflow the counter just
|
|
|
26ba25 |
because of mutual recursion, because AIO functions always yield at
|
|
|
26ba25 |
least once before invoking the callback.
|
|
|
26ba25 |
|
|
|
26ba25 |
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
|
|
|
26ba25 |
Reviewed-by: Fam Zheng <famz@redhat.com>
|
|
|
26ba25 |
Reviewed-by: Max Reitz <mreitz@redhat.com>
|
|
|
26ba25 |
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
|
|
|
26ba25 |
(cherry picked from commit 46aaf2a566e364a62315219255099cbf1c9b990d)
|
|
|
26ba25 |
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
|
|
|
26ba25 |
Signed-off-by: Danilo C. L. de Paula <ddepaula@redhat.com>
|
|
|
26ba25 |
---
|
|
|
26ba25 |
block/block-backend.c | 2 +-
|
|
|
26ba25 |
1 file changed, 1 insertion(+), 1 deletion(-)
|
|
|
26ba25 |
|
|
|
26ba25 |
diff --git a/block/block-backend.c b/block/block-backend.c
|
|
|
26ba25 |
index bfd0331..b8ea286 100644
|
|
|
26ba25 |
--- a/block/block-backend.c
|
|
|
26ba25 |
+++ b/block/block-backend.c
|
|
|
26ba25 |
@@ -1341,8 +1341,8 @@ static const AIOCBInfo blk_aio_em_aiocb_info = {
|
|
|
26ba25 |
static void blk_aio_complete(BlkAioEmAIOCB *acb)
|
|
|
26ba25 |
{
|
|
|
26ba25 |
if (acb->has_returned) {
|
|
|
26ba25 |
- blk_dec_in_flight(acb->rwco.blk);
|
|
|
26ba25 |
acb->common.cb(acb->common.opaque, acb->rwco.ret);
|
|
|
26ba25 |
+ blk_dec_in_flight(acb->rwco.blk);
|
|
|
26ba25 |
qemu_aio_unref(acb);
|
|
|
26ba25 |
}
|
|
|
26ba25 |
}
|
|
|
26ba25 |
--
|
|
|
26ba25 |
1.8.3.1
|
|
|
26ba25 |
|