From patchwork Wed Dec 6 10:53:04 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kevin Wolf X-Patchwork-Id: 845112 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=2001:4830:134:3::11; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3ysFrG1W7Wz9s9Y for ; Wed, 6 Dec 2017 21:55:10 +1100 (AEDT) Received: from localhost ([::1]:54858 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eMXM4-0002lV-8w for incoming@patchwork.ozlabs.org; Wed, 06 Dec 2017 05:55:08 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50687) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eMXKO-0001n8-G0 for qemu-devel@nongnu.org; Wed, 06 Dec 2017 05:53:26 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eMXKN-0006aN-Fl for qemu-devel@nongnu.org; Wed, 06 Dec 2017 05:53:24 -0500 Received: from mx1.redhat.com ([209.132.183.28]:59064) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1eMXKJ-0006X8-Vr; Wed, 06 Dec 2017 05:53:20 -0500 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 115BC7F7A9; Wed, 6 Dec 2017 10:53:19 +0000 (UTC) Received: from localhost.localdomain.com (unknown [10.36.117.0]) by smtp.corp.redhat.com (Postfix) with ESMTP id A2F70600C0; Wed, 6 Dec 2017 10:53:17 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Wed, 6 Dec 2017 11:53:04 +0100 Message-Id: <20171206105309.3468-2-kwolf@redhat.com> In-Reply-To: <20171206105309.3468-1-kwolf@redhat.com> References: <20171206105309.3468-1-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Wed, 06 Dec 2017 10:53:19 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v2 1/6] block: Make bdrv_drain_invoke() recursive 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, pbonzini@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, qemu-stable@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" This change separates bdrv_drain_invoke(), which calls the BlockDriver drain callbacks, from bdrv_drain_recurse(). Instead, the function performs its own recursion now. One reason for this is that bdrv_drain_recurse() can be called multiple times by bdrv_drain_all_begin(), but the callbacks may only be called once. The separation is necessary to fix this bug. The other reason is that we intend to go to a model where we call all driver callbacks first, and only then start polling. This is not fully achieved yet with this patch, as bdrv_drain_invoke() contains a BDRV_POLL_WHILE() loop for the block driver callbacks, which can still call callbacks for any unrelated event. It's a step in this direction anyway. Cc: qemu-stable@nongnu.org Signed-off-by: Kevin Wolf --- block/io.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/block/io.c b/block/io.c index 6773926fc1..096468b761 100644 --- a/block/io.c +++ b/block/io.c @@ -175,8 +175,10 @@ static void coroutine_fn bdrv_drain_invoke_entry(void *opaque) bdrv_wakeup(bs); } +/* Recursively call BlockDriver.bdrv_co_drain_begin/end callbacks */ static void bdrv_drain_invoke(BlockDriverState *bs, bool begin) { + BdrvChild *child, *tmp; BdrvCoDrainData data = { .bs = bs, .done = false, .begin = begin}; if (!bs->drv || (begin && !bs->drv->bdrv_co_drain_begin) || @@ -187,6 +189,10 @@ static void bdrv_drain_invoke(BlockDriverState *bs, bool begin) data.co = qemu_coroutine_create(bdrv_drain_invoke_entry, &data); bdrv_coroutine_enter(bs, data.co); BDRV_POLL_WHILE(bs, !data.done); + + QLIST_FOREACH_SAFE(child, &bs->children, next, tmp) { + bdrv_drain_invoke(child->bs, begin); + } } static bool bdrv_drain_recurse(BlockDriverState *bs, bool begin) @@ -194,9 +200,6 @@ static bool bdrv_drain_recurse(BlockDriverState *bs, bool begin) BdrvChild *child, *tmp; bool waited; - /* Ensure any pending metadata writes are submitted to bs->file. */ - bdrv_drain_invoke(bs, begin); - /* Wait for drained requests to finish */ waited = BDRV_POLL_WHILE(bs, atomic_read(&bs->in_flight) > 0); @@ -279,6 +282,7 @@ void bdrv_drained_begin(BlockDriverState *bs) bdrv_parent_drained_begin(bs); } + bdrv_drain_invoke(bs, true); bdrv_drain_recurse(bs, true); } @@ -294,6 +298,7 @@ void bdrv_drained_end(BlockDriverState *bs) } bdrv_parent_drained_end(bs); + bdrv_drain_invoke(bs, false); bdrv_drain_recurse(bs, false); aio_enable_external(bdrv_get_aio_context(bs)); } @@ -372,6 +377,8 @@ void bdrv_drain_all_begin(void) aio_context_acquire(aio_context); for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) { if (aio_context == bdrv_get_aio_context(bs)) { + /* FIXME Calling this multiple times is wrong */ + bdrv_drain_invoke(bs, true); waited |= bdrv_drain_recurse(bs, true); } } @@ -393,6 +400,7 @@ void bdrv_drain_all_end(void) aio_context_acquire(aio_context); aio_enable_external(aio_context); bdrv_parent_drained_end(bs); + bdrv_drain_invoke(bs, false); bdrv_drain_recurse(bs, false); aio_context_release(aio_context); } From patchwork Wed Dec 6 10:53:05 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kevin Wolf X-Patchwork-Id: 845111 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=2001:4830:134:3::11; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3ysFqH2pJlz9s84 for ; Wed, 6 Dec 2017 21:54:19 +1100 (AEDT) Received: from localhost ([::1]:54850 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eMXLF-0001wG-Bq for incoming@patchwork.ozlabs.org; Wed, 06 Dec 2017 05:54:17 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50690) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eMXKO-0001nB-Hq for qemu-devel@nongnu.org; Wed, 06 Dec 2017 05:53:25 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eMXKN-0006ak-P2 for qemu-devel@nongnu.org; Wed, 06 Dec 2017 05:53:24 -0500 Received: from mx1.redhat.com ([209.132.183.28]:59094) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1eMXKL-0006YN-M1; Wed, 06 Dec 2017 05:53:21 -0500 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C2D6A7F7B9; Wed, 6 Dec 2017 10:53:20 +0000 (UTC) Received: from localhost.localdomain.com (unknown [10.36.117.0]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5FB78600C0; Wed, 6 Dec 2017 10:53:19 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Wed, 6 Dec 2017 11:53:05 +0100 Message-Id: <20171206105309.3468-3-kwolf@redhat.com> In-Reply-To: <20171206105309.3468-1-kwolf@redhat.com> References: <20171206105309.3468-1-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Wed, 06 Dec 2017 10:53:20 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v2 2/6] block: Call .drain_begin only once in bdrv_drain_all_begin() 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, pbonzini@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, qemu-stable@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" bdrv_drain_all_begin() used to call the .bdrv_co_drain_begin() driver callback inside its polling loop. This means that how many times it got called for each node depended on long it had to poll the event loop. This is obviously not right and results in nodes that stay drained even after bdrv_drain_all_end(), which calls .bdrv_co_drain_begin() once per node. Fix bdrv_drain_all_begin() to call the callback only once, too. Cc: qemu-stable@nongnu.org Signed-off-by: Kevin Wolf --- block/io.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/block/io.c b/block/io.c index 096468b761..603f5b059e 100644 --- a/block/io.c +++ b/block/io.c @@ -355,6 +355,7 @@ void bdrv_drain_all_begin(void) aio_context_acquire(aio_context); bdrv_parent_drained_begin(bs); aio_disable_external(aio_context); + bdrv_drain_invoke(bs, true); aio_context_release(aio_context); if (!g_slist_find(aio_ctxs, aio_context)) { @@ -377,8 +378,6 @@ void bdrv_drain_all_begin(void) aio_context_acquire(aio_context); for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) { if (aio_context == bdrv_get_aio_context(bs)) { - /* FIXME Calling this multiple times is wrong */ - bdrv_drain_invoke(bs, true); waited |= bdrv_drain_recurse(bs, true); } } From patchwork Wed Dec 6 10:53:06 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kevin Wolf X-Patchwork-Id: 845121 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=2001:4830:134:3::11; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3ysFtd1sSrz9s84 for ; Wed, 6 Dec 2017 21:57:13 +1100 (AEDT) Received: from localhost ([::1]:54877 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eMXO3-0004al-BU for incoming@patchwork.ozlabs.org; Wed, 06 Dec 2017 05:57:11 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50784) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eMXKR-0001q1-S5 for qemu-devel@nongnu.org; Wed, 06 Dec 2017 05:53:30 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eMXKQ-0006co-Iq for qemu-devel@nongnu.org; Wed, 06 Dec 2017 05:53:27 -0500 Received: from mx1.redhat.com ([209.132.183.28]:40594) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1eMXKN-0006a4-EM; Wed, 06 Dec 2017 05:53:23 -0500 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 7FD96C013C5F; Wed, 6 Dec 2017 10:53:22 +0000 (UTC) Received: from localhost.localdomain.com (unknown [10.36.117.0]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1C452600C0; Wed, 6 Dec 2017 10:53:20 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Wed, 6 Dec 2017 11:53:06 +0100 Message-Id: <20171206105309.3468-4-kwolf@redhat.com> In-Reply-To: <20171206105309.3468-1-kwolf@redhat.com> References: <20171206105309.3468-1-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Wed, 06 Dec 2017 10:53:22 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v2 3/6] test-bdrv-drain: Test BlockDriver callbacks for 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, pbonzini@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, qemu-stable@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" This adds a test case that the BlockDriver callbacks for drain are called in bdrv_drained_all_begin/end(), and that both of them are called exactly once. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- tests/test-bdrv-drain.c | 137 ++++++++++++++++++++++++++++++++++++++++++++++++ tests/Makefile.include | 2 + 2 files changed, 139 insertions(+) create mode 100644 tests/test-bdrv-drain.c diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c new file mode 100644 index 0000000000..67541438c1 --- /dev/null +++ b/tests/test-bdrv-drain.c @@ -0,0 +1,137 @@ +/* + * Block node draining tests + * + * Copyright (c) 2017 Kevin Wolf + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#include "qemu/osdep.h" +#include "block/block.h" +#include "sysemu/block-backend.h" +#include "qapi/error.h" + +typedef struct BDRVTestState { + int drain_count; +} BDRVTestState; + +static void coroutine_fn bdrv_test_co_drain_begin(BlockDriverState *bs) +{ + BDRVTestState *s = bs->opaque; + s->drain_count++; +} + +static void coroutine_fn bdrv_test_co_drain_end(BlockDriverState *bs) +{ + BDRVTestState *s = bs->opaque; + s->drain_count--; +} + +static void bdrv_test_close(BlockDriverState *bs) +{ + BDRVTestState *s = bs->opaque; + g_assert_cmpint(s->drain_count, >, 0); +} + +static int coroutine_fn bdrv_test_co_preadv(BlockDriverState *bs, + uint64_t offset, uint64_t bytes, + QEMUIOVector *qiov, int flags) +{ + /* 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() comes + * first and polls its result, too, but it shouldn't accidentally complete + * this request yet. */ + co_aio_sleep_ns(qemu_get_aio_context(), QEMU_CLOCK_REALTIME, 100000); + + return 0; +} + +static BlockDriver bdrv_test = { + .format_name = "test", + .instance_size = sizeof(BDRVTestState), + + .bdrv_close = bdrv_test_close, + .bdrv_co_preadv = bdrv_test_co_preadv, + + .bdrv_co_drain_begin = bdrv_test_co_drain_begin, + .bdrv_co_drain_end = bdrv_test_co_drain_end, +}; + +static void aio_ret_cb(void *opaque, int ret) +{ + int *aio_ret = opaque; + *aio_ret = ret; +} + +static void test_drv_cb_drain_all(void) +{ + BlockBackend *blk; + BlockDriverState *bs; + BDRVTestState *s; + BlockAIOCB *acb; + int aio_ret; + + QEMUIOVector qiov; + struct iovec iov = { + .iov_base = NULL, + .iov_len = 0, + }; + qemu_iovec_init_external(&qiov, &iov, 1); + + blk = blk_new(BLK_PERM_ALL, BLK_PERM_ALL); + bs = bdrv_new_open_driver(&bdrv_test, "test-node", BDRV_O_RDWR, + &error_abort); + s = bs->opaque; + blk_insert_bs(blk, bs, &error_abort); + + /* Simple bdrv_drain_all_begin/end pair, check that CBs are called */ + g_assert_cmpint(s->drain_count, ==, 0); + bdrv_drain_all_begin(); + g_assert_cmpint(s->drain_count, ==, 1); + bdrv_drain_all_end(); + g_assert_cmpint(s->drain_count, ==, 0); + + /* Now do the same while a request is pending */ + aio_ret = -EINPROGRESS; + acb = blk_aio_preadv(blk, 0, &qiov, 0, aio_ret_cb, &aio_ret); + g_assert(acb != NULL); + g_assert_cmpint(aio_ret, ==, -EINPROGRESS); + + g_assert_cmpint(s->drain_count, ==, 0); + bdrv_drain_all_begin(); + g_assert_cmpint(aio_ret, ==, 0); + g_assert_cmpint(s->drain_count, ==, 1); + bdrv_drain_all_end(); + g_assert_cmpint(s->drain_count, ==, 0); + + bdrv_unref(bs); + blk_unref(blk); +} + +int main(int argc, char **argv) +{ + bdrv_init(); + qemu_init_main_loop(&error_abort); + + g_test_init(&argc, &argv, NULL); + + g_test_add_func("/bdrv-drain/driver-cb/drain_all", test_drv_cb_drain_all); + + return g_test_run(); +} diff --git a/tests/Makefile.include b/tests/Makefile.include index c002352134..c29db7616f 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -80,6 +80,7 @@ gcov-files-test-thread-pool-y = thread-pool.c gcov-files-test-hbitmap-y = util/hbitmap.c check-unit-y += tests/test-hbitmap$(EXESUF) gcov-files-test-hbitmap-y = blockjob.c +check-unit-y += tests/test-bdrv-drain$(EXESUF) check-unit-y += tests/test-blockjob$(EXESUF) check-unit-y += tests/test-blockjob-txn$(EXESUF) check-unit-y += tests/test-x86-cpuid$(EXESUF) @@ -592,6 +593,7 @@ tests/test-coroutine$(EXESUF): tests/test-coroutine.o $(test-block-obj-y) tests/test-aio$(EXESUF): tests/test-aio.o $(test-block-obj-y) tests/test-aio-multithread$(EXESUF): tests/test-aio-multithread.o $(test-block-obj-y) tests/test-throttle$(EXESUF): tests/test-throttle.o $(test-block-obj-y) +tests/test-bdrv-drain$(EXESUF): tests/test-bdrv-drain.o $(test-block-obj-y) $(test-util-obj-y) tests/test-blockjob$(EXESUF): tests/test-blockjob.o $(test-block-obj-y) $(test-util-obj-y) tests/test-blockjob-txn$(EXESUF): tests/test-blockjob-txn.o $(test-block-obj-y) $(test-util-obj-y) tests/test-thread-pool$(EXESUF): tests/test-thread-pool.o $(test-block-obj-y) From patchwork Wed Dec 6 10:53:07 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kevin Wolf X-Patchwork-Id: 845117 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=2001:4830:134:3::11; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3ysFsq2cXZz9s84 for ; Wed, 6 Dec 2017 21:56:30 +1100 (AEDT) Received: from localhost ([::1]:54869 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eMXNJ-0003zL-Un for incoming@patchwork.ozlabs.org; Wed, 06 Dec 2017 05:56:25 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50822) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eMXKU-0001sJ-9l for qemu-devel@nongnu.org; Wed, 06 Dec 2017 05:53:31 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eMXKR-0006df-J0 for qemu-devel@nongnu.org; Wed, 06 Dec 2017 05:53:30 -0500 Received: from mx1.redhat.com ([209.132.183.28]:35144) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1eMXKP-0006bJ-93; Wed, 06 Dec 2017 05:53:25 -0500 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 5773976527; Wed, 6 Dec 2017 10:53:24 +0000 (UTC) Received: from localhost.localdomain.com (unknown [10.36.117.0]) by smtp.corp.redhat.com (Postfix) with ESMTP id CE1AE600C0; Wed, 6 Dec 2017 10:53:22 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Wed, 6 Dec 2017 11:53:07 +0100 Message-Id: <20171206105309.3468-5-kwolf@redhat.com> In-Reply-To: <20171206105309.3468-1-kwolf@redhat.com> References: <20171206105309.3468-1-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Wed, 06 Dec 2017 10:53:24 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v2 4/6] block: bdrv_drain_recurse(): Remove unused begin parameter 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, pbonzini@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, qemu-stable@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" Now that the bdrv_drain_invoke() calls are pulled up to the callers of bdrv_drain_recurse(), the 'begin' parameter isn't needed any more. Signed-off-by: Kevin Wolf --- block/io.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/block/io.c b/block/io.c index 603f5b059e..390d463c71 100644 --- a/block/io.c +++ b/block/io.c @@ -195,7 +195,7 @@ static void bdrv_drain_invoke(BlockDriverState *bs, bool begin) } } -static bool bdrv_drain_recurse(BlockDriverState *bs, bool begin) +static bool bdrv_drain_recurse(BlockDriverState *bs) { BdrvChild *child, *tmp; bool waited; @@ -218,7 +218,7 @@ static bool bdrv_drain_recurse(BlockDriverState *bs, bool begin) */ bdrv_ref(bs); } - waited |= bdrv_drain_recurse(bs, begin); + waited |= bdrv_drain_recurse(bs); if (in_main_loop) { bdrv_unref(bs); } @@ -283,7 +283,7 @@ void bdrv_drained_begin(BlockDriverState *bs) } bdrv_drain_invoke(bs, true); - bdrv_drain_recurse(bs, true); + bdrv_drain_recurse(bs); } void bdrv_drained_end(BlockDriverState *bs) @@ -299,7 +299,7 @@ void bdrv_drained_end(BlockDriverState *bs) bdrv_parent_drained_end(bs); bdrv_drain_invoke(bs, false); - bdrv_drain_recurse(bs, false); + bdrv_drain_recurse(bs); aio_enable_external(bdrv_get_aio_context(bs)); } @@ -378,7 +378,7 @@ void bdrv_drain_all_begin(void) aio_context_acquire(aio_context); for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) { if (aio_context == bdrv_get_aio_context(bs)) { - waited |= bdrv_drain_recurse(bs, true); + waited |= bdrv_drain_recurse(bs); } } aio_context_release(aio_context); @@ -400,7 +400,7 @@ void bdrv_drain_all_end(void) aio_enable_external(aio_context); bdrv_parent_drained_end(bs); bdrv_drain_invoke(bs, false); - bdrv_drain_recurse(bs, false); + bdrv_drain_recurse(bs); aio_context_release(aio_context); } From patchwork Wed Dec 6 10:53:08 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kevin Wolf X-Patchwork-Id: 845125 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=2001:4830:134:3::11; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3ysFtm04X9z9s9Y for ; Wed, 6 Dec 2017 21:57:20 +1100 (AEDT) Received: from localhost ([::1]:54878 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eMXOA-0004gv-3C for incoming@patchwork.ozlabs.org; Wed, 06 Dec 2017 05:57:18 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50874) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eMXKW-0001uM-4J for qemu-devel@nongnu.org; Wed, 06 Dec 2017 05:53:33 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eMXKV-0006fw-EJ for qemu-devel@nongnu.org; Wed, 06 Dec 2017 05:53:32 -0500 Received: from mx1.redhat.com ([209.132.183.28]:44152) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1eMXKR-0006cc-23; Wed, 06 Dec 2017 05:53:27 -0500 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 235FB4E4C0; Wed, 6 Dec 2017 10:53:26 +0000 (UTC) Received: from localhost.localdomain.com (unknown [10.36.117.0]) by smtp.corp.redhat.com (Postfix) with ESMTP id A4EF4600C0; Wed, 6 Dec 2017 10:53:24 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Wed, 6 Dec 2017 11:53:08 +0100 Message-Id: <20171206105309.3468-6-kwolf@redhat.com> In-Reply-To: <20171206105309.3468-1-kwolf@redhat.com> References: <20171206105309.3468-1-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Wed, 06 Dec 2017 10:53:26 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v2 5/6] block: Don't wait for requests in bdrv_drain*_end() 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, pbonzini@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, qemu-stable@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" The device is drained, so there is no point in waiting for requests at the end of the drained section. Remove the bdrv_drain_recurse() calls there. The bdrv_drain_recurse() calls were introduced in commit 481cad48e5e in order to call the the .bdrv_co_drain_end() driver callback. This is now done by a separate bdrv_drain_invoke() call. Signed-off-by: Kevin Wolf Reviewed-by: Paolo Bonzini Reviewed-by: Paolo Bonzini --- block/io.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/block/io.c b/block/io.c index 390d463c71..5fdb92a15e 100644 --- a/block/io.c +++ b/block/io.c @@ -299,7 +299,6 @@ void bdrv_drained_end(BlockDriverState *bs) bdrv_parent_drained_end(bs); bdrv_drain_invoke(bs, false); - bdrv_drain_recurse(bs); aio_enable_external(bdrv_get_aio_context(bs)); } @@ -400,7 +399,6 @@ void bdrv_drain_all_end(void) aio_enable_external(aio_context); bdrv_parent_drained_end(bs); bdrv_drain_invoke(bs, false); - bdrv_drain_recurse(bs); aio_context_release(aio_context); } From patchwork Wed Dec 6 10:53:09 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kevin Wolf X-Patchwork-Id: 845128 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=2001:4830:134:3::11; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3ysFwh2x97z9s9Y for ; Wed, 6 Dec 2017 21:59:00 +1100 (AEDT) Received: from localhost ([::1]:54888 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eMXPm-0007U6-Ei for incoming@patchwork.ozlabs.org; Wed, 06 Dec 2017 05:58:58 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50882) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eMXKW-0001v8-Ht for qemu-devel@nongnu.org; Wed, 06 Dec 2017 05:53:33 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eMXKV-0006gH-JO for qemu-devel@nongnu.org; Wed, 06 Dec 2017 05:53:32 -0500 Received: from mx1.redhat.com ([209.132.183.28]:40748) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1eMXKT-0006eH-4u; Wed, 06 Dec 2017 05:53:29 -0500 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 2E3F4C04AC4F; Wed, 6 Dec 2017 10:53:28 +0000 (UTC) Received: from localhost.localdomain.com (unknown [10.36.117.0]) by smtp.corp.redhat.com (Postfix) with ESMTP id 72A5B600C0; Wed, 6 Dec 2017 10:53:26 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Wed, 6 Dec 2017 11:53:09 +0100 Message-Id: <20171206105309.3468-7-kwolf@redhat.com> In-Reply-To: <20171206105309.3468-1-kwolf@redhat.com> References: <20171206105309.3468-1-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Wed, 06 Dec 2017 10:53:28 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v2 6/6] block: Unify order in 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, pbonzini@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, qemu-stable@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" Drain requests are propagated to child nodes, parent nodes and directly to the AioContext. The order in which this happened was different between all combinations of drain/drain_all and begin/end. The correct order is to keep children only drained when their parents are also drained. This means that at the start of a drained section, the AioContext needs to be drained first, the parents second and only then the children. The correct order for the end of a drained section is the opposite. This patch changes the three other functions to follow the example of bdrv_drained_begin(), which is the only one that got it right. Signed-off-by: Kevin Wolf --- block/io.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/block/io.c b/block/io.c index 5fdb92a15e..1e92d2e5b2 100644 --- a/block/io.c +++ b/block/io.c @@ -277,6 +277,7 @@ void bdrv_drained_begin(BlockDriverState *bs) return; } + /* Stop things in parent-to-child order */ if (atomic_fetch_inc(&bs->quiesce_counter) == 0) { aio_disable_external(bdrv_get_aio_context(bs)); bdrv_parent_drained_begin(bs); @@ -297,8 +298,9 @@ void bdrv_drained_end(BlockDriverState *bs) return; } - bdrv_parent_drained_end(bs); + /* Re-enable things in child-to-parent order */ bdrv_drain_invoke(bs, false); + bdrv_parent_drained_end(bs); aio_enable_external(bdrv_get_aio_context(bs)); } @@ -351,9 +353,10 @@ void bdrv_drain_all_begin(void) for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) { AioContext *aio_context = bdrv_get_aio_context(bs); + /* Stop things in parent-to-child order */ aio_context_acquire(aio_context); - bdrv_parent_drained_begin(bs); aio_disable_external(aio_context); + bdrv_parent_drained_begin(bs); bdrv_drain_invoke(bs, true); aio_context_release(aio_context); @@ -395,10 +398,11 @@ void bdrv_drain_all_end(void) for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) { AioContext *aio_context = bdrv_get_aio_context(bs); + /* Re-enable things in child-to-parent order */ aio_context_acquire(aio_context); - aio_enable_external(aio_context); - bdrv_parent_drained_end(bs); bdrv_drain_invoke(bs, false); + bdrv_parent_drained_end(bs); + aio_enable_external(aio_context); aio_context_release(aio_context); }