From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57214) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aQ3PA-0005kF-Gw for qemu-devel@nongnu.org; Sun, 31 Jan 2016 20:35:49 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aQ3P6-0001c1-SO for qemu-devel@nongnu.org; Sun, 31 Jan 2016 20:35:48 -0500 Date: Mon, 1 Feb 2016 12:16:20 +1100 From: David Gibson Message-ID: <20160201011620.GZ23043@voom.redhat.com> References: <1454267976-27242-1-git-send-email-mark.cave-ayland@ilande.co.uk> <1454267976-27242-2-git-send-email-mark.cave-ayland@ilande.co.uk> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="uWAbeGC4mMoqIxdF" Content-Disposition: inline In-Reply-To: <1454267976-27242-2-git-send-email-mark.cave-ayland@ilande.co.uk> Subject: Re: [Qemu-devel] [PATCH 1/3] ppc: fix timebase adjustment during migration List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Mark Cave-Ayland Cc: aik@ozlabs.ru, qemu-ppc@nongnu.org, qemu-devel@nongnu.org, agraf@suse.de --uWAbeGC4mMoqIxdF Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sun, Jan 31, 2016 at 07:19:34PM +0000, Mark Cave-Ayland wrote: > ns_diff is already clamped to a minimum of 0 to prevent the timebase going > backwards during migration due to misaligned clocks. Following on from th= is > migration_duration_tb is also subject to the same constraint; hence the > expression MIN(0, migration_duration_tb) always evaluates to 0 and so no > timebase adjustment ever takes place. >=20 > Signed-off-by: Mark Cave-Ayland So, there are actually two problems here, which could be expressed a bit more clearly in the commit message. First, this clamping is redundant, because of the earlier clamp on ns_diff. Well.. probably.. I do wonder if we could get an overflow anywhere giving us a negative number again. More importantly, though, this is supposed to be a clamp below, which needs a MAX. MIN is Just Plain Wrong. > --- > hw/ppc/ppc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) >=20 > diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c > index ce90b09..19f4570 100644 > --- a/hw/ppc/ppc.c > +++ b/hw/ppc/ppc.c > @@ -877,7 +877,7 @@ static int timebase_post_load(void *opaque, int versi= on_id) > migration_duration_ns =3D MIN(NANOSECONDS_PER_SECOND, ns_diff); > migration_duration_tb =3D muldiv64(migration_duration_ns, freq, > NANOSECONDS_PER_SECOND); > - guest_tb =3D tb_remote->guest_timebase + MIN(0, migration_duration_t= b); > + guest_tb =3D tb_remote->guest_timebase + migration_duration_tb; > =20 > tb_off_adj =3D guest_tb - cpu_get_host_ticks(); > =20 --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --uWAbeGC4mMoqIxdF Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJWrrHkAAoJEGw4ysog2bOSJnAP/AhZWvSzq2geBI/x0sCHGZPz clOy/RnBVRBflJuqdqcUmzWkqvSzt15O9LO4HehmikjTOcX8Cea0WgLvtBbfQuPf h32SEjJyo70KR+tFiRjOlJu6o/sXPWEvZY/9atr+Zn3UnJ1l//Wo3yKSR3IObOSF EiJ8e3phR/7sCdGVY2ZffvwNs7coaQ/JhlN1YIslMcRIJUHUo/aIah7JaL/N35Of CciTxxkjoIQ0IdGZE4lhJb/85M1Hq3zFYxjA4l/HGRZI1W9sWZ14kWMB4bIon918 ERx8nUHzIlWoPZ1WO89u9foa0SIbGu0u2e7YEOY1KbbkfChOk2PPnZizQf9L0vd/ WRDt8CL12l9GVJgiZ2LpkeOjxbyzeQZwr3m2ZB/HVs1aEMH/+8EViWe4a2V7RkRc Ha7R+E3wvJ25FQB0ytU9GW6r605bR6AlrzDRsowaK5F6yqJ6+0olK+Xgn+w+8cqZ RW4C6ZPnuOU82F+U/xvoglk5PYyPuHD0Uw9mVtWqANtM/2ApW+ijpk2tRqRlkpn2 xHbCnsp5AMHe8AfNI6qAJN2AHmDZz09CsyugeWZKeqDslmIPcdmovxUj3TLMtNfy 6ByodgX42tzuzGwHHGFQVDtAunwDCubcbxG3y6czStOAzS5rIGh3rqSW4wlET7S2 o77coslLgEbHEoNPfext =2/P9 -----END PGP SIGNATURE----- --uWAbeGC4mMoqIxdF--