All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Stultz <john.stultz@linaro.org>
To: David Vrabel <david.vrabel@citrix.com>
Cc: "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 2/3] timekeeping: sync persistent clock and RTC on system time step changes
Date: Tue, 14 May 2013 10:15:07 -0700	[thread overview]
Message-ID: <5192711B.1070607@linaro.org> (raw)
In-Reply-To: <5192082D.4020400@citrix.com>

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



  parent reply	other threads:[~2013-05-14 17:15 UTC|newest]

Thread overview: 69+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-13 17:56 [PATCH 0/3] x86,time,xen: maintain an accurate persistent clock in more cases David Vrabel
2013-05-13 17:56 ` [PATCH 1/3] x86: increase precision of x86_platform.get/set_wallclock() David Vrabel
2013-05-13 17:56 ` David Vrabel
2013-05-14  0:57   ` John Stultz
2013-05-14  0:57   ` John Stultz
2013-05-14 17:52   ` John Stultz
2013-05-14 17:52   ` John Stultz
2013-05-29  0:18   ` John Stultz
2013-05-29  0:18   ` John Stultz
2013-05-29 12:16     ` David Vrabel
2013-05-29 12:16     ` David Vrabel
2013-05-13 17:56 ` [PATCH 2/3] timekeeping: sync persistent clock and RTC on system time step changes David Vrabel
2013-05-13 17:56 ` David Vrabel
2013-05-14  0:40   ` John Stultz
2013-05-14  0:40   ` John Stultz
2013-05-14  9:47     ` David Vrabel
2013-05-14  9:47     ` David Vrabel
2013-05-14 17:15       ` John Stultz
2013-05-14 17:15       ` John Stultz [this message]
2013-05-15  8:16         ` [Xen-devel] " Jan Beulich
2013-05-15 18:10           ` John Stultz
2013-05-15 18:10           ` [Xen-devel] " John Stultz
2013-05-28 18:26             ` David Vrabel
2013-05-28 18:31               ` Konrad Rzeszutek Wilk
2013-05-28 19:09                 ` John Stultz
2013-05-28 19:48                   ` Konrad Rzeszutek Wilk
2013-05-28 19:48                   ` [Xen-devel] " Konrad Rzeszutek Wilk
2013-05-28 20:03                     ` John Stultz
2013-05-28 20:03                     ` [Xen-devel] " John Stultz
2013-05-28 20:11                       ` John Stultz
2013-05-28 20:11                       ` [Xen-devel] " John Stultz
2013-05-28 20:25                         ` Konrad Rzeszutek Wilk
2013-05-28 20:25                           ` Konrad Rzeszutek Wilk
2013-05-28 20:30                           ` John Stultz
2013-05-28 20:30                           ` [Xen-devel] " John Stultz
2013-05-28 19:09                 ` John Stultz
2013-05-28 18:31               ` Konrad Rzeszutek Wilk
2013-05-28 19:06               ` John Stultz
2013-05-28 19:06               ` [Xen-devel] " John Stultz
2013-05-28 18:26             ` David Vrabel
2013-05-15  8:16         ` Jan Beulich
2013-05-13 17:56 ` [PATCH 3/3] x86/xen: sync the CMOS RTC as well as the Xen wallclock David Vrabel
2013-05-13 17:56 ` David Vrabel
2013-05-14  0:52   ` John Stultz
2013-05-14  0:52   ` John Stultz
2013-05-14  7:57     ` Jan Beulich
2013-05-14  7:57     ` [Xen-devel] " Jan Beulich
2013-05-14 15:59       ` John Stultz
2013-05-14 15:59       ` [Xen-devel] " John Stultz
2013-05-14 16:14         ` Jan Beulich
2013-05-14 16:17           ` John Stultz
2013-05-14 16:24             ` Konrad Rzeszutek Wilk
2013-05-14 16:28               ` John Stultz
2013-05-14 16:28               ` [Xen-devel] " John Stultz
2013-05-14 16:24             ` Konrad Rzeszutek Wilk
2013-05-14 16:17           ` John Stultz
2013-05-14 16:14         ` Jan Beulich
2013-05-14  9:55     ` David Vrabel
2013-05-14  9:55     ` David Vrabel
2013-05-14 17:24   ` John Stultz
2013-05-14 17:24   ` John Stultz
2013-05-14 18:00     ` David Vrabel
2013-05-14 18:00     ` David Vrabel
2013-05-14 18:03       ` John Stultz
2013-05-14 18:03       ` John Stultz
2013-05-15  8:19     ` [Xen-devel] " Jan Beulich
2013-05-15 18:13       ` John Stultz
2013-05-15 18:13       ` [Xen-devel] " John Stultz
2013-05-15  8:19     ` Jan Beulich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5192711B.1070607@linaro.org \
    --to=john.stultz@linaro.org \
    --cc=david.vrabel@citrix.com \
    --cc=konrad.wilk@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.