From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37079) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YflbH-0000Gz-HC for qemu-devel@nongnu.org; Wed, 08 Apr 2015 04:44:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YflbG-0001t0-Be for qemu-devel@nongnu.org; Wed, 08 Apr 2015 04:44:43 -0400 Date: Wed, 8 Apr 2015 09:44:33 +0100 From: Stefan Hajnoczi Message-ID: <20150408084433.GA28835@stefanha-thinkpad.redhat.com> References: <1426879023-18151-1-git-send-email-jsnow@redhat.com> <1426879023-18151-16-git-send-email-jsnow@redhat.com> <20150402133724.GJ25244@stefanha-thinkpad.redhat.com> <551D6707.20501@redhat.com> <20150407125742.GA21559@stefanha-thinkpad.redhat.com> <552409AA.1040101@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="cWoXeonUoKmBZSoM" Content-Disposition: inline In-Reply-To: <552409AA.1040101@redhat.com> Subject: Re: [Qemu-devel] [Qemu-block] [PATCH v4 15/20] block: Resize bitmaps on bdrv_truncate List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: John Snow Cc: famz@redhat.com, qemu-block@nongnu.org, Stefan Hajnoczi , qemu-devel@nongnu.org, armbru@redhat.com, vsementsov@parallels.com, mreitz@redhat.com --cWoXeonUoKmBZSoM Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Apr 07, 2015 at 12:45:30PM -0400, John Snow wrote: >=20 >=20 > On 04/07/2015 08:57 AM, Stefan Hajnoczi wrote: > >On Thu, Apr 02, 2015 at 11:57:59AM -0400, John Snow wrote: > >> > >> > >>On 04/02/2015 09:37 AM, Stefan Hajnoczi wrote: > >>>On Fri, Mar 20, 2015 at 03:16:58PM -0400, John Snow wrote: > >>>>+void hbitmap_truncate(HBitmap *hb, uint64_t size) > >>>>+{ > >>>>+ bool shrink; > >>>>+ unsigned i; > >>>>+ uint64_t num_elements =3D size; > >>>>+ uint64_t old; > >>>>+ > >>>>+ /* Size comes in as logical elements, adjust for granularity. */ > >>>>+ size =3D (size + (1ULL << hb->granularity) - 1) >> hb->granulari= ty; > >>>>+ assert(size <=3D ((uint64_t)1 << HBITMAP_LOG_MAX_SIZE)); > >>>>+ shrink =3D size < hb->size; > >>>>+ > >>>>+ /* bit sizes are identical; nothing to do. */ > >>>>+ if (size =3D=3D hb->size) { > >>>>+ return; > >>>>+ } > >>>>+ > >>>>+ /* If we're losing bits, let's clear those bits before we invali= date all of > >>>>+ * our invariants. This helps keep the bitcount consistent, and = will prevent > >>>>+ * us from carrying around garbage bits beyond the end of the ma= p. > >>>>+ * > >>>>+ * Because clearing bits past the end of map might reset bits we= care about > >>>>+ * within the array, record the current value of the last bit we= 're keeping. > >>>>+ */ > >>>>+ if (shrink) { > >>>>+ bool set =3D hbitmap_get(hb, num_elements - 1); > >>>>+ uint64_t fix_count =3D (hb->size << hb->granularity) - num_e= lements; > >>>>+ > >>>>+ assert(fix_count); > >>>>+ hbitmap_reset(hb, num_elements, fix_count); > >>>>+ if (set) { > >>>>+ hbitmap_set(hb, num_elements - 1, 1); > >>>>+ } > >>> > >>>Why is it necessary to set the last bit (if it was set)? The comment > >>>isn't clear to me. > >>> > >> > >>Sure. The granularity of the bitmap provides us with virtual bit groups= =2E for > >>a granularity of say g=3D2, we have 2^2 virtual bits per every real bit: > >> > >>101 in memory is treated, virtually, as 1111 0000 1111. > >> > >>The get/set calls operate on virtual bits, not concrete ones, so if we = were > >>to reset virtual bits 2-11: > >>11|11 0000 1111 > >> > >>We'd set the real bits to '000', because we clear or set the entire vir= tual > >>group. > >> > >>This is probably not what we really want, so as a shortcut I just read = and > >>then re-set the final bit. > >> > >>It is programmatically avoidable (Are we truncating into a granularity > >>group?) but in the case that we are, I'd need to read/reset the bit any= way, > >>so it seemed fine to just unconditionally apply the fix. > > > >I see. This is equivalent to: > > > >uint64_t start =3D QEMU_ALIGN_UP(num_elements, hb->granularity); >=20 > Probably you mean QEMU_ALIGN_UP(num_elements, 1 << hb->granularity) Yes, you are right. Stefan --cWoXeonUoKmBZSoM Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJVJOpxAAoJEJykq7OBq3PIgGcIAK0D4j8LKfOLQe6LM55fr1FG 4ebFkpUbJtzOqsk6Nmi5qDobvMh40HUkD1wGM/yTlVGoRL+pjx2TyHtnGHChEDpe SjCpQT21X7Nx1VSOG4ZZpFWKNA4kmr94p/G5ufTAwFKpZscuP8jZF0R/S4eTL8vv sibaV3A6ovTeTnIrDlFF3r9lhgwwL6nGhKOtkqfIlCE06SFHhARphNte8rMwEBrT pUyQjxeqT4Gbu1m+5KKKW8psnfW6q1nFviJqQY+MCT14mfOvqNCqRclwXi7M8pyT 7sUsjuX2CxDshFaZNrAtUpEEU6J8pHdBTghQWNPqkNatIIPQazLGhr1JtwzhDNc= =R24p -----END PGP SIGNATURE----- --cWoXeonUoKmBZSoM--