From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48510) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bqKiI-0001cR-03 for qemu-devel@nongnu.org; Sat, 01 Oct 2016 09:52:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bqKiF-0006xw-Ub for qemu-devel@nongnu.org; Sat, 01 Oct 2016 09:52:25 -0400 References: <1475232808-4852-1-git-send-email-vsementsov@virtuozzo.com> <1475232808-4852-2-git-send-email-vsementsov@virtuozzo.com> From: Max Reitz Message-ID: <213db4b2-4abe-9a50-7205-f6d96d11f4ae@redhat.com> Date: Sat, 1 Oct 2016 15:52:12 +0200 MIME-Version: 1.0 In-Reply-To: <1475232808-4852-2-git-send-email-vsementsov@virtuozzo.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="pcG2Bi6rGUbv8gJ8mfMfwPJAU0GFpOLqj" Subject: Re: [Qemu-devel] [PATCH 01/22] hbitmap: improve dirty iter List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladimir Sementsov-Ogievskiy , qemu-block@nongnu.org, qemu-devel@nongnu.org Cc: kwolf@redhat.com, armbru@redhat.com, eblake@redhat.com, jsnow@redhat.com, famz@redhat.com, den@openvz.org, stefanha@redhat.com, pbonzini@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --pcG2Bi6rGUbv8gJ8mfMfwPJAU0GFpOLqj From: Max Reitz To: Vladimir Sementsov-Ogievskiy , qemu-block@nongnu.org, qemu-devel@nongnu.org Cc: kwolf@redhat.com, armbru@redhat.com, eblake@redhat.com, jsnow@redhat.com, famz@redhat.com, den@openvz.org, stefanha@redhat.com, pbonzini@redhat.com Message-ID: <213db4b2-4abe-9a50-7205-f6d96d11f4ae@redhat.com> Subject: Re: [PATCH 01/22] hbitmap: improve dirty iter References: <1475232808-4852-1-git-send-email-vsementsov@virtuozzo.com> <1475232808-4852-2-git-send-email-vsementsov@virtuozzo.com> In-Reply-To: <1475232808-4852-2-git-send-email-vsementsov@virtuozzo.com> Content-Type: text/plain; charset=iso-8859-15 Content-Transfer-Encoding: quoted-printable On 30.09.2016 12:53, Vladimir Sementsov-Ogievskiy wrote: > Make dirty iter resistant to resetting bits in corresponding HBitmap. >=20 > Signed-off-by: Vladimir Sementsov-Ogievskiy > --- > include/qemu/hbitmap.h | 24 ++---------------------- > util/hbitmap.c | 23 ++++++++++++++++++++++- > 2 files changed, 24 insertions(+), 23 deletions(-) >=20 > diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h > index eb46475..9aa86c1 100644 > --- a/include/qemu/hbitmap.h > +++ b/include/qemu/hbitmap.h > @@ -243,10 +243,7 @@ void hbitmap_free(HBitmap *hb); > * the lowest-numbered bit that is set in @hb, starting at @first. > * > * Concurrent setting of bits is acceptable, and will at worst cause t= he > - * iteration to miss some of those bits. Resetting bits before the cu= rrent > - * position of the iterator is also okay. However, concurrent resetti= ng of > - * bits can lead to unexpected behavior if the iterator has not yet re= ached > - * those bits. > + * iteration to miss some of those bits. Concurrent bits resetting is = ok too. I'd prefer: Concurrent resetting of bits is OK, too. Or: Concurrently resetting bits is OK, too. Or: Resetting bits concurrently is OK, too. With or without that change: Reviewed-by: Max Reitz > */ > void hbitmap_iter_init(HBitmapIter *hbi, const HBitmap *hb, uint64_t f= irst); > =20 > @@ -285,24 +282,7 @@ void hbitmap_free_meta(HBitmap *hb); > * Return the next bit that is set in @hbi's associated HBitmap, > * or -1 if all remaining bits are zero. > */ > -static inline int64_t hbitmap_iter_next(HBitmapIter *hbi) > -{ > - unsigned long cur =3D hbi->cur[HBITMAP_LEVELS - 1]; > - int64_t item; > - > - if (cur =3D=3D 0) { > - cur =3D hbitmap_iter_skip_words(hbi); > - if (cur =3D=3D 0) { > - return -1; > - } > - } > - > - /* The next call will resume work from the next bit. */ > - hbi->cur[HBITMAP_LEVELS - 1] =3D cur & (cur - 1); > - item =3D ((uint64_t)hbi->pos << BITS_PER_LEVEL) + ctzl(cur); > - > - return item << hbi->granularity; > -} > +int64_t hbitmap_iter_next(HBitmapIter *hbi); > =20 > /** > * hbitmap_iter_next_word: > diff --git a/util/hbitmap.c b/util/hbitmap.c > index 6a13c12..4f5cf96 100644 > --- a/util/hbitmap.c > +++ b/util/hbitmap.c > @@ -106,8 +106,9 @@ unsigned long hbitmap_iter_skip_words(HBitmapIter *= hbi) > =20 > unsigned long cur; > do { > - cur =3D hbi->cur[--i]; > + i--; > pos >>=3D BITS_PER_LEVEL; > + cur =3D hbi->cur[i] & hb->levels[i][pos]; > } while (cur =3D=3D 0); > =20 > /* Check for end of iteration. We always use fewer than BITS_PER_= LONG > @@ -139,6 +140,26 @@ unsigned long hbitmap_iter_skip_words(HBitmapIter = *hbi) > return cur; > } > =20 > +int64_t hbitmap_iter_next(HBitmapIter *hbi) > +{ > + unsigned long cur =3D hbi->cur[HBITMAP_LEVELS - 1] & > + hbi->hb->levels[HBITMAP_LEVELS - 1][hbi->pos]; > + int64_t item; > + > + if (cur =3D=3D 0) { > + cur =3D hbitmap_iter_skip_words(hbi); > + if (cur =3D=3D 0) { > + return -1; > + } > + } > + > + /* The next call will resume work from the next bit. */ > + hbi->cur[HBITMAP_LEVELS - 1] =3D cur & (cur - 1); > + item =3D ((uint64_t)hbi->pos << BITS_PER_LEVEL) + ctzl(cur); > + > + return item << hbi->granularity; > +} > + > void hbitmap_iter_init(HBitmapIter *hbi, const HBitmap *hb, uint64_t f= irst) > { > unsigned i, bit; >=20 --pcG2Bi6rGUbv8gJ8mfMfwPJAU0GFpOLqj Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- iQEvBAEBCAAZBQJX77+MEhxtcmVpdHpAcmVkaGF0LmNvbQAKCRD0B9sAYdXPQI5M B/4kuO1Z3B5wIIDmk2Jp4VCBHEdBXev//amVS5/OJ9d3LinOaNJVOkr7Uv7tpsnP ehCWsagaUVtxk1uI2+91HdNGn1L5xR8YYOoojujqKyV86G30pg5bsHrB0S0Tw++q FFH6Oqds9ZHtEPcM2VPOKBwgITAK+DwnHjoRe+CCuzOlBUaC2TiadAP6KNJvzEVq 8RMtILndKk+Rz3I8Qx4qcPhYVvIJYIISwnU7U6UQq8HpA1kXw6vTKJqTQ8MsN9NZ lp1zUDyMj9tFBDXHNU77oEFnyCL3QJfPEbX2o4JFcFvcRcyymxl02O0TUAZLW/3C cFp7tyiUdxPfaqWB1AiolsbG =15pG -----END PGP SIGNATURE----- --pcG2Bi6rGUbv8gJ8mfMfwPJAU0GFpOLqj--