[PATCH 1/5] hrtimers: support resuming with two or more CPUs online (but stopped)

[PATCH 1/5] hrtimers: support resuming with two or more CPUs online (but stopped)

[David Vrabel]

From: David Vrabel <david.vrabel@citrix.com>

hrtimers_resume() only reprograms the timers for the current CPU as it
assumes that all other CPUs are offline at this point in the resume
process.  If other CPUs are online then their timers will not be
corrected and they may fire at the wrong time.

When running as a Xen guest, this assumption is not true.  Non-boot
CPUs are only stopped with IRQs disabled instead of offlining them.
This is a performance optimization as disabling the CPUs would add an
unacceptable amount of additional downtime during a live migration (>
200 ms for a 4 VCPU guest).

hrtimers_resume() cannot call on_each_cpu(retrigger_next_event,...)
as the other CPUs will be stopped with IRQs disabled.  Instead, defer
the call to the next softirq.

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/xen/manage.c |    3 ---
 kernel/hrtimer.c     |   15 ++++++++++++---
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c
index 412b96c..421da85 100644
--- a/drivers/xen/manage.c
+++ b/drivers/xen/manage.c
@@ -166,9 +166,6 @@ out_resume:
 
 	dpm_resume_end(si.cancelled ? PMSG_THAW : PMSG_RESTORE);
 
-	/* Make sure timer events get retriggered on all CPUs */
-	clock_was_set();
-
 out_thaw:
 #ifdef CONFIG_PREEMPT
 	thaw_processes();
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index fd4b13b..e86827e 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -773,15 +773,24 @@ void clock_was_set(void)
 
 /*
  * During resume we might have to reprogram the high resolution timer
- * interrupt (on the local CPU):
+ * interrupt on all online CPUs.  However, all other CPUs will be
+ * stopped with IRQs interrupts disabled so the clock_was_set() call
+ * must be deferred to the softirq.
+ *
+ * The one-shot timer has already been programmed to fire immediately
+ * (see tick_resume_oneshot()) and this interrupt will trigger the
+ * softirq to run early enough to correctly reprogram the timers on
+ * all CPUs.
  */
 void hrtimers_resume(void)
 {
+	struct hrtimer_cpu_base *cpu_base = &__get_cpu_var(hrtimer_bases);
+
 	WARN_ONCE(!irqs_disabled(),
 		  KERN_INFO "hrtimers_resume() called with IRQs enabled!");
 
-	retrigger_next_event(NULL);
-	timerfd_clock_was_set();
+	cpu_base->clock_was_set = 1;
+	__raise_softirq_irqoff(HRTIMER_SOFTIRQ);
 }
 
 static inline void timer_stats_hrtimer_set_start_info(struct hrtimer *timer)
-- 
1.7.2.5

[PATCH 2/5] time: pass flags instead of multiple bools to timekeeping_update()

[David Vrabel]

From: David Vrabel <david.vrabel@citrix.com>

Instead of passing multiple bools to timekeeping_updated(), define
flags and use a single 'action' parameter.  It is then more obvious
what each timekeeping_update() call does.

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
 kernel/time/timekeeping.c |   21 ++++++++++++---------
 1 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index baeeb5c..7aed2b0 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -26,6 +26,9 @@
 #include "tick-internal.h"
 #include "ntp_internal.h"
 
+#define TK_CLEAR_NTP (1 << 0)
+#define TK_MIRROR (1 << 1)
+
 static struct timekeeper timekeeper;
 static DEFINE_RAW_SPINLOCK(timekeeper_lock);
 static seqcount_t timekeeper_seq;
@@ -241,16 +244,16 @@ int pvclock_gtod_unregister_notifier(struct notifier_block *nb)
 EXPORT_SYMBOL_GPL(pvclock_gtod_unregister_notifier);
 
 /* must hold timekeeper_lock */
-static void timekeeping_update(struct timekeeper *tk, bool clearntp, bool mirror)
+static void timekeeping_update(struct timekeeper *tk, unsigned action)
 {
-	if (clearntp) {
+	if (action & TK_CLEAR_NTP) {
 		tk->ntp_error = 0;
 		ntp_clear();
 	}
 	update_vsyscall(tk);
 	update_pvclock_gtod(tk);
 
-	if (mirror)
+	if (action & TK_MIRROR)
 		memcpy(&shadow_timekeeper, &timekeeper, sizeof(timekeeper));
 }
 
@@ -508,7 +511,7 @@ int do_settimeofday(const struct timespec *tv)
 
 	tk_set_xtime(tk, tv);
 
-	timekeeping_update(tk, true, true);
+	timekeeping_update(tk, TK_CLEAR_NTP | TK_MIRROR);
 
 	write_seqcount_end(&timekeeper_seq);
 	raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
@@ -552,7 +555,7 @@ int timekeeping_inject_offset(struct timespec *ts)
 	tk_set_wall_to_mono(tk, timespec_sub(tk->wall_to_monotonic, *ts));
 
 error: /* even if we error out, we forwarded the time, so call update */
-	timekeeping_update(tk, true, true);
+	timekeeping_update(tk, TK_CLEAR_NTP | TK_MIRROR);
 
 	write_seqcount_end(&timekeeper_seq);
 	raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
@@ -633,7 +636,7 @@ static int change_clocksource(void *data)
 		if (old->disable)
 			old->disable(old);
 	}
-	timekeeping_update(tk, true, true);
+	timekeeping_update(tk, TK_CLEAR_NTP | TK_MIRROR);
 
 	write_seqcount_end(&timekeeper_seq);
 	raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
@@ -872,7 +875,7 @@ void timekeeping_inject_sleeptime(struct timespec *delta)
 
 	__timekeeping_inject_sleeptime(tk, delta);
 
-	timekeeping_update(tk, true, true);
+	timekeeping_update(tk, TK_CLEAR_NTP | TK_MIRROR);
 
 	write_seqcount_end(&timekeeper_seq);
 	raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
@@ -954,7 +957,7 @@ static void timekeeping_resume(void)
 	tk->cycle_last = clock->cycle_last = cycle_now;
 	tk->ntp_error = 0;
 	timekeeping_suspended = 0;
-	timekeeping_update(tk, false, true);
+	timekeeping_update(tk, TK_MIRROR);
 	write_seqcount_end(&timekeeper_seq);
 	raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
 
@@ -1415,7 +1418,7 @@ static void update_wall_time(void)
 	 * updating.
 	 */
 	memcpy(real_tk, tk, sizeof(*tk));
-	timekeeping_update(real_tk, false, false);
+	timekeeping_update(real_tk, 0);
 	write_seqcount_end(&timekeeper_seq);
 out:
 	raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
-- 
1.7.2.5

[PATCH 3/5] time: indicate that the clock was set in the pvclock gtod notifier chain

[David Vrabel]

From: David Vrabel <david.vrabel@citrix.com>

If the clock was set (stepped), set the action parameter to functions
in the pvclock gtod notifier chain to non-zero.  This allows the
callee to only do work if the clock was stepped.

This will be used on Xen as the synchronization of the Xen wallclock
to the control domain's (dom0) system time will be done with this
notifier and updating on every timer tick is unnecessary and too
expensive.

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
 include/linux/pvclock_gtod.h |    7 +++++++
 kernel/time/timekeeping.c    |   30 ++++++++++++++++++------------
 2 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/include/linux/pvclock_gtod.h b/include/linux/pvclock_gtod.h
index 0ca7582..a71d2db 100644
--- a/include/linux/pvclock_gtod.h
+++ b/include/linux/pvclock_gtod.h
@@ -3,6 +3,13 @@
 
 #include <linux/notifier.h>
 
+/*
+ * The pvclock gtod notifier is called when the system time is updated
+ * and is used to keep guest time synchronized with host time.
+ *
+ * The 'action' parameter in the notifier function is false (0), or
+ * true (non-zero) if system time was stepped.
+ */
 extern int pvclock_gtod_register_notifier(struct notifier_block *nb);
 extern int pvclock_gtod_unregister_notifier(struct notifier_block *nb);
 
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 7aed2b0..dd1b94d 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -28,6 +28,7 @@
 
 #define TK_CLEAR_NTP (1 << 0)
 #define TK_MIRROR (1 << 1)
+#define TK_CLOCK_WAS_SET (1 << 2)
 
 static struct timekeeper timekeeper;
 static DEFINE_RAW_SPINLOCK(timekeeper_lock);
@@ -203,9 +204,9 @@ static inline s64 timekeeping_get_ns_raw(struct timekeeper *tk)
 
 static RAW_NOTIFIER_HEAD(pvclock_gtod_chain);
 
-static void update_pvclock_gtod(struct timekeeper *tk)
+static void update_pvclock_gtod(struct timekeeper *tk, bool was_set)
 {
-	raw_notifier_call_chain(&pvclock_gtod_chain, 0, tk);
+	raw_notifier_call_chain(&pvclock_gtod_chain, was_set, tk);
 }
 
 /**
@@ -219,7 +220,7 @@ int pvclock_gtod_register_notifier(struct notifier_block *nb)
 
 	raw_spin_lock_irqsave(&timekeeper_lock, flags);
 	ret = raw_notifier_chain_register(&pvclock_gtod_chain, nb);
-	update_pvclock_gtod(tk);
+	update_pvclock_gtod(tk, true);
 	raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
 
 	return ret;
@@ -251,7 +252,7 @@ static void timekeeping_update(struct timekeeper *tk, unsigned action)
 		ntp_clear();
 	}
 	update_vsyscall(tk);
-	update_pvclock_gtod(tk);
+	update_pvclock_gtod(tk, action & TK_CLOCK_WAS_SET);
 
 	if (action & TK_MIRROR)
 		memcpy(&shadow_timekeeper, &timekeeper, sizeof(timekeeper));
@@ -511,7 +512,7 @@ int do_settimeofday(const struct timespec *tv)
 
 	tk_set_xtime(tk, tv);
 
-	timekeeping_update(tk, TK_CLEAR_NTP | TK_MIRROR);
+	timekeeping_update(tk, TK_CLEAR_NTP | TK_MIRROR | TK_CLOCK_WAS_SET);
 
 	write_seqcount_end(&timekeeper_seq);
 	raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
@@ -555,7 +556,7 @@ int timekeeping_inject_offset(struct timespec *ts)
 	tk_set_wall_to_mono(tk, timespec_sub(tk->wall_to_monotonic, *ts));
 
 error: /* even if we error out, we forwarded the time, so call update */
-	timekeeping_update(tk, TK_CLEAR_NTP | TK_MIRROR);
+	timekeeping_update(tk, TK_CLEAR_NTP | TK_MIRROR | TK_CLOCK_WAS_SET);
 
 	write_seqcount_end(&timekeeper_seq);
 	raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
@@ -636,7 +637,7 @@ static int change_clocksource(void *data)
 		if (old->disable)
 			old->disable(old);
 	}
-	timekeeping_update(tk, TK_CLEAR_NTP | TK_MIRROR);
+	timekeeping_update(tk, TK_CLEAR_NTP | TK_MIRROR | TK_CLOCK_WAS_SET);
 
 	write_seqcount_end(&timekeeper_seq);
 	raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
@@ -875,7 +876,7 @@ void timekeeping_inject_sleeptime(struct timespec *delta)
 
 	__timekeeping_inject_sleeptime(tk, delta);
 
-	timekeeping_update(tk, TK_CLEAR_NTP | TK_MIRROR);
+	timekeeping_update(tk, TK_CLEAR_NTP | TK_MIRROR | TK_CLOCK_WAS_SET);
 
 	write_seqcount_end(&timekeeper_seq);
 	raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
@@ -957,7 +958,7 @@ static void timekeeping_resume(void)
 	tk->cycle_last = clock->cycle_last = cycle_now;
 	tk->ntp_error = 0;
 	timekeeping_suspended = 0;
-	timekeeping_update(tk, TK_MIRROR);
+	timekeeping_update(tk, TK_MIRROR | TK_CLOCK_WAS_SET);
 	write_seqcount_end(&timekeeper_seq);
 	raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
 
@@ -1239,9 +1240,10 @@ out_adjust:
  * It also calls into the NTP code to handle leapsecond processing.
  *
  */
-static inline void accumulate_nsecs_to_secs(struct timekeeper *tk)
+static inline bool accumulate_nsecs_to_secs(struct timekeeper *tk)
 {
 	u64 nsecps = (u64)NSEC_PER_SEC << tk->shift;
+	unsigned action = 0;
 
 	while (tk->xtime_nsec >= nsecps) {
 		int leap;
@@ -1264,8 +1266,10 @@ static inline void accumulate_nsecs_to_secs(struct timekeeper *tk)
 			__timekeeping_set_tai_offset(tk, tk->tai_offset - leap);
 
 			clock_was_set_delayed();
+			action = TK_CLOCK_WAS_SET;
 		}
 	}
+	return action;
 }
 
 /**
@@ -1350,6 +1354,7 @@ static void update_wall_time(void)
 	struct timekeeper *tk = &shadow_timekeeper;
 	cycle_t offset;
 	int shift = 0, maxshift;
+	unsigned action;
 	unsigned long flags;
 
 	raw_spin_lock_irqsave(&timekeeper_lock, flags);
@@ -1402,7 +1407,7 @@ static void update_wall_time(void)
 	 * Finally, make sure that after the rounding
 	 * xtime_nsec isn't larger than NSEC_PER_SEC
 	 */
-	accumulate_nsecs_to_secs(tk);
+	action = accumulate_nsecs_to_secs(tk);
 
 	write_seqcount_begin(&timekeeper_seq);
 	/* Update clock->cycle_last with the new value */
@@ -1418,7 +1423,7 @@ static void update_wall_time(void)
 	 * updating.
 	 */
 	memcpy(real_tk, tk, sizeof(*tk));
-	timekeeping_update(real_tk, 0);
+	timekeeping_update(real_tk, action);
 	write_seqcount_end(&timekeeper_seq);
 out:
 	raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
@@ -1680,6 +1685,7 @@ int do_adjtimex(struct timex *txc)
 
 	if (tai != orig_tai) {
 		__timekeeping_set_tai_offset(tk, tai);
+		update_pvclock_gtod(tk, true);
 		clock_was_set_delayed();
 	}
 	write_seqcount_end(&timekeeper_seq);
-- 
1.7.2.5

Re: [PATCH 3/5] time: indicate that the clock was set in the pvclock gtod notifier chain

[John Stultz]

On 06/27/2013 03:35 AM, David Vrabel wrote:
> From: David Vrabel <david.vrabel@citrix.com>
>
> If the clock was set (stepped), set the action parameter to functions
> in the pvclock gtod notifier chain to non-zero.  This allows the
> callee to only do work if the clock was stepped.
>
> This will be used on Xen as the synchronization of the Xen wallclock
> to the control domain's (dom0) system time will be done with this
> notifier and updating on every timer tick is unnecessary and too
> expensive.
>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>

Looks pretty good. Minor note below.


>
> @@ -1239,9 +1240,10 @@ out_adjust:
>    * It also calls into the NTP code to handle leapsecond processing.
>    *
>    */
> -static inline void accumulate_nsecs_to_secs(struct timekeeper *tk)
> +static inline bool accumulate_nsecs_to_secs(struct timekeeper *tk)
Shouldn't this be unsigned instead of bool?


thanks
-john

Re: [PATCH 3/5] time: indicate that the clock was set in the pvclock gtod notifier chain

[David Vrabel]

On 27/06/13 18:37, John Stultz wrote:
> On 06/27/2013 03:35 AM, David Vrabel wrote:
>> From: David Vrabel <david.vrabel@citrix.com>
>>
>> If the clock was set (stepped), set the action parameter to functions
>> in the pvclock gtod notifier chain to non-zero.  This allows the
>> callee to only do work if the clock was stepped.
>>
>> This will be used on Xen as the synchronization of the Xen wallclock
>> to the control domain's (dom0) system time will be done with this
>> notifier and updating on every timer tick is unnecessary and too
>> expensive.
>>
>> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> 
> Looks pretty good. Minor note below.

I've fixed this and the other minor change you requested.

You can pull the wallclock-v7 branch from

   git://xenbits.xen.org/people/dvrabel/linux.git

or, if you prefer, I can repost the series.

David

[tip:timers/core] timekeeping: Indicate that clock was set in the pvclock gtod notifier

[tip-bot for David Vrabel]

Commit-ID:  780427f0e113b4c77dfff4d258c05a902cdb0eb9
Gitweb:     http://git.kernel.org/tip/780427f0e113b4c77dfff4d258c05a902cdb0eb9
Author:     David Vrabel <david.vrabel@citrix.com>
AuthorDate: Thu, 27 Jun 2013 11:35:46 +0100
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Fri, 28 Jun 2013 23:15:06 +0200

timekeeping: Indicate that clock was set in the pvclock gtod notifier

If the clock was set (stepped), set the action parameter to functions
in the pvclock gtod notifier chain to non-zero.  This allows the
callee to only do work if the clock was stepped.

This will be used on Xen as the synchronization of the Xen wallclock
to the control domain's (dom0) system time will be done with this
notifier and updating on every timer tick is unnecessary and too
expensive.

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: <xen-devel@lists.xen.org>
Link: http://lkml.kernel.org/r/1372329348-20841-4-git-send-email-david.vrabel@citrix.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/pvclock_gtod.h |  7 +++++++
 kernel/time/timekeeping.c    | 30 ++++++++++++++++++------------
 2 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/include/linux/pvclock_gtod.h b/include/linux/pvclock_gtod.h
index 0ca7582..a71d2db 100644
--- a/include/linux/pvclock_gtod.h
+++ b/include/linux/pvclock_gtod.h
@@ -3,6 +3,13 @@
 
 #include <linux/notifier.h>
 
+/*
+ * The pvclock gtod notifier is called when the system time is updated
+ * and is used to keep guest time synchronized with host time.
+ *
+ * The 'action' parameter in the notifier function is false (0), or
+ * true (non-zero) if system time was stepped.
+ */
 extern int pvclock_gtod_register_notifier(struct notifier_block *nb);
 extern int pvclock_gtod_unregister_notifier(struct notifier_block *nb);
 
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index d8b23a9..846d0a1 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -29,6 +29,7 @@
 
 #define TK_CLEAR_NTP		(1 << 0)
 #define TK_MIRROR		(1 << 1)
+#define TK_CLOCK_WAS_SET	(1 << 2)
 
 static struct timekeeper timekeeper;
 static DEFINE_RAW_SPINLOCK(timekeeper_lock);
@@ -204,9 +205,9 @@ static inline s64 timekeeping_get_ns_raw(struct timekeeper *tk)
 
 static RAW_NOTIFIER_HEAD(pvclock_gtod_chain);
 
-static void update_pvclock_gtod(struct timekeeper *tk)
+static void update_pvclock_gtod(struct timekeeper *tk, bool was_set)
 {
-	raw_notifier_call_chain(&pvclock_gtod_chain, 0, tk);
+	raw_notifier_call_chain(&pvclock_gtod_chain, was_set, tk);
 }
 
 /**
@@ -220,7 +221,7 @@ int pvclock_gtod_register_notifier(struct notifier_block *nb)
 
 	raw_spin_lock_irqsave(&timekeeper_lock, flags);
 	ret = raw_notifier_chain_register(&pvclock_gtod_chain, nb);
-	update_pvclock_gtod(tk);
+	update_pvclock_gtod(tk, true);
 	raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
 
 	return ret;
@@ -252,7 +253,7 @@ static void timekeeping_update(struct timekeeper *tk, unsigned int action)
 		ntp_clear();
 	}
 	update_vsyscall(tk);
-	update_pvclock_gtod(tk);
+	update_pvclock_gtod(tk, action & TK_CLOCK_WAS_SET);
 
 	if (action & TK_MIRROR)
 		memcpy(&shadow_timekeeper, &timekeeper, sizeof(timekeeper));
@@ -512,7 +513,7 @@ int do_settimeofday(const struct timespec *tv)
 
 	tk_set_xtime(tk, tv);
 
-	timekeeping_update(tk, TK_CLEAR_NTP | TK_MIRROR);
+	timekeeping_update(tk, TK_CLEAR_NTP | TK_MIRROR | TK_CLOCK_WAS_SET);
 
 	write_seqcount_end(&timekeeper_seq);
 	raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
@@ -556,7 +557,7 @@ int timekeeping_inject_offset(struct timespec *ts)
 	tk_set_wall_to_mono(tk, timespec_sub(tk->wall_to_monotonic, *ts));
 
 error: /* even if we error out, we forwarded the time, so call update */
-	timekeeping_update(tk, TK_CLEAR_NTP | TK_MIRROR);
+	timekeeping_update(tk, TK_CLEAR_NTP | TK_MIRROR | TK_CLOCK_WAS_SET);
 
 	write_seqcount_end(&timekeeper_seq);
 	raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
@@ -646,7 +647,7 @@ static int change_clocksource(void *data)
 			module_put(new->owner);
 		}
 	}
-	timekeeping_update(tk, TK_CLEAR_NTP | TK_MIRROR);
+	timekeeping_update(tk, TK_CLEAR_NTP | TK_MIRROR | TK_CLOCK_WAS_SET);
 
 	write_seqcount_end(&timekeeper_seq);
 	raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
@@ -887,7 +888,7 @@ void timekeeping_inject_sleeptime(struct timespec *delta)
 
 	__timekeeping_inject_sleeptime(tk, delta);
 
-	timekeeping_update(tk, TK_CLEAR_NTP | TK_MIRROR);
+	timekeeping_update(tk, TK_CLEAR_NTP | TK_MIRROR | TK_CLOCK_WAS_SET);
 
 	write_seqcount_end(&timekeeper_seq);
 	raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
@@ -969,7 +970,7 @@ static void timekeeping_resume(void)
 	tk->cycle_last = clock->cycle_last = cycle_now;
 	tk->ntp_error = 0;
 	timekeeping_suspended = 0;
-	timekeeping_update(tk, TK_MIRROR);
+	timekeeping_update(tk, TK_MIRROR | TK_CLOCK_WAS_SET);
 	write_seqcount_end(&timekeeper_seq);
 	raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
 
@@ -1243,9 +1244,10 @@ out_adjust:
  * It also calls into the NTP code to handle leapsecond processing.
  *
  */
-static inline void accumulate_nsecs_to_secs(struct timekeeper *tk)
+static inline unsigned int accumulate_nsecs_to_secs(struct timekeeper *tk)
 {
 	u64 nsecps = (u64)NSEC_PER_SEC << tk->shift;
+	unsigned int action = 0;
 
 	while (tk->xtime_nsec >= nsecps) {
 		int leap;
@@ -1268,8 +1270,10 @@ static inline void accumulate_nsecs_to_secs(struct timekeeper *tk)
 			__timekeeping_set_tai_offset(tk, tk->tai_offset - leap);
 
 			clock_was_set_delayed();
+			action = TK_CLOCK_WAS_SET;
 		}
 	}
+	return action;
 }
 
 /**
@@ -1354,6 +1358,7 @@ static void update_wall_time(void)
 	struct timekeeper *tk = &shadow_timekeeper;
 	cycle_t offset;
 	int shift = 0, maxshift;
+	unsigned int action;
 	unsigned long flags;
 
 	raw_spin_lock_irqsave(&timekeeper_lock, flags);
@@ -1406,7 +1411,7 @@ static void update_wall_time(void)
 	 * Finally, make sure that after the rounding
 	 * xtime_nsec isn't larger than NSEC_PER_SEC
 	 */
-	accumulate_nsecs_to_secs(tk);
+	action = accumulate_nsecs_to_secs(tk);
 
 	write_seqcount_begin(&timekeeper_seq);
 	/* Update clock->cycle_last with the new value */
@@ -1422,7 +1427,7 @@ static void update_wall_time(void)
 	 * updating.
 	 */
 	memcpy(real_tk, tk, sizeof(*tk));
-	timekeeping_update(real_tk, 0);
+	timekeeping_update(real_tk, action);
 	write_seqcount_end(&timekeeper_seq);
 out:
 	raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
@@ -1684,6 +1689,7 @@ int do_adjtimex(struct timex *txc)
 
 	if (tai != orig_tai) {
 		__timekeeping_set_tai_offset(tk, tai);
+		update_pvclock_gtod(tk, true);
 		clock_was_set_delayed();
 	}
 	write_seqcount_end(&timekeeper_seq);

[PATCH 4/5] x86/xen: sync the wallclock when the system time is set

[David Vrabel]

From: David Vrabel <david.vrabel@citrix.com>

Currently the Xen wallclock is only updated every 11 minutes if NTP is
synchronized to its clock source (using the sync_cmos_clock() work).
If a guest is started before NTP is synchronized it may see an
incorrect wallclock time.

Use the pvclock_gtod notifier chain to receive a notification when the
system time has changed and update the wallclock to match.

This chain is called on every timer tick and we want to avoid an extra
(expensive) hypercall on every tick.  Because dom0 has historically
never provided a very accurate wallclock and guests do not expect one,
we can do this simply: the wallclock is only updated if the clock was
set.

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
 arch/x86/xen/time.c |   28 ++++++++++++++++++++++++++++
 1 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
index a1947ac..3364850 100644
--- a/arch/x86/xen/time.c
+++ b/arch/x86/xen/time.c
@@ -14,6 +14,7 @@
 #include <linux/kernel_stat.h>
 #include <linux/math64.h>
 #include <linux/gfp.h>
+#include <linux/pvclock_gtod.h>
 
 #include <asm/pvclock.h>
 #include <asm/xen/hypervisor.h>
@@ -212,6 +213,30 @@ static int xen_set_wallclock(const struct timespec *now)
 	return HYPERVISOR_dom0_op(&op);
 }
 
+static int xen_pvclock_gtod_notify(struct notifier_block *nb, unsigned long was_set,
+				   void *priv)
+{
+	struct timespec now;
+	struct xen_platform_op op;
+
+	if (!was_set)
+		return NOTIFY_OK;
+
+	now = __current_kernel_time();
+
+	op.cmd = XENPF_settime;
+	op.u.settime.secs = now.tv_sec;
+	op.u.settime.nsecs = now.tv_nsec;
+	op.u.settime.system_time = xen_clocksource_read();
+
+	(void)HYPERVISOR_dom0_op(&op);
+	return NOTIFY_OK;
+}
+
+static struct notifier_block xen_pvclock_gtod_notifier = {
+	.notifier_call = xen_pvclock_gtod_notify,
+};
+
 static struct clocksource xen_clocksource __read_mostly = {
 	.name = "xen",
 	.rating = 400,
@@ -473,6 +498,9 @@ static void __init xen_time_init(void)
 	xen_setup_runstate_info(cpu);
 	xen_setup_timer(cpu);
 	xen_setup_cpu_clockevents();
+
+	if (xen_initial_domain())
+		pvclock_gtod_register_notifier(&xen_pvclock_gtod_notifier);
 }
 
 void __init xen_init_time_ops(void)
-- 
1.7.2.5

[PATCH 5/5] x86/xen: sync the CMOS RTC as well as the Xen wallclock

[David Vrabel]

From: David Vrabel <david.vrabel@citrix.com>

Adjustments to Xen's persistent clock via update_persistent_clock()
don't actually persist, as the Xen wallclock is a software only clock
and modifications to it do not modify the underlying CMOS RTC.

The x86_platform.set_wallclock hook can be used to keep the hardware
RTC synchronized (as on bare metal).  Because the Xen wallclock is now
kept synchronized by pvclock_gtod notifier, xen_set_wallclock() need
not do this and dom0 can simply use the native implementation.

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
 arch/x86/xen/time.c |   32 ++++++++++++++++----------------
 1 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
index 3364850..20e395a 100644
--- a/arch/x86/xen/time.c
+++ b/arch/x86/xen/time.c
@@ -199,37 +199,35 @@ static void xen_get_wallclock(struct timespec *now)
 
 static int xen_set_wallclock(const struct timespec *now)
 {
-	struct xen_platform_op op;
-
-	/* do nothing for domU */
-	if (!xen_initial_domain())
-		return -1;
-
-	op.cmd = XENPF_settime;
-	op.u.settime.secs = now->tv_sec;
-	op.u.settime.nsecs = now->tv_nsec;
-	op.u.settime.system_time = xen_clocksource_read();
-
-	return HYPERVISOR_dom0_op(&op);
+	return -1;
 }
 
 static int xen_pvclock_gtod_notify(struct notifier_block *nb, unsigned long was_set,
 				   void *priv)
 {
+	static struct timespec next;
 	struct timespec now;
 	struct xen_platform_op op;
 
-	if (!was_set)
-		return NOTIFY_OK;
-
 	now = __current_kernel_time();
 
+	if (!was_set && timespec_compare(&now, &next) < 0)
+		return NOTIFY_OK;
+
 	op.cmd = XENPF_settime;
 	op.u.settime.secs = now.tv_sec;
 	op.u.settime.nsecs = now.tv_nsec;
 	op.u.settime.system_time = xen_clocksource_read();
 
 	(void)HYPERVISOR_dom0_op(&op);
+
+	/*
+	 * Don't update the wallclock for another 11 minutes. This is
+	 * the same period as the sync_cmos_clock() work.
+	 */
+	next = now;
+	next.tv_sec += 11*60;
+
 	return NOTIFY_OK;
 }
 
@@ -513,7 +511,9 @@ void __init xen_init_time_ops(void)
 
 	x86_platform.calibrate_tsc = xen_tsc_khz;
 	x86_platform.get_wallclock = xen_get_wallclock;
-	x86_platform.set_wallclock = xen_set_wallclock;
+	/* Dom0 uses the native method to set the hardware RTC. */
+	if (!xen_initial_domain())
+		x86_platform.set_wallclock = xen_set_wallclock;
 }
 
 #ifdef CONFIG_XEN_PVHVM
-- 
1.7.2.5

Re: [PATCHv6 0/5] xen: maintain an accurate persistent clock in more cases

[Thomas Gleixner]

On Thu, 27 Jun 2013, David Vrabel wrote:
> These series fixes the above limitations and depends on "x86: increase
> precision of x86_platform.get/set_wallclock()" which was previously
> posted.

So I'd like to merge that in the following way:

I pick up patches 1-3 and stick them into tip timers/for-xen and merge
that branch into timers/core. When picking up 1/6, I'll drop the xen
part of that, so timers/core will not hold any xen specific bits.

Then the xen folks can pull timers/for-xen and apply the xen specific
stuff on top.

Thanks,

	tglx

Re: [PATCHv6 0/5] xen: maintain an accurate persistent clock in more cases

[Konrad Rzeszutek Wilk]

On Fri, Jun 28, 2013 at 03:14:42PM +0200, Thomas Gleixner wrote:
> On Thu, 27 Jun 2013, David Vrabel wrote:
> > These series fixes the above limitations and depends on "x86: increase
> > precision of x86_platform.get/set_wallclock()" which was previously
> > posted.
> 
> So I'd like to merge that in the following way:
> 
> I pick up patches 1-3 and stick them into tip timers/for-xen and merge
> that branch into timers/core. When picking up 1/6, I'll drop the xen
> part of that, so timers/core will not hold any xen specific bits.
> 
> Then the xen folks can pull timers/for-xen and apply the xen specific
> stuff on top.

Wouldn't it be easier for you to pick the "xen part of that" as well?
I am OK with you doing that and it all going through your tree.

But if it is easier for you to do the way you said - I can do that too.
Just tell me when to pull timers/for-xen
> 
> Thanks,
> 
> 	tglx
> 
>  

Re: [PATCHv6 0/5] xen: maintain an accurate persistent clock in more cases

[Thomas Gleixner]

On Fri, 28 Jun 2013, Konrad Rzeszutek Wilk wrote:
> On Fri, Jun 28, 2013 at 03:14:42PM +0200, Thomas Gleixner wrote:
> > On Thu, 27 Jun 2013, David Vrabel wrote:
> > > These series fixes the above limitations and depends on "x86: increase
> > > precision of x86_platform.get/set_wallclock()" which was previously
> > > posted.
> > 
> > So I'd like to merge that in the following way:
> > 
> > I pick up patches 1-3 and stick them into tip timers/for-xen and merge
> > that branch into timers/core. When picking up 1/6, I'll drop the xen
> > part of that, so timers/core will not hold any xen specific bits.
> > 
> > Then the xen folks can pull timers/for-xen and apply the xen specific
> > stuff on top.
> 
> Wouldn't it be easier for you to pick the "xen part of that" as well?
> I am OK with you doing that and it all going through your tree.

I can do that, if that's not conflicting with other xen/x86 stuff
outside of timers/core.

Thanks,

	tglx

Re: [PATCHv6 0/5] xen: maintain an accurate persistent clock in more cases

[Konrad Rzeszutek Wilk]

On Fri, Jun 28, 2013 at 05:12:35PM +0200, Thomas Gleixner wrote:
> On Fri, 28 Jun 2013, Konrad Rzeszutek Wilk wrote:
> > On Fri, Jun 28, 2013 at 03:14:42PM +0200, Thomas Gleixner wrote:
> > > On Thu, 27 Jun 2013, David Vrabel wrote:
> > > > These series fixes the above limitations and depends on "x86: increase
> > > > precision of x86_platform.get/set_wallclock()" which was previously
> > > > posted.
> > > 
> > > So I'd like to merge that in the following way:
> > > 
> > > I pick up patches 1-3 and stick them into tip timers/for-xen and merge
> > > that branch into timers/core. When picking up 1/6, I'll drop the xen
> > > part of that, so timers/core will not hold any xen specific bits.
> > > 
> > > Then the xen folks can pull timers/for-xen and apply the xen specific
> > > stuff on top.
> > 
> > Wouldn't it be easier for you to pick the "xen part of that" as well?
> > I am OK with you doing that and it all going through your tree.
> 
> I can do that, if that's not conflicting with other xen/x86 stuff
> outside of timers/core.

I got one change in there and it seems to apply cleanly (woot!).
But lets be on a safe side. There are also some Xen ARM changes that
are fiddling with arch/x86/xen/time.c (different maintainer)) - so both
me and Stefano can pull from the timers/for-xen and not have to worry about
conflict resolution.

Thanks!
> 
> Thanks,
> 
> 	tglx
> 

Re: [PATCHv6 0/5] xen: maintain an accurate persistent clock in more cases

[Thomas Gleixner]

On Fri, 28 Jun 2013, Konrad Rzeszutek Wilk wrote:
> On Fri, Jun 28, 2013 at 05:12:35PM +0200, Thomas Gleixner wrote:
> > On Fri, 28 Jun 2013, Konrad Rzeszutek Wilk wrote:
> > > On Fri, Jun 28, 2013 at 03:14:42PM +0200, Thomas Gleixner wrote:
> > > > On Thu, 27 Jun 2013, David Vrabel wrote:
> > > > > These series fixes the above limitations and depends on "x86: increase
> > > > > precision of x86_platform.get/set_wallclock()" which was previously
> > > > > posted.
> > > > 
> > > > So I'd like to merge that in the following way:
> > > > 
> > > > I pick up patches 1-3 and stick them into tip timers/for-xen and merge
> > > > that branch into timers/core. When picking up 1/6, I'll drop the xen
> > > > part of that, so timers/core will not hold any xen specific bits.
> > > > 
> > > > Then the xen folks can pull timers/for-xen and apply the xen specific
> > > > stuff on top.
> > > 
> > > Wouldn't it be easier for you to pick the "xen part of that" as well?
> > > I am OK with you doing that and it all going through your tree.
> > 
> > I can do that, if that's not conflicting with other xen/x86 stuff
> > outside of timers/core.
> 
> I got one change in there and it seems to apply cleanly (woot!).
> But lets be on a safe side. There are also some Xen ARM changes that
> are fiddling with arch/x86/xen/time.c (different maintainer)) - so both
> me and Stefano can pull from the timers/for-xen and not have to worry about
> conflict resolution.

Hrmpf, just noticed, that it depends on other stuff in timers/core:

ce0b098: x86: Fix vrtc_get_time/set_mmss to use new timespec interface
3565184: x86: Increase precision of x86_platform.get/set_wallclock()

/me rumages for frozen shark.

Re: [PATCH 2/5] time: pass flags instead of multiple bools to timekeeping_update()

[John Stultz]

On 06/27/2013 03:35 AM, David Vrabel wrote:
> From: David Vrabel <david.vrabel@citrix.com>
>
> Instead of passing multiple bools to timekeeping_updated(), define
> flags and use a single 'action' parameter.  It is then more obvious
> what each timekeeping_update() call does.
>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> ---
>   kernel/time/timekeeping.c |   21 ++++++++++++---------
>   1 files changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> index baeeb5c..7aed2b0 100644
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
> @@ -26,6 +26,9 @@
>   #include "tick-internal.h"
>   #include "ntp_internal.h"
>   
> +#define TK_CLEAR_NTP (1 << 0)
> +#define TK_MIRROR (1 << 1)
> +
>   static struct timekeeper timekeeper;
>   static DEFINE_RAW_SPINLOCK(timekeeper_lock);
>   static seqcount_t timekeeper_seq;
> @@ -241,16 +244,16 @@ int pvclock_gtod_unregister_notifier(struct notifier_block *nb)
>   EXPORT_SYMBOL_GPL(pvclock_gtod_unregister_notifier);
>   
>   /* must hold timekeeper_lock */
> -static void timekeeping_update(struct timekeeper *tk, bool clearntp, bool mirror)
> +static void timekeeping_update(struct timekeeper *tk, unsigned action)

Nit: Mind making this "unsigned int" just for consistency sake with the 
rest of the code?

thanks
-john

[tip:timers/core] timekeeping: Pass flags instead of multiple bools to timekeeping_update()

[tip-bot for David Vrabel]

Commit-ID:  04397fe94ad65289884b9862b6a0c722ececaadf
Gitweb:     http://git.kernel.org/tip/04397fe94ad65289884b9862b6a0c722ececaadf
Author:     David Vrabel <david.vrabel@citrix.com>
AuthorDate: Thu, 27 Jun 2013 11:35:45 +0100
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Fri, 28 Jun 2013 23:15:06 +0200

timekeeping: Pass flags instead of multiple bools to timekeeping_update()

Instead of passing multiple bools to timekeeping_updated(), define
flags and use a single 'action' parameter.  It is then more obvious
what each timekeeping_update() call does.

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: <xen-devel@lists.xen.org>
Link: http://lkml.kernel.org/r/1372329348-20841-3-git-send-email-david.vrabel@citrix.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/timekeeping.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 838fc07..d8b23a9 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -27,6 +27,9 @@
 #include "ntp_internal.h"
 #include "timekeeping_internal.h"
 
+#define TK_CLEAR_NTP		(1 << 0)
+#define TK_MIRROR		(1 << 1)
+
 static struct timekeeper timekeeper;
 static DEFINE_RAW_SPINLOCK(timekeeper_lock);
 static seqcount_t timekeeper_seq;
@@ -242,16 +245,16 @@ int pvclock_gtod_unregister_notifier(struct notifier_block *nb)
 EXPORT_SYMBOL_GPL(pvclock_gtod_unregister_notifier);
 
 /* must hold timekeeper_lock */
-static void timekeeping_update(struct timekeeper *tk, bool clearntp, bool mirror)
+static void timekeeping_update(struct timekeeper *tk, unsigned int action)
 {
-	if (clearntp) {
+	if (action & TK_CLEAR_NTP) {
 		tk->ntp_error = 0;
 		ntp_clear();
 	}
 	update_vsyscall(tk);
 	update_pvclock_gtod(tk);
 
-	if (mirror)
+	if (action & TK_MIRROR)
 		memcpy(&shadow_timekeeper, &timekeeper, sizeof(timekeeper));
 }
 
@@ -509,7 +512,7 @@ int do_settimeofday(const struct timespec *tv)
 
 	tk_set_xtime(tk, tv);
 
-	timekeeping_update(tk, true, true);
+	timekeeping_update(tk, TK_CLEAR_NTP | TK_MIRROR);
 
 	write_seqcount_end(&timekeeper_seq);
 	raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
@@ -553,7 +556,7 @@ int timekeeping_inject_offset(struct timespec *ts)
 	tk_set_wall_to_mono(tk, timespec_sub(tk->wall_to_monotonic, *ts));
 
 error: /* even if we error out, we forwarded the time, so call update */
-	timekeeping_update(tk, true, true);
+	timekeeping_update(tk, TK_CLEAR_NTP | TK_MIRROR);
 
 	write_seqcount_end(&timekeeper_seq);
 	raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
@@ -643,7 +646,7 @@ static int change_clocksource(void *data)
 			module_put(new->owner);
 		}
 	}
-	timekeeping_update(tk, true, true);
+	timekeeping_update(tk, TK_CLEAR_NTP | TK_MIRROR);
 
 	write_seqcount_end(&timekeeper_seq);
 	raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
@@ -884,7 +887,7 @@ void timekeeping_inject_sleeptime(struct timespec *delta)
 
 	__timekeeping_inject_sleeptime(tk, delta);
 
-	timekeeping_update(tk, true, true);
+	timekeeping_update(tk, TK_CLEAR_NTP | TK_MIRROR);
 
 	write_seqcount_end(&timekeeper_seq);
 	raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
@@ -966,7 +969,7 @@ static void timekeeping_resume(void)
 	tk->cycle_last = clock->cycle_last = cycle_now;
 	tk->ntp_error = 0;
 	timekeeping_suspended = 0;
-	timekeeping_update(tk, false, true);
+	timekeeping_update(tk, TK_MIRROR);
 	write_seqcount_end(&timekeeper_seq);
 	raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
 
@@ -1419,7 +1422,7 @@ static void update_wall_time(void)
 	 * updating.
 	 */
 	memcpy(real_tk, tk, sizeof(*tk));
-	timekeeping_update(real_tk, false, false);
+	timekeeping_update(real_tk, 0);
 	write_seqcount_end(&timekeeper_seq);
 out:
 	raw_spin_unlock_irqrestore(&timekeeper_lock, flags);

Re: [PATCH 5/5] x86/xen: sync the CMOS RTC as well as the Xen wallclock

[Thomas Gleixner]

On Thu, 27 Jun 2013, David Vrabel wrote:

> From: David Vrabel <david.vrabel@citrix.com>
> 
> Adjustments to Xen's persistent clock via update_persistent_clock()
> don't actually persist, as the Xen wallclock is a software only clock
> and modifications to it do not modify the underlying CMOS RTC.
> 
> The x86_platform.set_wallclock hook can be used to keep the hardware
> RTC synchronized (as on bare metal).  Because the Xen wallclock is now
> kept synchronized by pvclock_gtod notifier, xen_set_wallclock() need
> not do this and dom0 can simply use the native implementation.

I can understand that part, but ...
 
>  static int xen_pvclock_gtod_notify(struct notifier_block *nb, unsigned long was_set,
>  				   void *priv)
>  {
> +	static struct timespec next;
>  	struct timespec now;
>  	struct xen_platform_op op;
>  
> -	if (!was_set)
> -		return NOTIFY_OK;
> -
>  	now = __current_kernel_time();
>  
> +	if (!was_set && timespec_compare(&now, &next) < 0)
> +		return NOTIFY_OK;
> +
>  	op.cmd = XENPF_settime;
>  	op.u.settime.secs = now.tv_sec;
>  	op.u.settime.nsecs = now.tv_nsec;
>  	op.u.settime.system_time = xen_clocksource_read();
>  
>  	(void)HYPERVISOR_dom0_op(&op);
> +
> +	/*
> +	 * Don't update the wallclock for another 11 minutes. This is
> +	 * the same period as the sync_cmos_clock() work.
> +	 */
> +	next = now;
> +	next.tv_sec += 11*60;
> +

How is this related to the changelog? /me is confused .....

Re: [PATCH 5/5] x86/xen: sync the CMOS RTC as well as the Xen wallclock

[David Vrabel]

On 28/06/13 16:38, Thomas Gleixner wrote:
> On Thu, 27 Jun 2013, David Vrabel wrote:
> 
>> From: David Vrabel <david.vrabel@citrix.com>
>>
>> Adjustments to Xen's persistent clock via update_persistent_clock()
>> don't actually persist, as the Xen wallclock is a software only clock
>> and modifications to it do not modify the underlying CMOS RTC.
>>
>> The x86_platform.set_wallclock hook can be used to keep the hardware
>> RTC synchronized (as on bare metal).  Because the Xen wallclock is now
>> kept synchronized by pvclock_gtod notifier, xen_set_wallclock() need
>> not do this and dom0 can simply use the native implementation.
> 
> I can understand that part, but ...
>  
>>  static int xen_pvclock_gtod_notify(struct notifier_block *nb, unsigned long was_set,
>>  				   void *priv)
>>  {
>> +	static struct timespec next;
>>  	struct timespec now;
>>  	struct xen_platform_op op;
>>  
>> -	if (!was_set)
>> -		return NOTIFY_OK;
>> -
>>  	now = __current_kernel_time();
>>  
>> +	if (!was_set && timespec_compare(&now, &next) < 0)
>> +		return NOTIFY_OK;
>> +
>>  	op.cmd = XENPF_settime;
>>  	op.u.settime.secs = now.tv_sec;
>>  	op.u.settime.nsecs = now.tv_nsec;
>>  	op.u.settime.system_time = xen_clocksource_read();
>>  
>>  	(void)HYPERVISOR_dom0_op(&op);
>> +
>> +	/*
>> +	 * Don't update the wallclock for another 11 minutes. This is
>> +	 * the same period as the sync_cmos_clock() work.
>> +	 */
>> +	next = now;
>> +	next.tv_sec += 11*60;
>> +
> 
> How is this related to the changelog? /me is confused .....

Before:

Xen wallclock set when time is stepped.
Xen wallclock set every 11 minutes (by sync_cmos_clock()).
Hardware RTC never set.

After:

Xen wallclock set when time is stepped.
Xen wallclock set every 11 minutes (in pvclock gtod notifier).
Hardware RTC set every 11 minutes (by sync_cmos_clock()).

I'll update the changelog to be more descriptive:

  Adjustments to Xen's persistent clock via update_persistent_clock()
  don't actually persist, as the Xen wallclock is a software only clock
  and modifications to it do not modify the underlying CMOS RTC.

  The x86_platform.set_wallclock hook can be used to keep the hardware
  RTC synchronized (as on bare metal).  If (in dom0) we make the Xen
  wallclock periodically synchronized by the pvclock_gtod notifier, the
  set_wallclock hook need not update the Xen wallclock and the native
  implementation can be used.

Is that better?

David

Re: [PATCH 5/5] x86/xen: sync the CMOS RTC as well as the Xen wallclock

[Thomas Gleixner]

On Fri, 28 Jun 2013, David Vrabel wrote:
> 
> Before:
> 
> Xen wallclock set when time is stepped.
> Xen wallclock set every 11 minutes (by sync_cmos_clock()).
> Hardware RTC never set.
> 
> After:
> 
> Xen wallclock set when time is stepped.
> Xen wallclock set every 11 minutes (in pvclock gtod notifier).

Ah, you are emulating the sync_cmos_clock() behaviour for the xen
wallclock via the periodic pvclock_gtod notifier call.

> Hardware RTC set every 11 minutes (by sync_cmos_clock()).
> 
> I'll update the changelog to be more descriptive:
> 
>   Adjustments to Xen's persistent clock via update_persistent_clock()
>   don't actually persist, as the Xen wallclock is a software only clock
>   and modifications to it do not modify the underlying CMOS RTC.
> 
>   The x86_platform.set_wallclock hook can be used to keep the hardware
>   RTC synchronized (as on bare metal).  If (in dom0) we make the Xen
>   wallclock periodically synchronized by the pvclock_gtod notifier, the
>   set_wallclock hook need not update the Xen wallclock and the native
>   implementation can be used.

Yep. I'll pick that up.

Thanks,

	tglx

Re: [PATCH 5/5] x86/xen: sync the CMOS RTC as well as the Xen wallclock

[David Vrabel]

On 28/06/13 17:09, Thomas Gleixner wrote:
> On Fri, 28 Jun 2013, David Vrabel wrote:
>>
>> Before:
>>
>> Xen wallclock set when time is stepped.
>> Xen wallclock set every 11 minutes (by sync_cmos_clock()).
>> Hardware RTC never set.
>>
>> After:
>>
>> Xen wallclock set when time is stepped.
>> Xen wallclock set every 11 minutes (in pvclock gtod notifier).
> 
> Ah, you are emulating the sync_cmos_clock() behaviour for the xen
> wallclock via the periodic pvclock_gtod notifier call.
> 
>> Hardware RTC set every 11 minutes (by sync_cmos_clock()).
>>
>> I'll update the changelog to be more descriptive:
>>
>>   Adjustments to Xen's persistent clock via update_persistent_clock()
>>   don't actually persist, as the Xen wallclock is a software only clock
>>   and modifications to it do not modify the underlying CMOS RTC.
>>
>>   The x86_platform.set_wallclock hook can be used to keep the hardware
>>   RTC synchronized (as on bare metal).  If (in dom0) we make the Xen
>>   wallclock periodically synchronized by the pvclock_gtod notifier, the
>>   set_wallclock hook need not update the Xen wallclock and the native
>>   implementation can be used.
> 
> Yep. I'll pick that up.

If it helps, I've made this change as well as splitting the xen part of
the hrtimers patch out into a separate commit.

The following changes since commit 52efa9eb9ea36b457b23b1b5d3dd9ce64d110715:

  x86: increase precision of x86_platform.get/set_wallclock() (2013-06-26 18:07:16 +0100)

are available in the git repository at:
  git://xenbits.xen.org/people/dvrabel/linux.git wallclock-v8

David Vrabel (6):
      hrtimers: support resuming with two or more CPUs online (but stopped)
      time: pass flags instead of multiple bools to timekeeping_update()
      time: indicate that the clock was set in the pvclock gtod notifier chain
      xen: remove unnecessary call to clock_was_set() during resume
      x86/xen: sync the wallclock when the system time is set
      x86/xen: sync the CMOS RTC as well as the Xen wallclock

 arch/x86/xen/time.c          |   42 +++++++++++++++++++++++++++++++++++-------
 drivers/xen/manage.c         |    3 ---
 include/linux/pvclock_gtod.h |    7 +++++++
 kernel/hrtimer.c             |   15 ++++++++++++---
 kernel/time/timekeeping.c    |   39 ++++++++++++++++++++++++---------------
 5 files changed, 78 insertions(+), 28 deletions(-)

David

Re: [PATCH 5/5] x86/xen: sync the CMOS RTC as well as the Xen wallclock

[Thomas Gleixner]

On Fri, 28 Jun 2013, David Vrabel wrote:
> On 28/06/13 17:09, Thomas Gleixner wrote:
> > Yep. I'll pick that up.
> 
> If it helps, I've made this change as well as splitting the xen part of
> the hrtimers patch out into a separate commit.

Fun. I already have that same split in my pending queue :)

Thanks,

	tglx

[tip:timers/core] x86: xen: Sync the CMOS RTC as well as the Xen wallclock

[tip-bot for David Vrabel]

Commit-ID:  47433b8c9d7480a3eebd99df38e857ce85a37cee
Gitweb:     http://git.kernel.org/tip/47433b8c9d7480a3eebd99df38e857ce85a37cee
Author:     David Vrabel <david.vrabel@citrix.com>
AuthorDate: Thu, 27 Jun 2013 11:35:48 +0100
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Fri, 28 Jun 2013 23:15:07 +0200

x86: xen: Sync the CMOS RTC as well as the Xen wallclock

Adjustments to Xen's persistent clock via update_persistent_clock()
don't actually persist, as the Xen wallclock is a software only clock
and modifications to it do not modify the underlying CMOS RTC.

The x86_platform.set_wallclock hook is there to keep the hardware RTC
synchronized. On a guest this is pointless.

On Dom0 we can use the native implementaion which actually updates the
hardware RTC, but we still need to keep the software emulation of RTC
for the guests up to date. The subscription to the pvclock_notifier
allows us to emulate this easily. The notifier is called at every tick
and when the clock was set.

Right now we only use that notifier when the clock was set, but due to
the fact that it is called periodically from the timekeeping update
code, we can utilize it to emulate the NTP driven drift compensation
of update_persistant_clock() for the Xen wall (software) clock.

Add a 11 minutes periodic update to the pvclock_gtod notifier callback
to achieve that. The static variable 'next' which maintains that 11
minutes update cycle is protected by the core code serialization so
there is no need to add a Xen specific serialization mechanism.

[ tglx: Massaged changelog and added a few comments ]

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: <xen-devel@lists.xen.org>
Link: http://lkml.kernel.org/r/1372329348-20841-6-git-send-email-david.vrabel@citrix.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/xen/time.c | 45 ++++++++++++++++++++++++++-------------------
 1 file changed, 26 insertions(+), 19 deletions(-)

diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
index 3364850..7a5671b 100644
--- a/arch/x86/xen/time.c
+++ b/arch/x86/xen/time.c
@@ -199,37 +199,42 @@ static void xen_get_wallclock(struct timespec *now)
 
 static int xen_set_wallclock(const struct timespec *now)
 {
-	struct xen_platform_op op;
-
-	/* do nothing for domU */
-	if (!xen_initial_domain())
-		return -1;
-
-	op.cmd = XENPF_settime;
-	op.u.settime.secs = now->tv_sec;
-	op.u.settime.nsecs = now->tv_nsec;
-	op.u.settime.system_time = xen_clocksource_read();
-
-	return HYPERVISOR_dom0_op(&op);
+	return -1;
 }
 
-static int xen_pvclock_gtod_notify(struct notifier_block *nb, unsigned long was_set,
-				   void *priv)
+static int xen_pvclock_gtod_notify(struct notifier_block *nb,
+				   unsigned long was_set, void *priv)
 {
-	struct timespec now;
-	struct xen_platform_op op;
+	/* Protected by the calling core code serialization */
+	static struct timespec next_sync;
 
-	if (!was_set)
-		return NOTIFY_OK;
+	struct xen_platform_op op;
+	struct timespec now;
 
 	now = __current_kernel_time();
 
+	/*
+	 * We only take the expensive HV call when the clock was set
+	 * or when the 11 minutes RTC synchronization time elapsed.
+	 */
+	if (!was_set && timespec_compare(&now, &next_sync) < 0)
+		return NOTIFY_OK;
+
 	op.cmd = XENPF_settime;
 	op.u.settime.secs = now.tv_sec;
 	op.u.settime.nsecs = now.tv_nsec;
 	op.u.settime.system_time = xen_clocksource_read();
 
 	(void)HYPERVISOR_dom0_op(&op);
+
+	/*
+	 * Move the next drift compensation time 11 minutes
+	 * ahead. That's emulating the sync_cmos_clock() update for
+	 * the hardware RTC.
+	 */
+	next_sync = now;
+	next_sync.tv_sec += 11 * 60;
+
 	return NOTIFY_OK;
 }
 
@@ -513,7 +518,9 @@ void __init xen_init_time_ops(void)
 
 	x86_platform.calibrate_tsc = xen_tsc_khz;
 	x86_platform.get_wallclock = xen_get_wallclock;
-	x86_platform.set_wallclock = xen_set_wallclock;
+	/* Dom0 uses the native method to set the hardware RTC. */
+	if (!xen_initial_domain())
+		x86_platform.set_wallclock = xen_set_wallclock;
 }
 
 #ifdef CONFIG_XEN_PVHVM