User-Agent: Cyrus-JMAP/3.5.0-alpha0-4524-g5e5d2efdba-fm-20211214.001-g5e5d2efd
Mime-Version: 1.0
Message-Id: <273e60aa-d25c-4f29-a31a-97ea6c19ad67@www.fastmail.com>
In-Reply-To: <Ybp76D62Le2aEc5R@piout.net>
References: <5af5d2a5-767c-d313-3be6-cb6f426f1980@sent.com>
 <b074f506-2568-4506-9557-4a9bc9cbea83@www.fastmail.com> <87wnkbuuuz.ffs@tglx>
 <4bb238e1-e8fa-44e6-9f5e-d047d1d4a892@www.fastmail.com> <8735mvthk6.ffs@tglx>
 <2ab24da8-e37d-426a-9500-b7541d21f8a3@www.fastmail.com>
 <CALAqxLXf6TmOn_jCOv68oop=4On+CN-p_KkN-70BDt9OjQhzUw@mail.gmail.com>
 <83deaa79-39eb-4fd7-ad80-9d233fd6fdbb@www.fastmail.com>
 <CALAqxLX795pURb_aJTEAdq80LGiY=br88A+R3TN3HQh+HtS85Q@mail.gmail.com>
 <Ybp76D62Le2aEc5R@piout.net>
Date:   Wed, 15 Dec 2021 17:09:46 -0700
From:   "Joel Daniels" <jdaniels@sent.com>
To:     "Alexandre Belloni" <alexandre.belloni@bootlin.com>,
        "John Stultz" <john.stultz@linaro.org>
Cc:     "Thomas Gleixner" <tglx@linutronix.de>,
        "Stephen Boyd" <sboyd@kernel.org>, linux-kernel@vger.kernel.org,
        "Alessandro Zummo" <a.zummo@towertech.it>,
        linux-rtc@vger.kernel.org, x86@kernel.org
Subject: Re: Time keeping while suspended in the presence of persistent clock drift
Content-Type: text/plain
Precedence: bulk

>>> 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?