From nobody Sat May 18 22:14:49 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1527614669250878.6754086291965; Tue, 29 May 2018 10:24:29 -0700 (PDT) Received: from localhost ([::1]:34093 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNiMB-00064w-0U for importer@patchew.org; Tue, 29 May 2018 13:24:23 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58056) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNiK5-0004ij-Gd for qemu-devel@nongnu.org; Tue, 29 May 2018 13:22:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fNiK4-0001eF-A0 for qemu-devel@nongnu.org; Tue, 29 May 2018 13:22:13 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:38522 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fNiK0-0001bA-Q9; Tue, 29 May 2018 13:22:08 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 6FC7E400EB89; Tue, 29 May 2018 17:22:08 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-116-193.ams2.redhat.com [10.36.116.193]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1CE181117629; Tue, 29 May 2018 17:22:06 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Tue, 29 May 2018 19:21:37 +0200 Message-Id: <20180529172156.29311-2-kwolf@redhat.com> In-Reply-To: <20180529172156.29311-1-kwolf@redhat.com> References: <20180529172156.29311-1-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Tue, 29 May 2018 17:22:08 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Tue, 29 May 2018 17:22:08 +0000 (UTC) for IP:'10.11.54.3' DOMAIN:'int-mx03.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'kwolf@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PATCH v2 01/20] test-bdrv-drain: bdrv_drain() works with cross-AioContext events X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com, stefanha@redhat.com, pbonzini@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" As long as nobody keeps the other I/O thread from working, there is no reason why bdrv_drain() wouldn't work with cross-AioContext events. The key is that the root request we're waiting for is in the AioContext we're polling (which it always is for bdrv_drain()) so that aio_poll() is woken up in the end. Add a test case that shows that it works. Remove the comment in bdrv_drain() that claims otherwise. Signed-off-by: Kevin Wolf --- block/io.c | 4 -- tests/test-bdrv-drain.c | 187 ++++++++++++++++++++++++++++++++++++++++++++= +++- 2 files changed, 186 insertions(+), 5 deletions(-) diff --git a/block/io.c b/block/io.c index ca96b487eb..1e4e2f40ea 100644 --- a/block/io.c +++ b/block/io.c @@ -370,10 +370,6 @@ void bdrv_unapply_subtree_drain(BdrvChild *child, Bloc= kDriverState *old_parent) * * Note that unlike bdrv_drain_all(), the caller must hold the BlockDriver= State * AioContext. - * - * Only this BlockDriverState's AioContext is run, so in-flight requests m= ust - * not depend on events in other AioContexts. In that case, use - * bdrv_drain_all() instead. */ void coroutine_fn bdrv_co_drain(BlockDriverState *bs) { diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c index 2cba63b881..1682e2b72d 100644 --- a/tests/test-bdrv-drain.c +++ b/tests/test-bdrv-drain.c @@ -27,9 +27,13 @@ #include "block/blockjob_int.h" #include "sysemu/block-backend.h" #include "qapi/error.h" +#include "iothread.h" + +static QemuEvent done_event; =20 typedef struct BDRVTestState { int drain_count; + AioContext *bh_indirection_ctx; } BDRVTestState; =20 static void coroutine_fn bdrv_test_co_drain_begin(BlockDriverState *bs) @@ -50,16 +54,29 @@ static void bdrv_test_close(BlockDriverState *bs) g_assert_cmpint(s->drain_count, >, 0); } =20 +static void co_reenter_bh(void *opaque) +{ + aio_co_wake(opaque); +} + static int coroutine_fn bdrv_test_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t byte= s, QEMUIOVector *qiov, int flags) { + BDRVTestState *s =3D bs->opaque; + /* We want this request to stay until the polling loop in drain waits = for * it to complete. We need to sleep a while as bdrv_drain_invoke() com= es * first and polls its result, too, but it shouldn't accidentally comp= lete * this request yet. */ qemu_co_sleep_ns(QEMU_CLOCK_REALTIME, 100000); =20 + if (s->bh_indirection_ctx) { + aio_bh_schedule_oneshot(s->bh_indirection_ctx, co_reenter_bh, + qemu_coroutine_self()); + qemu_coroutine_yield(); + } + return 0; } =20 @@ -490,6 +507,164 @@ static void test_graph_change(void) blk_unref(blk_b); } =20 +struct test_iothread_data { + BlockDriverState *bs; + enum drain_type drain_type; + int *aio_ret; +}; + +static void test_iothread_drain_entry(void *opaque) +{ + struct test_iothread_data *data =3D opaque; + + aio_context_acquire(bdrv_get_aio_context(data->bs)); + do_drain_begin(data->drain_type, data->bs); + g_assert_cmpint(*data->aio_ret, =3D=3D, 0); + do_drain_end(data->drain_type, data->bs); + aio_context_release(bdrv_get_aio_context(data->bs)); + + qemu_event_set(&done_event); +} + +static void test_iothread_aio_cb(void *opaque, int ret) +{ + int *aio_ret =3D opaque; + *aio_ret =3D ret; + qemu_event_set(&done_event); +} + +/* + * Starts an AIO request on a BDS that runs in the AioContext of iothread = 1. + * The request involves a BH on iothread 2 before it can complete. + * + * @drain_thread =3D 0 means that do_drain_begin/end are called from the m= ain + * thread, @drain_thread =3D 1 means that they are called from iothread 1.= Drain + * for this BDS cannot be called from iothread 2 because only the main thr= ead + * may do cross-AioContext polling. + */ +static void test_iothread_common(enum drain_type drain_type, int drain_thr= ead) +{ + BlockBackend *blk; + BlockDriverState *bs; + BDRVTestState *s; + BlockAIOCB *acb; + int aio_ret; + struct test_iothread_data data; + + IOThread *a =3D iothread_new(); + IOThread *b =3D iothread_new(); + AioContext *ctx_a =3D iothread_get_aio_context(a); + AioContext *ctx_b =3D iothread_get_aio_context(b); + + QEMUIOVector qiov; + struct iovec iov =3D { + .iov_base =3D NULL, + .iov_len =3D 0, + }; + qemu_iovec_init_external(&qiov, &iov, 1); + + /* bdrv_drain_all() may only be called from the main loop thread */ + if (drain_type =3D=3D BDRV_DRAIN_ALL && drain_thread !=3D 0) { + goto out; + } + + blk =3D blk_new(BLK_PERM_ALL, BLK_PERM_ALL); + bs =3D bdrv_new_open_driver(&bdrv_test, "test-node", BDRV_O_RDWR, + &error_abort); + s =3D bs->opaque; + blk_insert_bs(blk, bs, &error_abort); + + blk_set_aio_context(blk, ctx_a); + aio_context_acquire(ctx_a); + + s->bh_indirection_ctx =3D ctx_b; + + aio_ret =3D -EINPROGRESS; + if (drain_thread =3D=3D 0) { + acb =3D blk_aio_preadv(blk, 0, &qiov, 0, test_iothread_aio_cb, &ai= o_ret); + } else { + acb =3D blk_aio_preadv(blk, 0, &qiov, 0, aio_ret_cb, &aio_ret); + } + g_assert(acb !=3D NULL); + g_assert_cmpint(aio_ret, =3D=3D, -EINPROGRESS); + + aio_context_release(ctx_a); + + data =3D (struct test_iothread_data) { + .bs =3D bs, + .drain_type =3D drain_type, + .aio_ret =3D &aio_ret, + }; + + switch (drain_thread) { + case 0: + if (drain_type !=3D BDRV_DRAIN_ALL) { + aio_context_acquire(ctx_a); + } + + /* The request is running on the IOThread a. Draining its block de= vice + * will make sure that it has completed as far as the BDS is conce= rned, + * but the drain in this thread can continue immediately after + * bdrv_dec_in_flight() and aio_ret might be assigned only slightly + * later. */ + qemu_event_reset(&done_event); + do_drain_begin(drain_type, bs); + g_assert_cmpint(bs->in_flight, =3D=3D, 0); + + if (drain_type !=3D BDRV_DRAIN_ALL) { + aio_context_release(ctx_a); + } + qemu_event_wait(&done_event); + if (drain_type !=3D BDRV_DRAIN_ALL) { + aio_context_acquire(ctx_a); + } + + g_assert_cmpint(aio_ret, =3D=3D, 0); + do_drain_end(drain_type, bs); + + if (drain_type !=3D BDRV_DRAIN_ALL) { + aio_context_release(ctx_a); + } + break; + case 1: + qemu_event_reset(&done_event); + aio_bh_schedule_oneshot(ctx_a, test_iothread_drain_entry, &data); + qemu_event_wait(&done_event); + break; + default: + g_assert_not_reached(); + } + + aio_context_acquire(ctx_a); + blk_set_aio_context(blk, qemu_get_aio_context()); + aio_context_release(ctx_a); + + bdrv_unref(bs); + blk_unref(blk); + +out: + iothread_join(a); + iothread_join(b); +} + +static void test_iothread_drain_all(void) +{ + test_iothread_common(BDRV_DRAIN_ALL, 0); + test_iothread_common(BDRV_DRAIN_ALL, 1); +} + +static void test_iothread_drain(void) +{ + test_iothread_common(BDRV_DRAIN, 0); + test_iothread_common(BDRV_DRAIN, 1); +} + +static void test_iothread_drain_subtree(void) +{ + test_iothread_common(BDRV_SUBTREE_DRAIN, 0); + test_iothread_common(BDRV_SUBTREE_DRAIN, 1); +} + =20 typedef struct TestBlockJob { BlockJob common; @@ -618,10 +793,13 @@ static void test_blockjob_drain_subtree(void) =20 int main(int argc, char **argv) { + int ret; + bdrv_init(); qemu_init_main_loop(&error_abort); =20 g_test_init(&argc, &argv, NULL); + qemu_event_init(&done_event, false); =20 g_test_add_func("/bdrv-drain/driver-cb/drain_all", test_drv_cb_drain_a= ll); g_test_add_func("/bdrv-drain/driver-cb/drain", test_drv_cb_drain); @@ -648,10 +826,17 @@ int main(int argc, char **argv) g_test_add_func("/bdrv-drain/multiparent", test_multiparent); g_test_add_func("/bdrv-drain/graph-change", test_graph_change); =20 + g_test_add_func("/bdrv-drain/iothread/drain_all", test_iothread_drain_= all); + g_test_add_func("/bdrv-drain/iothread/drain", test_iothread_drain); + g_test_add_func("/bdrv-drain/iothread/drain_subtree", + test_iothread_drain_subtree); + g_test_add_func("/bdrv-drain/blockjob/drain_all", test_blockjob_drain_= all); g_test_add_func("/bdrv-drain/blockjob/drain", test_blockjob_drain); g_test_add_func("/bdrv-drain/blockjob/drain_subtree", test_blockjob_drain_subtree); =20 - return g_test_run(); + ret =3D g_test_run(); + qemu_event_destroy(&done_event); + return ret; } --=20 2.13.6 From nobody Sat May 18 22:14:49 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1527614685301263.163737598033; Tue, 29 May 2018 10:24:45 -0700 (PDT) Received: from localhost ([::1]:34096 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNiMW-0006Js-AU for importer@patchew.org; Tue, 29 May 2018 13:24:44 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58065) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNiK5-0004j0-Qg for qemu-devel@nongnu.org; Tue, 29 May 2018 13:22:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fNiK4-0001eb-QG for qemu-devel@nongnu.org; Tue, 29 May 2018 13:22:13 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:53708 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fNiK2-0001cK-Cw; Tue, 29 May 2018 13:22:10 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 09D95BD9E; Tue, 29 May 2018 17:22:10 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-116-193.ams2.redhat.com [10.36.116.193]) by smtp.corp.redhat.com (Postfix) with ESMTP id AA3F31117629; Tue, 29 May 2018 17:22:08 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Tue, 29 May 2018 19:21:38 +0200 Message-Id: <20180529172156.29311-3-kwolf@redhat.com> In-Reply-To: <20180529172156.29311-1-kwolf@redhat.com> References: <20180529172156.29311-1-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Tue, 29 May 2018 17:22:10 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Tue, 29 May 2018 17:22:10 +0000 (UTC) for IP:'10.11.54.3' DOMAIN:'int-mx03.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'kwolf@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PATCH v2 02/20] block: Use bdrv_do_drain_begin/end in bdrv_drain_all() X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com, stefanha@redhat.com, pbonzini@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" bdrv_do_drain_begin/end() implement already everything that bdrv_drain_all_begin/end() need and currently still do manually: Disable external events, call parent drain callbacks, call block driver callbacks. It also does two more things: The first is incrementing bs->quiesce_counter. bdrv_drain_all() already stood out in the test case by behaving different from the other drain variants. Adding this is not only safe, but in fact a bug fix. The second is calling bdrv_drain_recurse(). We already do that later in the same function in a loop, so basically doing an early first iteration doesn't hurt. Signed-off-by: Kevin Wolf Reviewed-by: Stefan Hajnoczi --- block/io.c | 10 ++-------- tests/test-bdrv-drain.c | 14 ++++---------- 2 files changed, 6 insertions(+), 18 deletions(-) diff --git a/block/io.c b/block/io.c index 1e4e2f40ea..73579b31cf 100644 --- a/block/io.c +++ b/block/io.c @@ -413,11 +413,8 @@ void bdrv_drain_all_begin(void) for (bs =3D bdrv_first(&it); bs; bs =3D bdrv_next(&it)) { AioContext *aio_context =3D bdrv_get_aio_context(bs); =20 - /* Stop things in parent-to-child order */ aio_context_acquire(aio_context); - aio_disable_external(aio_context); - bdrv_parent_drained_begin(bs, NULL); - bdrv_drain_invoke(bs, true, true); + bdrv_do_drained_begin(bs, true, NULL); aio_context_release(aio_context); =20 if (!g_slist_find(aio_ctxs, aio_context)) { @@ -458,11 +455,8 @@ void bdrv_drain_all_end(void) for (bs =3D bdrv_first(&it); bs; bs =3D bdrv_next(&it)) { AioContext *aio_context =3D bdrv_get_aio_context(bs); =20 - /* Re-enable things in child-to-parent order */ aio_context_acquire(aio_context); - bdrv_drain_invoke(bs, false, true); - bdrv_parent_drained_end(bs, NULL); - aio_enable_external(aio_context); + bdrv_do_drained_end(bs, true, NULL); aio_context_release(aio_context); } } diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c index 1682e2b72d..4c3e93de0b 100644 --- a/tests/test-bdrv-drain.c +++ b/tests/test-bdrv-drain.c @@ -276,8 +276,7 @@ static void test_quiesce_common(enum drain_type drain_t= ype, bool recursive) =20 static void test_quiesce_drain_all(void) { - // XXX drain_all doesn't quiesce - //test_quiesce_common(BDRV_DRAIN_ALL, true); + test_quiesce_common(BDRV_DRAIN_ALL, true); } =20 static void test_quiesce_drain(void) @@ -319,12 +318,7 @@ static void test_nested(void) =20 for (outer =3D 0; outer < DRAIN_TYPE_MAX; outer++) { for (inner =3D 0; inner < DRAIN_TYPE_MAX; inner++) { - /* XXX bdrv_drain_all() doesn't increase the quiesce_counter */ - int bs_quiesce =3D (outer !=3D BDRV_DRAIN_ALL) + - (inner !=3D BDRV_DRAIN_ALL); - int backing_quiesce =3D (outer =3D=3D BDRV_SUBTREE_DRAIN) + - (inner =3D=3D BDRV_SUBTREE_DRAIN); - int backing_cb_cnt =3D (outer !=3D BDRV_DRAIN) + + int backing_quiesce =3D (outer !=3D BDRV_DRAIN) + (inner !=3D BDRV_DRAIN); =20 g_assert_cmpint(bs->quiesce_counter, =3D=3D, 0); @@ -335,10 +329,10 @@ static void test_nested(void) do_drain_begin(outer, bs); do_drain_begin(inner, bs); =20 - g_assert_cmpint(bs->quiesce_counter, =3D=3D, bs_quiesce); + g_assert_cmpint(bs->quiesce_counter, =3D=3D, 2); g_assert_cmpint(backing->quiesce_counter, =3D=3D, backing_quie= sce); g_assert_cmpint(s->drain_count, =3D=3D, 2); - g_assert_cmpint(backing_s->drain_count, =3D=3D, backing_cb_cnt= ); + g_assert_cmpint(backing_s->drain_count, =3D=3D, backing_quiesc= e); =20 do_drain_end(inner, bs); do_drain_end(outer, bs); --=20 2.13.6 From nobody Sat May 18 22:14:49 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1527614687150800.9430203541235; Tue, 29 May 2018 10:24:47 -0700 (PDT) Received: from localhost ([::1]:34095 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNiMU-0006JF-HG for importer@patchew.org; Tue, 29 May 2018 13:24:42 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58095) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNiK6-0004kP-R6 for qemu-devel@nongnu.org; Tue, 29 May 2018 13:22:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fNiK5-0001fd-VH for qemu-devel@nongnu.org; Tue, 29 May 2018 13:22:14 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:44872 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fNiK3-0001db-Ug; Tue, 29 May 2018 13:22:11 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 96D94808255B; Tue, 29 May 2018 17:22:11 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-116-193.ams2.redhat.com [10.36.116.193]) by smtp.corp.redhat.com (Postfix) with ESMTP id 441771117629; Tue, 29 May 2018 17:22:10 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Tue, 29 May 2018 19:21:39 +0200 Message-Id: <20180529172156.29311-4-kwolf@redhat.com> In-Reply-To: <20180529172156.29311-1-kwolf@redhat.com> References: <20180529172156.29311-1-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Tue, 29 May 2018 17:22:11 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Tue, 29 May 2018 17:22:11 +0000 (UTC) for IP:'10.11.54.3' DOMAIN:'int-mx03.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'kwolf@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PATCH v2 03/20] block: Remove 'recursive' parameter from bdrv_drain_invoke() X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com, stefanha@redhat.com, pbonzini@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" All callers pass false for the 'recursive' parameter now. Remove it. Signed-off-by: Kevin Wolf Reviewed-by: Stefan Hajnoczi --- block/io.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/block/io.c b/block/io.c index 73579b31cf..48d4d4df22 100644 --- a/block/io.c +++ b/block/io.c @@ -168,9 +168,8 @@ static void coroutine_fn bdrv_drain_invoke_entry(void *= opaque) } =20 /* Recursively call BlockDriver.bdrv_co_drain_begin/end callbacks */ -static void bdrv_drain_invoke(BlockDriverState *bs, bool begin, bool recur= sive) +static void bdrv_drain_invoke(BlockDriverState *bs, bool begin) { - BdrvChild *child, *tmp; BdrvCoDrainData data =3D { .bs =3D bs, .done =3D false, .begin =3D beg= in}; =20 if (!bs->drv || (begin && !bs->drv->bdrv_co_drain_begin) || @@ -181,12 +180,6 @@ static void bdrv_drain_invoke(BlockDriverState *bs, bo= ol begin, bool recursive) data.co =3D qemu_coroutine_create(bdrv_drain_invoke_entry, &data); bdrv_coroutine_enter(bs, data.co); BDRV_POLL_WHILE(bs, !data.done); - - if (recursive) { - QLIST_FOREACH_SAFE(child, &bs->children, next, tmp) { - bdrv_drain_invoke(child->bs, begin, true); - } - } } =20 static bool bdrv_drain_recurse(BlockDriverState *bs) @@ -287,7 +280,7 @@ void bdrv_do_drained_begin(BlockDriverState *bs, bool r= ecursive, } =20 bdrv_parent_drained_begin(bs, parent); - bdrv_drain_invoke(bs, true, false); + bdrv_drain_invoke(bs, true); bdrv_drain_recurse(bs); =20 if (recursive) { @@ -322,7 +315,7 @@ void bdrv_do_drained_end(BlockDriverState *bs, bool rec= ursive, old_quiesce_counter =3D atomic_fetch_dec(&bs->quiesce_counter); =20 /* Re-enable things in child-to-parent order */ - bdrv_drain_invoke(bs, false, false); + bdrv_drain_invoke(bs, false); bdrv_parent_drained_end(bs, parent); if (old_quiesce_counter =3D=3D 1) { aio_enable_external(bdrv_get_aio_context(bs)); --=20 2.13.6 From nobody Sat May 18 22:14:49 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1527614847436337.85921622896205; Tue, 29 May 2018 10:27:27 -0700 (PDT) Received: from localhost ([::1]:34117 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNiP8-0000MF-Hy for importer@patchew.org; Tue, 29 May 2018 13:27:26 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58119) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNiK8-0004n9-QR for qemu-devel@nongnu.org; Tue, 29 May 2018 13:22:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fNiK7-0001gU-QF for qemu-devel@nongnu.org; Tue, 29 May 2018 13:22:16 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:45534 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fNiK5-0001es-Gu; Tue, 29 May 2018 13:22:13 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 2FE2F401EF09; Tue, 29 May 2018 17:22:13 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-116-193.ams2.redhat.com [10.36.116.193]) by smtp.corp.redhat.com (Postfix) with ESMTP id D11731117629; Tue, 29 May 2018 17:22:11 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Tue, 29 May 2018 19:21:40 +0200 Message-Id: <20180529172156.29311-5-kwolf@redhat.com> In-Reply-To: <20180529172156.29311-1-kwolf@redhat.com> References: <20180529172156.29311-1-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.5]); Tue, 29 May 2018 17:22:13 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.5]); Tue, 29 May 2018 17:22:13 +0000 (UTC) for IP:'10.11.54.3' DOMAIN:'int-mx03.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'kwolf@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PATCH v2 04/20] block: Don't manually poll in bdrv_drain_all() X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com, stefanha@redhat.com, pbonzini@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" 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 --- block/io.c | 41 ++++++++++++----------------------------- 1 file changed, 12 insertions(+), 29 deletions(-) diff --git a/block/io.c b/block/io.c index 48d4d4df22..58f50ba253 100644 --- a/block/io.c +++ b/block/io.c @@ -377,6 +377,16 @@ void bdrv_drain(BlockDriverState *bs) bdrv_drained_end(bs); } =20 +static void bdrv_drain_assert_idle(BlockDriverState *bs) +{ + BdrvChild *child, *next; + + assert(atomic_read(&bs->in_flight) =3D=3D 0); + QLIST_FOREACH_SAFE(child, &bs->children, next, next) { + bdrv_drain_assert_idle(child->bs); + } +} + /* * Wait for pending requests to complete across all BlockDriverStates * @@ -391,11 +401,8 @@ void bdrv_drain(BlockDriverState *bs) */ void bdrv_drain_all_begin(void) { - /* Always run first iteration so any pending completion BHs run */ - bool waited =3D true; BlockDriverState *bs; BdrvNextIterator it; - GSList *aio_ctxs =3D NULL, *ctx; =20 /* BDRV_POLL_WHILE() for a node can only be called from its own I/O th= read * or the main loop AioContext. We potentially use BDRV_POLL_WHILE() on @@ -409,35 +416,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 =3D g_slist_prepend(aio_ctxs, aio_context); - } } =20 - /* Note that completion of an asynchronous I/O operation can trigger a= ny - * 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 =3D false; - - for (ctx =3D aio_ctxs; ctx !=3D NULL; ctx =3D ctx->next) { - AioContext *aio_context =3D ctx->data; - - aio_context_acquire(aio_context); - for (bs =3D bdrv_first(&it); bs; bs =3D bdrv_next(&it)) { - if (aio_context =3D=3D bdrv_get_aio_context(bs)) { - waited |=3D bdrv_drain_recurse(bs); - } - } - aio_context_release(aio_context); - } + for (bs =3D bdrv_first(&it); bs; bs =3D bdrv_next(&it)) { + bdrv_drain_assert_idle(bs); } - - g_slist_free(aio_ctxs); } =20 void bdrv_drain_all_end(void) --=20 2.13.6 From nobody Sat May 18 22:14:49 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1527614834420189.7004096028653; Tue, 29 May 2018 10:27:14 -0700 (PDT) Received: from localhost ([::1]:34116 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNiOv-0000A2-JQ for importer@patchew.org; Tue, 29 May 2018 13:27:13 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58151) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNiKC-0004qi-Dt for qemu-devel@nongnu.org; Tue, 29 May 2018 13:22:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fNiKB-0001i0-LQ for qemu-devel@nongnu.org; Tue, 29 May 2018 13:22:20 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:38550 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fNiK7-0001g8-3H; Tue, 29 May 2018 13:22:15 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id BDB0D402178A; Tue, 29 May 2018 17:22:14 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-116-193.ams2.redhat.com [10.36.116.193]) by smtp.corp.redhat.com (Postfix) with ESMTP id 6A6A61117629; Tue, 29 May 2018 17:22:13 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Tue, 29 May 2018 19:21:41 +0200 Message-Id: <20180529172156.29311-6-kwolf@redhat.com> In-Reply-To: <20180529172156.29311-1-kwolf@redhat.com> References: <20180529172156.29311-1-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Tue, 29 May 2018 17:22:14 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Tue, 29 May 2018 17:22:14 +0000 (UTC) for IP:'10.11.54.3' DOMAIN:'int-mx03.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'kwolf@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PATCH v2 05/20] tests/test-bdrv-drain: bdrv_drain_all() works in coroutines now X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com, stefanha@redhat.com, pbonzini@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Since we use bdrv_do_drained_begin/end() for bdrv_drain_all_begin/end(), coroutine context is automatically left with a BH, preventing the deadlocks that made bdrv_drain_all*() unsafe in coroutine context. Now that we even removed the old polling code as dead code, it's obvious that it's compatible now. Enable the coroutine test cases for bdrv_drain_all(). Signed-off-by: Kevin Wolf Reviewed-by: Stefan Hajnoczi --- tests/test-bdrv-drain.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c index 4c3e93de0b..17127e63e4 100644 --- a/tests/test-bdrv-drain.c +++ b/tests/test-bdrv-drain.c @@ -233,6 +233,11 @@ static void test_drv_cb_drain_subtree(void) test_drv_cb_common(BDRV_SUBTREE_DRAIN, true); } =20 +static void test_drv_cb_co_drain_all(void) +{ + call_in_coroutine(test_drv_cb_drain_all); +} + static void test_drv_cb_co_drain(void) { call_in_coroutine(test_drv_cb_drain); @@ -289,6 +294,11 @@ static void test_quiesce_drain_subtree(void) test_quiesce_common(BDRV_SUBTREE_DRAIN, true); } =20 +static void test_quiesce_co_drain_all(void) +{ + call_in_coroutine(test_quiesce_drain_all); +} + static void test_quiesce_co_drain(void) { call_in_coroutine(test_quiesce_drain); @@ -800,7 +810,8 @@ int main(int argc, char **argv) g_test_add_func("/bdrv-drain/driver-cb/drain_subtree", test_drv_cb_drain_subtree); =20 - // XXX bdrv_drain_all() doesn't work in coroutine context + g_test_add_func("/bdrv-drain/driver-cb/co/drain_all", + test_drv_cb_co_drain_all); g_test_add_func("/bdrv-drain/driver-cb/co/drain", test_drv_cb_co_drain= ); g_test_add_func("/bdrv-drain/driver-cb/co/drain_subtree", test_drv_cb_co_drain_subtree); @@ -811,7 +822,8 @@ int main(int argc, char **argv) g_test_add_func("/bdrv-drain/quiesce/drain_subtree", test_quiesce_drain_subtree); =20 - // XXX bdrv_drain_all() doesn't work in coroutine context + g_test_add_func("/bdrv-drain/quiesce/co/drain_all", + test_quiesce_co_drain_all); g_test_add_func("/bdrv-drain/quiesce/co/drain", test_quiesce_co_drain); g_test_add_func("/bdrv-drain/quiesce/co/drain_subtree", test_quiesce_co_drain_subtree); --=20 2.13.6 From nobody Sat May 18 22:14:49 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1527614900348548.779117812664; Tue, 29 May 2018 10:28:20 -0700 (PDT) Received: from localhost ([::1]:34119 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNiPz-00016h-Go for importer@patchew.org; Tue, 29 May 2018 13:28:19 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58178) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNiKD-0004rr-Tt for qemu-devel@nongnu.org; Tue, 29 May 2018 13:22:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fNiKC-0001ij-VG for qemu-devel@nongnu.org; Tue, 29 May 2018 13:22:21 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:44876 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fNiK8-0001gp-Mp; Tue, 29 May 2018 13:22:16 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 57DA1808255B; Tue, 29 May 2018 17:22:16 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-116-193.ams2.redhat.com [10.36.116.193]) by smtp.corp.redhat.com (Postfix) with ESMTP id 037931117629; Tue, 29 May 2018 17:22:14 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Tue, 29 May 2018 19:21:42 +0200 Message-Id: <20180529172156.29311-7-kwolf@redhat.com> In-Reply-To: <20180529172156.29311-1-kwolf@redhat.com> References: <20180529172156.29311-1-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Tue, 29 May 2018 17:22:16 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Tue, 29 May 2018 17:22:16 +0000 (UTC) for IP:'10.11.54.3' DOMAIN:'int-mx03.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'kwolf@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PATCH v2 06/20] block: Avoid unnecessary aio_poll() in AIO_WAIT_WHILE() X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com, stefanha@redhat.com, pbonzini@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" 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 Reviewed-by: Stefan Hajnoczi --- 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_ =3D false; \ - bool busy_ =3D true; \ AioWait *wait_ =3D (wait); \ AioContext *ctx_ =3D (ctx); \ if (in_aio_context_home_thread(ctx_)) { \ - while ((cond) || busy_) { \ - busy_ =3D aio_poll(ctx_, (cond)); \ - waited_ |=3D !!(cond) | busy_; \ + while ((cond)) { \ + aio_poll(ctx_, true); \ + waited_ =3D true; \ } \ } else { \ assert(qemu_get_current_aio_context() =3D=3D \ qemu_get_aio_context()); \ /* Increment wait_->num_waiters before evaluating cond. */ \ atomic_inc(&wait_->num_waiters); \ - while (busy_) { \ - if ((cond)) { \ - waited_ =3D busy_ =3D true; \ - aio_context_release(ctx_); \ - aio_poll(qemu_get_aio_context(), true); \ - aio_context_acquire(ctx_); \ - } else { \ - busy_ =3D aio_poll(ctx_, false); \ - waited_ |=3D busy_; \ - } \ + while ((cond)) { \ + aio_context_release(ctx_); \ + aio_poll(qemu_get_aio_context(), true); \ + aio_context_acquire(ctx_); \ + waited_ =3D 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, b= ool begin) BDRV_POLL_WHILE(bs, !data.done); } =20 +/* 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; =20 /* Wait for drained requests to finish */ - waited =3D BDRV_POLL_WHILE(bs, atomic_read(&bs->in_flight) > 0); + waited =3D BDRV_POLL_WHILE(bs, bdrv_drain_poll(bs)); =20 QLIST_FOREACH_SAFE(child, &bs->children, next, tmp) { BlockDriverState *bs =3D child->bs; --=20 2.13.6 From nobody Sat May 18 22:14:49 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1527615239673503.5794534495783; Tue, 29 May 2018 10:33:59 -0700 (PDT) Received: from localhost ([::1]:34163 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNiVS-0005b0-P8 for importer@patchew.org; Tue, 29 May 2018 13:33:58 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58213) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNiKI-0004wE-3p for qemu-devel@nongnu.org; Tue, 29 May 2018 13:22:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fNiKG-0001l3-9A for qemu-devel@nongnu.org; Tue, 29 May 2018 13:22:26 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:38554 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fNiKA-0001hN-A1; Tue, 29 May 2018 13:22:18 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E69AD402178A; Tue, 29 May 2018 17:22:17 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-116-193.ams2.redhat.com [10.36.116.193]) by smtp.corp.redhat.com (Postfix) with ESMTP id 9209C1117629; Tue, 29 May 2018 17:22:16 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Tue, 29 May 2018 19:21:43 +0200 Message-Id: <20180529172156.29311-8-kwolf@redhat.com> In-Reply-To: <20180529172156.29311-1-kwolf@redhat.com> References: <20180529172156.29311-1-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Tue, 29 May 2018 17:22:17 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Tue, 29 May 2018 17:22:17 +0000 (UTC) for IP:'10.11.54.3' DOMAIN:'int-mx03.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'kwolf@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PATCH v2 07/20] block: Really pause block jobs on drain X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com, stefanha@redhat.com, pbonzini@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" We already requested that block jobs be paused in .bdrv_drained_begin, but no guarantee was made that the job was actually inactive at the point where bdrv_drained_begin() returned. This introduces a new callback BdrvChildRole.bdrv_drained_poll() and uses it to make bdrv_drain_poll() consider block jobs using the node to be drained. For the test case to work as expected, we have to switch from block_job_sleep_ns() to qemu_co_sleep_ns() so that the test job is even considered active and must be waited for when draining the node. Signed-off-by: Kevin Wolf --- include/block/block.h | 8 ++++++++ include/block/block_int.h | 7 +++++++ include/block/blockjob_int.h | 8 ++++++++ block.c | 9 +++++++++ block/io.c | 40 ++++++++++++++++++++++++++++++++++------ block/mirror.c | 8 ++++++++ blockjob.c | 23 +++++++++++++++++++++++ tests/test-bdrv-drain.c | 18 ++++++++++-------- 8 files changed, 107 insertions(+), 14 deletions(-) diff --git a/include/block/block.h b/include/block/block.h index 3894edda9d..52f07da3d3 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -566,6 +566,14 @@ void bdrv_parent_drained_begin(BlockDriverState *bs, B= drvChild *ignore); void bdrv_parent_drained_end(BlockDriverState *bs, BdrvChild *ignore); =20 /** + * bdrv_drain_poll: + * + * Poll for pending requests in @bs and its parents (except for + * @ignore_parent). This is part of bdrv_drained_begin. + */ +bool bdrv_drain_poll(BlockDriverState *bs, BdrvChild *ignore_parent); + +/** * bdrv_drained_begin: * * Begin a quiesced section for exclusive access to the BDS, by disabling diff --git a/include/block/block_int.h b/include/block/block_int.h index 6c0927bce3..feba86888e 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -573,6 +573,13 @@ struct BdrvChildRole { void (*drained_begin)(BdrvChild *child); void (*drained_end)(BdrvChild *child); =20 + /* + * Returns whether the parent has pending requests for the child. This + * callback is polled after .drained_begin() has been called until all + * activity on the child has stopped. + */ + bool (*drained_poll)(BdrvChild *child); + /* Notifies the parent that the child has been activated/inactivated (= e.g. * when migration is completing) and it can start/stop requesting * permissions and doing I/O on it. */ diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h index 5cd50c6639..e4a318dd15 100644 --- a/include/block/blockjob_int.h +++ b/include/block/blockjob_int.h @@ -39,6 +39,14 @@ struct BlockJobDriver { JobDriver job_driver; =20 /* + * Returns whether the job has pending requests for the child or will + * submit new requests before the next pause point. This callback is p= olled + * in the context of draining a job node after requesting that the job= be + * paused, until all activity on the child has stopped. + */ + bool (*drained_poll)(BlockJob *job); + + /* * If the callback is not NULL, it will be invoked before the job is * resumed in a new AioContext. This is the place to move any resourc= es * besides job->blk to the new AioContext. diff --git a/block.c b/block.c index 501b64c819..3c654a8a63 100644 --- a/block.c +++ b/block.c @@ -820,6 +820,12 @@ static void bdrv_child_cb_drained_begin(BdrvChild *chi= ld) bdrv_drained_begin(bs); } =20 +static bool bdrv_child_cb_drained_poll(BdrvChild *child) +{ + BlockDriverState *bs =3D child->opaque; + return bdrv_drain_poll(bs, NULL); +} + static void bdrv_child_cb_drained_end(BdrvChild *child) { BlockDriverState *bs =3D child->opaque; @@ -904,6 +910,7 @@ const BdrvChildRole child_file =3D { .get_parent_desc =3D bdrv_child_get_parent_desc, .inherit_options =3D bdrv_inherited_options, .drained_begin =3D bdrv_child_cb_drained_begin, + .drained_poll =3D bdrv_child_cb_drained_poll, .drained_end =3D bdrv_child_cb_drained_end, .attach =3D bdrv_child_cb_attach, .detach =3D bdrv_child_cb_detach, @@ -928,6 +935,7 @@ const BdrvChildRole child_format =3D { .get_parent_desc =3D bdrv_child_get_parent_desc, .inherit_options =3D bdrv_inherited_fmt_options, .drained_begin =3D bdrv_child_cb_drained_begin, + .drained_poll =3D bdrv_child_cb_drained_poll, .drained_end =3D bdrv_child_cb_drained_end, .attach =3D bdrv_child_cb_attach, .detach =3D bdrv_child_cb_detach, @@ -1047,6 +1055,7 @@ const BdrvChildRole child_backing =3D { .detach =3D bdrv_backing_detach, .inherit_options =3D bdrv_backing_options, .drained_begin =3D bdrv_child_cb_drained_begin, + .drained_poll =3D bdrv_child_cb_drained_poll, .drained_end =3D bdrv_child_cb_drained_end, .inactivate =3D bdrv_child_cb_inactivate, .update_filename =3D bdrv_backing_update_filename, diff --git a/block/io.c b/block/io.c index aa973f1d4a..b76ef92009 100644 --- a/block/io.c +++ b/block/io.c @@ -69,6 +69,23 @@ void bdrv_parent_drained_end(BlockDriverState *bs, BdrvC= hild *ignore) } } =20 +static bool bdrv_parent_drained_poll(BlockDriverState *bs, BdrvChild *igno= re) +{ + BdrvChild *c, *next; + bool busy =3D false; + + QLIST_FOREACH_SAFE(c, &bs->parents, next_parent, next) { + if (c =3D=3D ignore) { + continue; + } + if (c->role->drained_poll) { + busy |=3D c->role->drained_poll(c); + } + } + + return busy; +} + static void bdrv_merge_limits(BlockLimits *dst, const BlockLimits *src) { dst->opt_transfer =3D MAX(dst->opt_transfer, src->opt_transfer); @@ -183,21 +200,32 @@ static void bdrv_drain_invoke(BlockDriverState *bs, b= ool begin) } =20 /* Returns true if BDRV_POLL_WHILE() should go into a blocking aio_poll() = */ -static bool bdrv_drain_poll(BlockDriverState *bs) +bool bdrv_drain_poll(BlockDriverState *bs, BdrvChild *ignore_parent) +{ + if (bdrv_parent_drained_poll(bs, ignore_parent)) { + return true; + } + + return atomic_read(&bs->in_flight); +} + +static bool bdrv_drain_poll_top_level(BlockDriverState *bs, + BdrvChild *ignore_parent) { /* 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); + + return bdrv_drain_poll(bs, ignore_parent); } =20 -static bool bdrv_drain_recurse(BlockDriverState *bs) +static bool bdrv_drain_recurse(BlockDriverState *bs, BdrvChild *parent) { BdrvChild *child, *tmp; bool waited; =20 /* Wait for drained requests to finish */ - waited =3D BDRV_POLL_WHILE(bs, bdrv_drain_poll(bs)); + waited =3D BDRV_POLL_WHILE(bs, bdrv_drain_poll_top_level(bs, parent)); =20 QLIST_FOREACH_SAFE(child, &bs->children, next, tmp) { BlockDriverState *bs =3D child->bs; @@ -214,7 +242,7 @@ static bool bdrv_drain_recurse(BlockDriverState *bs) */ bdrv_ref(bs); } - waited |=3D bdrv_drain_recurse(bs); + waited |=3D bdrv_drain_recurse(bs, child); if (in_main_loop) { bdrv_unref(bs); } @@ -290,7 +318,7 @@ void bdrv_do_drained_begin(BlockDriverState *bs, bool r= ecursive, =20 bdrv_parent_drained_begin(bs, parent); bdrv_drain_invoke(bs, true); - bdrv_drain_recurse(bs); + bdrv_drain_recurse(bs, parent); =20 if (recursive) { bs->recursive_quiesce_counter++; diff --git a/block/mirror.c b/block/mirror.c index dcb66ec3be..fb1d686ff6 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -964,6 +964,12 @@ static void mirror_pause(Job *job) mirror_wait_for_all_io(s); } =20 +static bool mirror_drained_poll(BlockJob *job) +{ + MirrorBlockJob *s =3D container_of(job, MirrorBlockJob, common); + return !!s->in_flight; +} + static void mirror_attached_aio_context(BlockJob *job, AioContext *new_con= text) { MirrorBlockJob *s =3D container_of(job, MirrorBlockJob, common); @@ -997,6 +1003,7 @@ static const BlockJobDriver mirror_job_driver =3D { .pause =3D mirror_pause, .complete =3D mirror_complete, }, + .drained_poll =3D mirror_drained_poll, .attached_aio_context =3D mirror_attached_aio_context, .drain =3D mirror_drain, }; @@ -1012,6 +1019,7 @@ static const BlockJobDriver commit_active_job_driver = =3D { .pause =3D mirror_pause, .complete =3D mirror_complete, }, + .drained_poll =3D mirror_drained_poll, .attached_aio_context =3D mirror_attached_aio_context, .drain =3D mirror_drain, }; diff --git a/blockjob.c b/blockjob.c index 0306533a2e..be5903aa96 100644 --- a/blockjob.c +++ b/blockjob.c @@ -155,6 +155,28 @@ static void child_job_drained_begin(BdrvChild *c) job_pause(&job->job); } =20 +static bool child_job_drained_poll(BdrvChild *c) +{ + BlockJob *bjob =3D c->opaque; + Job *job =3D &bjob->job; + const BlockJobDriver *drv =3D block_job_driver(bjob); + + /* An inactive or completed job doesn't have any pending requests. Jobs + * with !job->busy are either already paused or have a pause point aft= er + * being reentered, so no job driver code will run before they pause. = */ + if (!job->busy || job_is_completed(job) || job->deferred_to_main_loop)= { + return false; + } + + /* Otherwise, assume that it isn't fully stopped yet, but allow the jo= b to + * override this assumption. */ + if (drv->drained_poll) { + return drv->drained_poll(bjob); + } else { + return true; + } +} + static void child_job_drained_end(BdrvChild *c) { BlockJob *job =3D c->opaque; @@ -164,6 +186,7 @@ static void child_job_drained_end(BdrvChild *c) static const BdrvChildRole child_job =3D { .get_parent_desc =3D child_job_get_parent_desc, .drained_begin =3D child_job_drained_begin, + .drained_poll =3D child_job_drained_poll, .drained_end =3D child_job_drained_end, .stay_at_node =3D true, }; diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c index 17127e63e4..756e7a1a7f 100644 --- a/tests/test-bdrv-drain.c +++ b/tests/test-bdrv-drain.c @@ -686,7 +686,11 @@ static void coroutine_fn test_job_start(void *opaque) =20 job_transition_to_ready(&s->common.job); while (!s->should_complete) { - job_sleep_ns(&s->common.job, 100000); + /* Avoid block_job_sleep_ns() because it marks the job as !busy. We + * want to emulate some actual activity (probably some I/O) here so + * that drain has to wait for this acitivity to stop. */ + qemu_co_sleep_ns(QEMU_CLOCK_REALTIME, 100000); + job_pause_point(&s->common.job); } =20 job_defer_to_main_loop(&s->common.job, test_job_completed, NULL); @@ -733,7 +737,7 @@ static void test_blockjob_common(enum drain_type drain_= type) =20 g_assert_cmpint(job->job.pause_count, =3D=3D, 0); g_assert_false(job->job.paused); - g_assert_false(job->job.busy); /* We're in job_sleep_ns() */ + g_assert_true(job->job.busy); /* We're in job_sleep_ns() */ =20 do_drain_begin(drain_type, src); =20 @@ -743,15 +747,14 @@ static void test_blockjob_common(enum drain_type drai= n_type) } else { g_assert_cmpint(job->job.pause_count, =3D=3D, 1); } - /* XXX We don't wait until the job is actually paused. Is this okay? */ - /* g_assert_true(job->job.paused); */ + g_assert_true(job->job.paused); g_assert_false(job->job.busy); /* The job is paused */ =20 do_drain_end(drain_type, src); =20 g_assert_cmpint(job->job.pause_count, =3D=3D, 0); g_assert_false(job->job.paused); - g_assert_false(job->job.busy); /* We're in job_sleep_ns() */ + g_assert_true(job->job.busy); /* We're in qemu_co_sleep_ns() */ =20 do_drain_begin(drain_type, target); =20 @@ -761,15 +764,14 @@ static void test_blockjob_common(enum drain_type drai= n_type) } else { g_assert_cmpint(job->job.pause_count, =3D=3D, 1); } - /* XXX We don't wait until the job is actually paused. Is this okay? */ - /* g_assert_true(job->job.paused); */ + g_assert_true(job->job.paused); g_assert_false(job->job.busy); /* The job is paused */ =20 do_drain_end(drain_type, target); =20 g_assert_cmpint(job->job.pause_count, =3D=3D, 0); g_assert_false(job->job.paused); - g_assert_false(job->job.busy); /* We're in job_sleep_ns() */ + g_assert_true(job->job.busy); /* We're in job_sleep_ns() */ =20 ret =3D job_complete_sync(&job->job, &error_abort); g_assert_cmpint(ret, =3D=3D, 0); --=20 2.13.6 From nobody Sat May 18 22:14:49 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1527615060988533.6892992674891; Tue, 29 May 2018 10:31:00 -0700 (PDT) Received: from localhost ([::1]:34144 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNiSa-0003VJ-3y for importer@patchew.org; Tue, 29 May 2018 13:31:00 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58198) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNiKH-0004tj-7p for qemu-devel@nongnu.org; Tue, 29 May 2018 13:22:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fNiKG-0001kw-8f for qemu-devel@nongnu.org; Tue, 29 May 2018 13:22:25 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:38556 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fNiKB-0001hx-SN; Tue, 29 May 2018 13:22:19 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 7F75D402178A; Tue, 29 May 2018 17:22:19 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-116-193.ams2.redhat.com [10.36.116.193]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2D64B1117629; Tue, 29 May 2018 17:22:18 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Tue, 29 May 2018 19:21:44 +0200 Message-Id: <20180529172156.29311-9-kwolf@redhat.com> In-Reply-To: <20180529172156.29311-1-kwolf@redhat.com> References: <20180529172156.29311-1-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Tue, 29 May 2018 17:22:19 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Tue, 29 May 2018 17:22:19 +0000 (UTC) for IP:'10.11.54.3' DOMAIN:'int-mx03.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'kwolf@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PATCH v2 08/20] block: Remove bdrv_drain_recurse() X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com, stefanha@redhat.com, pbonzini@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" For bdrv_drain(), recursively waiting for child node requests is pointless because we didn't quiesce their parents, so new requests could come in anyway. Letting the function work only on a single node makes it more consistent. For subtree drains and drain_all, we already have the recursion in bdrv_do_drained_begin(), so the extra recursion doesn't add anything either. Remove the useless code. Signed-off-by: Kevin Wolf Reviewed-by: Stefan Hajnoczi --- block/io.c | 36 +++--------------------------------- 1 file changed, 3 insertions(+), 33 deletions(-) diff --git a/block/io.c b/block/io.c index b76ef92009..8303aebb58 100644 --- a/block/io.c +++ b/block/io.c @@ -219,38 +219,6 @@ static bool bdrv_drain_poll_top_level(BlockDriverState= *bs, return bdrv_drain_poll(bs, ignore_parent); } =20 -static bool bdrv_drain_recurse(BlockDriverState *bs, BdrvChild *parent) -{ - BdrvChild *child, *tmp; - bool waited; - - /* Wait for drained requests to finish */ - waited =3D BDRV_POLL_WHILE(bs, bdrv_drain_poll_top_level(bs, parent)); - - QLIST_FOREACH_SAFE(child, &bs->children, next, tmp) { - BlockDriverState *bs =3D child->bs; - bool in_main_loop =3D - qemu_get_current_aio_context() =3D=3D qemu_get_aio_context(); - assert(bs->refcnt > 0); - if (in_main_loop) { - /* In case the recursive bdrv_drain_recurse processes a - * block_job_defer_to_main_loop BH and modifies the graph, - * let's hold a reference to bs until we are done. - * - * IOThread doesn't have such a BH, and it is not safe to call - * bdrv_unref without BQL, so skip doing it there. - */ - bdrv_ref(bs); - } - waited |=3D bdrv_drain_recurse(bs, child); - if (in_main_loop) { - bdrv_unref(bs); - } - } - - return waited; -} - static void bdrv_do_drained_begin(BlockDriverState *bs, bool recursive, BdrvChild *parent); static void bdrv_do_drained_end(BlockDriverState *bs, bool recursive, @@ -318,7 +286,9 @@ void bdrv_do_drained_begin(BlockDriverState *bs, bool r= ecursive, =20 bdrv_parent_drained_begin(bs, parent); bdrv_drain_invoke(bs, true); - bdrv_drain_recurse(bs, parent); + + /* Wait for drained requests to finish */ + BDRV_POLL_WHILE(bs, bdrv_drain_poll_top_level(bs, parent)); =20 if (recursive) { bs->recursive_quiesce_counter++; --=20 2.13.6 From nobody Sat May 18 22:14:49 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1527615046389186.06630299916003; Tue, 29 May 2018 10:30:46 -0700 (PDT) Received: from localhost ([::1]:34140 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNiSH-00039Z-Bs for importer@patchew.org; Tue, 29 May 2018 13:30:41 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58268) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNiKN-00051U-7a for qemu-devel@nongnu.org; Tue, 29 May 2018 13:22:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fNiKM-0001oT-0e for qemu-devel@nongnu.org; Tue, 29 May 2018 13:22:31 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:38558 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fNiKH-0001lx-NC; Tue, 29 May 2018 13:22:25 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 5A9F5400EB89; Tue, 29 May 2018 17:22:25 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-116-193.ams2.redhat.com [10.36.116.193]) by smtp.corp.redhat.com (Postfix) with ESMTP id BAC2D1117629; Tue, 29 May 2018 17:22:19 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Tue, 29 May 2018 19:21:45 +0200 Message-Id: <20180529172156.29311-10-kwolf@redhat.com> In-Reply-To: <20180529172156.29311-1-kwolf@redhat.com> References: <20180529172156.29311-1-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Tue, 29 May 2018 17:22:25 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Tue, 29 May 2018 17:22:25 +0000 (UTC) for IP:'10.11.54.3' DOMAIN:'int-mx03.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'kwolf@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PATCH v2 09/20] test-bdrv-drain: Add test for node deletion X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com, stefanha@redhat.com, pbonzini@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" From: Max Reitz This patch adds two bdrv-drain tests for what happens if some BDS goes away during the drainage. The basic idea is that you have a parent BDS with some child nodes. Then, you drain one of the children. Because of that, the party who actually owns the parent decides to (A) delete it, or (B) detach all its children from it -- both while the child is still being drained. A real-world case where this can happen is the mirror block job, which may exit if you drain one of its children. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- tests/test-bdrv-drain.c | 169 ++++++++++++++++++++++++++++++++++++++++++++= ++++ 1 file changed, 169 insertions(+) diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c index 756e7a1a7f..df438f0084 100644 --- a/tests/test-bdrv-drain.c +++ b/tests/test-bdrv-drain.c @@ -797,6 +797,172 @@ static void test_blockjob_drain_subtree(void) test_blockjob_common(BDRV_SUBTREE_DRAIN); } =20 + +typedef struct BDRVTestTopState { + BdrvChild *wait_child; +} BDRVTestTopState; + +static void bdrv_test_top_close(BlockDriverState *bs) +{ + BdrvChild *c, *next_c; + QLIST_FOREACH_SAFE(c, &bs->children, next, next_c) { + bdrv_unref_child(bs, c); + } +} + +static int coroutine_fn bdrv_test_top_co_preadv(BlockDriverState *bs, + uint64_t offset, uint64_t = bytes, + QEMUIOVector *qiov, int fl= ags) +{ + BDRVTestTopState *tts =3D bs->opaque; + return bdrv_co_preadv(tts->wait_child, offset, bytes, qiov, flags); +} + +static BlockDriver bdrv_test_top_driver =3D { + .format_name =3D "test_top_driver", + .instance_size =3D sizeof(BDRVTestTopState), + + .bdrv_close =3D bdrv_test_top_close, + .bdrv_co_preadv =3D bdrv_test_top_co_preadv, + + .bdrv_child_perm =3D bdrv_format_default_perms, +}; + +typedef struct TestCoDeleteByDrainData { + BlockBackend *blk; + bool detach_instead_of_delete; + bool done; +} TestCoDeleteByDrainData; + +static void coroutine_fn test_co_delete_by_drain(void *opaque) +{ + TestCoDeleteByDrainData *dbdd =3D opaque; + BlockBackend *blk =3D dbdd->blk; + BlockDriverState *bs =3D blk_bs(blk); + BDRVTestTopState *tts =3D bs->opaque; + void *buffer =3D g_malloc(65536); + QEMUIOVector qiov; + struct iovec iov =3D { + .iov_base =3D buffer, + .iov_len =3D 65536, + }; + + qemu_iovec_init_external(&qiov, &iov, 1); + + /* Pretend some internal write operation from parent to child. + * Important: We have to read from the child, not from the parent! + * Draining works by first propagating it all up the tree to the + * root and then waiting for drainage from root to the leaves + * (protocol nodes). If we have a request waiting on the root, + * everything will be drained before we go back down the tree, but + * we do not want that. We want to be in the middle of draining + * when this following requests returns. */ + bdrv_co_preadv(tts->wait_child, 0, 65536, &qiov, 0); + + g_assert_cmpint(bs->refcnt, =3D=3D, 1); + + if (!dbdd->detach_instead_of_delete) { + blk_unref(blk); + } else { + BdrvChild *c, *next_c; + QLIST_FOREACH_SAFE(c, &bs->children, next, next_c) { + bdrv_unref_child(bs, c); + } + } + + dbdd->done =3D true; +} + +/** + * Test what happens when some BDS has some children, you drain one of + * them and this results in the BDS being deleted. + * + * If @detach_instead_of_delete is set, the BDS is not going to be + * deleted but will only detach all of its children. + */ +static void do_test_delete_by_drain(bool detach_instead_of_delete) +{ + BlockBackend *blk; + BlockDriverState *bs, *child_bs, *null_bs; + BDRVTestTopState *tts; + TestCoDeleteByDrainData dbdd; + Coroutine *co; + + bs =3D bdrv_new_open_driver(&bdrv_test_top_driver, "top", BDRV_O_RDWR, + &error_abort); + bs->total_sectors =3D 65536 >> BDRV_SECTOR_BITS; + tts =3D bs->opaque; + + null_bs =3D bdrv_open("null-co://", NULL, NULL, BDRV_O_RDWR | BDRV_O_P= ROTOCOL, + &error_abort); + bdrv_attach_child(bs, null_bs, "null-child", &child_file, &error_abort= ); + + /* This child will be the one to pass to requests through to, and + * it will stall until a drain occurs */ + child_bs =3D bdrv_new_open_driver(&bdrv_test, "child", BDRV_O_RDWR, + &error_abort); + child_bs->total_sectors =3D 65536 >> BDRV_SECTOR_BITS; + /* Takes our reference to child_bs */ + tts->wait_child =3D bdrv_attach_child(bs, child_bs, "wait-child", &chi= ld_file, + &error_abort); + + /* This child is just there to be deleted + * (for detach_instead_of_delete =3D=3D true) */ + null_bs =3D bdrv_open("null-co://", NULL, NULL, BDRV_O_RDWR | BDRV_O_P= ROTOCOL, + &error_abort); + bdrv_attach_child(bs, null_bs, "null-child", &child_file, &error_abort= ); + + blk =3D blk_new(BLK_PERM_ALL, BLK_PERM_ALL); + blk_insert_bs(blk, bs, &error_abort); + + /* Referenced by blk now */ + bdrv_unref(bs); + + g_assert_cmpint(bs->refcnt, =3D=3D, 1); + g_assert_cmpint(child_bs->refcnt, =3D=3D, 1); + g_assert_cmpint(null_bs->refcnt, =3D=3D, 1); + + + dbdd =3D (TestCoDeleteByDrainData){ + .blk =3D blk, + .detach_instead_of_delete =3D detach_instead_of_delete, + .done =3D false, + }; + co =3D qemu_coroutine_create(test_co_delete_by_drain, &dbdd); + qemu_coroutine_enter(co); + + /* Drain the child while the read operation is still pending. + * This should result in the operation finishing and + * test_co_delete_by_drain() resuming. Thus, @bs will be deleted + * and the coroutine will exit while this drain operation is still + * in progress. */ + bdrv_ref(child_bs); + bdrv_drain(child_bs); + bdrv_unref(child_bs); + + while (!dbdd.done) { + aio_poll(qemu_get_aio_context(), true); + } + + if (detach_instead_of_delete) { + /* Here, the reference has not passed over to the coroutine, + * so we have to delete the BB ourselves */ + blk_unref(blk); + } +} + + +static void test_delete_by_drain(void) +{ + do_test_delete_by_drain(false); +} + +static void test_detach_by_drain(void) +{ + do_test_delete_by_drain(true); +} + + int main(int argc, char **argv) { int ret; @@ -844,6 +1010,9 @@ int main(int argc, char **argv) g_test_add_func("/bdrv-drain/blockjob/drain_subtree", test_blockjob_drain_subtree); =20 + g_test_add_func("/bdrv-drain/deletion", test_delete_by_drain); + g_test_add_func("/bdrv-drain/detach", test_detach_by_drain); + ret =3D g_test_run(); qemu_event_destroy(&done_event); return ret; --=20 2.13.6 From nobody Sat May 18 22:14:49 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1527615499605106.78075356286604; Tue, 29 May 2018 10:38:19 -0700 (PDT) Received: from localhost ([::1]:34194 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNiZc-0000JV-R6 for importer@patchew.org; Tue, 29 May 2018 13:38:16 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58277) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNiKN-00051x-JX for qemu-devel@nongnu.org; Tue, 29 May 2018 13:22:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fNiKM-0001oi-AK for qemu-devel@nongnu.org; Tue, 29 May 2018 13:22:31 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:45536 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fNiKJ-0001mb-9t; Tue, 29 May 2018 13:22:27 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E788F401EF09; Tue, 29 May 2018 17:22:26 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-116-193.ams2.redhat.com [10.36.116.193]) by smtp.corp.redhat.com (Postfix) with ESMTP id 950E01117629; Tue, 29 May 2018 17:22:25 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Tue, 29 May 2018 19:21:46 +0200 Message-Id: <20180529172156.29311-11-kwolf@redhat.com> In-Reply-To: <20180529172156.29311-1-kwolf@redhat.com> References: <20180529172156.29311-1-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.5]); Tue, 29 May 2018 17:22:26 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.5]); Tue, 29 May 2018 17:22:26 +0000 (UTC) for IP:'10.11.54.3' DOMAIN:'int-mx03.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'kwolf@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PATCH v2 10/20] block: Drain recursively with a single BDRV_POLL_WHILE() X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com, stefanha@redhat.com, pbonzini@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Anything can happen inside BDRV_POLL_WHILE(), including graph changes that may interfere with its callers (e.g. child list iteration in recursive callers of bdrv_do_drained_begin). Switch to a single BDRV_POLL_WHILE() call for the whole subtree at the end of bdrv_do_drained_begin() to avoid such effects. The recursion happens now inside the loop condition. As the graph can only change between bdrv_drain_poll() calls, but not inside of it, doing the recursion here is safe. Signed-off-by: Kevin Wolf --- include/block/block.h | 9 +++++--- block.c | 2 +- block/io.c | 63 ++++++++++++++++++++++++++++++++++++-----------= ---- 3 files changed, 52 insertions(+), 22 deletions(-) diff --git a/include/block/block.h b/include/block/block.h index 52f07da3d3..56962e7ee7 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -568,10 +568,13 @@ void bdrv_parent_drained_end(BlockDriverState *bs, Bd= rvChild *ignore); /** * bdrv_drain_poll: * - * Poll for pending requests in @bs and its parents (except for - * @ignore_parent). This is part of bdrv_drained_begin. + * Poll for pending requests in @bs, its parents (except for @ignore_paren= t), + * and if @recursive is true its children as well. + * + * This is part of bdrv_drained_begin. */ -bool bdrv_drain_poll(BlockDriverState *bs, BdrvChild *ignore_parent); +bool bdrv_drain_poll(BlockDriverState *bs, bool recursive, + BdrvChild *ignore_parent); =20 /** * bdrv_drained_begin: diff --git a/block.c b/block.c index 3c654a8a63..8ca2cf27b4 100644 --- a/block.c +++ b/block.c @@ -823,7 +823,7 @@ static void bdrv_child_cb_drained_begin(BdrvChild *chil= d) static bool bdrv_child_cb_drained_poll(BdrvChild *child) { BlockDriverState *bs =3D child->opaque; - return bdrv_drain_poll(bs, NULL); + return bdrv_drain_poll(bs, false, NULL); } =20 static void bdrv_child_cb_drained_end(BdrvChild *child) diff --git a/block/io.c b/block/io.c index 8303aebb58..01784d1115 100644 --- a/block/io.c +++ b/block/io.c @@ -165,6 +165,7 @@ typedef struct { bool done; bool begin; bool recursive; + bool poll; BdrvChild *parent; } BdrvCoDrainData; =20 @@ -200,27 +201,42 @@ static void bdrv_drain_invoke(BlockDriverState *bs, b= ool begin) } =20 /* Returns true if BDRV_POLL_WHILE() should go into a blocking aio_poll() = */ -bool bdrv_drain_poll(BlockDriverState *bs, BdrvChild *ignore_parent) +bool bdrv_drain_poll(BlockDriverState *bs, bool recursive, + BdrvChild *ignore_parent) { + BdrvChild *child, *next; + if (bdrv_parent_drained_poll(bs, ignore_parent)) { return true; } =20 - return atomic_read(&bs->in_flight); + if (atomic_read(&bs->in_flight)) { + return true; + } + + if (recursive) { + QLIST_FOREACH_SAFE(child, &bs->children, next, next) { + if (bdrv_drain_poll(child->bs, recursive, child)) { + return true; + } + } + } + + return false; } =20 -static bool bdrv_drain_poll_top_level(BlockDriverState *bs, +static bool bdrv_drain_poll_top_level(BlockDriverState *bs, bool recursive, BdrvChild *ignore_parent) { /* Execute pending BHs first and check everything else only after the = BHs * have executed. */ while (aio_poll(bs->aio_context, false)); =20 - return bdrv_drain_poll(bs, ignore_parent); + return bdrv_drain_poll(bs, recursive, ignore_parent); } =20 static void bdrv_do_drained_begin(BlockDriverState *bs, bool recursive, - BdrvChild *parent); + BdrvChild *parent, bool poll); static void bdrv_do_drained_end(BlockDriverState *bs, bool recursive, BdrvChild *parent); =20 @@ -232,7 +248,7 @@ static void bdrv_co_drain_bh_cb(void *opaque) =20 bdrv_dec_in_flight(bs); if (data->begin) { - bdrv_do_drained_begin(bs, data->recursive, data->parent); + bdrv_do_drained_begin(bs, data->recursive, data->parent, data->pol= l); } else { bdrv_do_drained_end(bs, data->recursive, data->parent); } @@ -243,7 +259,7 @@ static void bdrv_co_drain_bh_cb(void *opaque) =20 static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs, bool begin, bool recursive, - BdrvChild *parent) + BdrvChild *parent, bool po= ll) { BdrvCoDrainData data; =20 @@ -258,6 +274,7 @@ static void coroutine_fn bdrv_co_yield_to_drain(BlockDr= iverState *bs, .begin =3D begin, .recursive =3D recursive, .parent =3D parent, + .poll =3D poll, }; bdrv_inc_in_flight(bs); aio_bh_schedule_oneshot(bdrv_get_aio_context(bs), @@ -270,12 +287,12 @@ static void coroutine_fn bdrv_co_yield_to_drain(Block= DriverState *bs, } =20 void bdrv_do_drained_begin(BlockDriverState *bs, bool recursive, - BdrvChild *parent) + BdrvChild *parent, bool poll) { BdrvChild *child, *next; =20 if (qemu_in_coroutine()) { - bdrv_co_yield_to_drain(bs, true, recursive, parent); + bdrv_co_yield_to_drain(bs, true, recursive, parent, poll); return; } =20 @@ -287,25 +304,35 @@ void bdrv_do_drained_begin(BlockDriverState *bs, bool= recursive, bdrv_parent_drained_begin(bs, parent); bdrv_drain_invoke(bs, true); =20 - /* Wait for drained requests to finish */ - BDRV_POLL_WHILE(bs, bdrv_drain_poll_top_level(bs, parent)); - if (recursive) { bs->recursive_quiesce_counter++; QLIST_FOREACH_SAFE(child, &bs->children, next, next) { - bdrv_do_drained_begin(child->bs, true, child); + bdrv_do_drained_begin(child->bs, true, child, false); } } + + /* + * Wait for drained requests to finish. + * + * Calling BDRV_POLL_WHILE() only once for the top-level node is okay:= The + * call is needed so things in this AioContext can make progress even + * though we don't return to the main AioContext loop - this automatic= ally + * includes other nodes in the same AioContext and therefore all child + * nodes. + */ + if (poll) { + BDRV_POLL_WHILE(bs, bdrv_drain_poll_top_level(bs, recursive, paren= t)); + } } =20 void bdrv_drained_begin(BlockDriverState *bs) { - bdrv_do_drained_begin(bs, false, NULL); + bdrv_do_drained_begin(bs, false, NULL, true); } =20 void bdrv_subtree_drained_begin(BlockDriverState *bs) { - bdrv_do_drained_begin(bs, true, NULL); + bdrv_do_drained_begin(bs, true, NULL, true); } =20 void bdrv_do_drained_end(BlockDriverState *bs, bool recursive, @@ -315,7 +342,7 @@ void bdrv_do_drained_end(BlockDriverState *bs, bool rec= ursive, int old_quiesce_counter; =20 if (qemu_in_coroutine()) { - bdrv_co_yield_to_drain(bs, false, recursive, parent); + bdrv_co_yield_to_drain(bs, false, recursive, parent, false); return; } assert(bs->quiesce_counter > 0); @@ -351,7 +378,7 @@ void bdrv_apply_subtree_drain(BdrvChild *child, BlockDr= iverState *new_parent) int i; =20 for (i =3D 0; i < new_parent->recursive_quiesce_counter; i++) { - bdrv_do_drained_begin(child->bs, true, child); + bdrv_do_drained_begin(child->bs, true, child, true); } } =20 @@ -421,7 +448,7 @@ void bdrv_drain_all_begin(void) AioContext *aio_context =3D bdrv_get_aio_context(bs); =20 aio_context_acquire(aio_context); - bdrv_do_drained_begin(bs, true, NULL); + bdrv_do_drained_begin(bs, true, NULL, true); aio_context_release(aio_context); } =20 --=20 2.13.6 From nobody Sat May 18 22:14:49 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1527615395618550.5659258372767; Tue, 29 May 2018 10:36:35 -0700 (PDT) Received: from localhost ([::1]:34185 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNiXy-0007V8-Rx for importer@patchew.org; Tue, 29 May 2018 13:36:34 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58287) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNiKO-00052M-2t for qemu-devel@nongnu.org; Tue, 29 May 2018 13:22:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fNiKN-0001p9-5g for qemu-devel@nongnu.org; Tue, 29 May 2018 13:22:32 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:59868 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fNiKK-0001nr-S7; Tue, 29 May 2018 13:22:28 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 8257D7D846; Tue, 29 May 2018 17:22:28 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-116-193.ams2.redhat.com [10.36.116.193]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2E6C7111762D; Tue, 29 May 2018 17:22:27 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Tue, 29 May 2018 19:21:47 +0200 Message-Id: <20180529172156.29311-12-kwolf@redhat.com> In-Reply-To: <20180529172156.29311-1-kwolf@redhat.com> References: <20180529172156.29311-1-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.2]); Tue, 29 May 2018 17:22:28 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.2]); Tue, 29 May 2018 17:22:28 +0000 (UTC) for IP:'10.11.54.3' DOMAIN:'int-mx03.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'kwolf@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PATCH v2 11/20] test-bdrv-drain: Test node deletion in subtree recursion X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com, stefanha@redhat.com, pbonzini@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" If bdrv_do_drained_begin() polls during its subtree recursion, the graph can change and mess up the bs->children iteration. Test that this doesn't happen. Signed-off-by: Kevin Wolf --- tests/test-bdrv-drain.c | 38 +++++++++++++++++++++++++++++--------- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c index df438f0084..4b788e323e 100644 --- a/tests/test-bdrv-drain.c +++ b/tests/test-bdrv-drain.c @@ -880,7 +880,8 @@ static void coroutine_fn test_co_delete_by_drain(void *= opaque) * If @detach_instead_of_delete is set, the BDS is not going to be * deleted but will only detach all of its children. */ -static void do_test_delete_by_drain(bool detach_instead_of_delete) +static void do_test_delete_by_drain(bool detach_instead_of_delete, + enum drain_type drain_type) { BlockBackend *blk; BlockDriverState *bs, *child_bs, *null_bs; @@ -936,9 +937,23 @@ static void do_test_delete_by_drain(bool detach_instea= d_of_delete) * test_co_delete_by_drain() resuming. Thus, @bs will be deleted * and the coroutine will exit while this drain operation is still * in progress. */ - bdrv_ref(child_bs); - bdrv_drain(child_bs); - bdrv_unref(child_bs); + switch (drain_type) { + case BDRV_DRAIN: + bdrv_ref(child_bs); + bdrv_drain(child_bs); + bdrv_unref(child_bs); + break; + case BDRV_SUBTREE_DRAIN: + /* Would have to ref/unref bs here for !detach_instead_of_delete, = but + * then the whole test becomes pointless because the graph changes + * don't occur during the drain any more. */ + assert(detach_instead_of_delete); + bdrv_subtree_drained_begin(bs); + bdrv_subtree_drained_end(bs); + break; + default: + g_assert_not_reached(); + } =20 while (!dbdd.done) { aio_poll(qemu_get_aio_context(), true); @@ -951,15 +966,19 @@ static void do_test_delete_by_drain(bool detach_inste= ad_of_delete) } } =20 - static void test_delete_by_drain(void) { - do_test_delete_by_drain(false); + do_test_delete_by_drain(false, BDRV_DRAIN); } =20 static void test_detach_by_drain(void) { - do_test_delete_by_drain(true); + do_test_delete_by_drain(true, BDRV_DRAIN); +} + +static void test_detach_by_drain_subtree(void) +{ + do_test_delete_by_drain(true, BDRV_SUBTREE_DRAIN); } =20 =20 @@ -1010,8 +1029,9 @@ int main(int argc, char **argv) g_test_add_func("/bdrv-drain/blockjob/drain_subtree", test_blockjob_drain_subtree); =20 - g_test_add_func("/bdrv-drain/deletion", test_delete_by_drain); - g_test_add_func("/bdrv-drain/detach", test_detach_by_drain); + g_test_add_func("/bdrv-drain/deletion/drain", test_delete_by_drain); + g_test_add_func("/bdrv-drain/detach/drain", test_detach_by_drain); + g_test_add_func("/bdrv-drain/detach/drain_subtree", test_detach_by_dra= in_subtree); =20 ret =3D g_test_run(); qemu_event_destroy(&done_event); --=20 2.13.6 From nobody Sat May 18 22:14:49 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1527614880163872.7443570399435; Tue, 29 May 2018 10:28:00 -0700 (PDT) Received: from localhost ([::1]:34118 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNiPf-0000pz-7J for importer@patchew.org; Tue, 29 May 2018 13:27:59 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58365) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNiKT-00055r-QQ for qemu-devel@nongnu.org; Tue, 29 May 2018 13:22:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fNiKR-0001rZ-9F for qemu-devel@nongnu.org; Tue, 29 May 2018 13:22:37 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:36786 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fNiKM-0001of-FM; Tue, 29 May 2018 13:22:30 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 1C3CA4023112; Tue, 29 May 2018 17:22:30 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-116-193.ams2.redhat.com [10.36.116.193]) by smtp.corp.redhat.com (Postfix) with ESMTP id BCF27111762D; Tue, 29 May 2018 17:22:28 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Tue, 29 May 2018 19:21:48 +0200 Message-Id: <20180529172156.29311-13-kwolf@redhat.com> In-Reply-To: <20180529172156.29311-1-kwolf@redhat.com> References: <20180529172156.29311-1-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.6]); Tue, 29 May 2018 17:22:30 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.6]); Tue, 29 May 2018 17:22:30 +0000 (UTC) for IP:'10.11.54.3' DOMAIN:'int-mx03.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'kwolf@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PATCH v2 12/20] block: Don't poll in parent drain callbacks X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com, stefanha@redhat.com, pbonzini@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" bdrv_do_drained_begin() is only safe if we have a single BDRV_POLL_WHILE() after quiescing all affected nodes. We cannot allow that parent callbacks introduce a nested polling loop that could cause graph changes while we're traversing the graph. Split off bdrv_do_drained_begin_quiesce(), which only quiesces a single node without waiting for its requests to complete. These requests will be waited for in the BDRV_POLL_WHILE() call down the call chain. Signed-off-by: Kevin Wolf --- include/block/block.h | 9 +++++++++ block.c | 2 +- block/io.c | 24 ++++++++++++++++-------- 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/include/block/block.h b/include/block/block.h index 56962e7ee7..6026c1385e 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -589,6 +589,15 @@ bool bdrv_drain_poll(BlockDriverState *bs, bool recurs= ive, void bdrv_drained_begin(BlockDriverState *bs); =20 /** + * bdrv_do_drained_begin_quiesce: + * + * Quiesces a BDS like bdrv_drained_begin(), but does not wait for already + * running requests to complete. + */ +void bdrv_do_drained_begin_quiesce(BlockDriverState *bs, + BdrvChild *parent); + +/** * Like bdrv_drained_begin, but recursively begins a quiesced section for * exclusive access to all child nodes as well. */ diff --git a/block.c b/block.c index 8ca2cf27b4..1405d1180b 100644 --- a/block.c +++ b/block.c @@ -817,7 +817,7 @@ static char *bdrv_child_get_parent_desc(BdrvChild *c) static void bdrv_child_cb_drained_begin(BdrvChild *child) { BlockDriverState *bs =3D child->opaque; - bdrv_drained_begin(bs); + bdrv_do_drained_begin_quiesce(bs, NULL); } =20 static bool bdrv_child_cb_drained_poll(BdrvChild *child) diff --git a/block/io.c b/block/io.c index 01784d1115..af14227af8 100644 --- a/block/io.c +++ b/block/io.c @@ -286,15 +286,10 @@ static void coroutine_fn bdrv_co_yield_to_drain(Block= DriverState *bs, assert(data.done); } =20 -void bdrv_do_drained_begin(BlockDriverState *bs, bool recursive, - BdrvChild *parent, bool poll) +void bdrv_do_drained_begin_quiesce(BlockDriverState *bs, + BdrvChild *parent) { - BdrvChild *child, *next; - - if (qemu_in_coroutine()) { - bdrv_co_yield_to_drain(bs, true, recursive, parent, poll); - return; - } + assert(!qemu_in_coroutine()); =20 /* Stop things in parent-to-child order */ if (atomic_fetch_inc(&bs->quiesce_counter) =3D=3D 0) { @@ -303,6 +298,19 @@ void bdrv_do_drained_begin(BlockDriverState *bs, bool = recursive, =20 bdrv_parent_drained_begin(bs, parent); bdrv_drain_invoke(bs, true); +} + +static void bdrv_do_drained_begin(BlockDriverState *bs, bool recursive, + BdrvChild *parent, bool poll) +{ + BdrvChild *child, *next; + + if (qemu_in_coroutine()) { + bdrv_co_yield_to_drain(bs, true, recursive, parent, poll); + return; + } + + bdrv_do_drained_begin_quiesce(bs, parent); =20 if (recursive) { bs->recursive_quiesce_counter++; --=20 2.13.6 From nobody Sat May 18 22:14:49 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 152761503321425.73813337883621; Tue, 29 May 2018 10:30:33 -0700 (PDT) Received: from localhost ([::1]:34137 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNiS2-0002x1-No for importer@patchew.org; Tue, 29 May 2018 13:30:26 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58367) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNiKT-00055t-QS for qemu-devel@nongnu.org; Tue, 29 May 2018 13:22:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fNiKR-0001rS-4C for qemu-devel@nongnu.org; Tue, 29 May 2018 13:22:37 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:59870 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fNiKO-0001pg-12; Tue, 29 May 2018 13:22:32 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id A8BBB7D846; Tue, 29 May 2018 17:22:31 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-116-193.ams2.redhat.com [10.36.116.193]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5616A111762D; Tue, 29 May 2018 17:22:30 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Tue, 29 May 2018 19:21:49 +0200 Message-Id: <20180529172156.29311-14-kwolf@redhat.com> In-Reply-To: <20180529172156.29311-1-kwolf@redhat.com> References: <20180529172156.29311-1-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.2]); Tue, 29 May 2018 17:22:31 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.2]); Tue, 29 May 2018 17:22:31 +0000 (UTC) for IP:'10.11.54.3' DOMAIN:'int-mx03.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'kwolf@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PATCH v2 13/20] test-bdrv-drain: Graph change through parent callback X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com, stefanha@redhat.com, pbonzini@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Signed-off-by: Kevin Wolf --- tests/test-bdrv-drain.c | 130 ++++++++++++++++++++++++++++++++++++++++++++= ++++ 1 file changed, 130 insertions(+) diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c index 4b788e323e..ca7b6a3fdf 100644 --- a/tests/test-bdrv-drain.c +++ b/tests/test-bdrv-drain.c @@ -982,6 +982,135 @@ static void test_detach_by_drain_subtree(void) } =20 =20 +struct detach_by_parent_data { + BlockDriverState *parent_b; + BdrvChild *child_b; + BlockDriverState *c; + BdrvChild *child_c; +}; + +static void detach_by_parent_aio_cb(void *opaque, int ret) +{ + struct detach_by_parent_data *data =3D opaque; + + g_assert_cmpint(ret, =3D=3D, 0); + bdrv_unref_child(data->parent_b, data->child_b); + + bdrv_ref(data->c); + data->child_c =3D bdrv_attach_child(data->parent_b, data->c, "PB-C", + &child_file, &error_abort); +} + +/* + * Initial graph: + * + * PA PB + * \ / \ + * A B C + * + * PA has a pending write request whose callback changes the child nodes o= f PB: + * It removes B and adds C instead. The subtree of PB is drained, which wi= ll + * indirectly drain the write request, too. + */ +static void test_detach_by_parent_cb(void) +{ + BlockBackend *blk; + BlockDriverState *parent_a, *parent_b, *a, *b, *c; + BdrvChild *child_a, *child_b; + BlockAIOCB *acb; + struct detach_by_parent_data data; + + QEMUIOVector qiov; + struct iovec iov =3D { + .iov_base =3D NULL, + .iov_len =3D 0, + }; + qemu_iovec_init_external(&qiov, &iov, 1); + + /* Create all involved nodes */ + parent_a =3D bdrv_new_open_driver(&bdrv_test, "parent-a", BDRV_O_RDWR, + &error_abort); + parent_b =3D bdrv_new_open_driver(&bdrv_test, "parent-b", 0, + &error_abort); + + a =3D bdrv_new_open_driver(&bdrv_test, "a", BDRV_O_RDWR, &error_abort); + b =3D bdrv_new_open_driver(&bdrv_test, "b", BDRV_O_RDWR, &error_abort); + c =3D bdrv_new_open_driver(&bdrv_test, "c", BDRV_O_RDWR, &error_abort); + + /* blk is a BB for parent-a */ + blk =3D blk_new(BLK_PERM_ALL, BLK_PERM_ALL); + blk_insert_bs(blk, parent_a, &error_abort); + bdrv_unref(parent_a); + + /* Set child relationships */ + bdrv_ref(b); + bdrv_ref(a); + child_b =3D bdrv_attach_child(parent_b, b, "PB-B", &child_file, &error= _abort); + child_a =3D bdrv_attach_child(parent_b, a, "PB-A", &child_backing, &er= ror_abort); + + bdrv_ref(a); + bdrv_attach_child(parent_a, a, "PA-A", &child_file, &error_abort); + + g_assert_cmpint(parent_a->refcnt, =3D=3D, 1); + g_assert_cmpint(parent_b->refcnt, =3D=3D, 1); + g_assert_cmpint(a->refcnt, =3D=3D, 3); + g_assert_cmpint(b->refcnt, =3D=3D, 2); + g_assert_cmpint(c->refcnt, =3D=3D, 1); + + g_assert(QLIST_FIRST(&parent_b->children) =3D=3D child_a); + g_assert(QLIST_NEXT(child_a, next) =3D=3D child_b); + g_assert(QLIST_NEXT(child_b, next) =3D=3D NULL); + + /* Start the evil write request */ + data =3D (struct detach_by_parent_data) { + .parent_b =3D parent_b, + .child_b =3D child_b, + .c =3D c, + }; + acb =3D blk_aio_preadv(blk, 0, &qiov, 0, detach_by_parent_aio_cb, &dat= a); + g_assert(acb !=3D NULL); + + /* Drain and check the expected result */ + bdrv_subtree_drained_begin(parent_b); + + g_assert(data.child_c !=3D NULL); + + g_assert_cmpint(parent_a->refcnt, =3D=3D, 1); + g_assert_cmpint(parent_b->refcnt, =3D=3D, 1); + g_assert_cmpint(a->refcnt, =3D=3D, 3); + g_assert_cmpint(b->refcnt, =3D=3D, 1); + g_assert_cmpint(c->refcnt, =3D=3D, 2); + + g_assert(QLIST_FIRST(&parent_b->children) =3D=3D data.child_c); + g_assert(QLIST_NEXT(data.child_c, next) =3D=3D child_a); + g_assert(QLIST_NEXT(child_a, next) =3D=3D NULL); + + g_assert_cmpint(parent_a->quiesce_counter, =3D=3D, 1); + g_assert_cmpint(parent_b->quiesce_counter, =3D=3D, 1); + g_assert_cmpint(a->quiesce_counter, =3D=3D, 1); + g_assert_cmpint(b->quiesce_counter, =3D=3D, 0); + g_assert_cmpint(c->quiesce_counter, =3D=3D, 1); + + bdrv_subtree_drained_end(parent_b); + + bdrv_unref(parent_b); + blk_unref(blk); + + /* XXX Once bdrv_close() unref's children instead of just detaching th= em, + * this won't be necessary any more. */ + bdrv_unref(a); + bdrv_unref(a); + bdrv_unref(c); + + g_assert_cmpint(a->refcnt, =3D=3D, 1); + g_assert_cmpint(b->refcnt, =3D=3D, 1); + g_assert_cmpint(c->refcnt, =3D=3D, 1); + bdrv_unref(a); + bdrv_unref(b); + bdrv_unref(c); +} + + int main(int argc, char **argv) { int ret; @@ -1032,6 +1161,7 @@ int main(int argc, char **argv) g_test_add_func("/bdrv-drain/deletion/drain", test_delete_by_drain); g_test_add_func("/bdrv-drain/detach/drain", test_detach_by_drain); g_test_add_func("/bdrv-drain/detach/drain_subtree", test_detach_by_dra= in_subtree); + g_test_add_func("/bdrv-drain/detach/parent_cb", test_detach_by_parent_= cb); =20 ret =3D g_test_run(); qemu_event_destroy(&done_event); --=20 2.13.6 From nobody Sat May 18 22:14:49 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 152761557841939.75624051605598; Tue, 29 May 2018 10:39:38 -0700 (PDT) Received: from localhost ([::1]:34201 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNiam-0000zC-LG for importer@patchew.org; Tue, 29 May 2018 13:39:28 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58401) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNiKV-00057n-1D for qemu-devel@nongnu.org; Tue, 29 May 2018 13:22:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fNiKT-0001tE-VJ for qemu-devel@nongnu.org; Tue, 29 May 2018 13:22:39 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:53724 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fNiKP-0001qQ-KV; Tue, 29 May 2018 13:22:33 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 41B65BB41B; Tue, 29 May 2018 17:22:33 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-116-193.ams2.redhat.com [10.36.116.193]) by smtp.corp.redhat.com (Postfix) with ESMTP id E321A111762D; Tue, 29 May 2018 17:22:31 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Tue, 29 May 2018 19:21:50 +0200 Message-Id: <20180529172156.29311-15-kwolf@redhat.com> In-Reply-To: <20180529172156.29311-1-kwolf@redhat.com> References: <20180529172156.29311-1-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Tue, 29 May 2018 17:22:33 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Tue, 29 May 2018 17:22:33 +0000 (UTC) for IP:'10.11.54.3' DOMAIN:'int-mx03.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'kwolf@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PATCH v2 14/20] block: Defer .bdrv_drain_begin callback to polling phase X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com, stefanha@redhat.com, pbonzini@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" We cannot allow aio_poll() in bdrv_drain_invoke(begin=3Dtrue) until we're done with propagating the drain through the graph and are doing the single final BDRV_POLL_WHILE(). Just schedule the coroutine with the callback and increase bs->in_flight to make sure that the polling phase will wait for it. Signed-off-by: Kevin Wolf --- block/io.c | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/block/io.c b/block/io.c index af14227af8..a5b899723a 100644 --- a/block/io.c +++ b/block/io.c @@ -182,22 +182,40 @@ static void coroutine_fn bdrv_drain_invoke_entry(void= *opaque) =20 /* Set data->done before reading bs->wakeup. */ atomic_mb_set(&data->done, true); - bdrv_wakeup(bs); + bdrv_dec_in_flight(bs); + + if (data->begin) { + g_free(data); + } } =20 /* Recursively call BlockDriver.bdrv_co_drain_begin/end callbacks */ static void bdrv_drain_invoke(BlockDriverState *bs, bool begin) { - BdrvCoDrainData data =3D { .bs =3D bs, .done =3D false, .begin =3D beg= in}; + BdrvCoDrainData *data; =20 if (!bs->drv || (begin && !bs->drv->bdrv_co_drain_begin) || (!begin && !bs->drv->bdrv_co_drain_end)) { return; } =20 - data.co =3D qemu_coroutine_create(bdrv_drain_invoke_entry, &data); - bdrv_coroutine_enter(bs, data.co); - BDRV_POLL_WHILE(bs, !data.done); + data =3D g_new(BdrvCoDrainData, 1); + *data =3D (BdrvCoDrainData) { + .bs =3D bs, + .done =3D false, + .begin =3D begin + }; + + /* Make sure the driver callback completes during the polling phase for + * drain_begin. */ + bdrv_inc_in_flight(bs); + data->co =3D qemu_coroutine_create(bdrv_drain_invoke_entry, data); + aio_co_schedule(bdrv_get_aio_context(bs), data->co); + + if (!begin) { + BDRV_POLL_WHILE(bs, !data->done); + g_free(data); + } } =20 /* Returns true if BDRV_POLL_WHILE() should go into a blocking aio_poll() = */ --=20 2.13.6 From nobody Sat May 18 22:14:49 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1527615709928319.94038817088995; Tue, 29 May 2018 10:41:49 -0700 (PDT) Received: from localhost ([::1]:34212 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNicy-0002OD-Mk for importer@patchew.org; Tue, 29 May 2018 13:41:44 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58437) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNiKW-0005A8-U0 for qemu-devel@nongnu.org; Tue, 29 May 2018 13:22:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fNiKV-0001uP-Fe for qemu-devel@nongnu.org; Tue, 29 May 2018 13:22:40 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:44886 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fNiKR-0001rL-6S; Tue, 29 May 2018 13:22:35 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id CFBCB808255B; Tue, 29 May 2018 17:22:34 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-116-193.ams2.redhat.com [10.36.116.193]) by smtp.corp.redhat.com (Postfix) with ESMTP id 7BBCA111762D; Tue, 29 May 2018 17:22:33 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Tue, 29 May 2018 19:21:51 +0200 Message-Id: <20180529172156.29311-16-kwolf@redhat.com> In-Reply-To: <20180529172156.29311-1-kwolf@redhat.com> References: <20180529172156.29311-1-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Tue, 29 May 2018 17:22:34 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Tue, 29 May 2018 17:22:34 +0000 (UTC) for IP:'10.11.54.3' DOMAIN:'int-mx03.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'kwolf@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PATCH v2 15/20] test-bdrv-drain: Test that bdrv_drain_invoke() doesn't poll X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com, stefanha@redhat.com, pbonzini@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" This adds a test case that goes wrong if bdrv_drain_invoke() calls aio_poll(). Signed-off-by: Kevin Wolf --- tests/test-bdrv-drain.c | 102 +++++++++++++++++++++++++++++++++++++++++---= ---- 1 file changed, 88 insertions(+), 14 deletions(-) diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c index ca7b6a3fdf..7b0454a193 100644 --- a/tests/test-bdrv-drain.c +++ b/tests/test-bdrv-drain.c @@ -34,12 +34,16 @@ static QemuEvent done_event; typedef struct BDRVTestState { int drain_count; AioContext *bh_indirection_ctx; + bool sleep_in_drain_begin; } BDRVTestState; =20 static void coroutine_fn bdrv_test_co_drain_begin(BlockDriverState *bs) { BDRVTestState *s =3D bs->opaque; s->drain_count++; + if (s->sleep_in_drain_begin) { + qemu_co_sleep_ns(QEMU_CLOCK_REALTIME, 100000); + } } =20 static void coroutine_fn bdrv_test_co_drain_end(BlockDriverState *bs) @@ -80,6 +84,22 @@ static int coroutine_fn bdrv_test_co_preadv(BlockDriverS= tate *bs, return 0; } =20 +static void bdrv_test_child_perm(BlockDriverState *bs, BdrvChild *c, + const BdrvChildRole *role, + BlockReopenQueue *reopen_queue, + uint64_t perm, uint64_t shared, + uint64_t *nperm, uint64_t *nshared) +{ + /* bdrv_format_default_perms() accepts only these two, so disguise + * detach_by_driver_cb_role as one of them. */ + if (role !=3D &child_file && role !=3D &child_backing) { + role =3D &child_file; + } + + bdrv_format_default_perms(bs, c, role, reopen_queue, perm, shared, + nperm, nshared); +} + static BlockDriver bdrv_test =3D { .format_name =3D "test", .instance_size =3D sizeof(BDRVTestState), @@ -90,7 +110,7 @@ static BlockDriver bdrv_test =3D { .bdrv_co_drain_begin =3D bdrv_test_co_drain_begin, .bdrv_co_drain_end =3D bdrv_test_co_drain_end, =20 - .bdrv_child_perm =3D bdrv_format_default_perms, + .bdrv_child_perm =3D bdrv_test_child_perm, }; =20 static void aio_ret_cb(void *opaque, int ret) @@ -987,13 +1007,14 @@ struct detach_by_parent_data { BdrvChild *child_b; BlockDriverState *c; BdrvChild *child_c; + bool by_parent_cb; }; +static struct detach_by_parent_data detach_by_parent_data; =20 -static void detach_by_parent_aio_cb(void *opaque, int ret) +static void detach_indirect_bh(void *opaque) { struct detach_by_parent_data *data =3D opaque; =20 - g_assert_cmpint(ret, =3D=3D, 0); bdrv_unref_child(data->parent_b, data->child_b); =20 bdrv_ref(data->c); @@ -1001,6 +1022,25 @@ static void detach_by_parent_aio_cb(void *opaque, in= t ret) &child_file, &error_abort); } =20 +static void detach_by_parent_aio_cb(void *opaque, int ret) +{ + struct detach_by_parent_data *data =3D &detach_by_parent_data; + + g_assert_cmpint(ret, =3D=3D, 0); + if (data->by_parent_cb) { + detach_indirect_bh(data); + } +} + +static void detach_by_driver_cb_drained_begin(BdrvChild *child) +{ + aio_bh_schedule_oneshot(qemu_get_current_aio_context(), + detach_indirect_bh, &detach_by_parent_data); + child_file.drained_begin(child); +} + +static BdrvChildRole detach_by_driver_cb_role; + /* * Initial graph: * @@ -1008,17 +1048,25 @@ static void detach_by_parent_aio_cb(void *opaque, i= nt ret) * \ / \ * A B C * - * PA has a pending write request whose callback changes the child nodes o= f PB: - * It removes B and adds C instead. The subtree of PB is drained, which wi= ll - * indirectly drain the write request, too. + * by_parent_cb =3D=3D true: Test that parent callbacks don't poll + * + * PA has a pending write request whose callback changes the child nod= es of + * PB: It removes B and adds C instead. The subtree of PB is drained, = which + * will indirectly drain the write request, too. + * + * by_parent_cb =3D=3D false: Test that bdrv_drain_invoke() doesn't poll + * + * PA's BdrvChildRole has a .drained_begin callback that schedules a BH + * that does the same graph change. If bdrv_drain_invoke() calls it, t= he + * state is messed up, but if it is only polled in the single + * BDRV_POLL_WHILE() at the end of the drain, this should work fine. */ -static void test_detach_by_parent_cb(void) +static void test_detach_indirect(bool by_parent_cb) { BlockBackend *blk; BlockDriverState *parent_a, *parent_b, *a, *b, *c; BdrvChild *child_a, *child_b; BlockAIOCB *acb; - struct detach_by_parent_data data; =20 QEMUIOVector qiov; struct iovec iov =3D { @@ -1027,6 +1075,12 @@ static void test_detach_by_parent_cb(void) }; qemu_iovec_init_external(&qiov, &iov, 1); =20 + if (!by_parent_cb) { + detach_by_driver_cb_role =3D child_file; + detach_by_driver_cb_role.drained_begin =3D + detach_by_driver_cb_drained_begin; + } + /* Create all involved nodes */ parent_a =3D bdrv_new_open_driver(&bdrv_test, "parent-a", BDRV_O_RDWR, &error_abort); @@ -1042,6 +1096,13 @@ static void test_detach_by_parent_cb(void) blk_insert_bs(blk, parent_a, &error_abort); bdrv_unref(parent_a); =20 + /* If we want to get bdrv_drain_invoke() to call aio_poll(), the driver + * callback must not return immediately. */ + if (!by_parent_cb) { + BDRVTestState *s =3D parent_a->opaque; + s->sleep_in_drain_begin =3D true; + } + /* Set child relationships */ bdrv_ref(b); bdrv_ref(a); @@ -1049,7 +1110,9 @@ static void test_detach_by_parent_cb(void) child_a =3D bdrv_attach_child(parent_b, a, "PB-A", &child_backing, &er= ror_abort); =20 bdrv_ref(a); - bdrv_attach_child(parent_a, a, "PA-A", &child_file, &error_abort); + bdrv_attach_child(parent_a, a, "PA-A", + by_parent_cb ? &child_file : &detach_by_driver_cb_ro= le, + &error_abort); =20 g_assert_cmpint(parent_a->refcnt, =3D=3D, 1); g_assert_cmpint(parent_b->refcnt, =3D=3D, 1); @@ -1062,18 +1125,19 @@ static void test_detach_by_parent_cb(void) g_assert(QLIST_NEXT(child_b, next) =3D=3D NULL); =20 /* Start the evil write request */ - data =3D (struct detach_by_parent_data) { + detach_by_parent_data =3D (struct detach_by_parent_data) { .parent_b =3D parent_b, .child_b =3D child_b, .c =3D c, + .by_parent_cb =3D by_parent_cb, }; - acb =3D blk_aio_preadv(blk, 0, &qiov, 0, detach_by_parent_aio_cb, &dat= a); + acb =3D blk_aio_preadv(blk, 0, &qiov, 0, detach_by_parent_aio_cb, NULL= ); g_assert(acb !=3D NULL); =20 /* Drain and check the expected result */ bdrv_subtree_drained_begin(parent_b); =20 - g_assert(data.child_c !=3D NULL); + g_assert(detach_by_parent_data.child_c !=3D NULL); =20 g_assert_cmpint(parent_a->refcnt, =3D=3D, 1); g_assert_cmpint(parent_b->refcnt, =3D=3D, 1); @@ -1081,8 +1145,8 @@ static void test_detach_by_parent_cb(void) g_assert_cmpint(b->refcnt, =3D=3D, 1); g_assert_cmpint(c->refcnt, =3D=3D, 2); =20 - g_assert(QLIST_FIRST(&parent_b->children) =3D=3D data.child_c); - g_assert(QLIST_NEXT(data.child_c, next) =3D=3D child_a); + g_assert(QLIST_FIRST(&parent_b->children) =3D=3D detach_by_parent_data= .child_c); + g_assert(QLIST_NEXT(detach_by_parent_data.child_c, next) =3D=3D child_= a); g_assert(QLIST_NEXT(child_a, next) =3D=3D NULL); =20 g_assert_cmpint(parent_a->quiesce_counter, =3D=3D, 1); @@ -1110,6 +1174,15 @@ static void test_detach_by_parent_cb(void) bdrv_unref(c); } =20 +static void test_detach_by_parent_cb(void) +{ + test_detach_indirect(true); +} + +static void test_detach_by_driver_cb(void) +{ + test_detach_indirect(false); +} =20 int main(int argc, char **argv) { @@ -1162,6 +1235,7 @@ int main(int argc, char **argv) g_test_add_func("/bdrv-drain/detach/drain", test_detach_by_drain); g_test_add_func("/bdrv-drain/detach/drain_subtree", test_detach_by_dra= in_subtree); g_test_add_func("/bdrv-drain/detach/parent_cb", test_detach_by_parent_= cb); + g_test_add_func("/bdrv-drain/detach/driver_cb", test_detach_by_driver_= cb); =20 ret =3D g_test_run(); qemu_event_destroy(&done_event); --=20 2.13.6 From nobody Sat May 18 22:14:49 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1527615637670593.5218252890606; Tue, 29 May 2018 10:40:37 -0700 (PDT) Received: from localhost ([::1]:34209 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNibs-0001lF-RT for importer@patchew.org; Tue, 29 May 2018 13:40:36 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58422) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNiKV-00058n-Uq for qemu-devel@nongnu.org; Tue, 29 May 2018 13:22:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fNiKU-0001tw-VH for qemu-devel@nongnu.org; Tue, 29 May 2018 13:22:39 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:53726 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fNiKS-0001sB-Oh; Tue, 29 May 2018 13:22:36 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 681B9BD9E; Tue, 29 May 2018 17:22:36 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-116-193.ams2.redhat.com [10.36.116.193]) by smtp.corp.redhat.com (Postfix) with ESMTP id 16859111762D; Tue, 29 May 2018 17:22:34 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Tue, 29 May 2018 19:21:52 +0200 Message-Id: <20180529172156.29311-17-kwolf@redhat.com> In-Reply-To: <20180529172156.29311-1-kwolf@redhat.com> References: <20180529172156.29311-1-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Tue, 29 May 2018 17:22:36 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Tue, 29 May 2018 17:22:36 +0000 (UTC) for IP:'10.11.54.3' DOMAIN:'int-mx03.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'kwolf@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PATCH v2 16/20] block: Allow AIO_WAIT_WHILE with NULL ctx X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com, stefanha@redhat.com, pbonzini@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" bdrv_drain_all() wants to have a single polling loop for draining the in-flight requests of all nodes. This means that the AIO_WAIT_WHILE() condition relies on activity in multiple AioContexts, which is polled from the mainloop context. We must therefore call AIO_WAIT_WHILE() from the mainloop thread and use the AioWait notification mechanism. Just randomly picking the AioContext of any non-mainloop thread would work, but instead of bothering to find such a context in the caller, we can just as well accept NULL for ctx. Signed-off-by: Kevin Wolf --- include/block/aio-wait.h | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/include/block/aio-wait.h b/include/block/aio-wait.h index 783d3678dd..c85a62f798 100644 --- a/include/block/aio-wait.h +++ b/include/block/aio-wait.h @@ -57,7 +57,8 @@ typedef struct { /** * AIO_WAIT_WHILE: * @wait: the aio wait object - * @ctx: the aio context + * @ctx: the aio context, or NULL if multiple aio contexts (for which the + * caller does not hold a lock) are involved in the polling conditio= n. * @cond: wait while this conditional expression is true * * Wait while a condition is true. Use this to implement synchronous @@ -75,7 +76,7 @@ typedef struct { bool waited_ =3D false; \ AioWait *wait_ =3D (wait); \ AioContext *ctx_ =3D (ctx); \ - if (in_aio_context_home_thread(ctx_)) { \ + if (ctx_ && in_aio_context_home_thread(ctx_)) { \ while ((cond)) { \ aio_poll(ctx_, true); \ waited_ =3D true; \ @@ -86,9 +87,13 @@ typedef struct { /* Increment wait_->num_waiters before evaluating cond. */ \ atomic_inc(&wait_->num_waiters); \ while ((cond)) { \ - aio_context_release(ctx_); \ + if (ctx_) { \ + aio_context_release(ctx_); \ + } \ aio_poll(qemu_get_aio_context(), true); \ - aio_context_acquire(ctx_); \ + if (ctx_) { \ + aio_context_acquire(ctx_); \ + } \ waited_ =3D true; \ } \ atomic_dec(&wait_->num_waiters); \ --=20 2.13.6 From nobody Sat May 18 22:14:49 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1527615233268255.91496622973432; Tue, 29 May 2018 10:33:53 -0700 (PDT) Received: from localhost ([::1]:34162 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNiVM-0005XL-Ff for importer@patchew.org; Tue, 29 May 2018 13:33:52 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58443) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNiKX-0005Am-9c for qemu-devel@nongnu.org; Tue, 29 May 2018 13:22:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fNiKW-0001v9-Cb for qemu-devel@nongnu.org; Tue, 29 May 2018 13:22:41 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:53730 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fNiKU-0001tJ-AU; Tue, 29 May 2018 13:22:38 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id F3914BD9E; Tue, 29 May 2018 17:22:37 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-116-193.ams2.redhat.com [10.36.116.193]) by smtp.corp.redhat.com (Postfix) with ESMTP id A1BD8111762D; Tue, 29 May 2018 17:22:36 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Tue, 29 May 2018 19:21:53 +0200 Message-Id: <20180529172156.29311-18-kwolf@redhat.com> In-Reply-To: <20180529172156.29311-1-kwolf@redhat.com> References: <20180529172156.29311-1-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Tue, 29 May 2018 17:22:38 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Tue, 29 May 2018 17:22:38 +0000 (UTC) for IP:'10.11.54.3' DOMAIN:'int-mx03.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'kwolf@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PATCH v2 17/20] block: Move bdrv_drain_all_begin() out of coroutine context X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com, stefanha@redhat.com, pbonzini@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Before we can introduce a single polling loop for all nodes in bdrv_drain_all_begin(), we must make sure to run it outside of coroutine context like we already do for bdrv_do_drained_begin(). Signed-off-by: Kevin Wolf --- block/io.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/block/io.c b/block/io.c index a5b899723a..2c06aaf6d8 100644 --- a/block/io.c +++ b/block/io.c @@ -264,11 +264,16 @@ static void bdrv_co_drain_bh_cb(void *opaque) Coroutine *co =3D data->co; BlockDriverState *bs =3D data->bs; =20 - bdrv_dec_in_flight(bs); - if (data->begin) { - bdrv_do_drained_begin(bs, data->recursive, data->parent, data->pol= l); + if (bs) { + bdrv_dec_in_flight(bs); + if (data->begin) { + bdrv_do_drained_begin(bs, data->recursive, data->parent, data-= >poll); + } else { + bdrv_do_drained_end(bs, data->recursive, data->parent); + } } else { - bdrv_do_drained_end(bs, data->recursive, data->parent); + assert(data->begin); + bdrv_drain_all_begin(); } =20 data->done =3D true; @@ -294,7 +299,9 @@ static void coroutine_fn bdrv_co_yield_to_drain(BlockDr= iverState *bs, .parent =3D parent, .poll =3D poll, }; - bdrv_inc_in_flight(bs); + if (bs) { + bdrv_inc_in_flight(bs); + } aio_bh_schedule_oneshot(bdrv_get_aio_context(bs), bdrv_co_drain_bh_cb, &data); =20 @@ -464,6 +471,11 @@ void bdrv_drain_all_begin(void) BlockDriverState *bs; BdrvNextIterator it; =20 + if (qemu_in_coroutine()) { + bdrv_co_yield_to_drain(NULL, true, false, NULL, true); + return; + } + /* BDRV_POLL_WHILE() for a node can only be called from its own I/O th= read * or the main loop AioContext. We potentially use BDRV_POLL_WHILE() on * nodes in several different AioContexts, so make sure we're in the m= ain --=20 2.13.6 From nobody Sat May 18 22:14:49 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (208.118.235.17 [208.118.235.17]) by mx.zohomail.com with SMTPS id 1527615249831712.9798460964279; Tue, 29 May 2018 10:34:09 -0700 (PDT) Received: from localhost ([::1]:34164 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNiVV-0005fG-TH for importer@patchew.org; Tue, 29 May 2018 13:34:01 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58548) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNiKi-0005NB-F9 for qemu-devel@nongnu.org; Tue, 29 May 2018 13:22:54 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fNiKg-0001zd-7n for qemu-devel@nongnu.org; Tue, 29 May 2018 13:22:52 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:38564 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fNiKa-0001x9-FU; Tue, 29 May 2018 13:22:44 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 19673402178A; Tue, 29 May 2018 17:22:44 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-116-193.ams2.redhat.com [10.36.116.193]) by smtp.corp.redhat.com (Postfix) with ESMTP id 3A2BA111762D; Tue, 29 May 2018 17:22:38 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Tue, 29 May 2018 19:21:54 +0200 Message-Id: <20180529172156.29311-19-kwolf@redhat.com> In-Reply-To: <20180529172156.29311-1-kwolf@redhat.com> References: <20180529172156.29311-1-kwolf@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Tue, 29 May 2018 17:22:44 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Tue, 29 May 2018 17:22:44 +0000 (UTC) for IP:'10.11.54.3' DOMAIN:'int-mx03.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'kwolf@redhat.com' RCPT:'' Content-Transfer-Encoding: quoted-printable X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PATCH v2 18/20] block: ignore_bds_parents parameter for drain functions X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com, stefanha@redhat.com, pbonzini@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" In the future, bdrv_drained_all_begin/end() will drain all invidiual nodes separately rather than whole subtrees. This means that we don't want to propagate the drain to all parents any more: If the parent is a BDS, it will already be drained separately. Recursing to all parents is unnecessary work and would make it an O(n=C2=B2) operation. Prepare the drain function for the changed drain_all by adding an ignore_bds_parents parameter to the internal implementation that prevents the propagation of the drain to BDS parents. We still (have to) propagate it to non-BDS parents like BlockBackends or Jobs because those are not drained separately. Signed-off-by: Kevin Wolf --- include/block/block.h | 16 ++++++--- include/block/block_int.h | 6 ++++ block.c | 11 +++--- block/io.c | 88 ++++++++++++++++++++++++++++---------------= ---- block/vvfat.c | 1 + 5 files changed, 78 insertions(+), 44 deletions(-) diff --git a/include/block/block.h b/include/block/block.h index 6026c1385e..a678556608 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -555,7 +555,8 @@ void bdrv_io_unplug(BlockDriverState *bs); * Begin a quiesced section of all users of @bs. This is part of * bdrv_drained_begin. */ -void bdrv_parent_drained_begin(BlockDriverState *bs, BdrvChild *ignore); +void bdrv_parent_drained_begin(BlockDriverState *bs, BdrvChild *ignore, + bool ignore_bds_parents); =20 /** * bdrv_parent_drained_end: @@ -563,18 +564,23 @@ void bdrv_parent_drained_begin(BlockDriverState *bs, = BdrvChild *ignore); * End a quiesced section of all users of @bs. This is part of * bdrv_drained_end. */ -void bdrv_parent_drained_end(BlockDriverState *bs, BdrvChild *ignore); +void bdrv_parent_drained_end(BlockDriverState *bs, BdrvChild *ignore, + bool ignore_bds_parents); =20 /** * bdrv_drain_poll: * * Poll for pending requests in @bs, its parents (except for @ignore_paren= t), - * and if @recursive is true its children as well. + * and if @recursive is true its children as well (used for subtree drain). + * + * If @ignore_bds_parents is true, parents that are BlockDriverStates must + * ignore the drain request because they will be drained separately (used = for + * drain_all). * * This is part of bdrv_drained_begin. */ bool bdrv_drain_poll(BlockDriverState *bs, bool recursive, - BdrvChild *ignore_parent); + BdrvChild *ignore_parent, bool ignore_bds_parents); =20 /** * bdrv_drained_begin: @@ -595,7 +601,7 @@ void bdrv_drained_begin(BlockDriverState *bs); * running requests to complete. */ void bdrv_do_drained_begin_quiesce(BlockDriverState *bs, - BdrvChild *parent); + BdrvChild *parent, bool ignore_bds_pare= nts); =20 /** * Like bdrv_drained_begin, but recursively begins a quiesced section for diff --git a/include/block/block_int.h b/include/block/block_int.h index feba86888e..edb16df015 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -545,6 +545,12 @@ struct BdrvChildRole { * points to. */ bool stay_at_node; =20 + /* If true, the parent is a BlockDriverState and bdrv_next_all_states() + * will return it. This information is used for drain_all, where every= node + * will be drained separately, so the drain only needs to be propagate= d to + * non-BDS parents. */ + bool parent_is_bds; + void (*inherit_options)(int *child_flags, QDict *child_options, int parent_flags, QDict *parent_options); =20 diff --git a/block.c b/block.c index 1405d1180b..ce6eb53ad9 100644 --- a/block.c +++ b/block.c @@ -817,13 +817,13 @@ static char *bdrv_child_get_parent_desc(BdrvChild *c) static void bdrv_child_cb_drained_begin(BdrvChild *child) { BlockDriverState *bs =3D child->opaque; - bdrv_do_drained_begin_quiesce(bs, NULL); + bdrv_do_drained_begin_quiesce(bs, NULL, false); } =20 static bool bdrv_child_cb_drained_poll(BdrvChild *child) { BlockDriverState *bs =3D child->opaque; - return bdrv_drain_poll(bs, false, NULL); + return bdrv_drain_poll(bs, false, NULL, false); } =20 static void bdrv_child_cb_drained_end(BdrvChild *child) @@ -907,6 +907,7 @@ static void bdrv_inherited_options(int *child_flags, QD= ict *child_options, } =20 const BdrvChildRole child_file =3D { + .parent_is_bds =3D true, .get_parent_desc =3D bdrv_child_get_parent_desc, .inherit_options =3D bdrv_inherited_options, .drained_begin =3D bdrv_child_cb_drained_begin, @@ -932,6 +933,7 @@ static void bdrv_inherited_fmt_options(int *child_flags= , QDict *child_options, } =20 const BdrvChildRole child_format =3D { + .parent_is_bds =3D true, .get_parent_desc =3D bdrv_child_get_parent_desc, .inherit_options =3D bdrv_inherited_fmt_options, .drained_begin =3D bdrv_child_cb_drained_begin, @@ -1050,6 +1052,7 @@ static int bdrv_backing_update_filename(BdrvChild *c,= BlockDriverState *base, } =20 const BdrvChildRole child_backing =3D { + .parent_is_bds =3D true, .get_parent_desc =3D bdrv_child_get_parent_desc, .attach =3D bdrv_backing_attach, .detach =3D bdrv_backing_detach, @@ -4945,7 +4948,7 @@ void bdrv_set_aio_context(BlockDriverState *bs, AioCo= ntext *new_context) AioContext *ctx =3D bdrv_get_aio_context(bs); =20 aio_disable_external(ctx); - bdrv_parent_drained_begin(bs, NULL); + bdrv_parent_drained_begin(bs, NULL, false); bdrv_drain(bs); /* ensure there are no in-flight requests */ =20 while (aio_poll(ctx, false)) { @@ -4959,7 +4962,7 @@ void bdrv_set_aio_context(BlockDriverState *bs, AioCo= ntext *new_context) */ aio_context_acquire(new_context); bdrv_attach_aio_context(bs, new_context); - bdrv_parent_drained_end(bs, NULL); + bdrv_parent_drained_end(bs, NULL, false); aio_enable_external(ctx); aio_context_release(new_context); } diff --git a/block/io.c b/block/io.c index 2c06aaf6d8..d71e278289 100644 --- a/block/io.c +++ b/block/io.c @@ -41,12 +41,13 @@ static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs, int64_t offset, int bytes, BdrvRequestFlags flags); =20 -void bdrv_parent_drained_begin(BlockDriverState *bs, BdrvChild *ignore) +void bdrv_parent_drained_begin(BlockDriverState *bs, BdrvChild *ignore, + bool ignore_bds_parents) { BdrvChild *c, *next; =20 QLIST_FOREACH_SAFE(c, &bs->parents, next_parent, next) { - if (c =3D=3D ignore) { + if (c =3D=3D ignore || (ignore_bds_parents && c->role->parent_is_b= ds)) { continue; } if (c->role->drained_begin) { @@ -55,12 +56,13 @@ void bdrv_parent_drained_begin(BlockDriverState *bs, Bd= rvChild *ignore) } } =20 -void bdrv_parent_drained_end(BlockDriverState *bs, BdrvChild *ignore) +void bdrv_parent_drained_end(BlockDriverState *bs, BdrvChild *ignore, + bool ignore_bds_parents) { BdrvChild *c, *next; =20 QLIST_FOREACH_SAFE(c, &bs->parents, next_parent, next) { - if (c =3D=3D ignore) { + if (c =3D=3D ignore || (ignore_bds_parents && c->role->parent_is_b= ds)) { continue; } if (c->role->drained_end) { @@ -69,13 +71,14 @@ void bdrv_parent_drained_end(BlockDriverState *bs, Bdrv= Child *ignore) } } =20 -static bool bdrv_parent_drained_poll(BlockDriverState *bs, BdrvChild *igno= re) +static bool bdrv_parent_drained_poll(BlockDriverState *bs, BdrvChild *igno= re, + bool ignore_bds_parents) { BdrvChild *c, *next; bool busy =3D false; =20 QLIST_FOREACH_SAFE(c, &bs->parents, next_parent, next) { - if (c =3D=3D ignore) { + if (c =3D=3D ignore || (ignore_bds_parents && c->role->parent_is_b= ds)) { continue; } if (c->role->drained_poll) { @@ -167,6 +170,7 @@ typedef struct { bool recursive; bool poll; BdrvChild *parent; + bool ignore_bds_parents; } BdrvCoDrainData; =20 static void coroutine_fn bdrv_drain_invoke_entry(void *opaque) @@ -220,11 +224,11 @@ static void bdrv_drain_invoke(BlockDriverState *bs, b= ool begin) =20 /* Returns true if BDRV_POLL_WHILE() should go into a blocking aio_poll() = */ bool bdrv_drain_poll(BlockDriverState *bs, bool recursive, - BdrvChild *ignore_parent) + BdrvChild *ignore_parent, bool ignore_bds_parents) { BdrvChild *child, *next; =20 - if (bdrv_parent_drained_poll(bs, ignore_parent)) { + if (bdrv_parent_drained_poll(bs, ignore_parent, ignore_bds_parents)) { return true; } =20 @@ -233,8 +237,9 @@ bool bdrv_drain_poll(BlockDriverState *bs, bool recursi= ve, } =20 if (recursive) { + assert(!ignore_bds_parents); QLIST_FOREACH_SAFE(child, &bs->children, next, next) { - if (bdrv_drain_poll(child->bs, recursive, child)) { + if (bdrv_drain_poll(child->bs, recursive, child, false)) { return true; } } @@ -250,13 +255,14 @@ static bool bdrv_drain_poll_top_level(BlockDriverStat= e *bs, bool recursive, * have executed. */ while (aio_poll(bs->aio_context, false)); =20 - return bdrv_drain_poll(bs, recursive, ignore_parent); + return bdrv_drain_poll(bs, recursive, ignore_parent, false); } =20 static void bdrv_do_drained_begin(BlockDriverState *bs, bool recursive, - BdrvChild *parent, bool poll); + BdrvChild *parent, bool ignore_bds_paren= ts, + bool poll); static void bdrv_do_drained_end(BlockDriverState *bs, bool recursive, - BdrvChild *parent); + BdrvChild *parent, bool ignore_bds_parents= ); =20 static void bdrv_co_drain_bh_cb(void *opaque) { @@ -267,9 +273,11 @@ static void bdrv_co_drain_bh_cb(void *opaque) if (bs) { bdrv_dec_in_flight(bs); if (data->begin) { - bdrv_do_drained_begin(bs, data->recursive, data->parent, data-= >poll); + bdrv_do_drained_begin(bs, data->recursive, data->parent, + data->ignore_bds_parents, data->poll); } else { - bdrv_do_drained_end(bs, data->recursive, data->parent); + bdrv_do_drained_end(bs, data->recursive, data->parent, + data->ignore_bds_parents); } } else { assert(data->begin); @@ -282,7 +290,9 @@ static void bdrv_co_drain_bh_cb(void *opaque) =20 static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs, bool begin, bool recursive, - BdrvChild *parent, bool po= ll) + BdrvChild *parent, + bool ignore_bds_parents, + bool poll) { BdrvCoDrainData data; =20 @@ -297,6 +307,7 @@ static void coroutine_fn bdrv_co_yield_to_drain(BlockDr= iverState *bs, .begin =3D begin, .recursive =3D recursive, .parent =3D parent, + .ignore_bds_parents =3D ignore_bds_parents, .poll =3D poll, }; if (bs) { @@ -312,7 +323,7 @@ static void coroutine_fn bdrv_co_yield_to_drain(BlockDr= iverState *bs, } =20 void bdrv_do_drained_begin_quiesce(BlockDriverState *bs, - BdrvChild *parent) + BdrvChild *parent, bool ignore_bds_pare= nts) { assert(!qemu_in_coroutine()); =20 @@ -321,26 +332,30 @@ void bdrv_do_drained_begin_quiesce(BlockDriverState *= bs, aio_disable_external(bdrv_get_aio_context(bs)); } =20 - bdrv_parent_drained_begin(bs, parent); + bdrv_parent_drained_begin(bs, parent, ignore_bds_parents); bdrv_drain_invoke(bs, true); } =20 static void bdrv_do_drained_begin(BlockDriverState *bs, bool recursive, - BdrvChild *parent, bool poll) + BdrvChild *parent, bool ignore_bds_paren= ts, + bool poll) { BdrvChild *child, *next; =20 if (qemu_in_coroutine()) { - bdrv_co_yield_to_drain(bs, true, recursive, parent, poll); + bdrv_co_yield_to_drain(bs, true, recursive, parent, ignore_bds_par= ents, + poll); return; } =20 - bdrv_do_drained_begin_quiesce(bs, parent); + bdrv_do_drained_begin_quiesce(bs, parent, ignore_bds_parents); =20 if (recursive) { + assert(!ignore_bds_parents); bs->recursive_quiesce_counter++; QLIST_FOREACH_SAFE(child, &bs->children, next, next) { - bdrv_do_drained_begin(child->bs, true, child, false); + bdrv_do_drained_begin(child->bs, true, child, ignore_bds_paren= ts, + false); } } =20 @@ -354,28 +369,30 @@ static void bdrv_do_drained_begin(BlockDriverState *b= s, bool recursive, * nodes. */ if (poll) { + assert(!ignore_bds_parents); BDRV_POLL_WHILE(bs, bdrv_drain_poll_top_level(bs, recursive, paren= t)); } } =20 void bdrv_drained_begin(BlockDriverState *bs) { - bdrv_do_drained_begin(bs, false, NULL, true); + bdrv_do_drained_begin(bs, false, NULL, false, true); } =20 void bdrv_subtree_drained_begin(BlockDriverState *bs) { - bdrv_do_drained_begin(bs, true, NULL, true); + bdrv_do_drained_begin(bs, true, NULL, false, true); } =20 -void bdrv_do_drained_end(BlockDriverState *bs, bool recursive, - BdrvChild *parent) +static void bdrv_do_drained_end(BlockDriverState *bs, bool recursive, + BdrvChild *parent, bool ignore_bds_parents) { BdrvChild *child, *next; int old_quiesce_counter; =20 if (qemu_in_coroutine()) { - bdrv_co_yield_to_drain(bs, false, recursive, parent, false); + bdrv_co_yield_to_drain(bs, false, recursive, parent, ignore_bds_pa= rents, + false); return; } assert(bs->quiesce_counter > 0); @@ -383,27 +400,28 @@ void bdrv_do_drained_end(BlockDriverState *bs, bool r= ecursive, =20 /* Re-enable things in child-to-parent order */ bdrv_drain_invoke(bs, false); - bdrv_parent_drained_end(bs, parent); + bdrv_parent_drained_end(bs, parent, ignore_bds_parents); if (old_quiesce_counter =3D=3D 1) { aio_enable_external(bdrv_get_aio_context(bs)); } =20 if (recursive) { + assert(!ignore_bds_parents); bs->recursive_quiesce_counter--; QLIST_FOREACH_SAFE(child, &bs->children, next, next) { - bdrv_do_drained_end(child->bs, true, child); + bdrv_do_drained_end(child->bs, true, child, ignore_bds_parents= ); } } } =20 void bdrv_drained_end(BlockDriverState *bs) { - bdrv_do_drained_end(bs, false, NULL); + bdrv_do_drained_end(bs, false, NULL, false); } =20 void bdrv_subtree_drained_end(BlockDriverState *bs) { - bdrv_do_drained_end(bs, true, NULL); + bdrv_do_drained_end(bs, true, NULL, false); } =20 void bdrv_apply_subtree_drain(BdrvChild *child, BlockDriverState *new_pare= nt) @@ -411,7 +429,7 @@ void bdrv_apply_subtree_drain(BdrvChild *child, BlockDr= iverState *new_parent) int i; =20 for (i =3D 0; i < new_parent->recursive_quiesce_counter; i++) { - bdrv_do_drained_begin(child->bs, true, child, true); + bdrv_do_drained_begin(child->bs, true, child, false, true); } } =20 @@ -420,7 +438,7 @@ void bdrv_unapply_subtree_drain(BdrvChild *child, Block= DriverState *old_parent) int i; =20 for (i =3D 0; i < old_parent->recursive_quiesce_counter; i++) { - bdrv_do_drained_end(child->bs, true, child); + bdrv_do_drained_end(child->bs, true, child, false); } } =20 @@ -472,7 +490,7 @@ void bdrv_drain_all_begin(void) BdrvNextIterator it; =20 if (qemu_in_coroutine()) { - bdrv_co_yield_to_drain(NULL, true, false, NULL, true); + bdrv_co_yield_to_drain(NULL, true, false, NULL, false, true); return; } =20 @@ -486,7 +504,7 @@ void bdrv_drain_all_begin(void) AioContext *aio_context =3D bdrv_get_aio_context(bs); =20 aio_context_acquire(aio_context); - bdrv_do_drained_begin(bs, true, NULL, true); + bdrv_do_drained_begin(bs, true, NULL, false, true); aio_context_release(aio_context); } =20 @@ -504,7 +522,7 @@ void bdrv_drain_all_end(void) AioContext *aio_context =3D bdrv_get_aio_context(bs); =20 aio_context_acquire(aio_context); - bdrv_do_drained_end(bs, true, NULL); + bdrv_do_drained_end(bs, true, NULL, false); aio_context_release(aio_context); } } diff --git a/block/vvfat.c b/block/vvfat.c index 662dca0114..384c954d70 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -3133,6 +3133,7 @@ static void vvfat_qcow_options(int *child_flags, QDic= t *child_options, } =20 static const BdrvChildRole child_vvfat_qcow =3D { + .parent_is_bds =3D true, .inherit_options =3D vvfat_qcow_options, }; =20 --=20 2.13.6 From nobody Sat May 18 22:14:49 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1527615383926829.4137260609187; Tue, 29 May 2018 10:36:23 -0700 (PDT) Received: from localhost ([::1]:34184 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNiXm-0007M8-W0 for importer@patchew.org; Tue, 29 May 2018 13:36:23 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58532) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNiKh-0005LX-5g for qemu-devel@nongnu.org; Tue, 29 May 2018 13:22:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fNiKf-0001zF-HH for qemu-devel@nongnu.org; Tue, 29 May 2018 13:22:51 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:53736 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fNiKc-0001xe-0y; Tue, 29 May 2018 13:22:46 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id A5DFFBB41B; Tue, 29 May 2018 17:22:45 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-116-193.ams2.redhat.com [10.36.116.193]) by smtp.corp.redhat.com (Postfix) with ESMTP id 53C18111762D; Tue, 29 May 2018 17:22:44 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Tue, 29 May 2018 19:21:55 +0200 Message-Id: <20180529172156.29311-20-kwolf@redhat.com> In-Reply-To: <20180529172156.29311-1-kwolf@redhat.com> References: <20180529172156.29311-1-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Tue, 29 May 2018 17:22:45 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Tue, 29 May 2018 17:22:45 +0000 (UTC) for IP:'10.11.54.3' DOMAIN:'int-mx03.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'kwolf@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PATCH v2 19/20] block: Allow graph changes in bdrv_drain_all_begin/end sections X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com, stefanha@redhat.com, pbonzini@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" bdrv_drain_all_*() used bdrv_next() to iterate over all root nodes and did a subtree drain for each of them. This works fine as long as the graph is static, but sadly, reality looks different. If the graph changes so that root nodes are added or removed, we would have to compensate for this. bdrv_next() returns each root node only once even if it's the root node for multiple BlockBackends or for a monitor-owned block driver tree, which would only complicate things. The much easier and more obviously correct way is to fundamentally change the way the functions work: Iterate over all BlockDriverStates, no matter who owns them, and drain them individually. Compensation is only necessary when a new BDS is created inside a drain_all section. Removal of a BDS doesn't require any action because it's gone afterwards anyway. Signed-off-by: Kevin Wolf --- include/block/block.h | 1 + include/block/block_int.h | 1 + block.c | 34 ++++++++++++++++++++++++--- block/io.c | 60 ++++++++++++++++++++++++++++++++++++-------= ---- 4 files changed, 79 insertions(+), 17 deletions(-) diff --git a/include/block/block.h b/include/block/block.h index a678556608..1ff13854d8 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -419,6 +419,7 @@ BlockDriverState *bdrv_lookup_bs(const char *device, Error **errp); bool bdrv_chain_contains(BlockDriverState *top, BlockDriverState *base); BlockDriverState *bdrv_next_node(BlockDriverState *bs); +BlockDriverState *bdrv_next_all_states(BlockDriverState *bs); =20 typedef struct BdrvNextIterator { enum { diff --git a/include/block/block_int.h b/include/block/block_int.h index edb16df015..212694a0c2 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -822,6 +822,7 @@ int coroutine_fn bdrv_co_pwritev(BdrvChild *child, int64_t offset, unsigned int bytes, QEMUIOVector *qiov, BdrvRequestFlags flags); =20 +extern unsigned int bdrv_drain_all_count; void bdrv_apply_subtree_drain(BdrvChild *child, BlockDriverState *new_pare= nt); void bdrv_unapply_subtree_drain(BdrvChild *child, BlockDriverState *old_pa= rent); =20 diff --git a/block.c b/block.c index ce6eb53ad9..5c58a19646 100644 --- a/block.c +++ b/block.c @@ -332,6 +332,10 @@ BlockDriverState *bdrv_new(void) =20 qemu_co_queue_init(&bs->flush_queue); =20 + for (i =3D 0; i < bdrv_drain_all_count; i++) { + bdrv_drained_begin(bs); + } + QTAILQ_INSERT_TAIL(&all_bdrv_states, bs, bs_list); =20 return bs; @@ -1163,7 +1167,7 @@ static int bdrv_open_driver(BlockDriverState *bs, Blo= ckDriver *drv, int open_flags, Error **errp) { Error *local_err =3D NULL; - int ret; + int i, ret; =20 bdrv_assign_node_name(bs, node_name, &local_err); if (local_err) { @@ -1211,6 +1215,12 @@ static int bdrv_open_driver(BlockDriverState *bs, Bl= ockDriver *drv, assert(bdrv_min_mem_align(bs) !=3D 0); assert(is_power_of_2(bs->bl.request_alignment)); =20 + for (i =3D 0; i < bs->quiesce_counter; i++) { + if (drv->bdrv_co_drain_begin) { + drv->bdrv_co_drain_begin(bs); + } + } + return 0; open_failed: bs->drv =3D NULL; @@ -2021,7 +2031,12 @@ static void bdrv_replace_child_noperm(BdrvChild *chi= ld, child->role->detach(child); } if (old_bs->quiesce_counter && child->role->drained_end) { - for (i =3D 0; i < old_bs->quiesce_counter; i++) { + int num =3D old_bs->quiesce_counter; + if (child->role->parent_is_bds) { + num -=3D bdrv_drain_all_count; + } + assert(num >=3D 0); + for (i =3D 0; i < num; i++) { child->role->drained_end(child); } } @@ -2033,7 +2048,12 @@ static void bdrv_replace_child_noperm(BdrvChild *chi= ld, if (new_bs) { QLIST_INSERT_HEAD(&new_bs->parents, child, next_parent); if (new_bs->quiesce_counter && child->role->drained_begin) { - for (i =3D 0; i < new_bs->quiesce_counter; i++) { + int num =3D new_bs->quiesce_counter; + if (child->role->parent_is_bds) { + num -=3D bdrv_drain_all_count; + } + assert(num >=3D 0); + for (i =3D 0; i < num; i++) { child->role->drained_begin(child); } } @@ -4037,6 +4057,14 @@ BlockDriverState *bdrv_next_node(BlockDriverState *b= s) return QTAILQ_NEXT(bs, node_list); } =20 +BlockDriverState *bdrv_next_all_states(BlockDriverState *bs) +{ + if (!bs) { + return QTAILQ_FIRST(&all_bdrv_states); + } + return QTAILQ_NEXT(bs, bs_list); +} + const char *bdrv_get_node_name(const BlockDriverState *bs) { return bs->node_name; diff --git a/block/io.c b/block/io.c index d71e278289..08298f815b 100644 --- a/block/io.c +++ b/block/io.c @@ -38,6 +38,8 @@ /* Maximum bounce buffer for copy-on-read and write zeroes, in bytes */ #define MAX_BOUNCE_BUFFER (32768 << BDRV_SECTOR_BITS) =20 +static AioWait drain_all_aio_wait; + static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs, int64_t offset, int bytes, BdrvRequestFlags flags); =20 @@ -472,6 +474,29 @@ static void bdrv_drain_assert_idle(BlockDriverState *b= s) } } =20 +unsigned int bdrv_drain_all_count =3D 0; + +static bool bdrv_drain_all_poll(void) +{ + BlockDriverState *bs =3D NULL; + bool result =3D false; + + /* Execute pending BHs first (may modify the graph) and check everythi= ng + * else only after the BHs have executed. */ + while (aio_poll(qemu_get_aio_context(), false)); + + /* bdrv_drain_poll() can't make changes to the graph and we are holdin= g the + * main AioContext lock, so iterating bdrv_next_all_states() is safe. = */ + while ((bs =3D bdrv_next_all_states(bs))) { + AioContext *aio_context =3D bdrv_get_aio_context(bs); + aio_context_acquire(aio_context); + result |=3D bdrv_drain_poll(bs, false, NULL, true); + aio_context_release(aio_context); + } + + return result; +} + /* * Wait for pending requests to complete across all BlockDriverStates * @@ -486,45 +511,51 @@ static void bdrv_drain_assert_idle(BlockDriverState *= bs) */ void bdrv_drain_all_begin(void) { - BlockDriverState *bs; - BdrvNextIterator it; + BlockDriverState *bs =3D NULL; =20 if (qemu_in_coroutine()) { - bdrv_co_yield_to_drain(NULL, true, false, NULL, false, true); + bdrv_co_yield_to_drain(NULL, true, false, NULL, true, true); return; } =20 - /* BDRV_POLL_WHILE() for a node can only be called from its own I/O th= read - * or the main loop AioContext. We potentially use BDRV_POLL_WHILE() on - * nodes in several different AioContexts, so make sure we're in the m= ain - * context. */ + /* AIO_WAIT_WHILE() with a NULL context can only be called from the ma= in + * loop AioContext, so make sure we're in the main context. */ assert(qemu_get_current_aio_context() =3D=3D qemu_get_aio_context()); + assert(bdrv_drain_all_count < INT_MAX); + bdrv_drain_all_count++; =20 - for (bs =3D bdrv_first(&it); bs; bs =3D bdrv_next(&it)) { + /* Quiesce all nodes, without polling in-flight requests yet. The graph + * cannot change during this loop. */ + while ((bs =3D bdrv_next_all_states(bs))) { AioContext *aio_context =3D bdrv_get_aio_context(bs); =20 aio_context_acquire(aio_context); - bdrv_do_drained_begin(bs, true, NULL, false, true); + bdrv_do_drained_begin(bs, false, NULL, true, false); aio_context_release(aio_context); } =20 - for (bs =3D bdrv_first(&it); bs; bs =3D bdrv_next(&it)) { + /* Now poll the in-flight requests */ + AIO_WAIT_WHILE(&drain_all_aio_wait, NULL, bdrv_drain_all_poll()); + + while ((bs =3D bdrv_next_all_states(bs))) { bdrv_drain_assert_idle(bs); } } =20 void bdrv_drain_all_end(void) { - BlockDriverState *bs; - BdrvNextIterator it; + BlockDriverState *bs =3D NULL; =20 - for (bs =3D bdrv_first(&it); bs; bs =3D bdrv_next(&it)) { + while ((bs =3D bdrv_next_all_states(bs))) { AioContext *aio_context =3D bdrv_get_aio_context(bs); =20 aio_context_acquire(aio_context); - bdrv_do_drained_end(bs, true, NULL, false); + bdrv_do_drained_end(bs, false, NULL, true); aio_context_release(aio_context); } + + assert(bdrv_drain_all_count > 0); + bdrv_drain_all_count--; } =20 void bdrv_drain_all(void) @@ -647,6 +678,7 @@ void bdrv_inc_in_flight(BlockDriverState *bs) void bdrv_wakeup(BlockDriverState *bs) { aio_wait_kick(bdrv_get_aio_wait(bs)); + aio_wait_kick(&drain_all_aio_wait); } =20 void bdrv_dec_in_flight(BlockDriverState *bs) --=20 2.13.6 From nobody Sat May 18 22:14:49 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 152761506753056.15466528054185; Tue, 29 May 2018 10:31:07 -0700 (PDT) Received: from localhost ([::1]:34145 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNiSd-0003WW-R4 for importer@patchew.org; Tue, 29 May 2018 13:31:03 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58534) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNiKh-0005Ln-Dd for qemu-devel@nongnu.org; Tue, 29 May 2018 13:22:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fNiKg-0001zm-8y for qemu-devel@nongnu.org; Tue, 29 May 2018 13:22:51 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:38566 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fNiKd-0001yL-KU; Tue, 29 May 2018 13:22:47 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 3E81A402178A; Tue, 29 May 2018 17:22:47 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-116-193.ams2.redhat.com [10.36.116.193]) by smtp.corp.redhat.com (Postfix) with ESMTP id E0529111762D; Tue, 29 May 2018 17:22:45 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Tue, 29 May 2018 19:21:56 +0200 Message-Id: <20180529172156.29311-21-kwolf@redhat.com> In-Reply-To: <20180529172156.29311-1-kwolf@redhat.com> References: <20180529172156.29311-1-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Tue, 29 May 2018 17:22:47 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Tue, 29 May 2018 17:22:47 +0000 (UTC) for IP:'10.11.54.3' DOMAIN:'int-mx03.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'kwolf@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PATCH v2 20/20] test-bdrv-drain: Test graph changes in drain_all section X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com, stefanha@redhat.com, pbonzini@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" This tests both adding and remove a node between bdrv_drain_all_begin() and bdrv_drain_all_end(), and enabled the existing detach test for drain_all. Signed-off-by: Kevin Wolf --- tests/test-bdrv-drain.c | 75 +++++++++++++++++++++++++++++++++++++++++++++= ++-- 1 file changed, 73 insertions(+), 2 deletions(-) diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c index 7b0454a193..51088f318c 100644 --- a/tests/test-bdrv-drain.c +++ b/tests/test-bdrv-drain.c @@ -452,7 +452,7 @@ static void test_multiparent(void) blk_unref(blk_b); } =20 -static void test_graph_change(void) +static void test_graph_change_drain_subtree(void) { BlockBackend *blk_a, *blk_b; BlockDriverState *bs_a, *bs_b, *backing; @@ -531,6 +531,63 @@ static void test_graph_change(void) blk_unref(blk_b); } =20 +static void test_graph_change_drain_all(void) +{ + BlockBackend *blk_a, *blk_b; + BlockDriverState *bs_a, *bs_b; + BDRVTestState *a_s, *b_s; + + /* Create node A with a BlockBackend */ + blk_a =3D blk_new(BLK_PERM_ALL, BLK_PERM_ALL); + bs_a =3D bdrv_new_open_driver(&bdrv_test, "test-node-a", BDRV_O_RDWR, + &error_abort); + a_s =3D bs_a->opaque; + blk_insert_bs(blk_a, bs_a, &error_abort); + + g_assert_cmpint(bs_a->quiesce_counter, =3D=3D, 0); + g_assert_cmpint(a_s->drain_count, =3D=3D, 0); + + /* Call bdrv_drain_all_begin() */ + bdrv_drain_all_begin(); + + g_assert_cmpint(bs_a->quiesce_counter, =3D=3D, 1); + g_assert_cmpint(a_s->drain_count, =3D=3D, 1); + + /* Create node B with a BlockBackend */ + blk_b =3D blk_new(BLK_PERM_ALL, BLK_PERM_ALL); + bs_b =3D bdrv_new_open_driver(&bdrv_test, "test-node-b", BDRV_O_RDWR, + &error_abort); + b_s =3D bs_b->opaque; + blk_insert_bs(blk_b, bs_b, &error_abort); + + g_assert_cmpint(bs_a->quiesce_counter, =3D=3D, 1); + g_assert_cmpint(bs_b->quiesce_counter, =3D=3D, 1); + g_assert_cmpint(a_s->drain_count, =3D=3D, 1); + g_assert_cmpint(b_s->drain_count, =3D=3D, 1); + + /* Unref and finally delete node A */ + blk_unref(blk_a); + + g_assert_cmpint(bs_a->quiesce_counter, =3D=3D, 1); + g_assert_cmpint(bs_b->quiesce_counter, =3D=3D, 1); + g_assert_cmpint(a_s->drain_count, =3D=3D, 1); + g_assert_cmpint(b_s->drain_count, =3D=3D, 1); + + bdrv_unref(bs_a); + + g_assert_cmpint(bs_b->quiesce_counter, =3D=3D, 1); + g_assert_cmpint(b_s->drain_count, =3D=3D, 1); + + /* End the drained section */ + bdrv_drain_all_end(); + + g_assert_cmpint(bs_b->quiesce_counter, =3D=3D, 0); + g_assert_cmpint(b_s->drain_count, =3D=3D, 0); + + bdrv_unref(bs_b); + blk_unref(blk_b); +} + struct test_iothread_data { BlockDriverState *bs; enum drain_type drain_type; @@ -971,6 +1028,10 @@ static void do_test_delete_by_drain(bool detach_inste= ad_of_delete, bdrv_subtree_drained_begin(bs); bdrv_subtree_drained_end(bs); break; + case BDRV_DRAIN_ALL: + bdrv_drain_all_begin(); + bdrv_drain_all_end(); + break; default: g_assert_not_reached(); } @@ -991,6 +1052,11 @@ static void test_delete_by_drain(void) do_test_delete_by_drain(false, BDRV_DRAIN); } =20 +static void test_detach_by_drain_all(void) +{ + do_test_delete_by_drain(true, BDRV_DRAIN_ALL); +} + static void test_detach_by_drain(void) { do_test_delete_by_drain(true, BDRV_DRAIN); @@ -1219,7 +1285,11 @@ int main(int argc, char **argv) =20 g_test_add_func("/bdrv-drain/nested", test_nested); g_test_add_func("/bdrv-drain/multiparent", test_multiparent); - g_test_add_func("/bdrv-drain/graph-change", test_graph_change); + + g_test_add_func("/bdrv-drain/graph-change/drain_subtree", + test_graph_change_drain_subtree); + g_test_add_func("/bdrv-drain/graph-change/drain_all", + test_graph_change_drain_all); =20 g_test_add_func("/bdrv-drain/iothread/drain_all", test_iothread_drain_= all); g_test_add_func("/bdrv-drain/iothread/drain", test_iothread_drain); @@ -1232,6 +1302,7 @@ int main(int argc, char **argv) test_blockjob_drain_subtree); =20 g_test_add_func("/bdrv-drain/deletion/drain", test_delete_by_drain); + g_test_add_func("/bdrv-drain/detach/drain_all", test_detach_by_drain_a= ll); g_test_add_func("/bdrv-drain/detach/drain", test_detach_by_drain); g_test_add_func("/bdrv-drain/detach/drain_subtree", test_detach_by_dra= in_subtree); g_test_add_func("/bdrv-drain/detach/parent_cb", test_detach_by_parent_= cb); --=20 2.13.6