From patchwork Wed Mar 16 14:16:50 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Bonzini X-Patchwork-Id: 8601031 Return-Path: X-Original-To: patchwork-qemu-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 1CB139F6E1 for ; Wed, 16 Mar 2016 14:21:24 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 4210F202F2 for ; Wed, 16 Mar 2016 14:21:23 +0000 (UTC) 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.kernel.org (Postfix) with ESMTPS id 339DD202AE for ; Wed, 16 Mar 2016 14:21:22 +0000 (UTC) Received: from localhost ([::1]:56630 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1agCK9-0001r1-Hu for patchwork-qemu-devel@patchwork.kernel.org; Wed, 16 Mar 2016 10:21:21 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42122) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1agCGH-00039c-BS for qemu-devel@nongnu.org; Wed, 16 Mar 2016 10:17:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1agCGC-000648-SH for qemu-devel@nongnu.org; Wed, 16 Mar 2016 10:17:21 -0400 Received: from mx1.redhat.com ([209.132.183.28]:35754) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1agCGC-00063m-I8 for qemu-devel@nongnu.org; Wed, 16 Mar 2016 10:17:16 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (Postfix) with ESMTPS id 4A21E7F09C for ; Wed, 16 Mar 2016 14:17:16 +0000 (UTC) Received: from donizetti.redhat.com (ovpn-112-54.ams2.redhat.com [10.36.112.54]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u2GEGwc2001673; Wed, 16 Mar 2016 10:17:14 -0400 From: Paolo Bonzini To: qemu-devel@nongnu.org Date: Wed, 16 Mar 2016 15:16:50 +0100 Message-Id: <1458137817-15383-10-git-send-email-pbonzini@redhat.com> In-Reply-To: <1458137817-15383-1-git-send-email-pbonzini@redhat.com> References: <1458137817-15383-1-git-send-email-pbonzini@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: famz@redhat.com, stefanha@redhat.com Subject: [Qemu-devel] [PATCH v2 09/16] block: wait for all pending I/O when doing synchronous requests X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Synchronous I/O should in general happen either in the main thread (e.g. for bdrv_open and bdrv_create) or between bdrv_drained_begin and bdrv_drained_end. Therefore, the simplest way to wait for it to finish is to wait for _all_ pending I/O to complete. In fact, there was one case in bdrv_close where we explicitly drained after bdrv_flush; this is now unnecessary. And we should probably have called bdrv_drain_all after calls to bdrv_flush_all, which is now unnecessary too. This decouples synchronous I/O from aio_poll. When the request used not to be tracked as part of bdrv_drain (e.g. bdrv_co_get_block_status) we need to update the in_flight count. Signed-off-by: Paolo Bonzini Reviewed-by: Fam Zheng --- v1->v2: remove unnecessary qed.c hunk [Fam] block.c | 1 - block/io.c | 39 ++++++++++++--------------------------- block/qed-table.c | 16 ++++------------ tests/qemu-iotests/060 | 8 ++++++-- tests/qemu-iotests/060.out | 4 +++- 5 files changed, 25 insertions(+), 43 deletions(-) diff --git a/block.c b/block.c index bbd7de6..47f2367 100644 --- a/block.c +++ b/block.c @@ -2142,7 +2142,6 @@ static void bdrv_close(BlockDriverState *bs) bdrv_drained_begin(bs); /* complete I/O */ bdrv_flush(bs); - bdrv_drain(bs); /* in case flush left pending I/O */ bdrv_release_named_dirty_bitmaps(bs); assert(QLIST_EMPTY(&bs->dirty_bitmaps)); diff --git a/block/io.c b/block/io.c index db975ab..0a99131 100644 --- a/block/io.c +++ b/block/io.c @@ -583,13 +583,9 @@ static int bdrv_prwv_co(BlockDriverState *bs, int64_t offset, /* Fast-path if already in coroutine context */ bdrv_rw_co_entry(&rwco); } else { - AioContext *aio_context = bdrv_get_aio_context(bs); - co = qemu_coroutine_create(bdrv_rw_co_entry); qemu_coroutine_enter(co, &rwco); - while (rwco.ret == NOT_DONE) { - aio_poll(aio_context, true); - } + bdrv_drain(bs); } bdrv_no_throttling_end(bs); @@ -1535,17 +1531,19 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs, } *file = NULL; + bdrv_inc_in_flight(bs); ret = bs->drv->bdrv_co_get_block_status(bs, sector_num, nb_sectors, pnum, file); if (ret < 0) { *pnum = 0; - return ret; + goto out; } if (ret & BDRV_BLOCK_RAW) { assert(ret & BDRV_BLOCK_OFFSET_VALID); - return bdrv_get_block_status(bs->file->bs, ret >> BDRV_SECTOR_BITS, - *pnum, pnum, file); + ret = bdrv_get_block_status(bs->file->bs, ret >> BDRV_SECTOR_BITS, + *pnum, pnum, file); + goto out; } if (ret & (BDRV_BLOCK_DATA | BDRV_BLOCK_ZERO)) { @@ -1587,6 +1585,8 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs, } } +out: + bdrv_dec_in_flight(bs); return ret; } @@ -1652,13 +1652,9 @@ int64_t bdrv_get_block_status_above(BlockDriverState *bs, /* Fast-path if already in coroutine context */ bdrv_get_block_status_above_co_entry(&data); } else { - AioContext *aio_context = bdrv_get_aio_context(bs); - co = qemu_coroutine_create(bdrv_get_block_status_above_co_entry); qemu_coroutine_enter(co, &data); - while (!data.done) { - aio_poll(aio_context, true); - } + bdrv_drain(bs); } return data.ret; } @@ -2459,13 +2455,9 @@ int bdrv_flush(BlockDriverState *bs) /* Fast-path if already in coroutine context */ bdrv_flush_co_entry(&rwco); } else { - AioContext *aio_context = bdrv_get_aio_context(bs); - co = qemu_coroutine_create(bdrv_flush_co_entry); qemu_coroutine_enter(co, &rwco); - while (rwco.ret == NOT_DONE) { - aio_poll(aio_context, true); - } + bdrv_drain(bs); } return rwco.ret; @@ -2582,13 +2574,9 @@ int bdrv_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors) /* Fast-path if already in coroutine context */ bdrv_discard_co_entry(&rwco); } else { - AioContext *aio_context = bdrv_get_aio_context(bs); - co = qemu_coroutine_create(bdrv_discard_co_entry); qemu_coroutine_enter(co, &rwco); - while (rwco.ret == NOT_DONE) { - aio_poll(aio_context, true); - } + bdrv_drain(bs); } return rwco.ret; @@ -2663,11 +2651,8 @@ int bdrv_ioctl(BlockDriverState *bs, unsigned long int req, void *buf) bdrv_co_ioctl_entry(&data); } else { Coroutine *co = qemu_coroutine_create(bdrv_co_ioctl_entry); - qemu_coroutine_enter(co, &data); - while (data.ret == -EINPROGRESS) { - aio_poll(bdrv_get_aio_context(bs), true); - } + bdrv_drain(bs); } return data.ret; } diff --git a/block/qed-table.c b/block/qed-table.c index 802945f..3a8566f 100644 --- a/block/qed-table.c +++ b/block/qed-table.c @@ -173,9 +173,7 @@ int qed_read_l1_table_sync(BDRVQEDState *s) qed_read_table(s, s->header.l1_table_offset, s->l1_table, qed_sync_cb, &ret); - while (ret == -EINPROGRESS) { - aio_poll(bdrv_get_aio_context(s->bs), true); - } + bdrv_drain(s->bs); return ret; } @@ -194,9 +192,7 @@ int qed_write_l1_table_sync(BDRVQEDState *s, unsigned int index, int ret = -EINPROGRESS; qed_write_l1_table(s, index, n, qed_sync_cb, &ret); - while (ret == -EINPROGRESS) { - aio_poll(bdrv_get_aio_context(s->bs), true); - } + bdrv_drain(s->bs); return ret; } @@ -267,9 +263,7 @@ int qed_read_l2_table_sync(BDRVQEDState *s, QEDRequest *request, uint64_t offset int ret = -EINPROGRESS; qed_read_l2_table(s, request, offset, qed_sync_cb, &ret); - while (ret == -EINPROGRESS) { - aio_poll(bdrv_get_aio_context(s->bs), true); - } + bdrv_drain(s->bs); return ret; } @@ -289,9 +283,7 @@ int qed_write_l2_table_sync(BDRVQEDState *s, QEDRequest *request, int ret = -EINPROGRESS; qed_write_l2_table(s, request, index, n, flush, qed_sync_cb, &ret); - while (ret == -EINPROGRESS) { - aio_poll(bdrv_get_aio_context(s->bs), true); - } + bdrv_drain(s->bs); return ret; } diff --git a/tests/qemu-iotests/060 b/tests/qemu-iotests/060 index c81319c..dffe35a 100755 --- a/tests/qemu-iotests/060 +++ b/tests/qemu-iotests/060 @@ -164,8 +164,12 @@ echo "open -o file.driver=blkdebug $TEST_IMG break cow_read 0 aio_write 0k 1k wait_break 0 -write 64k 64k -resume 0" | $QEMU_IO | _filter_qemu_io +break pwritev_done 1 +aio_write 64k 64k +wait_break 1 +resume 1 +resume 0 +aio_flush" | $QEMU_IO | _filter_qemu_io echo echo "=== Testing unallocated image header ===" diff --git a/tests/qemu-iotests/060.out b/tests/qemu-iotests/060.out index 5d40206..a0a9a11 100644 --- a/tests/qemu-iotests/060.out +++ b/tests/qemu-iotests/060.out @@ -105,7 +105,9 @@ discard 65536/65536 bytes at offset 0 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) qcow2: Marking image as corrupt: Preventing invalid write on metadata (overlaps with active L2 table); further corruption events will be suppressed blkdebug: Suspended request '0' -write failed: Input/output error +blkdebug: Suspended request '1' +blkdebug: Resuming request '1' +aio_write failed: Input/output error blkdebug: Resuming request '0' aio_write failed: No medium found