Jeremy Fitzhardinge
2010-Oct-26 16:59 UTC
[PATCH] x86/pvclock-xen: zero last_value on resume
If the guest domain has been suspend/resumed or migrated, then the
system clock backing the pvclock clocksource may revert to a smaller
value (ie, can be non-monotonic across the migration/save-restore).
Make sure we zero last_value in that case so that the domain
continues to see clock updates.
[ I don't know if kvm needs an analogous fix or not. ]
Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge at citrix.com>
Cc: Stable Kernel <stable at kernel.org>
Reported-by: Eelco Dolstra <e.dolstra at tudelft.nl>
Reported-by: Olivier Hanesse <olivier.hanesse at gmail.com>
Bisected-by: C?dric Schieli <cschieli at gmail.com>
Tested-by: C?dric Schieli <cschieli at gmail.com>
diff --git a/arch/x86/include/asm/pvclock.h b/arch/x86/include/asm/pvclock.h
index cd02f32..6226870 100644
--- a/arch/x86/include/asm/pvclock.h
+++ b/arch/x86/include/asm/pvclock.h
@@ -11,5 +11,6 @@ unsigned long pvclock_tsc_khz(struct pvclock_vcpu_time_info
*src);
void pvclock_read_wallclock(struct pvclock_wall_clock *wall,
struct pvclock_vcpu_time_info *vcpu,
struct timespec *ts);
+void pvclock_resume(void);
#endif /* _ASM_X86_PVCLOCK_H */
diff --git a/arch/x86/kernel/pvclock.c b/arch/x86/kernel/pvclock.c
index 239427c..a4f07c1 100644
--- a/arch/x86/kernel/pvclock.c
+++ b/arch/x86/kernel/pvclock.c
@@ -120,6 +120,11 @@ unsigned long pvclock_tsc_khz(struct pvclock_vcpu_time_info
*src)
static atomic64_t last_value = ATOMIC64_INIT(0);
+void pvclock_resume(void)
+{
+ atomic64_set(&last_value, 0);
+}
+
cycle_t pvclock_clocksource_read(struct pvclock_vcpu_time_info *src)
{
struct pvclock_shadow_time shadow;
diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
index b2bb5aa..5da5e53 100644
--- a/arch/x86/xen/time.c
+++ b/arch/x86/xen/time.c
@@ -426,6 +426,8 @@ void xen_timer_resume(void)
{
int cpu;
+ pvclock_resume();
+
if (xen_clockevent != &xen_vcpuop_clockevent)
return;
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: x86-pvclock-resume.patch
Url:
http://lists.linux-foundation.org/pipermail/virtualization/attachments/20101026/d1730f99/attachment.txt
On Tue, 2010-10-26 at 09:59 -0700, Jeremy Fitzhardinge wrote:> > If the guest domain has been suspend/resumed or migrated, then the > system clock backing the pvclock clocksource may revert to a smaller > value (ie, can be non-monotonic across the migration/save-restore). > Make sure we zero last_value in that case so that the domain > continues to see clock updates. > > [ I don't know if kvm needs an analogous fix or not. ]After migration, save/restore, etc, we issue an ioctl where we tell the host the last clock value. That (in theory) guarantees monotonicity. I am not opposed to this patch in any way, however.
Jeremy Fitzhardinge
2010-Nov-22 20:16 UTC
[PATCH] x86/pvclock-xen: zero last_value on resume
On 10/26/2010 09:59 AM, Jeremy Fitzhardinge wrote:> > If the guest domain has been suspend/resumed or migrated, then the > system clock backing the pvclock clocksource may revert to a smaller > value (ie, can be non-monotonic across the migration/save-restore). > Make sure we zero last_value in that case so that the domain > continues to see clock updates.Ping? Looks like this fell through the gaps. J> [ I don't know if kvm needs an analogous fix or not. ] > > Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge at citrix.com> > Cc: Stable Kernel <stable at kernel.org> > Reported-by: Eelco Dolstra <e.dolstra at tudelft.nl> > Reported-by: Olivier Hanesse <olivier.hanesse at gmail.com> > Bisected-by: C?dric Schieli <cschieli at gmail.com> > Tested-by: C?dric Schieli <cschieli at gmail.com> > > diff --git a/arch/x86/include/asm/pvclock.h b/arch/x86/include/asm/pvclock.h > index cd02f32..6226870 100644 > --- a/arch/x86/include/asm/pvclock.h > +++ b/arch/x86/include/asm/pvclock.h > @@ -11,5 +11,6 @@ unsigned long pvclock_tsc_khz(struct pvclock_vcpu_time_info *src); > void pvclock_read_wallclock(struct pvclock_wall_clock *wall, > struct pvclock_vcpu_time_info *vcpu, > struct timespec *ts); > +void pvclock_resume(void); > > #endif /* _ASM_X86_PVCLOCK_H */ > diff --git a/arch/x86/kernel/pvclock.c b/arch/x86/kernel/pvclock.c > index 239427c..a4f07c1 100644 > --- a/arch/x86/kernel/pvclock.c > +++ b/arch/x86/kernel/pvclock.c > @@ -120,6 +120,11 @@ unsigned long pvclock_tsc_khz(struct pvclock_vcpu_time_info *src) > > static atomic64_t last_value = ATOMIC64_INIT(0); > > +void pvclock_resume(void) > +{ > + atomic64_set(&last_value, 0); > +} > + > cycle_t pvclock_clocksource_read(struct pvclock_vcpu_time_info *src) > { > struct pvclock_shadow_time shadow; > diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c > index b2bb5aa..5da5e53 100644 > --- a/arch/x86/xen/time.c > +++ b/arch/x86/xen/time.c > @@ -426,6 +426,8 @@ void xen_timer_resume(void) > { > int cpu; > > + pvclock_resume(); > + > if (xen_clockevent != &xen_vcpuop_clockevent) > return; > > > > > > _______________________________________________ > Virtualization mailing list > Virtualization at lists.linux-foundation.org > https://lists.linux-foundation.org/mailman/listinfo/virtualization
* Jeremy Fitzhardinge <jeremy at goop.org> wrote:> On 10/26/2010 09:59 AM, Jeremy Fitzhardinge wrote: > > > > If the guest domain has been suspend/resumed or migrated, then the > > system clock backing the pvclock clocksource may revert to a smaller > > value (ie, can be non-monotonic across the migration/save-restore). > > Make sure we zero last_value in that case so that the domain > > continues to see clock updates. > > Ping? Looks like this fell through the gaps.Does not apply cleanly here - mind resending the latest version? Thanks, Ingo
Jeremy Fitzhardinge
2010-Nov-26 22:29 UTC
[PATCH] x86/pvclock-xen: zero last_value on resume
On 11/26/2010 02:40 AM, Ingo Molnar wrote:> * Jeremy Fitzhardinge <jeremy at goop.org> wrote: > >> On 10/26/2010 09:59 AM, Jeremy Fitzhardinge wrote: >>> If the guest domain has been suspend/resumed or migrated, then the >>> system clock backing the pvclock clocksource may revert to a smaller >>> value (ie, can be non-monotonic across the migration/save-restore). >>> Make sure we zero last_value in that case so that the domain >>> continues to see clock updates. >> Ping? Looks like this fell through the gaps. > Does not apply cleanly here - mind resending the latest version?It rebased cleanly to 2.6.37-rc3. You can pull it from: git://git.kernel.org/pub/scm/linux/kernel/git/jeremy/xen.git pvclock-resume Jeremy Fitzhardinge (1): x86/pvclock: zero last_value on resume arch/x86/include/asm/pvclock.h | 1 + arch/x86/kernel/pvclock.c | 5 +++++ arch/x86/xen/time.c | 2 ++ 3 files changed, 8 insertions(+), 0 deletions(-) Thanks, J Subject: [PATCH] x86/pvclock: zero last_value on resume If the guest domain has been suspend/resumed or migrated, then the system clock backing the pvclock clocksource may revert to a smaller value (ie, can be non-monotonic across the migration/save-restore). Make sure we zero last_value in that case so that the domain continues to see clock updates. Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge at citrix.com> diff --git a/arch/x86/include/asm/pvclock.h b/arch/x86/include/asm/pvclock.h index 7f7e577..31d84ac 100644 --- a/arch/x86/include/asm/pvclock.h +++ b/arch/x86/include/asm/pvclock.h @@ -11,6 +11,7 @@ unsigned long pvclock_tsc_khz(struct pvclock_vcpu_time_info *src); void pvclock_read_wallclock(struct pvclock_wall_clock *wall, struct pvclock_vcpu_time_info *vcpu, struct timespec *ts); +void pvclock_resume(void); /* * Scale a 64-bit delta by scaling and multiplying by a 32-bit fraction, diff --git a/arch/x86/kernel/pvclock.c b/arch/x86/kernel/pvclock.c index 008b91e..42eb330 100644 --- a/arch/x86/kernel/pvclock.c +++ b/arch/x86/kernel/pvclock.c @@ -83,6 +83,11 @@ unsigned long pvclock_tsc_khz(struct pvclock_vcpu_time_info *src) static atomic64_t last_value = ATOMIC64_INIT(0); +void pvclock_resume(void) +{ + atomic64_set(&last_value, 0); +} + cycle_t pvclock_clocksource_read(struct pvclock_vcpu_time_info *src) { struct pvclock_shadow_time shadow; diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c index b2bb5aa..5da5e53 100644 --- a/arch/x86/xen/time.c +++ b/arch/x86/xen/time.c @@ -426,6 +426,8 @@ void xen_timer_resume(void) { int cpu; + pvclock_resume(); + if (xen_clockevent != &xen_vcpuop_clockevent) return;