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 <[email protected]> > --- > 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 > > >
