From patchwork Wed Dec 12 13:46:28 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Bonzini X-Patchwork-Id: 205569 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id E61CB2C009A for ; Thu, 13 Dec 2012 02:51:02 +1100 (EST) Received: from localhost ([::1]:50552 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Timfc-0000w1-20 for incoming@patchwork.ozlabs.org; Wed, 12 Dec 2012 08:48:20 -0500 Received: from eggs.gnu.org ([208.118.235.92]:59044) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Timeu-0007s5-16 for qemu-devel@nongnu.org; Wed, 12 Dec 2012 08:47:41 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Timem-0003n5-Tk for qemu-devel@nongnu.org; Wed, 12 Dec 2012 08:47:35 -0500 Received: from mail-ie0-f173.google.com ([209.85.223.173]:35767) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Timem-0003mv-NM for qemu-devel@nongnu.org; Wed, 12 Dec 2012 08:47:28 -0500 Received: by mail-ie0-f173.google.com with SMTP id e13so1769044iej.4 for ; Wed, 12 Dec 2012 05:47:28 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:from:to:cc:subject:date:message-id:x-mailer:in-reply-to :references; bh=ThzbqUZ76vbs9xrdTmZlpjYoePZ1OsgmqztJFu6j1ng=; b=lSjMQFaXOX+fosJR4MxLdlMopmfNB4yB+g+sqIyfddpSRiCvEG80UUUjhxO7QSNnby m3CX3cCMnXIEcEcsuY/QnLPED0C8EsOgJRp6ENPiPOLMd38NcViooDl3MDiG8Pr16Xub 7x3hUak7bxT+op58pjSGfsHlChzGhN8sjbF53L9Z4Ra5EdFFLT47VDHj9YuP4sdDx+ZL WHlcXb55gq5aGR8Bg/KC8FTxjtT6DXI6DIk8rA9uazbSXiLgGK9Ee7R8ESxmANFb4ueG xNdF+XGjD4WFIJB/02XPvwBPoTikV/ZUOEo0YibjexPhCgVm+11JN3x78v5vNJIc8WgC nh0w== Received: by 10.42.46.141 with SMTP id k13mr757033icf.46.1355320048223; Wed, 12 Dec 2012 05:47:28 -0800 (PST) Received: from yakj.usersys.redhat.com (93-34-219-150.ip51.fastwebnet.it. [93.34.219.150]) by mx.google.com with ESMTPS id fv6sm1786818igc.17.2012.12.12.05.47.24 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 12 Dec 2012 05:47:26 -0800 (PST) From: Paolo Bonzini To: qemu-devel@nongnu.org Date: Wed, 12 Dec 2012 14:46:28 +0100 Message-Id: <1355319999-30627-10-git-send-email-pbonzini@redhat.com> X-Mailer: git-send-email 1.8.0.1 In-Reply-To: <1355319999-30627-1-git-send-email-pbonzini@redhat.com> References: <1355319999-30627-1-git-send-email-pbonzini@redhat.com> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x [fuzzy] X-Received-From: 209.85.223.173 Cc: kwolf@redhat.com, jcody@redhat.com, stefanha@redhat.com Subject: [Qemu-devel] [PATCH 09/20] mirror: switch mirror_iteration to AIO 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+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org There is really no change in the behavior of the job here, since there is still a maximum of one in-flight I/O operation between the source and the target. However, this patch already introduces the AIO callbacks (which are unmodified in the next patch) and some of the logic to count in-flight operations and only complete the job when there is none. Signed-off-by: Paolo Bonzini Reviewed-by: Eric Blake --- block/mirror.c | 155 +++++++++++++++++++++++++++++++++++++++++++-------------- trace-events | 2 + 2 files changed, 119 insertions(+), 38 deletions(-) diff --git a/block/mirror.c b/block/mirror.c index 93240eb..f02ffb3 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -33,8 +33,19 @@ typedef struct MirrorBlockJob { unsigned long *cow_bitmap; HBitmapIter hbi; uint8_t *buf; + + int in_flight; + int ret; } MirrorBlockJob; +typedef struct MirrorOp { + MirrorBlockJob *s; + QEMUIOVector qiov; + struct iovec iov; + int64_t sector_num; + int nb_sectors; +} MirrorOp; + static BlockErrorAction mirror_error_action(MirrorBlockJob *s, bool read, int error) { @@ -48,15 +59,60 @@ static BlockErrorAction mirror_error_action(MirrorBlockJob *s, bool read, } } -static int coroutine_fn mirror_iteration(MirrorBlockJob *s, - BlockErrorAction *p_action) +static void mirror_iteration_done(MirrorOp *op) +{ + MirrorBlockJob *s = op->s; + + s->in_flight--; + trace_mirror_iteration_done(s, op->sector_num, op->nb_sectors); + g_slice_free(MirrorOp, op); + qemu_coroutine_enter(s->common.co, NULL); +} + +static void mirror_write_complete(void *opaque, int ret) +{ + MirrorOp *op = opaque; + MirrorBlockJob *s = op->s; + if (ret < 0) { + BlockDriverState *source = s->common.bs; + BlockErrorAction action; + + bdrv_set_dirty(source, op->sector_num, op->nb_sectors); + action = mirror_error_action(s, false, -ret); + if (action == BDRV_ACTION_REPORT && s->ret >= 0) { + s->ret = ret; + } + } + mirror_iteration_done(op); +} + +static void mirror_read_complete(void *opaque, int ret) +{ + MirrorOp *op = opaque; + MirrorBlockJob *s = op->s; + if (ret < 0) { + BlockDriverState *source = s->common.bs; + BlockErrorAction action; + + bdrv_set_dirty(source, op->sector_num, op->nb_sectors); + action = mirror_error_action(s, true, -ret); + if (action == BDRV_ACTION_REPORT && s->ret >= 0) { + s->ret = ret; + } + + mirror_iteration_done(op); + return; + } + bdrv_aio_writev(s->target, op->sector_num, &op->qiov, op->nb_sectors, + mirror_write_complete, op); +} + +static void coroutine_fn mirror_iteration(MirrorBlockJob *s) { BlockDriverState *source = s->common.bs; - BlockDriverState *target = s->target; - QEMUIOVector qiov; - int ret, nb_sectors, nb_sectors_chunk; + int nb_sectors, nb_sectors_chunk; int64_t end, sector_num, cluster_num; - struct iovec iov; + MirrorOp *op; s->sector_num = hbitmap_iter_next(&s->hbi); if (s->sector_num < 0) { @@ -87,31 +143,30 @@ static int coroutine_fn mirror_iteration(MirrorBlockJob *s, end = s->common.len >> BDRV_SECTOR_BITS; nb_sectors = MIN(nb_sectors, end - sector_num); + + /* Allocate a MirrorOp that is used as an AIO callback. */ + op = g_slice_new(MirrorOp); + op->s = s; + op->iov.iov_base = s->buf; + op->iov.iov_len = nb_sectors * 512; + op->sector_num = sector_num; + op->nb_sectors = nb_sectors; + qemu_iovec_init_external(&op->qiov, &op->iov, 1); + bdrv_reset_dirty(source, sector_num, nb_sectors); /* Copy the dirty cluster. */ - iov.iov_base = s->buf; - iov.iov_len = nb_sectors * 512; - qemu_iovec_init_external(&qiov, &iov, 1); - + s->in_flight++; trace_mirror_one_iteration(s, sector_num, nb_sectors); - ret = bdrv_co_readv(source, sector_num, nb_sectors, &qiov); - if (ret < 0) { - *p_action = mirror_error_action(s, true, -ret); - goto fail; - } - ret = bdrv_co_writev(target, sector_num, nb_sectors, &qiov); - if (ret < 0) { - *p_action = mirror_error_action(s, false, -ret); - s->synced = false; - goto fail; - } - return 0; + bdrv_aio_readv(source, sector_num, &op->qiov, nb_sectors, + mirror_read_complete, op); +} -fail: - /* Try again later. */ - bdrv_set_dirty(source, sector_num, nb_sectors); - return ret; +static void mirror_drain(MirrorBlockJob *s) +{ + while (s->in_flight > 0) { + qemu_coroutine_yield(); + } } static void coroutine_fn mirror_run(void *opaque) @@ -119,6 +174,7 @@ static void coroutine_fn mirror_run(void *opaque) MirrorBlockJob *s = opaque; BlockDriverState *bs = s->common.bs; int64_t sector_num, end, nb_sectors_chunk, length; + uint64_t last_pause_ns; BlockDriverInfo bdi; char backing_filename[1024]; int ret = 0; @@ -177,28 +233,43 @@ static void coroutine_fn mirror_run(void *opaque) } bdrv_dirty_iter_init(bs, &s->hbi); + last_pause_ns = qemu_get_clock_ns(rt_clock); for (;;) { uint64_t delay_ns; int64_t cnt; bool should_complete; + if (s->ret < 0) { + ret = s->ret; + break; + } + cnt = bdrv_get_dirty_count(bs); - if (cnt != 0) { - BlockErrorAction action = BDRV_ACTION_REPORT; - ret = mirror_iteration(s, &action); - if (ret < 0 && action == BDRV_ACTION_REPORT) { - goto immediate_exit; + + /* Note that even when no rate limit is applied we need to yield + * periodically with no pending I/O so that qemu_aio_flush() returns. + * We do so every SLICE_TIME milliseconds, or when there is an error, + * or when the source is clean, whichever comes first. + */ + if (qemu_get_clock_ns(rt_clock) - last_pause_ns < SLICE_TIME && + s->common.iostatus == BLOCK_DEVICE_IO_STATUS_OK) { + if (s->in_flight > 0) { + trace_mirror_yield(s, s->in_flight, cnt); + qemu_coroutine_yield(); + continue; + } else if (cnt != 0) { + mirror_iteration(s); + continue; } - cnt = bdrv_get_dirty_count(bs); } should_complete = false; - if (cnt == 0) { + if (s->in_flight == 0 && cnt == 0) { trace_mirror_before_flush(s); ret = bdrv_flush(s->target); if (ret < 0) { if (mirror_error_action(s, false, -ret) == BDRV_ACTION_REPORT) { - goto immediate_exit; + break; } } else { /* We're out of the streaming phase. From now on, if the job @@ -244,15 +315,12 @@ static void coroutine_fn mirror_run(void *opaque) delay_ns = 0; } - /* Note that even when no rate limit is applied we need to yield - * with no pending I/O here so that qemu_aio_flush() returns. - */ block_job_sleep_ns(&s->common, rt_clock, delay_ns); if (block_job_is_cancelled(&s->common)) { break; } } else if (!should_complete) { - delay_ns = (cnt == 0 ? SLICE_TIME : 0); + delay_ns = (s->in_flight == 0 && cnt == 0 ? SLICE_TIME : 0); block_job_sleep_ns(&s->common, rt_clock, delay_ns); } else if (cnt == 0) { /* The two disks are in sync. Exit and report successful @@ -262,9 +330,20 @@ static void coroutine_fn mirror_run(void *opaque) s->common.cancelled = false; break; } + last_pause_ns = qemu_get_clock_ns(rt_clock); } immediate_exit: + if (s->in_flight > 0) { + /* We get here only if something went wrong. Either the job failed, + * or it was cancelled prematurely so that we do not guarantee that + * the target is a copy of the source. + */ + assert(ret < 0 || (!s->synced && block_job_is_cancelled(&s->common))); + mirror_drain(s); + } + + assert(s->in_flight == 0); g_free(s->buf); g_free(s->cow_bitmap); bdrv_set_dirty_tracking(bs, 0); diff --git a/trace-events b/trace-events index b318b4e..ca50812 100644 --- a/trace-events +++ b/trace-events @@ -85,6 +85,8 @@ mirror_before_drain(void *s, int64_t cnt) "s %p dirty count %"PRId64 mirror_before_sleep(void *s, int64_t cnt, int synced) "s %p dirty count %"PRId64" synced %d" mirror_one_iteration(void *s, int64_t sector_num, int nb_sectors) "s %p sector_num %"PRId64" nb_sectors %d" mirror_cow(void *s, int64_t sector_num) "s %p sector_num %"PRId64 +mirror_iteration_done(void *s, int64_t sector_num, int nb_sectors) "s %p sector_num %"PRId64" nb_sectors %d" +mirror_yield(void *s, int64_t cnt, int in_flight) "s %p dirty count %"PRId64" in_flight %d" # blockdev.c qmp_block_job_cancel(void *job) "job %p"