kernel/time/ntp.c | 10 +++++----- kernel/time/ntp_internal.h | 4 ++-- kernel/time/timekeeping.c | 7 ++++--- 3 files changed, 11 insertions(+), 10 deletions(-)
Follow-up of commit 35b603f8a78b ("ntp: Make sure RTC is synchronized
when time goes backwards").
sync_hw_clock() is normally called every 11 minutes when time is
synchronized. This issue is that this periodic timer uses the REALTIME
clock, so when time moves backwards, the timer expires late.
If the timer expires late, which can be days later, the RTC will no longer
be updated, which is an issue if the device is abruptly powered OFF during
this period. When the device will restart (when powered ON), it will have
the date prior to the time jump.
This follow-up handles all kernel API (syscall) that can trigger a time
jump. Cancel periodic timer on any time jump, if and only if STA_UNSYNC
flag was previously set (net_clear() was called).
The timer will be relaunched at the end of ntp_notify_cmos_timer() if
NTP is synced again somehow: This is possible since stopping the timer is
done outside of a locked section. Otherwise the timer will be relaunched
later when NTP is synced. This way, when the time is synchronized again,
the RTC is updated after less than 2 seconds.
Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
---
kernel/time/ntp.c | 10 +++++-----
kernel/time/ntp_internal.h | 4 ++--
kernel/time/timekeeping.c | 7 ++++---
3 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 802b336f4b8c..b296c71af09b 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -660,14 +660,14 @@ static void sync_hw_clock(struct work_struct *work)
sched_sync_hw_clock(offset_nsec, res != 0);
}
-void ntp_notify_cmos_timer(bool offset_set)
+void ntp_notify_cmos_timer(bool ntp_was_cleared)
{
/*
- * If the time jumped (using ADJ_SETOFFSET) cancels sync timer,
- * which may have been running if the time was synchronized
- * prior to the ADJ_SETOFFSET call.
+ * If time jumped (clock set), and if ntp_clear() was called,
+ * cancels sync timer, which may have been running if time was
+ * previously synchronized.
*/
- if (offset_set)
+ if (ntp_was_cleared)
hrtimer_cancel(&sync_hrtimer);
/*
diff --git a/kernel/time/ntp_internal.h b/kernel/time/ntp_internal.h
index 5a633dce9057..0615ed904119 100644
--- a/kernel/time/ntp_internal.h
+++ b/kernel/time/ntp_internal.h
@@ -14,9 +14,9 @@ extern int __do_adjtimex(struct __kernel_timex *txc,
extern void __hardpps(const struct timespec64 *phase_ts, const struct timespec64 *raw_ts);
#if defined(CONFIG_GENERIC_CMOS_UPDATE) || defined(CONFIG_RTC_SYSTOHC)
-extern void ntp_notify_cmos_timer(bool offset_set);
+extern void ntp_notify_cmos_timer(bool ntp_was_cleared);
#else
-static inline void ntp_notify_cmos_timer(bool offset_set) { }
+static inline void ntp_notify_cmos_timer(bool ntp_was_cleared) { }
#endif
#endif /* _LINUX_NTP_INTERNAL_H */
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 7e6f409bf311..602775aa24c7 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1472,6 +1472,7 @@ int do_settimeofday64(const struct timespec64 *ts)
/* Signal hrtimers about time change */
clock_was_set(CLOCK_SET_WALL);
+ ntp_notify_cmos_timer(true);
if (!ret) {
audit_tk_injoffset(ts_delta);
@@ -1522,6 +1523,7 @@ static int timekeeping_inject_offset(const struct timespec64 *ts)
/* Signal hrtimers about time change */
clock_was_set(CLOCK_SET_WALL);
+ ntp_notify_cmos_timer(true);
return ret;
}
@@ -1897,6 +1899,7 @@ void timekeeping_inject_sleeptime64(const struct timespec64 *delta)
/* Signal hrtimers about time change */
clock_was_set(CLOCK_SET_WALL | CLOCK_SET_BOOT);
+ ntp_notify_cmos_timer(true);
}
#endif
@@ -2553,7 +2556,6 @@ int do_adjtimex(struct __kernel_timex *txc)
{
struct timekeeper *tk = &tk_core.timekeeper;
struct audit_ntp_data ad;
- bool offset_set = false;
bool clock_set = false;
struct timespec64 ts;
unsigned long flags;
@@ -2576,7 +2578,6 @@ int do_adjtimex(struct __kernel_timex *txc)
if (ret)
return ret;
- offset_set = delta.tv_sec != 0;
audit_tk_injoffset(delta);
}
@@ -2610,7 +2611,7 @@ int do_adjtimex(struct __kernel_timex *txc)
if (clock_set)
clock_was_set(CLOCK_SET_WALL);
- ntp_notify_cmos_timer(offset_set);
+ ntp_notify_cmos_timer(false);
return ret;
}
--
2.46.1On Sun, Oct 06 2024 at 18:58, Benjamin ROBIN wrote:
> @@ -2553,7 +2556,6 @@ int do_adjtimex(struct __kernel_timex *txc)
> {
> struct timekeeper *tk = &tk_core.timekeeper;
> struct audit_ntp_data ad;
> - bool offset_set = false;
> bool clock_set = false;
> struct timespec64 ts;
> unsigned long flags;
> @@ -2576,7 +2578,6 @@ int do_adjtimex(struct __kernel_timex *txc)
> if (ret)
> return ret;
>
> - offset_set = delta.tv_sec != 0;
> audit_tk_injoffset(delta);
> }
>
> @@ -2610,7 +2611,7 @@ int do_adjtimex(struct __kernel_timex *txc)
> if (clock_set)
> clock_was_set(CLOCK_SET_WALL);
>
> - ntp_notify_cmos_timer(offset_set);
> + ntp_notify_cmos_timer(false);
Can you please add a comment why this does not need to cancel the
hrtimer? It took me a few looks to validate that this is correct. Six
months down the road we both forgot about it. :)
Also can you please rebase this on top of
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers/core
which has a larger change of the NTP code.
Thanks,
tglx
On Wed, Oct 16, 2024 at 12:28:31AM +0200, Thomas Gleixner wrote: Sorry for replying with such delay, I was not really available... > > @@ -2610,7 +2611,7 @@ int do_adjtimex(struct __kernel_timex *txc) > > if (clock_set) > > clock_was_set(CLOCK_SET_WALL); > > > > - ntp_notify_cmos_timer(offset_set); > > + ntp_notify_cmos_timer(false); > > Can you please add a comment why this does not need to cancel the > hrtimer? It took me a few looks to validate that this is correct. Six > months down the road we both forgot about it. :) I tried to add a comment that explain it in the v2. I hope this is "good enough". From my understanding of why this is not needed to cancel NTP sync hrtimer here: Because NTP sync flag was not cleared since (either): - The frequency was adjusted, which has no impact on NTP sync hrtimer. - TAI offset was adjusted, and it should not have any impact on REALTIME. - Leap second was handled, which does not require to restart NTP sync hrtimer. > Also can you please rebase this on top of > > git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers/core > > which has a larger change of the NTP code. I did rebase on top of this branch. I will send the v2 patch in a separate email. Thanks, Benjamin
© 2016 - 2024 Red Hat, Inc.