From nobody Wed Apr 16 03:21:50 2025 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 1531238556055718.8866972765115; Tue, 10 Jul 2018 09:02:36 -0700 (PDT) Received: from localhost ([::1]:48567 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fcv5z-0004Xr-Bu for importer@patchew.org; Tue, 10 Jul 2018 12:02:31 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44189) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fcunT-00051N-RE for qemu-devel@nongnu.org; Tue, 10 Jul 2018 11:43:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fcunS-0001aN-Oy for qemu-devel@nongnu.org; Tue, 10 Jul 2018 11:43:23 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:38806 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 1fcunN-0001Wn-Pv; Tue, 10 Jul 2018 11:43:17 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 53D3185629; Tue, 10 Jul 2018 15:43:17 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-116-156.ams2.redhat.com [10.36.116.156]) by smtp.corp.redhat.com (Postfix) with ESMTP id 80F612026D6B; Tue, 10 Jul 2018 15:43:16 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Tue, 10 Jul 2018 17:42:41 +0200 Message-Id: <20180710154304.18304-2-kwolf@redhat.com> In-Reply-To: <20180710154304.18304-1-kwolf@redhat.com> References: <20180710154304.18304-1-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.2]); Tue, 10 Jul 2018 15:43:17 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.2]); Tue, 10 Jul 2018 15:43:17 +0000 (UTC) for IP:'10.11.54.4' DOMAIN:'int-mx04.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] [PULL 01/24] block: Poll after drain on attaching a node 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, peter.maydell@linaro.org, qemu-devel@nongnu.org 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 dcf94a23b1 ('block: Don't poll in parent drain callbacks') removed polling in bdrv_child_cb_drained_begin() on the grounds that the original bdrv_drain() already will poll and BdrvChildRole.drained_begin calls must not cause graph changes (and therefore must not call aio_poll() or the recursion through the graph will break. This reasoning is correct for calls through bdrv_do_drained_begin(). However, BdrvChildRole.drained_begin is also called when a node that is already in a drained section (i.e. bdrv_do_drained_begin() has already returned and therefore can't poll any more) is attached to a new parent. In this case, we must explicitly poll to have all requests completed before the drained new child can be attached to the parent. In bdrv_replace_child_noperm(), we know that we're not inside the recursion of bdrv_do_drained_begin() because graph changes are not allowed there, and bdrv_replace_child_noperm() is a graph change. The call of BdrvChildRole.drained_begin() must therefore be followed by a BDRV_POLL_WHILE() that waits for the completion of requests. Reported-by: Max Reitz Signed-off-by: Kevin Wolf --- include/block/block.h | 8 ++++++++ include/block/block_int.h | 3 +++ block.c | 2 +- block/io.c | 26 ++++++++++++++++++++------ 4 files changed, 32 insertions(+), 7 deletions(-) diff --git a/include/block/block.h b/include/block/block.h index bc76b1e59f..706ef009ad 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -569,6 +569,14 @@ void bdrv_parent_drained_begin(BlockDriverState *bs, B= drvChild *ignore, bool ignore_bds_parents); =20 /** + * bdrv_parent_drained_begin_single: + * + * Begin a quiesced section for the parent of @c. If @poll is true, wait f= or + * any pending activity to cease. + */ +void bdrv_parent_drained_begin_single(BdrvChild *c, bool poll); + +/** * bdrv_parent_drained_end: * * End a quiesced section of all users of @bs. This is part of diff --git a/include/block/block_int.h b/include/block/block_int.h index af71b414be..81cd3db7a9 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -606,6 +606,9 @@ struct BdrvChildRole { * requests after returning from .drained_begin() until .drained_end()= is * called. * + * These functions must not change the graph (and therefore also must = not + * call aio_poll(), which could change the graph indirectly). + * * Note that this can be nested. If drained_begin() was called twice, = new * I/O is allowed only after drained_end() was called twice, too. */ diff --git a/block.c b/block.c index ac8b3a3511..a2fe05ea96 100644 --- a/block.c +++ b/block.c @@ -2066,7 +2066,7 @@ static void bdrv_replace_child_noperm(BdrvChild *chil= d, } assert(num >=3D 0); for (i =3D 0; i < num; i++) { - child->role->drained_begin(child); + bdrv_parent_drained_begin_single(child, true); } } =20 diff --git a/block/io.c b/block/io.c index 1a2272fad3..038449f81f 100644 --- a/block/io.c +++ b/block/io.c @@ -52,9 +52,7 @@ void bdrv_parent_drained_begin(BlockDriverState *bs, Bdrv= Child *ignore, if (c =3D=3D ignore || (ignore_bds_parents && c->role->parent_is_b= ds)) { continue; } - if (c->role->drained_begin) { - c->role->drained_begin(c); - } + bdrv_parent_drained_begin_single(c, false); } } =20 @@ -73,6 +71,14 @@ void bdrv_parent_drained_end(BlockDriverState *bs, BdrvC= hild *ignore, } } =20 +static bool bdrv_parent_drained_poll_single(BdrvChild *c) +{ + if (c->role->drained_poll) { + return c->role->drained_poll(c); + } + return false; +} + static bool bdrv_parent_drained_poll(BlockDriverState *bs, BdrvChild *igno= re, bool ignore_bds_parents) { @@ -83,14 +89,22 @@ static bool bdrv_parent_drained_poll(BlockDriverState *= bs, BdrvChild *ignore, if (c =3D=3D ignore || (ignore_bds_parents && c->role->parent_is_b= ds)) { continue; } - if (c->role->drained_poll) { - busy |=3D c->role->drained_poll(c); - } + busy |=3D bdrv_parent_drained_poll_single(c); } =20 return busy; } =20 +void bdrv_parent_drained_begin_single(BdrvChild *c, bool poll) +{ + if (c->role->drained_begin) { + c->role->drained_begin(c); + } + if (poll) { + BDRV_POLL_WHILE(c->bs, bdrv_parent_drained_poll_single(c)); + } +} + static void bdrv_merge_limits(BlockLimits *dst, const BlockLimits *src) { dst->opt_transfer =3D MAX(dst->opt_transfer, src->opt_transfer); --=20 2.13.6