From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44056) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZSRDj-0006yZ-Pl for qemu-devel@nongnu.org; Thu, 20 Aug 2015 10:53:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZSRDf-0001FW-J7 for qemu-devel@nongnu.org; Thu, 20 Aug 2015 10:53:35 -0400 Received: from mx1.redhat.com ([209.132.183.28]:35449) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZSRDf-0001FM-6d for qemu-devel@nongnu.org; Thu, 20 Aug 2015 10:53:31 -0400 Date: Thu, 20 Aug 2015 07:53:26 -0700 From: Stefan Hajnoczi Message-ID: <20150820145326.GC21642@stefanha-thinkpad> References: <1438939964-12584-1-git-send-email-vsementsov@virtuozzo.com> <1438939964-12584-2-git-send-email-vsementsov@virtuozzo.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="ZmUaFz6apKcXQszQ" Content-Disposition: inline In-Reply-To: <1438939964-12584-2-git-send-email-vsementsov@virtuozzo.com> Subject: Re: [Qemu-devel] [PATCH 01/12] hbitmap: serialization List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladimir Sementsov-Ogievskiy Cc: kwolf@redhat.com, peter.maydell@linaro.org, quintela@redhat.com, qemu-devel@nongnu.org, dgilbert@redhat.com, vsementsov@parallels.com, den@openvz.org, amit.shah@redhat.com, pbonzini@redhat.com, jsnow@redhat.com --ZmUaFz6apKcXQszQ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Aug 07, 2015 at 12:32:33PM +0300, Vladimir Sementsov-Ogievskiy wrot= e: > +/** > + * hbitmap_serialize_part > + * @hb: HBitmap to oprate on. s/oprate/operate/ > + * @buf: Buffer to store serialized bitmap. > + * @start: First bit to store. > + * @count: Number of bits to store. > + * > + * Stores HBitmap data corresponding to given region. The format of save= d data > + * is linear sequence of bits, so it can be used by hbitmap_deserialize_= part > + * independently of endianness and size of HBitmap level array elements These functions *are* dependent of HBitmap level array element size. They always assign full array elements (unsigned long). If count < BITS_PER_LONG at some point before the end, the bitmap will be corrupt because leading bits will be zeroed when the next hbitmap_deserialize_part() call is made! > + */ > +void hbitmap_serialize_part(const HBitmap *hb, uint8_t *buf, > + uint64_t start, uint64_t count); > + > +/** > + * hbitmap_deserialize_part > + * @hb: HBitmap to operate on. > + * @buf: Buffer to restore bitmap data from. > + * @start: First bit to restore. > + * @count: Number of bits to restore. > + * > + * Retores HBitmap data corresponding to given region. The format is the= same s/Retores/Restores/ > + * as for hbitmap_serialize_part. > + * > + * ! The bitmap becomes inconsistent after this operation. > + * hbitmap_serialize_finish should be called before using the bitmap aft= er > + * data restoring. > + */ > +void hbitmap_deserialize_part(HBitmap *hb, uint8_t *buf, > + uint64_t start, uint64_t count); > + > +/** > + * hbitmap_deserialize_zeroes > + * @hb: HBitmap to operate on. > + * @start: First bit to restore. > + * @count: Number of bits to restore. > + * > + * Same as hbitmap_serialize_part, but fills the bitmap with zeroes. > + */ > +void hbitmap_deserialize_zeroes(HBitmap *hb, uint64_t start, uint64_t co= unt); > + > +/** > + * hbitmap_deserialize_finish > + * @hb: HBitmap to operate on. > + * > + * Repair HBitmap after calling hbitmap_deserialize_data. Actually, all = HBitmap > + * layers are restored here. > + */ > +void hbitmap_deserialize_finish(HBitmap *hb); > + > +/** > * hbitmap_free: > * @hb: HBitmap to operate on. > * > diff --git a/util/hbitmap.c b/util/hbitmap.c > index 50b888f..c7c21fe 100644 > --- a/util/hbitmap.c > +++ b/util/hbitmap.c > @@ -378,6 +378,104 @@ bool hbitmap_get(const HBitmap *hb, uint64_t item) > return (hb->levels[HBITMAP_LEVELS - 1][pos >> BITS_PER_LEVEL] & bit)= !=3D 0; > } > =20 > +uint64_t hbitmap_data_size(const HBitmap *hb, uint64_t count) > +{ > + uint64_t size, gran; > + > + if (count =3D=3D 0) { > + return 0; > + } > + > + gran =3D 1ll << hb->granularity; > + size =3D (((gran + count - 2) >> hb->granularity) >> BITS_PER_LEVEL)= + 1; > + > + return size * sizeof(unsigned long); > +} > + > +void hbitmap_serialize_part(const HBitmap *hb, uint8_t *buf, > + uint64_t start, uint64_t count) > +{ > + uint64_t i; > + uint64_t last =3D start + count - 1; > + unsigned long *out =3D (unsigned long *)buf; I'm not sure if we care but this can lead to unaligned stores if buf isn't aligned to sizeof(unsigned long). Unaligned stores are best avoided: https://www.linux-mips.org/wiki/Alignment If you replace out[i - start] =3D ... with a memcpy() call then the alignment problem is solved. If you are worried that all these memcpy() calls are slow, check the compiler output since gcc probably optimizes away the memcpy(). > + > + if (count =3D=3D 0) { > + return; > + } > + > + start =3D (start >> hb->granularity) >> BITS_PER_LEVEL; > + last =3D (last >> hb->granularity) >> BITS_PER_LEVEL; > + count =3D last - start + 1; > + > + for (i =3D start; i <=3D last; ++i) { > + unsigned long el =3D hb->levels[HBITMAP_LEVELS - 1][i]; > + out[i - start] =3D > + (BITS_PER_LONG =3D=3D 32 ? cpu_to_le32(el) : cpu_to_le64(el)= ); > + } > +} > + > +void hbitmap_deserialize_part(HBitmap *hb, uint8_t *buf, > + uint64_t start, uint64_t count) > +{ > + uint64_t i; > + uint64_t last =3D start + count - 1; > + unsigned long *in =3D (unsigned long *)buf; Same here. --ZmUaFz6apKcXQszQ Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJV1enmAAoJEJykq7OBq3PIJgsH/R9JQzmzZd9Dnk7R4srQ2t3Y FoEycgZOD4yx+100m7ckepJso4ouySjUzIp/zvOquH91+VqVI8KIq+wnfkLlNdpC wFcz12GAeJki0AiVhiP0ZT2TnXlf6zztqXhD8o9S9bel77HsS6vYkvfbxeS4EW07 kycd7Aj4tf77Aow26NFoOh2lqPDhniAZMQR1IvHUTVG2FXoR1Ry5XwN72gf9Uqji 9McGtoqrWwCOluRm9pkVvBm+LyAKJ9iaaKiVD6+QSG3NpvrT4QLls0pkVtrK1143 WgEK+ezBzQ4uzUjhjuX/WnWblkAiiAhykzDckaaf5LaLCZV12XQcVPC6eul1fNs= =ZCc0 -----END PGP SIGNATURE----- --ZmUaFz6apKcXQszQ--