Time keeping while suspended in the presence of persistent clock drift

Time keeping while suspended in the presence of persistent clock drift

[Joel Daniels]

Hi,
I have an x86 laptop whose CMOS (RTC) clock gains an extra 3.75 seconds 
per day that it is suspended (S3) or off. It keeps time quite accurately 
while awake using the TSC clock source. I use the machine about 1 hour 
per day with the machine in the S3 sleep state for the remaining 23 
hours. The machine is not usually connected to a network and I do not 
run an NTP daemon (though I do not believe this is relevant). When cold 
booting, I correct for the CMOS clock drift using hwclock before making 
the filesystem writable. When resuming from suspend-to-ram (S3), 
however, I must either use hwclock again (causing the system time to 
jump backwards and potentially upsetting programs like make) or use a 
large slew rate (absolute value greater than 1000 PPM) to correct the 
system clock. As far as I can tell there is currently no way to inform 
the kernel of my CMOS clock drift. Is this correct? I am considering 
writing a patch to make the kernel compensate for the drift of the 
persistent and/or RTC clock(s) when injecting sleep time. The patch 
would require user space to inform the kernel of the drift (probably via 
sysfs). Does this seem like a good approach? Regards, Joel Daniels

Re: Time keeping while suspended in the presence of persistent clock drift

[Joel Daniels]

My email client just stripped all the newlines from my message making
it unreadable. I apologize and am resending it using a different
client. The original message (properly wrapped) is below.

=========

Hi,

I have an x86 laptop whose CMOS (RTC) clock gains an extra 3.75 seconds
per day that it is suspended (S3) or off. It keeps time quite accurately
while awake using the TSC clock source. I use the machine about 1 hour
per day with the machine in the S3 sleep state for the remaining 23
hours.

The machine is not usually connected to a network and I do not run an
NTP daemon (though I do not believe this is relevant). When cold
booting, I correct for the CMOS clock drift using hwclock before making
the filesystem writable. When resuming from suspend-to-ram (S3),
however, I must either use hwclock again (causing the system time to
jump backwards and potentially upsetting programs like make) or use a
large slew rate (absolute value greater than 1000 PPM) to correct the
system clock.

As far as I can tell there is currently no way to inform the kernel of
my CMOS clock drift. Is this correct?

I am considering writing a patch to make the kernel compensate for the
drift of the persistent and/or RTC clock(s) when injecting sleep time.
The patch would require user space to inform the kernel of the drift
(probably via sysfs). Does this seem like a good approach?

Regards,
Joel Daniels

Re: Time keeping while suspended in the presence of persistent clock drift

[Thomas Gleixner]

Joel!

On Thu, Dec 09 2021 at 11:06, Joel Daniels wrote:
> I have an x86 laptop whose CMOS (RTC) clock gains an extra 3.75 seconds
> per day that it is suspended (S3) or off. It keeps time quite accurately
> while awake using the TSC clock source. I use the machine about 1 hour
> per day with the machine in the S3 sleep state for the remaining 23
> hours.
>
> The machine is not usually connected to a network and I do not run an
> NTP daemon (though I do not believe this is relevant). When cold

Believe does not help much for analyzing technical problems. :)

Can you please verify that the problem persists with NTP enabled and
synchronized?

Thanks,

        tglx

Re: Time keeping while suspended in the presence of persistent clock drift

[Joel Daniels]

Hi Thomas,

On Sat, 11 Dec 2021 14:36 +0100, Thomas Gleixner wrote:
> Can you please verify that the problem persists with NTP enabled and
> synchronized?

Yes, I just verified that the problem still exists while
synchronized to NTP. Specifically, I connected the machine to the
internet and launched chrony with:

    $ sudo chronyd -d -L 0

using this /etc/chrony.conf:

    server time.cloudflare.com iburst
    driftfile /var/lib/chrony/drift
    maxslewrate 500
    rtcsync

I waited for chrony to stabilize:

    $ chronyc tracking && echo && chronyc sources
    [...]
    Ref time (UTC)  : Sun Dec 12 00:18:00 2021
    System time     : 0.000099906 seconds slow of NTP time
    [...]
    Frequency       : 2.096 ppm fast
    Residual freq   : -0.041 ppm
    Skew            : 0.985 ppm
    [...]

    MS Name/IP address         Stratum Poll Reach LastRx Last sample               
    ====================================================================
    ^* time.cloudflare.com           3   6   377    26   -282us[ -424us]

    $ sntp -K/dev/null time.cloudflare.com
    [...]
    2021-12-11 17:18:57.339207 (+0700) +0.001868 +/- 0.012247 [...]

Then I suspended the computer. I woke it 37 hours later and my system
clock was ahead by ~6 seconds:

    $ sntp -K/dev/null time.cloudflare.com
    [...]
    2021-12-13 06:24:28.425752 (+0700) -5.767757 +/- 3.856178 [...]

Chrony, of course, started to slew away the error and at the current
rate my system time will be correct in about 3 hours:

    $ chronyc tracking && echo && chronyc sources
    [...]
    Ref time (UTC)  : Mon Dec 13 13:30:52 2021
    System time     : 5.597892284 seconds fast of NTP time
    [...]
    Frequency       : 2.252 ppm fast
    Residual freq   : +0.603 ppm
    Skew            : 4.259 ppm
    [...]

    MS Name/IP address         Stratum Poll Reach LastRx Last sample               
    ====================================================================
    ^* time.cloudflare.com           3   6   377    15    +80us[ +197us]

This behavior is expected from my reading of the timekeeping_resume
function (kernel/time/timekeeping.c). Specifically these lines:

    read_persistent_clock64(&ts_new);
    
    ...
    
    cycle_now = tk_clock_read(&tk->tkr_mono);
    nsec = clocksource_stop_suspend_timing(clock, cycle_now);
    if (nsec > 0) {
            ts_delta = ns_to_timespec64(nsec);
            inject_sleeptime = true;
    } else if (timespec64_compare(&ts_new, &timekeeping_suspend_time) > 0) {
            ts_delta = timespec64_sub(ts_new, timekeeping_suspend_time);
            inject_sleeptime = true;
    }
    
    if (inject_sleeptime) {
            suspend_timing_needed = false;
            __timekeeping_inject_sleeptime(tk, &ts_delta);
    }

The "if" branch does not apply as I have no clock sources flagged as
CLOCK_SOURCE_SUSPEND_NONSTOP but the "else if" branch does apply.

The kernel seems to believe that the time spent sleeping is exactly
the difference of two calls to read_persistent_clock64 with no option
to adjust for persistent clock drift.

I would like to provide a way for user space to inform the kernel
that the persistent clock drifts so it can make a corresponding
adjustment when resuming from a long suspend period.

In my use case it would be enough for me to set this parameter on
boot. In use cases with continuous network access, NTP daemons
could be enhanced to periodically update this parameter with the
daemon's best estimate of the persistent clock drift.

Regards,
Joel

Re: Time keeping while suspended in the presence of persistent clock drift

[Thomas Gleixner]

Joel,

On Mon, Dec 13 2021 at 06:39, Joel Daniels wrote:
> On Sat, 11 Dec 2021 14:36 +0100, Thomas Gleixner wrote:
>> Can you please verify that the problem persists with NTP enabled and
>> synchronized?
>
> Yes, I just verified that the problem still exists while
> synchronized to NTP.
...
>     $ chronyc tracking && echo && chronyc sources
>     [...]
>     Ref time (UTC)  : Mon Dec 13 13:30:52 2021
>     System time     : 5.597892284 seconds fast of NTP time

thanks for making sure that this is really a RTC issue on that machine.

> The "if" branch does not apply as I have no clock sources flagged as
> CLOCK_SOURCE_SUSPEND_NONSTOP but the "else if" branch does apply.

Which CPU is in that box?

> The kernel seems to believe that the time spent sleeping is exactly
> the difference of two calls to read_persistent_clock64 with no option
> to adjust for persistent clock drift.

The kernel does not believe. It relies on the accuracy of the CMOS clock
which is usually pretty good.

> I would like to provide a way for user space to inform the kernel
> that the persistent clock drifts so it can make a corresponding
> adjustment when resuming from a long suspend period.
>
> In my use case it would be enough for me to set this parameter on
> boot. In use cases with continuous network access, NTP daemons
> could be enhanced to periodically update this parameter with the
> daemon's best estimate of the persistent clock drift.

That needs some thought. The RTC people (cc'ed now) might have opionions
on that.

Thanks,

        tglx

Re: Time keeping while suspended in the presence of persistent clock drift

[Joel Daniels]

Thomas,

On Tue, Dec 14, 2021, at 6:57 AM, Thomas Gleixner wrote:
> thanks for making sure that this is really a RTC issue on that machine.

And thank you for taking an interest. I've measured the RTC drift over
a number of days and it is stable at around 3.8 seconds per day (or 44
ppm).

>> The "if" branch does not apply as I have no clock sources flagged as
>> CLOCK_SOURCE_SUSPEND_NONSTOP but the "else if" branch does apply.
>
> Which CPU is in that box?

Intel Celeron N4120. This is a Gemini Lake Refresh (Atom) chip.

The relevant bit from the early_init_intel function
(linux/arch/x86/kernel/cpu/intel.c) is:

    /* Penwell and Cloverview have the TSC which doesn't sleep on S3 */
    if (c->x86 == 6) {
            switch (c->x86_model) {
            case INTEL_FAM6_ATOM_SALTWELL_MID:
            case INTEL_FAM6_ATOM_SALTWELL_TABLET:
            case INTEL_FAM6_ATOM_SILVERMONT_MID:
            case INTEL_FAM6_ATOM_AIRMONT_NP:
                    set_cpu_cap(c, X86_FEATURE_NONSTOP_TSC_S3);
                    break;
            default:
                    break;
            }
    }

> The kernel does not believe. It relies on the accuracy of the CMOS clock
> which is usually pretty good.

The references I have found on CMOS clock accuracy [1, 2, 3, 4]
indicate that a drift of 1 or 2 seconds per day (10 to 20 ppm) is
typical. Hopefully people on linux-rtc can confirm?

If that is correct then my clock, at +44ppm, is an outlier but I
suspect that people with a consistent drift of only 1 second per day
would still benefit from being able to correct for it. Indeed, people
have been using hwclock and /etc/adjtime to correct for CMOS RTC
drift for decades.

> > I would like to provide a way for user space to inform the kernel
> > that the persistent clock drifts so it can make a corresponding
> > adjustment when resuming from a long suspend period.
> >
> > ...
> 
> That needs some thought. The RTC people (cc'ed now) might have opinions
> on that.

I agree that this needs thought. Three issues that I am particularly
worried about:

   [A] On machines with a persistent clock how is userspace supposed
       to be sure what drift to measure? Can it assume that the drift
       of the persistent clock used for sleep time injection is the
       same as the drift of /dev/rtc? This seems dangerous.

   [B] Sleep time injection can come from the "persistent clock" or,
       if there is no persistent clock, from an RTC driver. I'd like
       to correct for drift from the perisistant clock but not touch
       the RTC driver sleep time injection mechanism. Is this
       acceptable or do people feel that any drift correction should
       work with both mechanisms in order to ensure a polished
       interface?
       
   [C] Some users may want to correct for drift during suspend-to-RAM
       but during suspend-to-disk they might boot into some other
       operating system which itself sets the CMOS RTC. Hopefully,
       this could be solved from userspace by changing the drift
       correction parameter to 0 just before a suspend-to-disk
       operation.

I suspect that there are other things about which I should also be
worried if only I were less ignorant. That is why I am asking here.

Thanks,
Joel Daniels

[1] http://www.ntp.org/ntpfaq/NTP-s-trbl-spec.htm#AEN5674 :
    "A PC used a stratum 1 server with PPS had had a hardware fault,
    and it had been powered off for about 18 days. ... when the system
    was rebooted the RTC clock was off by 18 seconds. That would be
    an error of roughly 12 PPM."

[2] https://www.hindawi.com/journals/jcnc/2008/583162/ :
    "In IBM PC compatible computers, the RTC circuit is the Motorola
    146818, with a resolution of approximately one second and a
    significant drift"

[3] https://www.maximintegrated.com/en/design/
            technical-documents/app-notes/5/58.html :
    Tables 1, 2 and 3 list 32.768Khz crystals with typical frequency
    tolerances of around +/- 20ppm at 25 degrees celsius.

[4] https://www.greyware.com/software/domaintime/technical/
            accuracy/pcclocks.asp:
    "The resolution of most PC real-time clocks is one full second,
    and most RTCs drift considerably over time."

Re: Time keeping while suspended in the presence of persistent clock drift

[John Stultz]

On Tue, Dec 14, 2021 at 9:43 AM Joel Daniels <jdaniels@sent.com> wrote:
> On Tue, Dec 14, 2021, at 6:57 AM, Thomas Gleixner wrote:
> > thanks for making sure that this is really a RTC issue on that machine.
>
> And thank you for taking an interest. I've measured the RTC drift over
> a number of days and it is stable at around 3.8 seconds per day (or 44
> ppm).
>
> >> The "if" branch does not apply as I have no clock sources flagged as
> >> CLOCK_SOURCE_SUSPEND_NONSTOP but the "else if" branch does apply.
> >
> > Which CPU is in that box?
>
> Intel Celeron N4120. This is a Gemini Lake Refresh (Atom) chip.
>
> The relevant bit from the early_init_intel function
> (linux/arch/x86/kernel/cpu/intel.c) is:
>
>     /* Penwell and Cloverview have the TSC which doesn't sleep on S3 */
>     if (c->x86 == 6) {
>             switch (c->x86_model) {
>             case INTEL_FAM6_ATOM_SALTWELL_MID:
>             case INTEL_FAM6_ATOM_SALTWELL_TABLET:
>             case INTEL_FAM6_ATOM_SILVERMONT_MID:
>             case INTEL_FAM6_ATOM_AIRMONT_NP:
>                     set_cpu_cap(c, X86_FEATURE_NONSTOP_TSC_S3);
>                     break;
>             default:
>                     break;
>             }
>     }
>
> > The kernel does not believe. It relies on the accuracy of the CMOS clock
> > which is usually pretty good.
>
> The references I have found on CMOS clock accuracy [1, 2, 3, 4]
> indicate that a drift of 1 or 2 seconds per day (10 to 20 ppm) is
> typical. Hopefully people on linux-rtc can confirm?
>
> If that is correct then my clock, at +44ppm, is an outlier but I
> suspect that people with a consistent drift of only 1 second per day
> would still benefit from being able to correct for it. Indeed, people
> have been using hwclock and /etc/adjtime to correct for CMOS RTC
> drift for decades.
>
> > > I would like to provide a way for user space to inform the kernel
> > > that the persistent clock drifts so it can make a corresponding
> > > adjustment when resuming from a long suspend period.
> > >
> > > ...
> >
> > That needs some thought. The RTC people (cc'ed now) might have opinions
> > on that.
>
> I agree that this needs thought. Three issues that I am particularly
> worried about:

I'm not really active in this space much anymore, but a few of my
(possibly wrongheaded) thoughts:

>    [A] On machines with a persistent clock how is userspace supposed
>        to be sure what drift to measure? Can it assume that the drift
>        of the persistent clock used for sleep time injection is the
>        same as the drift of /dev/rtc? This seems dangerous.

Yea, there can be multiple RTCs as well.

>    [B] Sleep time injection can come from the "persistent clock" or,
>        if there is no persistent clock, from an RTC driver. I'd like
>        to correct for drift from the perisistant clock but not touch
>        the RTC driver sleep time injection mechanism. Is this
>        acceptable or do people feel that any drift correction should
>        work with both mechanisms in order to ensure a polished
>        interface?

This dual interface comes from the desire to support both the more
atomic/earlier correction we can do w/ the persistent_clock interface
while holding the timekeeping lock, while also supporting RTC devices
that may sleep when being read, or may have dependencies that aren't
ready that early in resume.

Admittedly having two separate abstractions here is a bit of a pain,
and fixing just one side doesn't make it better.

>    [C] Some users may want to correct for drift during suspend-to-RAM
>        but during suspend-to-disk they might boot into some other
>        operating system which itself sets the CMOS RTC. Hopefully,
>        this could be solved from userspace by changing the drift
>        correction parameter to 0 just before a suspend-to-disk
>        operation.

Oof. This feels particularly complex and fragile to try to address.

> I suspect that there are other things about which I should also be
> worried if only I were less ignorant. That is why I am asking here.

Personally, I'm not sure this warrants adding new userland interfaces
for. I'd probably lean towards having the RTC framework internally
measure and correct for drift, rather than adding an extra knob in
userland.

Alternatively I'd go very simple and just put the correction factor in
a boot argument.

thanks
-john

Re: Time keeping while suspended in the presence of persistent clock drift

[Alexandre Belloni]

On 15/12/2021 13:06:30-0800, John Stultz wrote:
> I'm not really active in this space much anymore, but a few of my
> (possibly wrongheaded) thoughts:
> 
> >    [A] On machines with a persistent clock how is userspace supposed
> >        to be sure what drift to measure? Can it assume that the drift
> >        of the persistent clock used for sleep time injection is the
> >        same as the drift of /dev/rtc? This seems dangerous.
> 
> Yea, there can be multiple RTCs as well.
> 
> >    [B] Sleep time injection can come from the "persistent clock" or,
> >        if there is no persistent clock, from an RTC driver. I'd like
> >        to correct for drift from the perisistant clock but not touch
> >        the RTC driver sleep time injection mechanism. Is this
> >        acceptable or do people feel that any drift correction should
> >        work with both mechanisms in order to ensure a polished
> >        interface?
> 
> This dual interface comes from the desire to support both the more
> atomic/earlier correction we can do w/ the persistent_clock interface
> while holding the timekeeping lock, while also supporting RTC devices
> that may sleep when being read, or may have dependencies that aren't
> ready that early in resume.
> 
> Admittedly having two separate abstractions here is a bit of a pain,
> and fixing just one side doesn't make it better.
> 
> >    [C] Some users may want to correct for drift during suspend-to-RAM
> >        but during suspend-to-disk they might boot into some other
> >        operating system which itself sets the CMOS RTC. Hopefully,
> >        this could be solved from userspace by changing the drift
> >        correction parameter to 0 just before a suspend-to-disk
> >        operation.
> 
> Oof. This feels particularly complex and fragile to try to address.
> 
> > I suspect that there are other things about which I should also be
> > worried if only I were less ignorant. That is why I am asking here.
> 
> Personally, I'm not sure this warrants adding new userland interfaces
> for. I'd probably lean towards having the RTC framework internally
> measure and correct for drift, rather than adding an extra knob in
> userland.
> 

I'd rather lean towards the timekeeping code doing that. The RTC
subsystem doesn't know which RTC has to be used.

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: Time keeping while suspended in the presence of persistent clock drift

[Alexandre Belloni]

On 14/12/2021 14:57:45+0100, Thomas Gleixner wrote:
> Joel,
> 
> On Mon, Dec 13 2021 at 06:39, Joel Daniels wrote:
> > On Sat, 11 Dec 2021 14:36 +0100, Thomas Gleixner wrote:
> >> Can you please verify that the problem persists with NTP enabled and
> >> synchronized?
> >
> > Yes, I just verified that the problem still exists while
> > synchronized to NTP.
> ...
> >     $ chronyc tracking && echo && chronyc sources
> >     [...]
> >     Ref time (UTC)  : Mon Dec 13 13:30:52 2021
> >     System time     : 5.597892284 seconds fast of NTP time
> 
> thanks for making sure that this is really a RTC issue on that machine.
> 
> > The "if" branch does not apply as I have no clock sources flagged as
> > CLOCK_SOURCE_SUSPEND_NONSTOP but the "else if" branch does apply.
> 
> Which CPU is in that box?
> 
> > The kernel seems to believe that the time spent sleeping is exactly
> > the difference of two calls to read_persistent_clock64 with no option
> > to adjust for persistent clock drift.
> 
> The kernel does not believe. It relies on the accuracy of the CMOS clock
> which is usually pretty good.
> 
> > I would like to provide a way for user space to inform the kernel
> > that the persistent clock drifts so it can make a corresponding
> > adjustment when resuming from a long suspend period.
> >
> > In my use case it would be enough for me to set this parameter on
> > boot. In use cases with continuous network access, NTP daemons
> > could be enhanced to periodically update this parameter with the
> > daemon's best estimate of the persistent clock drift.
> 
> That needs some thought. The RTC people (cc'ed now) might have opionions
> on that.
> 

The RTC subsystem already has two interfaces to correct the drift of an
RTC. However, this is currently limited to RTC that have hardware
support for this feature. I guess we could had software emulation of the
feature to be able to correct for any RTCs  but this will raise many
design questions, like how often the correction has to happen, what to
do with RTC that have a counter that doesn't reset when setting their
time, etc...

I guess this would be able to solve your particular issue has you will
need a mechanism to handle when you overshoot the regular correction
timer.

However, everything falls down once the machine is turned off, making
the whole effort moot...


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: Time keeping while suspended in the presence of persistent clock drift

[John Stultz]

On Wed, Dec 15, 2021 at 1:32 PM Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
>
> On 15/12/2021 13:06:30-0800, John Stultz wrote:
> > I'm not really active in this space much anymore, but a few of my
> > (possibly wrongheaded) thoughts:
> >
> > >    [A] On machines with a persistent clock how is userspace supposed
> > >        to be sure what drift to measure? Can it assume that the drift
> > >        of the persistent clock used for sleep time injection is the
> > >        same as the drift of /dev/rtc? This seems dangerous.
> >
> > Yea, there can be multiple RTCs as well.
> >
> > >    [B] Sleep time injection can come from the "persistent clock" or,
> > >        if there is no persistent clock, from an RTC driver. I'd like
> > >        to correct for drift from the perisistant clock but not touch
> > >        the RTC driver sleep time injection mechanism. Is this
> > >        acceptable or do people feel that any drift correction should
> > >        work with both mechanisms in order to ensure a polished
> > >        interface?
> >
> > This dual interface comes from the desire to support both the more
> > atomic/earlier correction we can do w/ the persistent_clock interface
> > while holding the timekeeping lock, while also supporting RTC devices
> > that may sleep when being read, or may have dependencies that aren't
> > ready that early in resume.
> >
> > Admittedly having two separate abstractions here is a bit of a pain,
> > and fixing just one side doesn't make it better.
> >
> > >    [C] Some users may want to correct for drift during suspend-to-RAM
> > >        but during suspend-to-disk they might boot into some other
> > >        operating system which itself sets the CMOS RTC. Hopefully,
> > >        this could be solved from userspace by changing the drift
> > >        correction parameter to 0 just before a suspend-to-disk
> > >        operation.
> >
> > Oof. This feels particularly complex and fragile to try to address.
> >
> > > I suspect that there are other things about which I should also be
> > > worried if only I were less ignorant. That is why I am asking here.
> >
> > Personally, I'm not sure this warrants adding new userland interfaces
> > for. I'd probably lean towards having the RTC framework internally
> > measure and correct for drift, rather than adding an extra knob in
> > userland.
> >
>
> I'd rather lean towards the timekeeping code doing that. The RTC

Heh, touche'!  :)

> subsystem doesn't know which RTC has to be used.

Though the RTC layer *is* the one that tracks which RTC is used, via
the logic in drivers/rtc/class.c, and the timekeeping core already has
adjtimex for timekeeping corrections, so if we're correcting
underlying RTCs it seems such tuning would best be done in the RTC
layer.

Though how the persistent_clock interface ties into such corrections
would be a separate thing.

thanks
-john

Re: Time keeping while suspended in the presence of persistent clock drift

[Joel Daniels]

>> > I would like to provide a way for user space to inform the kernel
>> > that the persistent clock drifts so it can make a corresponding
>> > adjustment when resuming from a long suspend period.
>> >
>> > In my use case it would be enough for me to set this parameter on
>> > boot. In use cases with continuous network access, NTP daemons
>> > could be enhanced to periodically update this parameter with the
>> > daemon's best estimate of the persistent clock drift.
>> 
>> That needs some thought. The RTC people (cc'ed now) might have opionions
>> on that.
>> 
>
> The RTC subsystem already has two interfaces to correct the drift of an
> RTC. However, this is currently limited to RTC that have hardware
> support for this feature. I guess we could had software emulation of the
> feature to be able to correct for any RTCs  but this will raise many
> design questions, like how often the correction has to happen, what to
> do with RTC that have a counter that doesn't reset when setting their
> time, etc...
>
> I guess this would be able to solve your particular issue has you will
> need a mechanism to handle when you overshoot the regular correction
> timer.
>
> However, everything falls down once the machine is turned off, making
> the whole effort moot...

Today two mechanism are regularly used to correct for rtc drift while
the machine is powered off: the hwclock program and chronyd with the
"-s" option. They both rely on the RTC running at the same rate when
the machine is on or off. So I agree with you that trying to emulate
hardware RTC drift correction in software is not going to work well.

Re: Time keeping while suspended in the presence of persistent clock drift

[Thomas Gleixner]

On Wed, Dec 15 2021 at 14:02, John Stultz wrote:
> On Wed, Dec 15, 2021 at 1:32 PM Alexandre Belloni
>> I'd rather lean towards the timekeeping code doing that. The RTC
>
> Heh, touche'!  :)
>
>> subsystem doesn't know which RTC has to be used.
>
> Though the RTC layer *is* the one that tracks which RTC is used, via
> the logic in drivers/rtc/class.c, and the timekeeping core already has
> adjtimex for timekeeping corrections, so if we're correcting
> underlying RTCs it seems such tuning would best be done in the RTC
> layer.
>
> Though how the persistent_clock interface ties into such corrections
> would be a separate thing.

Might be the final trigger to get rid of that leftover from the last
millenium?

Thanks,

        tglx

Re: Time keeping while suspended in the presence of persistent clock drift

[Joel Daniels]

Hi John,

Thanks for your feedback.

>>    [A] On machines with a persistent clock how is userspace supposed
>>        to be sure what drift to measure? Can it assume that the drift
>>        of the persistent clock used for sleep time injection is the
>>        same as the drift of /dev/rtc? This seems dangerous.
>
> Yea, there can be multiple RTCs as well.
>
>>    [B] Sleep time injection can come from the "persistent clock" or,
>>        if there is no persistent clock, from an RTC driver. I'd like
>>        to correct for drift from the perisistant clock but not touch
>>        the RTC driver sleep time injection mechanism. Is this
>>        acceptable or do people feel that any drift correction should
>>        work with both mechanisms in order to ensure a polished
>>        interface?
>
> This dual interface comes from the desire to support both the more
> atomic/earlier correction we can do w/ the persistent_clock interface
> while holding the timekeeping lock, while also supporting RTC devices
> that may sleep when being read, or may have dependencies that aren't
> ready that early in resume.
> 
> Admittedly having two separate abstractions here is a bit of a pain,
> and fixing just one side doesn't make it better.

Thanks; that makes sense to me. I suppose I ought to have a separate
sleep-time-injection drift correction parameter per RTC? That way the
kernel wouldn't do something silly if somebody hotplugs one RTC while
removing another. The persistent clock is almost always exposed as an
RTC as well, so either I could try to be very clever and make the
persistent clock share the drift correction parameter of its
corresponding RTC or I could just maintain a separate correction for
the persistent clock.

>>    [C] Some users may want to correct for drift during suspend-to-RAM
>>        but during suspend-to-disk they might boot into some other
>>        operating system which itself sets the CMOS RTC. Hopefully,
>>        this could be solved from userspace by changing the drift
>>        correction parameter to 0 just before a suspend-to-disk
>>        operation.
>
> Oof. This feels particularly complex and fragile to try to address.

Yes, I think we should ignore this issue and treat all suspend/resume
cycles identically. People who regularly dual-boot can just not use
the new feature.

> Personally, I'm not sure this warrants adding new userland interfaces
> for. I'd probably lean towards having the RTC framework internally
> measure and correct for drift, rather than adding an extra knob in
> userland.

Measuring RTC drift is hard. The standard PC RTC has only one second
resolution so you have to wait for the "edge" of a tick and measure
drift over an extended period of time. If you have some NTP daemon
slewing your system clock while you try to measure RTC drift then
you will get garbage. If your motherboard gets hot enough then your
RTC will run at a different rate while the machine is on than while
it is off.

I know of three programs that measure RTC drift today:

  # hwclock: you must use it to set the RTC twice, the second time
    with the "--update-drift" argument. The manual suggests waiting
    one day between calls. The drift and offset information is
    stored in /etc/adjtime. On boot "hwclock --hctosys" will use this
    to set the system clock correctly.

  # adjtimex (program not syscall) when run with the "--compare"
    option. It uses a least squares estimate from multiple samples
    which by default are each 10 seconds apart.

  # chrony with the "rtcfile" directive. It tracks the RTC over time
    to measure its offset and drift similarly to how it tracks the
    system clock drift. Tracking information is saved into
      /var/lib/chrony/rtc
    and can be used (via "chronyd -s") to set the system clock
    correctly on next boot.

Any method of measuring the drift is going to need to persist the
drift coefficient to disk so that it can set the system clock
correctly on boot. I think it would be best for the kernel to use this
same coefficient.

> Alternatively I'd go very simple and just put the correction factor in
> a boot argument.

This works for my use case though it won't be useful to a general
distro. Would you have one argument being used regardless of where the
sleep injection was coming from or would you try to tie it to the
persistent clock and/or a specific RTC?

Re: Time keeping while suspended in the presence of persistent clock drift

[John Stultz]

On Wed, Dec 15, 2021 at 2:33 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> On Wed, Dec 15 2021 at 14:02, John Stultz wrote:
> > On Wed, Dec 15, 2021 at 1:32 PM Alexandre Belloni
> >> I'd rather lean towards the timekeeping code doing that. The RTC
> >
> > Heh, touche'!  :)
> >
> >> subsystem doesn't know which RTC has to be used.
> >
> > Though the RTC layer *is* the one that tracks which RTC is used, via
> > the logic in drivers/rtc/class.c, and the timekeeping core already has
> > adjtimex for timekeeping corrections, so if we're correcting
> > underlying RTCs it seems such tuning would best be done in the RTC
> > layer.
> >
> > Though how the persistent_clock interface ties into such corrections
> > would be a separate thing.
>
> Might be the final trigger to get rid of that leftover from the last
> millenium?
>

Yeah. Simplifying probably helps for consistency and maintainability.
(on top of the rtc and persistent clock, we also have the nonstop
clocksources that keep running through suspend and can be used. :)

It's just that window after resume but before the sleep time injection
where time would be incorrect always made me uncomfortable, so it was
nice to have some correct way to avoid that, even if all hardware
couldn't utilize it.  But as I'm less involved here, maybe someone
else can simplify things and live with that worry. :)

thanks
-john

Re: Time keeping while suspended in the presence of persistent clock drift

[John Stultz]

On Wed, Dec 15, 2021 at 2:44 PM Joel Daniels <jdaniels@sent.com> wrote:
> >>    [A] On machines with a persistent clock how is userspace supposed
> >>        to be sure what drift to measure? Can it assume that the drift
> >>        of the persistent clock used for sleep time injection is the
> >>        same as the drift of /dev/rtc? This seems dangerous.
> >
> > Yea, there can be multiple RTCs as well.
> >
> >>    [B] Sleep time injection can come from the "persistent clock" or,
> >>        if there is no persistent clock, from an RTC driver. I'd like
> >>        to correct for drift from the perisistant clock but not touch
> >>        the RTC driver sleep time injection mechanism. Is this
> >>        acceptable or do people feel that any drift correction should
> >>        work with both mechanisms in order to ensure a polished
> >>        interface?
> >
> > This dual interface comes from the desire to support both the more
> > atomic/earlier correction we can do w/ the persistent_clock interface
> > while holding the timekeeping lock, while also supporting RTC devices
> > that may sleep when being read, or may have dependencies that aren't
> > ready that early in resume.
> >
> > Admittedly having two separate abstractions here is a bit of a pain,
> > and fixing just one side doesn't make it better.
>
> Thanks; that makes sense to me. I suppose I ought to have a separate
> sleep-time-injection drift correction parameter per RTC? That way the
> kernel wouldn't do something silly if somebody hotplugs one RTC while
> removing another. The persistent clock is almost always exposed as an
> RTC as well, so either I could try to be very clever and make the
> persistent clock share the drift correction parameter of its
> corresponding RTC or I could just maintain a separate correction for
> the persistent clock.
>
> >>    [C] Some users may want to correct for drift during suspend-to-RAM
> >>        but during suspend-to-disk they might boot into some other
> >>        operating system which itself sets the CMOS RTC. Hopefully,
> >>        this could be solved from userspace by changing the drift
> >>        correction parameter to 0 just before a suspend-to-disk
> >>        operation.
> >
> > Oof. This feels particularly complex and fragile to try to address.
>
> Yes, I think we should ignore this issue and treat all suspend/resume
> cycles identically. People who regularly dual-boot can just not use
> the new feature.
>
> > Personally, I'm not sure this warrants adding new userland interfaces
> > for. I'd probably lean towards having the RTC framework internally
> > measure and correct for drift, rather than adding an extra knob in
> > userland.
>
> Measuring RTC drift is hard. The standard PC RTC has only one second
> resolution so you have to wait for the "edge" of a tick and measure
> drift over an extended period of time. If you have some NTP daemon
> slewing your system clock while you try to measure RTC drift then
> you will get garbage. If your motherboard gets hot enough then your
> RTC will run at a different rate while the machine is on than while
> it is off.
>
> I know of three programs that measure RTC drift today:
>
>   # hwclock: you must use it to set the RTC twice, the second time
>     with the "--update-drift" argument. The manual suggests waiting
>     one day between calls. The drift and offset information is
>     stored in /etc/adjtime. On boot "hwclock --hctosys" will use this
>     to set the system clock correctly.
>
>   # adjtimex (program not syscall) when run with the "--compare"
>     option. It uses a least squares estimate from multiple samples
>     which by default are each 10 seconds apart.
>
>   # chrony with the "rtcfile" directive. It tracks the RTC over time
>     to measure its offset and drift similarly to how it tracks the
>     system clock drift. Tracking information is saved into
>       /var/lib/chrony/rtc
>     and can be used (via "chronyd -s") to set the system clock
>     correctly on next boot.
>
> Any method of measuring the drift is going to need to persist the
> drift coefficient to disk so that it can set the system clock
> correctly on boot. I think it would be best for the kernel to use this
> same coefficient.

My initial thought was for the rtc class layer to do the estimation
internally against the system time (assuming it was NTP corrected) to
try establish a close enough correction factor as the system was up
and running, but you're right that would be reset on every reboot, and
with second granular RTCs accurate error calculations will take awhile
(though polling for the second-edge might work well enough, but would
be cpu heavy for a background task).

Maybe that's a good enough argument for having an ADJ ioctl added to
the rtc chardev?

But it also seems to suggest that if chronyd already has all this
logic in userland, maybe having it calculate and call settimeofday()
on resume from userland instead of pushing half of that logic into the
kernel?

> > Alternatively I'd go very simple and just put the correction factor in
> > a boot argument.
>
> This works for my use case though it won't be useful to a general
> distro. Would you have one argument being used regardless of where the
> sleep injection was coming from or would you try to tie it to the
> persistent clock and/or a specific RTC?

I agree it is an important thing to consider how to generalize this
for common use (which is why I prefer the approach that works
*without* any distro integration).

But it's also important to consider if the added complexity is
*really* needed in the general case.

thanks
-john

Re: Time keeping while suspended in the presence of persistent clock drift

[Alexandre Belloni]

On 15/12/2021 15:42:05-0700, Joel Daniels wrote:
> Measuring RTC drift is hard. The standard PC RTC has only one second
> resolution so you have to wait for the "edge" of a tick and measure
> drift over an extended period of time.

Having a one second resolution and measuring drift is orthogonal. The
proper way to read the time from an RTC is not to read its time but to
program an alarm or an update interrupt.

> If you have some NTP daemon
> slewing your system clock while you try to measure RTC drift then
> you will get garbage.

Yeah, the issue is that CLOCK_MONOTONIC is affected by adjtime.

> If your motherboard gets hot enough then your
> RTC will run at a different rate while the machine is on than while
> it is off.

Indeed, you get about 1ppm per degree Celsius on a typical quartz. the
farther away you get from 25°C, the worse it gets.
You also lose about 5ppm per year due to aging.

> 
> I know of three programs that measure RTC drift today:
> 
>   # hwclock: you must use it to set the RTC twice, the second time
>     with the "--update-drift" argument. The manual suggests waiting
>     one day between calls. The drift and offset information is
>     stored in /etc/adjtime. On boot "hwclock --hctosys" will use this
>     to set the system clock correctly.
> 
>   # adjtimex (program not syscall) when run with the "--compare"
>     option. It uses a least squares estimate from multiple samples
>     which by default are each 10 seconds apart.
> 
>   # chrony with the "rtcfile" directive. It tracks the RTC over time
>     to measure its offset and drift similarly to how it tracks the
>     system clock drift. Tracking information is saved into
>       /var/lib/chrony/rtc
>     and can be used (via "chronyd -s") to set the system clock
>     correctly on next boot.
> 
> Any method of measuring the drift is going to need to persist the
> drift coefficient to disk so that it can set the system clock
> correctly on boot. I think it would be best for the kernel to use this
> same coefficient.
> 

I usually use chrony to measure the drift and then get the RTC to
correct its own drift when it is supported by the hardware. The value is
then stored directly on the RTC and nothing more is needed.

Anything that is not using NTP will actually measure the drift
difference between the clocksource and the RTC and I've seen systems
where the RTC was drifting less than the clocksource so you may be
overcompensating in the wrong direction.

Note that on any system using systemd, the kernel hctosys will be used
to the the system time at boot so you may be already more than a second
away from the actual time.

> > Alternatively I'd go very simple and just put the correction factor in
> > a boot argument.
> 
> This works for my use case though it won't be useful to a general
> distro. Would you have one argument being used regardless of where the
> sleep injection was coming from or would you try to tie it to the
> persistent clock and/or a specific RTC?

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: Time keeping while suspended in the presence of persistent clock drift

[Alexandre Belloni]

On 15/12/2021 15:26:28-0800, John Stultz wrote:
> > Any method of measuring the drift is going to need to persist the
> > drift coefficient to disk so that it can set the system clock
> > correctly on boot. I think it would be best for the kernel to use this
> > same coefficient.
> 
> My initial thought was for the rtc class layer to do the estimation
> internally against the system time (assuming it was NTP corrected) to
> try establish a close enough correction factor as the system was up
> and running, but you're right that would be reset on every reboot, and
> with second granular RTCs accurate error calculations will take awhile
> (though polling for the second-edge might work well enough, but would
> be cpu heavy for a background task).
> 
> Maybe that's a good enough argument for having an ADJ ioctl added to
> the rtc chardev?
> 

Then why not got for the correction software emulation? that would avoid
duplicating interfaces and we'd avoid to use it on RTCs with hardware
support.

> But it also seems to suggest that if chronyd already has all this
> logic in userland, maybe having it calculate and call settimeofday()
> on resume from userland instead of pushing half of that logic into the
> kernel?

My suggestion would leave the correction calculation to userspace which
is definitively where it should stay.

> 
> > > Alternatively I'd go very simple and just put the correction factor in
> > > a boot argument.
> >
> > This works for my use case though it won't be useful to a general
> > distro. Would you have one argument being used regardless of where the
> > sleep injection was coming from or would you try to tie it to the
> > persistent clock and/or a specific RTC?
> 
> I agree it is an important thing to consider how to generalize this
> for common use (which is why I prefer the approach that works
> *without* any distro integration).
> 
> But it's also important to consider if the added complexity is
> *really* needed in the general case.
> 
> thanks
> -john

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: Time keeping while suspended in the presence of persistent clock drift

[Joel Daniels]

>>> Any method of measuring the drift is going to need to persist the
>>> drift coefficient to disk so that it can set the system clock
>>> correctly on boot. I think it would be best for the kernel to use this
>>> same coefficient.
>> 
>> My initial thought was for the rtc class layer to do the estimation
>> internally against the system time (assuming it was NTP corrected) to
>> try establish a close enough correction factor as the system was up
>> and running, but you're right that would be reset on every reboot, and
>> with second granular RTCs accurate error calculations will take awhile
>> (though polling for the second-edge might work well enough, but would
>> be cpu heavy for a background task).
>> 
>> Maybe that's a good enough argument for having an ADJ ioctl added to
>> the rtc chardev?
>> 
>
> Then why not got for the correction software emulation? that would avoid
> duplicating interfaces and we'd avoid to use it on RTCs with hardware
> support.
>

Emulated drift correction would affect the time reported by the RTC
wouldn't it? If I want to get a drift-adjusted time using hwclock
I would need it to use /etc/adjtime to adjust for RTC drift until
boot but then trust the kernel to adjust for RTC drift after boot?

If the software emulation ever wrote back a new value to the RTC then
hwclock would have no chance of working since (during boot) it needs
to know the last time the RTC was set.

>> But it also seems to suggest that if chronyd already has all this
>> logic in userland, maybe having it calculate and call settimeofday()
>> on resume from userland instead of pushing half of that logic into the
>> kernel?
>
> My suggestion would leave the correction calculation to userspace which
> is definitively where it should stay.

So this is what I am setting up now and it kind of works. By the
time a time daemon can do anything the system clock is already several
seconds in the future and other userspace programs may have already
observed this wrong value so calling settimeofday() is dangerous.
Instead I use adjtimex to set the ADJ_TICK parameter in order to slew
the system clock aggressively (30,000 ppm or more) so as to fix the
clock quickly.

I do not like having the system clock running 30,000 ppm too slowly
for a couple of minutes after a resume but I can live with it if you
think an in-kernel solution is not appropriate. The default chrony
config uses a slew rate as large as 83,000 ppm so maybe I am too
nervous about this.

Also, if the time daemon dies (e.g. it gets killed by the OOM killer)
before it has a chance to reset ADJ_TICK then the system clock will
run 30,000 ppm too slowly forever! I'd rather use ADJ_OFFSET_SINGLESHOT
to avoid this but ADJ_OFFSET_SINGLESHOT will slew the clock at only
500 ppm and it will take hours before the time is correct.

>>>> Alternatively I'd go very simple and just put the correction factor in
>>>> a boot argument.
>>>
>>> This works for my use case though it won't be useful to a general
>>> distro. Would you have one argument being used regardless of where the
>>> sleep injection was coming from or would you try to tie it to the
>>> persistent clock and/or a specific RTC?