From nobody Sun Oct 5 21:06:23 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 Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1499289538919434.53360847747024; Wed, 5 Jul 2017 14:18:58 -0700 (PDT) Received: from localhost ([::1]:48105 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dSrhJ-0004v6-FK for importer@patchew.org; Wed, 05 Jul 2017 17:18:57 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37960) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dSrXu-0004xG-CV for qemu-devel@nongnu.org; Wed, 05 Jul 2017 17:09:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dSrXs-0002i1-F6 for qemu-devel@nongnu.org; Wed, 05 Jul 2017 17:09:14 -0400 Received: from mx1.redhat.com ([209.132.183.28]:35012) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dSrXm-0002eI-1S; Wed, 05 Jul 2017 17:09:06 -0400 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 1AC5FC04D28D; Wed, 5 Jul 2017 21:09:05 +0000 (UTC) Received: from red.redhat.com (ovpn-120-36.rdu2.redhat.com [10.10.120.36]) by smtp.corp.redhat.com (Postfix) with ESMTP id EB172600CC; Wed, 5 Jul 2017 21:09:03 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 1AC5FC04D28D Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=eblake@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 1AC5FC04D28D From: Eric Blake To: qemu-devel@nongnu.org Date: Wed, 5 Jul 2017 16:08:29 -0500 Message-Id: <20170705210842.960-9-eblake@redhat.com> In-Reply-To: <20170705210842.960-1-eblake@redhat.com> References: <20170705210842.960-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Wed, 05 Jul 2017 21:09:05 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v4 08/21] mirror: Switch MirrorBlockJob to byte-based 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, jcody@redhat.com, jsnow@redhat.com, qemu-block@nongnu.org, Max Reitz 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" We are gradually converting to byte-based interfaces, as they are easier to reason about than sector-based. Continue by converting an internal structure (no semantic change), and all references to the buffer size. Add an assertion that our use of s->granularity >> BDRV_SECTOR_BITS (necessary for interaction with sector-based dirty bitmaps, until a later patch converts those to be byte-based) does not suffer from truncation problems. [checkpatch has a false positive on use of MIN() in this patch] Signed-off-by: Eric Blake Reviewed-by: John Snow Reviewed-by: Kevin Wolf --- v4: add assertion and formatting tweak [Kevin], R-b dropped v2-v3: no change --- block/mirror.c | 82 +++++++++++++++++++++++++++++-------------------------= ---- 1 file changed, 41 insertions(+), 41 deletions(-) diff --git a/block/mirror.c b/block/mirror.c index b4dfe95..9aca0cb 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -24,9 +24,8 @@ #define SLICE_TIME 100000000ULL /* ns */ #define MAX_IN_FLIGHT 16 -#define MAX_IO_SECTORS ((1 << 20) >> BDRV_SECTOR_BITS) /* 1 Mb */ -#define DEFAULT_MIRROR_BUF_SIZE \ - (MAX_IN_FLIGHT * MAX_IO_SECTORS * BDRV_SECTOR_SIZE) +#define MAX_IO_BYTES (1 << 20) /* 1 Mb */ +#define DEFAULT_MIRROR_BUF_SIZE (MAX_IN_FLIGHT * MAX_IO_BYTES) /* The mirroring buffer is a list of granularity-sized chunks. * Free chunks are organized in a list. @@ -67,11 +66,11 @@ typedef struct MirrorBlockJob { uint64_t last_pause_ns; unsigned long *in_flight_bitmap; int in_flight; - int64_t sectors_in_flight; + int64_t bytes_in_flight; int ret; bool unmap; bool waiting_for_io; - int target_cluster_sectors; + int target_cluster_size; int max_iov; bool initial_zeroing_ongoing; } MirrorBlockJob; @@ -79,8 +78,8 @@ typedef struct MirrorBlockJob { typedef struct MirrorOp { MirrorBlockJob *s; QEMUIOVector qiov; - int64_t sector_num; - int nb_sectors; + int64_t offset; + uint64_t bytes; } MirrorOp; static BlockErrorAction mirror_error_action(MirrorBlockJob *s, bool read, @@ -101,13 +100,12 @@ static void mirror_iteration_done(MirrorOp *op, int r= et) MirrorBlockJob *s =3D op->s; struct iovec *iov; int64_t chunk_num; - int i, nb_chunks, sectors_per_chunk; + int i, nb_chunks; - trace_mirror_iteration_done(s, op->sector_num * BDRV_SECTOR_SIZE, - op->nb_sectors * BDRV_SECTOR_SIZE, ret); + trace_mirror_iteration_done(s, op->offset, op->bytes, ret); s->in_flight--; - s->sectors_in_flight -=3D op->nb_sectors; + s->bytes_in_flight -=3D op->bytes; iov =3D op->qiov.iov; for (i =3D 0; i < op->qiov.niov; i++) { MirrorBuffer *buf =3D (MirrorBuffer *) iov[i].iov_base; @@ -115,16 +113,15 @@ static void mirror_iteration_done(MirrorOp *op, int r= et) s->buf_free_count++; } - sectors_per_chunk =3D s->granularity >> BDRV_SECTOR_BITS; - chunk_num =3D op->sector_num / sectors_per_chunk; - nb_chunks =3D DIV_ROUND_UP(op->nb_sectors, sectors_per_chunk); + chunk_num =3D op->offset / s->granularity; + nb_chunks =3D DIV_ROUND_UP(op->bytes, s->granularity); bitmap_clear(s->in_flight_bitmap, chunk_num, nb_chunks); if (ret >=3D 0) { if (s->cow_bitmap) { bitmap_set(s->cow_bitmap, chunk_num, nb_chunks); } if (!s->initial_zeroing_ongoing) { - s->common.offset +=3D (uint64_t)op->nb_sectors * BDRV_SECTOR_S= IZE; + s->common.offset +=3D op->bytes; } } qemu_iovec_destroy(&op->qiov); @@ -144,7 +141,8 @@ static void mirror_write_complete(void *opaque, int ret) if (ret < 0) { BlockErrorAction action; - bdrv_set_dirty_bitmap(s->dirty_bitmap, op->sector_num, op->nb_sect= ors); + bdrv_set_dirty_bitmap(s->dirty_bitmap, op->offset >> BDRV_SECTOR_B= ITS, + op->bytes >> BDRV_SECTOR_BITS); action =3D mirror_error_action(s, false, -ret); if (action =3D=3D BLOCK_ERROR_ACTION_REPORT && s->ret >=3D 0) { s->ret =3D ret; @@ -163,7 +161,8 @@ static void mirror_read_complete(void *opaque, int ret) if (ret < 0) { BlockErrorAction action; - bdrv_set_dirty_bitmap(s->dirty_bitmap, op->sector_num, op->nb_sect= ors); + bdrv_set_dirty_bitmap(s->dirty_bitmap, op->offset >> BDRV_SECTOR_B= ITS, + op->bytes >> BDRV_SECTOR_BITS); action =3D mirror_error_action(s, true, -ret); if (action =3D=3D BLOCK_ERROR_ACTION_REPORT && s->ret >=3D 0) { s->ret =3D ret; @@ -171,7 +170,7 @@ static void mirror_read_complete(void *opaque, int ret) mirror_iteration_done(op, ret); } else { - blk_aio_pwritev(s->target, op->sector_num * BDRV_SECTOR_SIZE, &op-= >qiov, + blk_aio_pwritev(s->target, op->offset, &op->qiov, 0, mirror_write_complete, op); } aio_context_release(blk_get_aio_context(s->common.blk)); @@ -211,7 +210,8 @@ static int mirror_cow_align(MirrorBlockJob *s, align_nb_sectors =3D max_sectors; if (need_cow) { align_nb_sectors =3D QEMU_ALIGN_DOWN(align_nb_sectors, - s->target_cluster_sectors); + s->target_cluster_size >> + BDRV_SECTOR_BITS); } } /* Clipping may result in align_nb_sectors unaligned to chunk boundary= , but @@ -277,8 +277,8 @@ static int mirror_do_read(MirrorBlockJob *s, int64_t se= ctor_num, /* Allocate a MirrorOp that is used as an AIO callback. */ op =3D g_new(MirrorOp, 1); op->s =3D s; - op->sector_num =3D sector_num; - op->nb_sectors =3D nb_sectors; + op->offset =3D sector_num * BDRV_SECTOR_SIZE; + op->bytes =3D nb_sectors * BDRV_SECTOR_SIZE; /* Now make a QEMUIOVector taking enough granularity-sized chunks * from s->buf_free. @@ -295,7 +295,7 @@ static int mirror_do_read(MirrorBlockJob *s, int64_t se= ctor_num, /* Copy the dirty cluster. */ s->in_flight++; - s->sectors_in_flight +=3D nb_sectors; + s->bytes_in_flight +=3D nb_sectors * BDRV_SECTOR_SIZE; trace_mirror_one_iteration(s, sector_num * BDRV_SECTOR_SIZE, nb_sectors * BDRV_SECTOR_SIZE); @@ -315,19 +315,17 @@ static void mirror_do_zero_or_discard(MirrorBlockJob = *s, * so the freeing in mirror_iteration_done is nop. */ op =3D g_new0(MirrorOp, 1); op->s =3D s; - op->sector_num =3D sector_num; - op->nb_sectors =3D nb_sectors; + op->offset =3D sector_num * BDRV_SECTOR_SIZE; + op->bytes =3D nb_sectors * BDRV_SECTOR_SIZE; s->in_flight++; - s->sectors_in_flight +=3D nb_sectors; + s->bytes_in_flight +=3D nb_sectors * BDRV_SECTOR_SIZE; if (is_discard) { blk_aio_pdiscard(s->target, sector_num << BDRV_SECTOR_BITS, - op->nb_sectors << BDRV_SECTOR_BITS, - mirror_write_complete, op); + op->bytes, mirror_write_complete, op); } else { blk_aio_pwrite_zeroes(s->target, sector_num * BDRV_SECTOR_SIZE, - op->nb_sectors * BDRV_SECTOR_SIZE, - s->unmap ? BDRV_REQ_MAY_UNMAP : 0, + op->bytes, s->unmap ? BDRV_REQ_MAY_UNMAP : 0, mirror_write_complete, op); } } @@ -342,8 +340,7 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlo= ckJob *s) int64_t end =3D s->bdev_length / BDRV_SECTOR_SIZE; int sectors_per_chunk =3D s->granularity >> BDRV_SECTOR_BITS; bool write_zeroes_ok =3D bdrv_can_write_zeroes_with_unmap(blk_bs(s->ta= rget)); - int max_io_sectors =3D MAX((s->buf_size >> BDRV_SECTOR_BITS) / MAX_IN_= FLIGHT, - MAX_IO_SECTORS); + int max_io_bytes =3D MAX(s->buf_size / MAX_IN_FLIGHT, MAX_IO_BYTES); bdrv_dirty_bitmap_lock(s->dirty_bitmap); sector_num =3D bdrv_dirty_iter_next(s->dbi); @@ -415,9 +412,10 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBl= ockJob *s) nb_chunks * sectors_per_chunk, &io_sectors, &file); if (ret < 0) { - io_sectors =3D MIN(nb_chunks * sectors_per_chunk, max_io_secto= rs); + io_sectors =3D MIN(nb_chunks * sectors_per_chunk, + max_io_bytes >> BDRV_SECTOR_BITS); } else if (ret & BDRV_BLOCK_DATA) { - io_sectors =3D MIN(io_sectors, max_io_sectors); + io_sectors =3D MIN(io_sectors, max_io_bytes >> BDRV_SECTOR_BIT= S); } io_sectors -=3D io_sectors % sectors_per_chunk; @@ -719,7 +717,6 @@ static void coroutine_fn mirror_run(void *opaque) char backing_filename[2]; /* we only need 2 characters because we are = only checking for a NULL string */ int ret =3D 0; - int target_cluster_size =3D BDRV_SECTOR_SIZE; if (block_job_is_cancelled(&s->common)) { goto immediate_exit; @@ -771,14 +768,15 @@ static void coroutine_fn mirror_run(void *opaque) bdrv_get_backing_filename(target_bs, backing_filename, sizeof(backing_filename)); if (!bdrv_get_info(target_bs, &bdi) && bdi.cluster_size) { - target_cluster_size =3D bdi.cluster_size; + s->target_cluster_size =3D bdi.cluster_size; + } else { + s->target_cluster_size =3D BDRV_SECTOR_SIZE; } if (backing_filename[0] && !target_bs->backing - && s->granularity < target_cluster_size) { - s->buf_size =3D MAX(s->buf_size, target_cluster_size); + && s->granularity < s->target_cluster_size) { + s->buf_size =3D MAX(s->buf_size, s->target_cluster_size); s->cow_bitmap =3D bitmap_new(length); } - s->target_cluster_sectors =3D target_cluster_size >> BDRV_SECTOR_BITS; s->max_iov =3D MIN(bs->bl.max_iov, target_bs->bl.max_iov); s->buf =3D qemu_try_blockalign(bs, s->buf_size); @@ -814,10 +812,10 @@ static void coroutine_fn mirror_run(void *opaque) cnt =3D bdrv_get_dirty_count(s->dirty_bitmap); /* s->common.offset contains the number of bytes already processed= so * far, cnt is the number of dirty sectors remaining and - * s->sectors_in_flight is the number of sectors currently being + * s->bytes_in_flight is the number of bytes currently being * processed; together those are the current total operation lengt= h */ - s->common.len =3D s->common.offset + - (cnt + s->sectors_in_flight) * BDRV_SECTOR_SIZE; + s->common.len =3D s->common.offset + s->bytes_in_flight + + cnt * BDRV_SECTOR_SIZE; /* Note that even when no rate limit is applied we need to yield * periodically with no pending I/O so that bdrv_drain_all() retur= ns. @@ -1150,6 +1148,8 @@ static void mirror_start_job(const char *job_id, Bloc= kDriverState *bs, } assert ((granularity & (granularity - 1)) =3D=3D 0); + /* Granularity must be large enough for sector-based dirty bitmap */ + assert(granularity >=3D BDRV_SECTOR_SIZE); if (buf_size < 0) { error_setg(errp, "Invalid parameter 'buf-size'"); --=20 2.9.4