From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53682) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1adV0D-0008Nb-J9 for qemu-devel@nongnu.org; Tue, 08 Mar 2016 22:41:38 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1adV0C-0005JD-IC for qemu-devel@nongnu.org; Tue, 08 Mar 2016 22:41:37 -0500 Date: Wed, 9 Mar 2016 11:41:27 +0800 From: Fam Zheng Message-ID: <20160309034126.GF17947@ad.usersys.redhat.com> References: <1455645388-32401-1-git-send-email-pbonzini@redhat.com> <1455645388-32401-8-git-send-email-pbonzini@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1455645388-32401-8-git-send-email-pbonzini@redhat.com> Subject: Re: [Qemu-devel] [PATCH 07/16] block: change drain to look only at one child at a time List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: stefanha@redhat.com, qemu-devel@nongnu.org, qemu-block@nongnu.org On Tue, 02/16 18:56, Paolo Bonzini wrote: > bdrv_requests_pending is checking children to also wait until internal > requests (such as metadata writes) have completed. However, checking > children is in general overkill because, apart from this special case, > the parent's in_flight count will always be incremented by at least one > for every request in the child. While the patch looks good, I'm not sure I understand this: aren't children's requests generated by the child itself, how is parent's in_flight incremented? > > Since internal requests are only generated by the parent in the child, > instead visit the tree parent first, and then wait for internal I/O in > the children to complete. > > Signed-off-by: Paolo Bonzini > --- > block/io.c | 27 +++++++++++++++++++-------- > 1 file changed, 19 insertions(+), 8 deletions(-) > > diff --git a/block/io.c b/block/io.c > index a9a23a6..e0c9215 100644 > --- a/block/io.c > +++ b/block/io.c > @@ -249,6 +249,23 @@ static void bdrv_drain_recurse(BlockDriverState *bs) > } > } > > +static bool bdrv_drain_io_recurse(BlockDriverState *bs) > +{ > + BdrvChild *child; > + bool waited = false; > + > + while (atomic_read(&bs->in_flight) > 0) { > + aio_poll(bdrv_get_aio_context(bs), true); > + waited = true; > + } > + > + QLIST_FOREACH(child, &bs->children, next) { > + waited |= bdrv_drain_io_recurse(child->bs); > + } > + > + return waited; > +} > + > /* > * Wait for pending requests to complete on a single BlockDriverState subtree, > * and suspend block driver's internal I/O until next request arrives. > @@ -265,10 +282,7 @@ void bdrv_drain(BlockDriverState *bs) > bdrv_no_throttling_begin(bs); > bdrv_io_unplugged_begin(bs); > bdrv_drain_recurse(bs); > - while (bdrv_requests_pending(bs)) { > - /* Keep iterating */ > - aio_poll(bdrv_get_aio_context(bs), true); > - } > + bdrv_drain_io_recurse(bs); > bdrv_io_unplugged_end(bs); > bdrv_no_throttling_end(bs); > } > @@ -319,10 +333,7 @@ void bdrv_drain_all(void) > aio_context_acquire(aio_context); > while ((bs = bdrv_next(bs))) { > if (aio_context == bdrv_get_aio_context(bs)) { > - if (bdrv_requests_pending(bs)) { > - aio_poll(aio_context, true); > - waited = true; > - } > + waited |= bdrv_drain_io_recurse(bs); > } > } > aio_context_release(aio_context); > -- > 2.5.0 > > >