From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:53832) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SUixr-0005Ox-JB for qemu-devel@nongnu.org; Wed, 16 May 2012 14:28:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SUixo-0007dr-TC for qemu-devel@nongnu.org; Wed, 16 May 2012 14:28:47 -0400 Received: from mx1.redhat.com ([209.132.183.28]:13876) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SUixo-0007dP-Lc for qemu-devel@nongnu.org; Wed, 16 May 2012 14:28:44 -0400 Message-ID: <4FB3D9A7.2060704@redhat.com> Date: Wed, 16 May 2012 10:45:27 -0600 From: Eric Blake MIME-Version: 1.0 References: <1337169582-28312-1-git-send-email-owasserm@redhat.com> <1337169582-28312-9-git-send-email-owasserm@redhat.com> In-Reply-To: <1337169582-28312-9-git-send-email-owasserm@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="------------enig5D96AE672F81A8518833ACA1" Subject: Re: [Qemu-devel] [PATCH v10 8/9] Add set_cachesize command List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Orit Wasserman Cc: aliguori@us.ibm.com, quintela@redhat.com, stefanha@gmail.com, mdroth@linux.vnet.ibm.com, qemu-devel@nongnu.org, blauwirbel@gmail.com, Petter Svard , Benoit Hudzia , avi@redhat.com, pbonzini@redhat.com, Aidan Shribman This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig5D96AE672F81A8518833ACA1 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 05/16/2012 05:59 AM, Orit Wasserman wrote: > Change XBZRLE cache size in bytes (the size should be a power of 2). > If XBZRLE cache size is too small there will be many cache miss. >=20 > Signed-off-by: Benoit Hudzia > Signed-off-by: Petter Svard > Signed-off-by: Aidan Shribman > Signed-off-by: Orit Wasserman > =20 > + > +void xbzrle_cache_resize(int64_t order) > +{ > + cache_resize(XBZRLE.cache, pow(2, order)); '1 << order' is much more efficient than a call to pow(). > +void qmp_migrate_set_cachesize(int64_t value, Error **errp) > + > + /* power of 2 */ > + if (value !=3D 1 && !is_power_of_2(value)) { > + error_set(errp, QERR_INVALID_PARAMETER_VALUE, "cache size", > + "needs to be power of 2"); We already have QERR_PROPERTY_VALUE_NOT_POWER_OF_2, why aren't you using that here? > + return; > + } > + > + s->xbzrle_cache_size =3D value; > + xbzrle_cache_resize(log2(value)); log2() is rather expensive, ffs() from is more efficient at converting a single bit into the appropriate order. > ## > +# @migrate_set_cachesize > +# > +# Set XBZRLE cache size > +# > +# @value: cache size in bytes > +# > +# Returns: nothing on success Document the error for a non-power-of-2 or for overflow. Document whether this command is safe for an ongoing migration, or whether it must be called in advance of a migration. > +# > +# Since: 1.1 1.2. > +static inline bool is_power_of_2(int64_t value) > +{ > + return !(value & (value - 1)); > +} This says '0' is a power of 2, which is not true. Either fix the logic to exclude 0, or fix the function name to state that you are really checking that at most one bit is set. Also, if value is 0x8000000000000000, you are triggering unspecified behavior per C99. Is it worth using uint64_t for defined behavior, or do you need to take precautions regarding negative values? > +SQMP > +migrate_set_cachesize > +--------------------- > + > +Set cache size to be used by XBZRLE migration > + > +Arguments: > + > +- "value": cache size in bytes (json-int) Would it be any easier to take 'order' (log2 of the size) instead of the actual cache size? That is, instead of calling "value":1048576, I would rather type "value":20. > + > +Example: > + > +-> { "execute": "migrate_set_cachesize", "arguments": { "value": 512 }= } Isn't 512 bytes rather small? And given my argument about taking order rather than bytes as being easier to use, don't you really mean 512 megabytes (order 29) rather than 512 bytes (order 9)? --=20 Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --------------enig5D96AE672F81A8518833ACA1 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iQEcBAEBCAAGBQJPs9mnAAoJEKeha0olJ0NqNHMH/jFbpkI0cBKRDsBmw07NB1/q lm7ZMASyPFqrzReikN+efhBjMTcIWZ/zk1nNW16SPh4jSvLUdSkD5Qx+aDgF+I1u iYZLSetDIlJ2z49Jc3L9CZEZ01OSEjvoN4J5CWMHqX6Oa8XHxB0QBCe2iVGbHmOS FxxAQSI3SMp2/h9NkB7s1Z/NpWqty2BV48Dtf1ILTsCXrktPq9Q3YodaMfS29/wr SIf/9EG3YaqOaJaw1LPPEEDMvPCLM0Q0+8x5MCx6EJBf+Xf49KePqiQPxX86q8CB Aczgq0Vgo+bsv1+FOewvLBhATQLD2T+9AG+Wa5NtifFsF20BCqrDDfVsVyxzjWw= =yGfh -----END PGP SIGNATURE----- --------------enig5D96AE672F81A8518833ACA1--