Xen devel - May 2013 - [PATCH 2/3] timekeeping: sync persistent clock and RTC on system time step changes

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

The persistent clock or the RTC is only synchronized with system time
every 11 minutes if NTP is running.  This gives a window where the
persistent clock may be incorrect after a step change in the time
(such as on first boot).

This particularly affects Xen guests as until an update to the control
domain''s persistent clock, new guests will start with the incorrect
system time.

When there is a step change in the system time, call
update_persistent_clock or rtc_set_ntp_time() to synchronize the
persistent clock or RTC to the new system time.

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

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 98cd470..56f2349 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -240,12 +240,54 @@ int pvclock_gtod_unregister_notifier(struct notifier_block
*nb)
 }
 EXPORT_SYMBOL_GPL(pvclock_gtod_unregister_notifier);
 
+#if defined(CONFIG_GENERIC_CMOS_UPDATE) || defined(CONFIG_RTC_SYSTOHC)
+
+static void __sync_persistent_clock(struct work_struct *work)
+{
+	struct timespec now;
+	int ret = 0;
+
+	getnstimeofday(&now);
+
+#ifdef CONFIG_GENERIC_CMOS_UPDATE
+	ret = update_persistent_clock(now);
+#endif
+#ifdef CONFIG_RTC_SYSTOHC
+	if (ret == -ENODEV)
+		rtc_set_ntp_time(now);
+#endif
+}
+
+static DECLARE_DELAYED_WORK(sync_persistent_clock_work,
__sync_persistent_clock);
+
+static void sync_persistent_clock(struct timekeeper *tk)
+{
+	u64 nsecs;
+	u32 remainder;
+
+	/* Many RTCs require updates 500 ms before the next second. */
+	nsecs = timekeeping_get_ns(tk);
+	div_u64_rem(nsecs, NSEC_PER_SEC, &remainder);
+	if (remainder > NSEC_PER_SEC / 2)
+		nsecs = remainder - NSEC_PER_SEC / 2;
+	else
+		nsecs = remainder + NSEC_PER_SEC / 2;
+
+	if (system_wq)
+		schedule_delayed_work(&sync_persistent_clock_work,
nsecs_to_jiffies(nsecs));
+}
+
+#endif
+
 /* must hold timekeeper_lock */
-static void timekeeping_update(struct timekeeper *tk, bool clearntp, bool
mirror)
+static void timekeeping_update(struct timekeeper *tk, bool step, bool mirror)
 {
-	if (clearntp) {
+	if (step) {
 		tk->ntp_error = 0;
 		ntp_clear();
+#if defined(CONFIG_GENERIC_CMOS_UPDATE) || defined(CONFIG_RTC_SYSTOHC)
+		sync_persistent_clock(tk);
+#endif
 	}
 	update_vsyscall(tk);
 	update_pvclock_gtod(tk);
-- 
1.7.2.5

On 05/13/2013 10:56 AM, David Vrabel wrote:
> From: David Vrabel <david.vrabel@citrix.com>
>
> The persistent clock or the RTC is only synchronized with system time
> every 11 minutes if NTP is running.  This gives a window where the
> persistent clock may be incorrect after a step change in the time
> (such as on first boot).
>
> This particularly affects Xen guests as until an update to the control
> domain''s persistent clock, new guests will start with the
incorrect
> system time.
>
> When there is a step change in the system time, call
> update_persistent_clock or rtc_set_ntp_time() to synchronize the
> persistent clock or RTC to the new system time.
I''m sorry, this isn''t quite making sense to me. Could you
further
describe the exact problematic behavior you''re seeing here, and why its
a problem?

You seem to be saying we should always set the RTC any time settimeofday 
is called (regardless of the NTP sync state), which doesn''t seem right 
to me. Also I worry that this would cause the RTC to be set when the RTC 
hctosys() code (or hwclock) sets the time to the RTC clock, which is a 
bit circular.

I suspect once the problem is better understood, there will be a better 
solution then trying to always set the RTC on any settimeofday() call.

thanks
-john


>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> ---
>   kernel/time/timekeeping.c |   46
+++++++++++++++++++++++++++++++++++++++++++-
>   1 files changed, 44 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> index 98cd470..56f2349 100644
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
> @@ -240,12 +240,54 @@ int pvclock_gtod_unregister_notifier(struct
notifier_block *nb)
>   }
>   EXPORT_SYMBOL_GPL(pvclock_gtod_unregister_notifier);
>   
> +#if defined(CONFIG_GENERIC_CMOS_UPDATE) || defined(CONFIG_RTC_SYSTOHC)
> +
> +static void __sync_persistent_clock(struct work_struct *work)
> +{
> +	struct timespec now;
> +	int ret = 0;
> +
> +	getnstimeofday(&now);
> +
> +#ifdef CONFIG_GENERIC_CMOS_UPDATE
> +	ret = update_persistent_clock(now);
> +#endif
> +#ifdef CONFIG_RTC_SYSTOHC
> +	if (ret == -ENODEV)
> +		rtc_set_ntp_time(now);
> +#endif
> +}
> +
> +static DECLARE_DELAYED_WORK(sync_persistent_clock_work,
__sync_persistent_clock);
> +
> +static void sync_persistent_clock(struct timekeeper *tk)
> +{
> +	u64 nsecs;
> +	u32 remainder;
> +
> +	/* Many RTCs require updates 500 ms before the next second. */
> +	nsecs = timekeeping_get_ns(tk);
> +	div_u64_rem(nsecs, NSEC_PER_SEC, &remainder);
> +	if (remainder > NSEC_PER_SEC / 2)
> +		nsecs = remainder - NSEC_PER_SEC / 2;
> +	else
> +		nsecs = remainder + NSEC_PER_SEC / 2;
> +
> +	if (system_wq)
> +		schedule_delayed_work(&sync_persistent_clock_work,
nsecs_to_jiffies(nsecs));
> +}
> +
> +#endif
> +
>   /* must hold timekeeper_lock */
> -static void timekeeping_update(struct timekeeper *tk, bool clearntp, bool
mirror)
> +static void timekeeping_update(struct timekeeper *tk, bool step, bool
mirror)
>   {
> -	if (clearntp) {
> +	if (step) {
>   		tk->ntp_error = 0;
>   		ntp_clear();
> +#if defined(CONFIG_GENERIC_CMOS_UPDATE) || defined(CONFIG_RTC_SYSTOHC)
> +		sync_persistent_clock(tk);
> +#endif
>   	}
>   	update_vsyscall(tk);
>   	update_pvclock_gtod(tk);

On 14/05/13 01:40, John Stultz wrote:
> On 05/13/2013 10:56 AM, David Vrabel wrote:
>> From: David Vrabel <david.vrabel@citrix.com>
>>
>> The persistent clock or the RTC is only synchronized with system time
>> every 11 minutes if NTP is running.  This gives a window where the
>> persistent clock may be incorrect after a step change in the time
>> (such as on first boot).
>>
>> This particularly affects Xen guests as until an update to the control
>> domain''s persistent clock, new guests will start with the
incorrect
>> system time.
>>
>> When there is a step change in the system time, call
>> update_persistent_clock or rtc_set_ntp_time() to synchronize the
>> persistent clock or RTC to the new system time.
> 
> I''m sorry, this isn''t quite making sense to me. Could you
further
> describe the exact problematic behavior you''re seeing here, and
why its
> a problem?
The Xen wallclock is used as the persistent clock for Xen guests.  This
is initialized (by Xen) with the CMOS RTC at the start of day.  If the
RTC is incorrect then guests will see an incorrect wallclock time until
dom0 has corrected it.

Currently dom0 only updates the Xen wallclock with the 11 min periodic
work when NTP is synced.  This leaves a window where newly started
guests will see an incorrect wallclock time.  This can cause guests to
fail to start correctly if the wallclock is now behind what it was when
the guest last started. (e.g., fsck of its disk fails as its last mount
time appears to be far into the future).

Similarly (but less problematic), if a bare metal system is rebooted
before the RTC is updated it will still have the incorrect time.
> You seem to be saying we should always set the RTC any time settimeofday 
> is called (regardless of the NTP sync state), which doesn''t seem
right
> to me. Also I worry that this would cause the RTC to be set when the RTC 
> hctosys() code (or hwclock) sets the time to the RTC clock, which is a 
> bit circular.
I''m not too concerned about the behaviour of manual syncs of the RTC
because: a) if the kernel does this automatically then the use of manual
syncs is no longer necessary; and b) the RTC will still end up with the
correct time.
> I suspect once the problem is better understood, there will be a better 
> solution then trying to always set the RTC on any settimeofday() call.
David

On 05/14/2013 02:47 AM, David Vrabel wrote:
> On 14/05/13 01:40, John Stultz wrote:
>> On 05/13/2013 10:56 AM, David Vrabel wrote:
>>> From: David Vrabel <david.vrabel@citrix.com>
>>>
>>> The persistent clock or the RTC is only synchronized with system
time
>>> every 11 minutes if NTP is running.  This gives a window where the
>>> persistent clock may be incorrect after a step change in the time
>>> (such as on first boot).
>>>
>>> This particularly affects Xen guests as until an update to the
control
>>> domain''s persistent clock, new guests will start with the
incorrect
>>> system time.
>>>
>>> When there is a step change in the system time, call
>>> update_persistent_clock or rtc_set_ntp_time() to synchronize the
>>> persistent clock or RTC to the new system time.
>> I''m sorry, this isn''t quite making sense to me. Could
you further
>> describe the exact problematic behavior you''re seeing here,
and why its
>> a problem?
> The Xen wallclock is used as the persistent clock for Xen guests.  This
> is initialized (by Xen) with the CMOS RTC at the start of day.
Start of the day? I assume you mean on dom0 bootup? Or is it done 
pre-dom0 bootup by Xen itself?
>    If the
> RTC is incorrect then guests will see an incorrect wallclock time until
> dom0 has corrected it.

Sorry, just a bit more clarifying context here: So there is a 1:1 
relationship between xen_wall_clock and the RTC for all domN guests? And 
even if dom0 has set its system time properly, domN guests will 
initialize (in effect) from the hardware RTC and not from dom0''s system
time?


So, let me see if I''m getting this right:

* Hardware has misconfigured RTC, set to the wrong date/time
* Xen boots up dom0 (or Xen itself) initializes the xen_wall_clock to 
the RTC
* dom0 finishes booting, and uses NTP to correct the system time
* dom1 starts up, uses xen_wall_clock to initialize its system time, but 
the RTC is still wrong, so it boots with the wrong time.
* After 11 minutes of sync w/ NTP, dom0 sets the RTC fixing the time.
* dom2 starts up, uses xen_wall_clock to initialize its system time 
correctly.

At this point, dom0 and dom2 have the correct time and dom1 is 
incorrect, right?

And with your other patches, after the next boot up (assuming dom0 is 
synced with NTP for > 11 minutes) everything will be fine (and if NTP 
doesn''t set dom0''s RTC, or the hwclock isn''t
otherwise corrected we''ll
see the same behavior).

Is this correct?

> Currently dom0 only updates the Xen wallclock with the 11 min periodic
> work when NTP is synced.  This leaves a window where newly started
> guests will see an incorrect wallclock time.  This can cause guests to
> fail to start correctly if the wallclock is now behind what it was when
> the guest last started. (e.g., fsck of its disk fails as its last mount
> time appears to be far into the future).
>
> Similarly (but less problematic), if a bare metal system is rebooted
> before the RTC is updated it will still have the incorrect time.
So this has been the existing behavior for quite some time. If the RTC 
is misconfigured, it has to be corrected either explicitly by the admin 
via hwclock or by the kernel but only if we''re well synced with NTP.


>> You seem to be saying we should always set the RTC any time
settimeofday
>> is called (regardless of the NTP sync state), which doesn''t
seem right
>> to me. Also I worry that this would cause the RTC to be set when the
RTC
>> hctosys() code (or hwclock) sets the time to the RTC clock, which is a
>> bit circular.
> I''m not too concerned about the behaviour of manual syncs of the
RTC
> because: a) if the kernel does this automatically then the use of manual
> syncs is no longer necessary;
Well, we can''t break existing interface behavior. Even if its 
unnecessary at that point.
> and b) the RTC will still end up with the
> correct time.
But this isn''t in fact the case. Imagine an networkless embedded system
that''s system clock drifts. Its setup to use a cron job to set the time
a few times a day to correct this (its a bit naive, I know, but this is 
how these embedded systems often are configured).

The problem is, that every time it uses hwclock --hctosys, we read the 
RTC, and write it to the system time, which will then write that value 
back to the RTC. Since there will be some delay between the RTC read and 
the RTC write, we will inject some slight error into the RTC, such that 
it may be a second behind where it ought to be.

We do this regularly enough, and now the RTC clock is drifting behind.

And sure, this is somewhat of a contrived an example, but we can''t
break
folks using these approaches.


So I think the other patches in this series are fine, and should help 
limit the effect of the problematic case of a  mis-configured RTC. But 
this one I don''t think we can do reasonably.

If you really feel this tight-binding of the system time and the RTC is 
necessary (if NTP synced or not), you might continue working the 
approach with following modifications:

1) Limit the RTC syncing to settimeofday() and inject_time_offset(). 
Where it is now, we''d call it on every resume from suspend, which would
cause the same problematic circular RTC drift on systems that do 
frequent suspend/resumes.

2) Modify the logic so we only set the RTC on settimeofday() if the 
current RTC value is more then N seconds off. This would limit the 
circular drift, allowing time being set by the RTC to not result in 
modifying the RTC.


With those two changes you might have a better chance, but I''m still 
hesitant. There may be use cases where the RTC and the system time are 
intentionally kept out of sync.

thanks
-john

>>> On 14.05.13 at 19:15, John Stultz <john.stultz@linaro.org>
wrote:
> On 05/14/2013 02:47 AM, David Vrabel wrote:
>> On 14/05/13 01:40, John Stultz wrote:
>>> I''m sorry, this isn''t quite making sense to me.
Could you further
>>> describe the exact problematic behavior you''re seeing
here, and why its
>>> a problem?
>> The Xen wallclock is used as the persistent clock for Xen guests.  This
>> is initialized (by Xen) with the CMOS RTC at the start of day.
> 
> Start of the day? I assume you mean on dom0 bootup? Or is it done 
> pre-dom0 bootup by Xen itself?
It is, indeed - Xen reads the CMOS clock (or consults EFI) once
when it starts up, but leaves those alone as soon as it launched
Dom0.
>>    If the
>> RTC is incorrect then guests will see an incorrect wallclock time until
>> dom0 has corrected it.
> 
> 
> Sorry, just a bit more clarifying context here: So there is a 1:1 
> relationship between xen_wall_clock and the RTC for all domN guests? And 
> even if dom0 has set its system time properly, domN guests will 
> initialize (in effect) from the hardware RTC and not from dom0''s
system
> time?
No, (PV) DomU-s will get their time from the software clock that
Xen maintains, which Dom0 is helping keep in good shape when
NTP-synced.

So the hypervisor really doesn''t care about the actual RTC getting
updated in hardware, all it needs is for Dom0 to notify it of wall
clock corrections.

Jan

On 05/15/2013 01:16 AM, Jan Beulich wrote:
>>>> On 14.05.13 at 19:15, John Stultz
<john.stultz@linaro.org> wrote:
>> On 05/14/2013 02:47 AM, David Vrabel wrote:
>>> On 14/05/13 01:40, John Stultz wrote:
>>>> I''m sorry, this isn''t quite making sense to
me. Could you further
>>>> describe the exact problematic behavior you''re seeing
here, and why its
>>>> a problem?
>>> The Xen wallclock is used as the persistent clock for Xen guests. 
This
>>> is initialized (by Xen) with the CMOS RTC at the start of day.
>> Start of the day? I assume you mean on dom0 bootup? Or is it done
>> pre-dom0 bootup by Xen itself?
> It is, indeed - Xen reads the CMOS clock (or consults EFI) once
> when it starts up, but leaves those alone as soon as it launched
> Dom0.
>
>>>     If the
>>> RTC is incorrect then guests will see an incorrect wallclock time
until
>>> dom0 has corrected it.
>>
>> Sorry, just a bit more clarifying context here: So there is a 1:1
>> relationship between xen_wall_clock and the RTC for all domN guests?
And
>> even if dom0 has set its system time properly, domN guests will
>> initialize (in effect) from the hardware RTC and not from
dom0''s system
>> time?
> No, (PV) DomU-s will get their time from the software clock that
> Xen maintains, which Dom0 is helping keep in good shape when
> NTP-synced.
Ok, so really, as soon as the Dom0 time is set by NTP, all guests will 
see the right time? That makes more sense, and means the window for 
these sorts of issues is reasonably quite small.

David: So I''m less inclined to merge this individual change, but if you
still feel strongly about it, let me know and we can circle around on it 
after you''ve addressed the specific issues I pointed out earlier.

thanks
-john

On 15/05/13 19:10, John Stultz wrote:
> 
> Ok, so really, as soon as the Dom0 time is set by NTP, all guests will 
> see the right time? That makes more sense, and means the window for 
> these sorts of issues is reasonably quite small.
It''s a small window but it''s occurring in our automated test
system.
> David: So I''m less inclined to merge this individual change, but
if you
> still feel strongly about it, let me know and we can circle around on it 
> after you''ve addressed the specific issues I pointed out earlier.
This patch was the actual bug fix but I''ve reworked it to use the
pvclock_gtod notifier chain as this seemed to be what KVM hosts were
using to maintain a clock for guests.  Please review the new series, thanks.

David

On Tue, May 28, 2013 at 07:26:14PM +0100, David Vrabel
wrote:
> On 15/05/13 19:10, John Stultz wrote:
> > 
> > Ok, so really, as soon as the Dom0 time is set by NTP, all guests will
> > see the right time? That makes more sense, and means the window for 
> > these sorts of issues is reasonably quite small.
> 
> It''s a small window but it''s occurring in our automated
test system.
> 
> > David: So I''m less inclined to merge this individual change,
but if you
> > still feel strongly about it, let me know and we can circle around on
it
> > after you''ve addressed the specific issues I pointed out
earlier.
> 
> This patch was the actual bug fix but I''ve reworked it to use the
> pvclock_gtod notifier chain as this seemed to be what KVM hosts were
> using to maintain a clock for guests.  Please review the new series,
thanks.
Looks good.

John if you are OK I am thinking to push this to Linus shortly as it is
fixing a bug.
> 
> David

On 05/28/2013 11:26 AM, David Vrabel wrote:
> On 15/05/13 19:10, John Stultz wrote:
>> Ok, so really, as soon as the Dom0 time is set by NTP, all guests will
>> see the right time? That makes more sense, and means the window for
>> these sorts of issues is reasonably quite small.
> It''s a small window but it''s occurring in our automated
test system.
What is it with about your automated test system that causes a 
misconfigured RTC & system such that xen guests are started before Dom0 
has finished being initialized? Or am I still misunderstanding the 
situation?

1) Dom0 boots w/ misconfigured RTC
2) Dom0 starts ntp, correcting the date
3) After 11 minutes of sync the RTC is synced to NTP time.

 From earlier discussions, the only problem would be with guests 
starting between 1 and 2 (which seems a bit early for guests to be 
starting). And after 3, with your patch 1/2 (that I''ve queued) the
issue
should not recur again, as the RTC is corrected.

>
>> David: So I''m less inclined to merge this individual change,
but if you
>> still feel strongly about it, let me know and we can circle around on
it
>> after you''ve addressed the specific issues I pointed out
earlier.
> This patch was the actual bug fix but I''ve reworked it to use the
> pvclock_gtod notifier chain as this seemed to be what KVM hosts were
> using to maintain a clock for guests.  Please review the new series,
thanks.
I''m still not convinced that we should be changing the behavior
we''ve
had for quite a long time.

thanks
-john

On 05/28/2013 11:31 AM, Konrad Rzeszutek Wilk wrote:
> On Tue, May 28, 2013 at 07:26:14PM +0100, David Vrabel wrote:
>> On 15/05/13 19:10, John Stultz wrote:
>>> Ok, so really, as soon as the Dom0 time is set by NTP, all guests
will
>>> see the right time? That makes more sense, and means the window for
>>> these sorts of issues is reasonably quite small.
>> It''s a small window but it''s occurring in our
automated test system.
>>
>>> David: So I''m less inclined to merge this individual
change, but if you
>>> still feel strongly about it, let me know and we can circle around
on it
>>> after you''ve addressed the specific issues I pointed out
earlier.
>> This patch was the actual bug fix but I''ve reworked it to use
the
>> pvclock_gtod notifier chain as this seemed to be what KVM hosts were
>> using to maintain a clock for guests.  Please review the new series,
thanks.
> Looks good.
>
> John if you are OK I am thinking to push this to Linus shortly as it is
> fixing a bug.
I''m really not sure I''d call this a bug.  That seems like an 
over-reaction to a misconfigured system.

Or if there is a bug, I''m not sure its been clearly explained.

thanks
-john

On Tue, May 28, 2013 at 12:09:14PM -0700, John Stultz
wrote:
> On 05/28/2013 11:31 AM, Konrad Rzeszutek Wilk wrote:
> >On Tue, May 28, 2013 at 07:26:14PM +0100, David Vrabel wrote:
> >>On 15/05/13 19:10, John Stultz wrote:
> >>>Ok, so really, as soon as the Dom0 time is set by NTP, all
guests will
> >>>see the right time? That makes more sense, and means the window
for
> >>>these sorts of issues is reasonably quite small.
> >>It''s a small window but it''s occurring in our
automated test system.
> >>
> >>>David: So I''m less inclined to merge this individual
change, but if you
> >>>still feel strongly about it, let me know and we can circle
around on it
> >>>after you''ve addressed the specific issues I pointed
out earlier.
> >>This patch was the actual bug fix but I''ve reworked it to
use the
> >>pvclock_gtod notifier chain as this seemed to be what KVM hosts
were
> >>using to maintain a clock for guests.  Please review the new
series, thanks.
> >Looks good.
> >
> >John if you are OK I am thinking to push this to Linus shortly as it is
> >fixing a bug.
> 
> I''m really not sure I''d call this a bug.  That seems like
an
> over-reaction to a misconfigured system.
> 
> Or if there is a bug, I''m not sure its been clearly explained.
The #1 patch - b/c you try to set the RTC time and it actually never takes.
Meaning
on the next time the machine is booted the time is again
off.
> 
> thanks
> -john
> 
> 
>

On 05/28/2013 12:48 PM, Konrad Rzeszutek Wilk wrote:
> On Tue, May 28, 2013 at 12:09:14PM -0700, John Stultz wrote:
>> On 05/28/2013 11:31 AM, Konrad Rzeszutek Wilk wrote:
>>> On Tue, May 28, 2013 at 07:26:14PM +0100, David Vrabel wrote:
>>>> On 15/05/13 19:10, John Stultz wrote:
>>>>> Ok, so really, as soon as the Dom0 time is set by NTP, all
guests will
>>>>> see the right time? That makes more sense, and means the
window for
>>>>> these sorts of issues is reasonably quite small.
>>>> It''s a small window but it''s occurring in our
automated test system.
>>>>
>>>>> David: So I''m less inclined to merge this
individual change, but if you
>>>>> still feel strongly about it, let me know and we can circle
around on it
>>>>> after you''ve addressed the specific issues I
pointed out earlier.
>>>> This patch was the actual bug fix but I''ve reworked it
to use the
>>>> pvclock_gtod notifier chain as this seemed to be what KVM hosts
were
>>>> using to maintain a clock for guests.  Please review the new
series, thanks.
>>> Looks good.
>>>
>>> John if you are OK I am thinking to push this to Linus shortly as
it is
>>> fixing a bug.
>> I''m really not sure I''d call this a bug.  That seems
like an
>> over-reaction to a misconfigured system.
>>
>> Or if there is a bug, I''m not sure its been clearly explained.
> The #1 patch - b/c you try to set the RTC time and it actually never takes.
Meaning
> on the next time the machine is booted the time is again off.
Ok. Yea, that one I''m fine with and have queued for 3.11.

If you want to push it as a bug fix for 3.10, I''ll leave it to you to 
push to Linus.

thanks
-john

On 05/28/2013 01:03 PM, John Stultz wrote:
> On 05/28/2013 12:48 PM, Konrad Rzeszutek Wilk wrote:
>> On Tue, May 28, 2013 at 12:09:14PM -0700, John Stultz wrote:
>>> On 05/28/2013 11:31 AM, Konrad Rzeszutek Wilk wrote:
>>>> On Tue, May 28, 2013 at 07:26:14PM +0100, David Vrabel wrote:
>>>>> On 15/05/13 19:10, John Stultz wrote:
>>>>>> Ok, so really, as soon as the Dom0 time is set by NTP,
all guests
>>>>>> will
>>>>>> see the right time? That makes more sense, and means
the window for
>>>>>> these sorts of issues is reasonably quite small.
>>>>> It''s a small window but it''s occurring in
our automated test system.
>>>>>
>>>>>> David: So I''m less inclined to merge this
individual change, but
>>>>>> if you
>>>>>> still feel strongly about it, let me know and we can
circle
>>>>>> around on it
>>>>>> after you''ve addressed the specific issues I
pointed out earlier.
>>>>> This patch was the actual bug fix but I''ve
reworked it to use the
>>>>> pvclock_gtod notifier chain as this seemed to be what KVM
hosts were
>>>>> using to maintain a clock for guests.  Please review the
new
>>>>> series, thanks.
>>>> Looks good.
>>>>
>>>> John if you are OK I am thinking to push this to Linus shortly
as
>>>> it is
>>>> fixing a bug.
>>> I''m really not sure I''d call this a bug.  That
seems like an
>>> over-reaction to a misconfigured system.
>>>
>>> Or if there is a bug, I''m not sure its been clearly
explained.
>> The #1 patch - b/c you try to set the RTC time and it actually never 
>> takes. Meaning
>> on the next time the machine is booted the time is again off.
>
> Ok. Yea, that one I''m fine with and have queued for 3.11.
>
> If you want to push it as a bug fix for 3.10, I''ll leave it to you
to
> push to Linus.
Probably obvious, but just in case its not clear:

Though if that one patch goes to linus for 3.10, it needs to be reworked 
to not depend on the mach_set_rtc_mmss() interface change it currently 
depends on. The interface change is not bugfix material.

thanks
-john

On Tue, May 28, 2013 at 01:11:33PM -0700, John Stultz
wrote:
> On 05/28/2013 01:03 PM, John Stultz wrote:
> >On 05/28/2013 12:48 PM, Konrad Rzeszutek Wilk wrote:
> >>On Tue, May 28, 2013 at 12:09:14PM -0700, John Stultz wrote:
> >>>On 05/28/2013 11:31 AM, Konrad Rzeszutek Wilk wrote:
> >>>>On Tue, May 28, 2013 at 07:26:14PM +0100, David Vrabel
wrote:
> >>>>>On 15/05/13 19:10, John Stultz wrote:
> >>>>>>Ok, so really, as soon as the Dom0 time is set by
NTP,
> >>>>>>all guests will
> >>>>>>see the right time? That makes more sense, and
means the window for
> >>>>>>these sorts of issues is reasonably quite small.
> >>>>>It''s a small window but it''s
occurring in our automated test system.
> >>>>>
> >>>>>>David: So I''m less inclined to merge this
individual
> >>>>>>change, but if you
> >>>>>>still feel strongly about it, let me know and we
can
> >>>>>>circle around on it
> >>>>>>after you''ve addressed the specific issues
I pointed out earlier.
> >>>>>This patch was the actual bug fix but I''ve
reworked it to use the
> >>>>>pvclock_gtod notifier chain as this seemed to be what
KVM hosts were
> >>>>>using to maintain a clock for guests.  Please review
the
> >>>>>new series, thanks.
> >>>>Looks good.
> >>>>
> >>>>John if you are OK I am thinking to push this to Linus
> >>>>shortly as it is
> >>>>fixing a bug.
> >>>I''m really not sure I''d call this a bug. 
That seems like an
> >>>over-reaction to a misconfigured system.
> >>>
> >>>Or if there is a bug, I''m not sure its been clearly
explained.
> >>The #1 patch - b/c you try to set the RTC time and it actually
> >>never takes. Meaning
> >>on the next time the machine is booted the time is again off.
> >
> >Ok. Yea, that one I''m fine with and have queued for 3.11.
> >
> >If you want to push it as a bug fix for 3.10, I''ll leave it to
you
> >to push to Linus.
> 
> Probably obvious, but just in case its not clear:
> 
> Though if that one patch goes to linus for 3.10, it needs to be
> reworked to not depend on the mach_set_rtc_mmss() interface change
> it currently depends on. The interface change is not bugfix
> material.
You are referring to commit 3195ef59cb42cda3aeeb24a7fd2ba1b900c4a3cc
Author: Prarit Bhargava <prarit@redhat.com>
Date:   Thu Feb 14 12:02:54 2013 -0500

    x86: Do full rtc synchronization with ntp

which is not CC-ed to stable@vger.kernel.org, so the #1 patch would
not be able to be back-ported to any kernel right (so v3.9, v3.8..)?

But it could go to v3.10 - as that change is there. But you are saying
it can''t go to v3.10 b/c the interface change is not a bugfix
material.  Is that b/c said git commit might cause regressions and
you wouldn''t want to do two reverts?

That makes sense -  which point I  you are right that it makes
sense to stick said patch (#1) in your 3.11 queue and skip the v3.10
train for it.

On 05/28/2013 01:25 PM, Konrad Rzeszutek Wilk wrote:
> On Tue, May 28, 2013 at 01:11:33PM -0700, John Stultz wrote:
>> On 05/28/2013 01:03 PM, John Stultz wrote:
>>> On 05/28/2013 12:48 PM, Konrad Rzeszutek Wilk wrote:
>>>> On Tue, May 28, 2013 at 12:09:14PM -0700, John Stultz wrote:
>>>>> On 05/28/2013 11:31 AM, Konrad Rzeszutek Wilk wrote:
>>>>>> On Tue, May 28, 2013 at 07:26:14PM +0100, David Vrabel
wrote:
>>>>>>> On 15/05/13 19:10, John Stultz wrote:
>>>>>>>> Ok, so really, as soon as the Dom0 time is set
by NTP,
>>>>>>>> all guests will
>>>>>>>> see the right time? That makes more sense, and
means the window for
>>>>>>>> these sorts of issues is reasonably quite
small.
>>>>>>> It''s a small window but it''s
occurring in our automated test system.
>>>>>>>
>>>>>>>> David: So I''m less inclined to merge
this individual
>>>>>>>> change, but if you
>>>>>>>> still feel strongly about it, let me know and
we can
>>>>>>>> circle around on it
>>>>>>>> after you''ve addressed the specific
issues I pointed out earlier.
>>>>>>> This patch was the actual bug fix but I''ve
reworked it to use the
>>>>>>> pvclock_gtod notifier chain as this seemed to be
what KVM hosts were
>>>>>>> using to maintain a clock for guests.  Please
review the
>>>>>>> new series, thanks.
>>>>>> Looks good.
>>>>>>
>>>>>> John if you are OK I am thinking to push this to Linus
>>>>>> shortly as it is
>>>>>> fixing a bug.
>>>>> I''m really not sure I''d call this a bug. 
That seems like an
>>>>> over-reaction to a misconfigured system.
>>>>>
>>>>> Or if there is a bug, I''m not sure its been
clearly explained.
>>>> The #1 patch - b/c you try to set the RTC time and it actually
>>>> never takes. Meaning
>>>> on the next time the machine is booted the time is again off.
>>> Ok. Yea, that one I''m fine with and have queued for 3.11.
>>>
>>> If you want to push it as a bug fix for 3.10, I''ll leave
it to you
>>> to push to Linus.
>> Probably obvious, but just in case its not clear:
>>
>> Though if that one patch goes to linus for 3.10, it needs to be
>> reworked to not depend on the mach_set_rtc_mmss() interface change
>> it currently depends on. The interface change is not bugfix
>> material.
> You are referring to commit 3195ef59cb42cda3aeeb24a7fd2ba1b900c4a3cc
> Author: Prarit Bhargava <prarit@redhat.com>
> Date:   Thu Feb 14 12:02:54 2013 -0500
>
>      x86: Do full rtc synchronization with ntp
No. I mean David''s patch:
     x86: Increase precision of x86_platform.get/set_wallclock()

That one should be held back for 3.11.

But the calling of mach_set_rtc_mmss() from the Xen set_wallclock hook 
would still be reasonable to go in 3.10 (limited impact to only Xen, 
fixing a bug), but it would have to limit the call to the second 
granular mach_set_rtc_mmss() in 3.10.

thanks
-john