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