| Message ID | 1454267976-27242-2-git-send-email-mark.cave-ayland@ilande.co.uk |
|---|---|
| State | New |
| Headers | show |
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 this > 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. > > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> 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(-) > > 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 version_id) > migration_duration_ns = MIN(NANOSECONDS_PER_SECOND, ns_diff); > migration_duration_tb = muldiv64(migration_duration_ns, freq, > NANOSECONDS_PER_SECOND); > - guest_tb = tb_remote->guest_timebase + MIN(0, migration_duration_tb); > + guest_tb = tb_remote->guest_timebase + migration_duration_tb; > > tb_off_adj = guest_tb - cpu_get_host_ticks(); >
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 version_id) migration_duration_ns = MIN(NANOSECONDS_PER_SECOND, ns_diff); migration_duration_tb = muldiv64(migration_duration_ns, freq, NANOSECONDS_PER_SECOND); - guest_tb = tb_remote->guest_timebase + MIN(0, migration_duration_tb); + guest_tb = tb_remote->guest_timebase + migration_duration_tb; tb_off_adj = guest_tb - cpu_get_host_ticks();
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 this 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. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> --- hw/ppc/ppc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)