From patchwork Wed Apr 11 16:39:25 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kevin Wolf X-Patchwork-Id: 10335861 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 29E4E6053B for ; Wed, 11 Apr 2018 16:42:17 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 17FE21FF13 for ; Wed, 11 Apr 2018 16:42:17 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 0CA6822701; Wed, 11 Apr 2018 16:42:17 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00, MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 9EEDB1FF13 for ; Wed, 11 Apr 2018 16:42:16 +0000 (UTC) Received: from localhost ([::1]:39623 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1f6Ip5-0004QL-Rs for patchwork-qemu-devel@patchwork.kernel.org; Wed, 11 Apr 2018 12:42:15 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41087) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1f6In9-0002hC-Vg for qemu-devel@nongnu.org; Wed, 11 Apr 2018 12:40:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1f6In6-0004V5-Vs for qemu-devel@nongnu.org; Wed, 11 Apr 2018 12:40:15 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:33568 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 1f6Imw-0004Ji-U9; Wed, 11 Apr 2018 12:40:03 -0400 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 79FC04023113; Wed, 11 Apr 2018 16:40:02 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-117-181.ams2.redhat.com [10.36.117.181]) by smtp.corp.redhat.com (Postfix) with ESMTP id 3FD4CBDC4D; Wed, 11 Apr 2018 16:40:01 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Wed, 11 Apr 2018 18:39:25 +0200 Message-Id: <20180411163940.2523-5-kwolf@redhat.com> In-Reply-To: <20180411163940.2523-1-kwolf@redhat.com> References: <20180411163940.2523-1-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.11.54.5 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.6]); Wed, 11 Apr 2018 16:40:02 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.6]); Wed, 11 Apr 2018 16:40:02 +0000 (UTC) for IP:'10.11.54.5' DOMAIN:'int-mx05.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 04/19] 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+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP All involved nodes are already idle, we called bdrv_do_draine_begin() on them. The comment in the code suggested that this were 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 d2bd89c3bb..ea6f9f023a 100644 --- a/block/io.c +++ b/block/io.c @@ -376,6 +376,16 @@ void bdrv_drain(BlockDriverState *bs) bdrv_drained_end(bs); } +static void bdrv_drain_assert_idle(BlockDriverState *bs) +{ + BdrvChild *child, *next; + + assert(atomic_read(&bs->in_flight) == 0); + QLIST_FOREACH_SAFE(child, &bs->children, next, next) { + bdrv_drain_assert_idle(child->bs); + } +} + /* * Wait for pending requests to complete across all BlockDriverStates * @@ -390,11 +400,8 @@ void bdrv_drain(BlockDriverState *bs) */ void bdrv_drain_all_begin(void) { - /* Always run first iteration so any pending completion BHs run */ - bool waited = true; BlockDriverState *bs; BdrvNextIterator it; - GSList *aio_ctxs = NULL, *ctx; /* BDRV_POLL_WHILE() for a node can only be called from its own I/O thread * or the main loop AioContext. We potentially use BDRV_POLL_WHILE() on @@ -408,35 +415,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 = g_slist_prepend(aio_ctxs, aio_context); - } } - /* Note that completion of an asynchronous I/O operation can trigger any - * 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 = false; - - for (ctx = aio_ctxs; ctx != NULL; ctx = ctx->next) { - AioContext *aio_context = ctx->data; - - 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); - } - } - aio_context_release(aio_context); - } + for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) { + bdrv_drain_assert_idle(bs); } - - g_slist_free(aio_ctxs); } void bdrv_drain_all_end(void)