From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36361) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aGAnD-00005j-Kk for qemu-devel@nongnu.org; Mon, 04 Jan 2016 14:27:48 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aGAnC-0000ZP-Kn for qemu-devel@nongnu.org; Mon, 04 Jan 2016 14:27:47 -0500 References: <1450926914-12509-1-git-send-email-famz@redhat.com> <1450926914-12509-2-git-send-email-famz@redhat.com> From: Max Reitz Message-ID: <568AC7A9.4080708@redhat.com> Date: Mon, 4 Jan 2016 20:27:37 +0100 MIME-Version: 1.0 In-Reply-To: <1450926914-12509-2-git-send-email-famz@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="UFAwCSObVhCi6a0hfgA8XfkfqfhlSIhpd" Subject: Re: [Qemu-devel] [PATCH v8 1/2] mirror: Rewrite mirror_iteration List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng , qemu-devel@nongnu.org Cc: Kevin Wolf , pbonzini@redhat.com, Jeff Cody , qemu-block@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --UFAwCSObVhCi6a0hfgA8XfkfqfhlSIhpd Content-Type: text/plain; charset=iso-8859-15 Content-Transfer-Encoding: quoted-printable On 24.12.2015 04:15, Fam Zheng wrote: > The "pnum < nb_sectors" condition in deciding whether to actually copy > data is unnecessarily strict, and the qiov initialization is > unnecessarily for bdrv_aio_write_zeroes and bdrv_aio_discard. >=20 > Rewrite mirror_iteration to fix both flaws. >=20 > Signed-off-by: Fam Zheng > --- > block/mirror.c | 344 +++++++++++++++++++++++++++++++++++--------------= -------- > trace-events | 1 - > 2 files changed, 213 insertions(+), 132 deletions(-) >=20 > diff --git a/block/mirror.c b/block/mirror.c > index f201f2b..0081c2e 100644 > --- a/block/mirror.c > +++ b/block/mirror.c > @@ -46,7 +46,6 @@ typedef struct MirrorBlockJob { > BlockdevOnError on_source_error, on_target_error; > bool synced; > bool should_complete; > - int64_t sector_num; > int64_t granularity; > size_t buf_size; > int64_t bdev_length; > @@ -63,6 +62,8 @@ typedef struct MirrorBlockJob { > int ret; > bool unmap; > bool waiting_for_io; > + int target_cluster_sectors; > + int max_iov; > } MirrorBlockJob; > =20 > typedef struct MirrorOp { > @@ -158,115 +159,90 @@ static void mirror_read_complete(void *opaque, i= nt ret) > mirror_write_complete, op); > } > =20 > -static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s) > +/* Round sector_num and/or nb_sectors to target cluster if COW is need= ed, and > + * return the offset of the adjusted tail sector against original. */ > +static int mirror_cow_align(MirrorBlockJob *s, > + int64_t *sector_num, > + int *nb_sectors) > +{ > + bool head_need_cow, tail_need_cow; > + int diff =3D 0; > + int chunk_sectors =3D s->granularity >> BDRV_SECTOR_BITS; > + > + head_need_cow =3D !test_bit(*sector_num / chunk_sectors, s->cow_bi= tmap); > + tail_need_cow =3D !test_bit((*sector_num + *nb_sectors - 1) / chun= k_sectors, > + s->cow_bitmap); > + if (head_need_cow || tail_need_cow) { > + int64_t align_sector_num; > + int align_nb_sectors; > + bdrv_round_to_clusters(s->target, *sector_num, *nb_sectors, > + &align_sector_num, &align_nb_sectors); > + if (tail_need_cow) { > + diff =3D align_sector_num + align_nb_sectors > + - (*sector_num + *nb_sectors); > + assert(diff >=3D 0); > + *nb_sectors +=3D diff; > + } > + if (head_need_cow) { > + int d =3D *sector_num - align_sector_num; > + assert(d >=3D 0); > + *sector_num =3D align_sector_num; > + *nb_sectors +=3D d; > + } > + } > + > + /* If the resulting chunks are more than max_iov, we have to shrin= k it > + * under the alignment restriction. */ > + if (*nb_sectors / chunk_sectors > s->max_iov) { > + int shrink =3D *nb_sectors / chunk_sectors - s->max_iov; Isn't this missing a "shrink *=3D chunk_sectors"? Because after this line= , shrink's unit seems to be chunks, but the following code seems to presume= it > + if (tail_need_cow) { > + shrink -=3D shrink % s->target_cluster_sectors; > + } > + diff -=3D shrink; > + *nb_sectors -=3D shrink; > + } Max (The rest looks fine.) --UFAwCSObVhCi6a0hfgA8XfkfqfhlSIhpd Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAEBCAAGBQJWisepAAoJEDuxQgLoOKytiUYIAIOj5sy7p4zoWe8vryaf3ufX sAjahhFbKu0bfzzfyuiB370bXlbi1HhC/5w7mCk6a6qmPBbPiYAU0L5lxQCd/MwI kRLMAXOLJkplQo24SMJNbZLQlRjcgqzTtrlD5Zwe2bMwxrUbWHIHnygYk9l6p217 U2aUVHRS4Ju4RJcrnNFUWSEl5mcpf2+2Fs4/9lrCXXtqCOnubbI39sNMMnt/BLw2 T4ZJ5bAXuqRq9TQKnBakOwZIseOjh+lthZ055WhhgG0TW8wK9d0rEcPBswTsUvyj 8P9h52qABBfMZcuqpwq4k+c9yCwm1BF+URKLh10eZ6p6r9FiG77j87G/hpa7szU= =WJpq -----END PGP SIGNATURE----- --UFAwCSObVhCi6a0hfgA8XfkfqfhlSIhpd--