[PATCH 0/5 v2] Fixes for leapsecond expiring early ABS_TIME CLOCK_REALTIME timers

[PATCH 0/5 v2] Fixes for leapsecond expiring early ABS_TIME CLOCK_REALTIME timers

[John Stultz]

So this is a second round at trying to address the issue, trying
to integrate feedback from Ingo and Thomas, trying to simplify
what I can. I've also split out the changes so each can be
more easily reviewed. Its still not tiny, but its simpler.

This series is against tip/timers/core, and the first patch isn't
strictly related but is a fix that is needed in tip/timers/core.

As Prarit reported here:
https://lkml.org/lkml/2015/5/27/458

Since the leapsecond is applied at timer tick time, and not
the actual second edge, ABS_TIME CLOCK_REALTIME timers set for
right after the leapsecond could fire a second early, since
some timers may be expired before we trigger the timekeeping
timer, which then applies the leapsecond.

Thus this patch series tries to address this issue, including
extending the leap-a-day test to catch this problem, as well
as other relevant fixups I found while working on the code.

This series has only had limited testing, so I wanted to send
it out for initial review and comment. Folks can grab this tree
via git for testing here:
https://git.linaro.org/people/john.stultz/linux.git dev/early-leap-timer

Thoughts and feedback would be appreciated!
thanks
-john

Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Jan Kara <jack@suse.cz>
Cc: Jiri Bohac <jbohac@suse.cz>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Shuah Khan <shuahkh@osg.samsung.com>


John Stultz (5):
  time: Move clock_was_set_seq update to before we update the
    shadow-timekeeper
  ntp: Introduce and use SECS_PER_DAY macro instead of 86400
  time: Do leapsecond adjustment to avoid early timer expirations
  ntp: Do leapsecond adjustment in adjtimex read path
  selftests: timers: Add leap-second timer edge testing to leap-a-day.c

 include/linux/time64.h                      |  1 +
 include/linux/timekeeper_internal.h         |  2 +
 kernel/time/ntp.c                           | 61 ++++++++++++++++++++---
 kernel/time/ntp_internal.h                  |  1 +
 kernel/time/timekeeping.c                   | 35 +++++++++++--
 tools/testing/selftests/timers/leap-a-day.c | 76 +++++++++++++++++++++++++++--
 6 files changed, 160 insertions(+), 16 deletions(-)

-- 
1.9.1

[PATCH 1/5] time: Move clock_was_set_seq update to before we update the shadow-timekeeper

[John Stultz]

It was reported that 868a3e915f7f5eba (hrtimer: Make offset
update smarter) was causing timer problems after suspend/resume.

The problem with that change is the modification to
clock_was_set_seq in timekeeping_update is done prior to
mirroring the time state to the shadow-timekeeper. Thus the
next time we do update_wall_time() the updated sequence is
overwritten by whats in the shadow copy.

This patch moves the shadow-timekeeper mirroring to the end
of the function, after all updates have been made, so all data
is kept in sync.

(This patch also affects the update_fast_timekeeper calls which
were also problematically done prior to the mirroring).

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Preeti U Murthy <preeti@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Marcelo Tosatti <mtosatti@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Reported-by: Jeremiah Mahler <jmmahler@gmail.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 kernel/time/timekeeping.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 90ed5db..849b932 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -585,15 +585,19 @@ static void timekeeping_update(struct timekeeper *tk, unsigned int action)
 	update_vsyscall(tk);
 	update_pvclock_gtod(tk, action & TK_CLOCK_WAS_SET);
 
-	if (action & TK_MIRROR)
-		memcpy(&shadow_timekeeper, &tk_core.timekeeper,
-		       sizeof(tk_core.timekeeper));
-
 	update_fast_timekeeper(&tk->tkr_mono, &tk_fast_mono);
 	update_fast_timekeeper(&tk->tkr_raw,  &tk_fast_raw);
 
 	if (action & TK_CLOCK_WAS_SET)
 		tk->clock_was_set_seq++;
+	/*
+	 * The mirroring of the data to the shadow-timekeeper needs
+	 * to happen last here to ensure we don't over-write the
+	 * timekeeper structure on the next update with stale data
+	 */
+	if (action & TK_MIRROR)
+		memcpy(&shadow_timekeeper, &tk_core.timekeeper,
+		       sizeof(tk_core.timekeeper));
 }
 
 /**
-- 
1.9.1

[PATCH 2/5] ntp: Introduce and use SECS_PER_DAY macro instead of 86400

[John Stultz]

Currently the leapsecond logic uses what looks like magic values.

Improve this by defining SECS_PER_DAY and using that macro
to make the logic more clear.

Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Jan Kara <jack@suse.cz>
Cc: Jiri Bohac <jbohac@suse.cz>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 kernel/time/ntp.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 7a68100..7aa2161 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -35,6 +35,7 @@ unsigned long			tick_nsec;
 static u64			tick_length;
 static u64			tick_length_base;
 
+#define SECS_PER_DAY		86400
 #define MAX_TICKADJ		500LL		/* usecs */
 #define MAX_TICKADJ_SCALED \
 	(((MAX_TICKADJ * NSEC_PER_USEC) << NTP_SCALE_SHIFT) / NTP_INTERVAL_FREQ)
@@ -390,7 +391,7 @@ int second_overflow(unsigned long secs)
 	case TIME_INS:
 		if (!(time_status & STA_INS))
 			time_state = TIME_OK;
-		else if (secs % 86400 == 0) {
+		else if (secs % SECS_PER_DAY == 0) {
 			leap = -1;
 			time_state = TIME_OOP;
 			printk(KERN_NOTICE
@@ -400,7 +401,7 @@ int second_overflow(unsigned long secs)
 	case TIME_DEL:
 		if (!(time_status & STA_DEL))
 			time_state = TIME_OK;
-		else if ((secs + 1) % 86400 == 0) {
+		else if ((secs + 1) % SECS_PER_DAY == 0) {
 			leap = 1;
 			time_state = TIME_WAIT;
 			printk(KERN_NOTICE
-- 
1.9.1

[PATCH 3/5] time: Do leapsecond adjustment to avoid early timer expirations

[John Stultz]

Currently, leapsecond adjustments are done at tick time.

As a result, the leapsecond was applied at the first timer
tick *after* the leapsecond (~1-10ms late depending on HZ),
rather then exactly on the second edge.

This was in part historical from back when we were always
tick based, but correcting this since has been avoided since
it adds extra conditional checks in the gettime fastpath,
which has performance overhead.

However, it was recently pointed out that ABS_TIME
CLOCK_REALTIME timers set for right after the leapsecond
could fire a second early, since some timers may be expired
before we trigger the timekeeping timer, which then applies
the leapsecond.

This isn't quite as bad as it sounds, since behaviorally
it is similar to what is possible w/ ntpd made leapsecond
adjustments done w/o using the kernel discipline. Where
due to latencies, timers may fire just prior to the
settimeofday call. (Also, one should note that all
applications using CLOCK_REALTIME timers should always be
careful, since they are prone to quirks from settimeofday()
disturbances.)

However, the purpose of having the kernel do the leap adjustment
is to avoid such latencies, so I think this is worth fixing.

So in order to properly keep those timers from firing a second
early, this patch modifies the ntp and timekeeping logic so
that we keep enough state so that the update_base_offsets_now
accessor, which provides the hrtimer core the current time,
can check and apply the leapsecond adjustment on the second
edge. This prevents the hrtimer core from expiring timers too
early.

This patch does not modify any other time read path, so no
additional overhead is incurred. However, this also means
that the leap-second continues to be applied at tick time
for all other read-paths.

Apologies to Richard Cochran, who pushed for similar changes
years ago, which I resisted due to the concerns about the
performance overhead.

While I suspect this isn't extremely critical, folks who
care about strict leap-second correctness will likely
want to watch this. Potentially a -stable candidate
eventually.

Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Jan Kara <jack@suse.cz>
Cc: Jiri Bohac <jbohac@suse.cz>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Shuah Khan <shuahkh@osg.samsung.com>
Originally-suggested-by: Richard Cochran <richardcochran@gmail.com>
Reported-by: Daniel Bristot de Oliveira <bristot@redhat.com>
Reported-by: Prarit Bhargava <prarit@redhat.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>

New in v2:
* Removed redundent state update calls
* Simplified logic to only handle leap insertion
* Reduced scope to only address hrtimer expiration issue
* Incorporated logic suggestions from Thomas.
---
 include/linux/time64.h              |  1 +
 include/linux/timekeeper_internal.h |  2 ++
 kernel/time/ntp.c                   | 42 ++++++++++++++++++++++++++++++-------
 kernel/time/ntp_internal.h          |  1 +
 kernel/time/timekeeping.c           | 23 +++++++++++++++++++-
 5 files changed, 61 insertions(+), 8 deletions(-)

diff --git a/include/linux/time64.h b/include/linux/time64.h
index 12d4e82..77b5df2 100644
--- a/include/linux/time64.h
+++ b/include/linux/time64.h
@@ -29,6 +29,7 @@ struct timespec64 {
 #define FSEC_PER_SEC	1000000000000000LL
 
 /* Located here for timespec[64]_valid_strict */
+#define TIME64_MAX			((s64)~((u64)1 << 63))
 #define KTIME_MAX			((s64)~((u64)1 << 63))
 #define KTIME_SEC_MAX			(KTIME_MAX / NSEC_PER_SEC)
 
diff --git a/include/linux/timekeeper_internal.h b/include/linux/timekeeper_internal.h
index e1f5a11..2524722 100644
--- a/include/linux/timekeeper_internal.h
+++ b/include/linux/timekeeper_internal.h
@@ -50,6 +50,7 @@ struct tk_read_base {
  * @offs_tai:		Offset clock monotonic -> clock tai
  * @tai_offset:		The current UTC to TAI offset in seconds
  * @clock_was_set_seq:	The sequence number of clock was set events
+ * @next_leap_ktime:	CLOCK_MONOTONIC time value of a pending leap-second
  * @raw_time:		Monotonic raw base time in timespec64 format
  * @cycle_interval:	Number of clock cycles in one NTP interval
  * @xtime_interval:	Number of clock shifted nano seconds in one NTP
@@ -90,6 +91,7 @@ struct timekeeper {
 	ktime_t			offs_tai;
 	s32			tai_offset;
 	unsigned int		clock_was_set_seq;
+	ktime_t			next_leap_ktime;
 	struct timespec64	raw_time;
 
 	/* The following members are for timekeeping internal use */
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 7aa2161..033743e 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -77,6 +77,9 @@ static long			time_adjust;
 /* constant (boot-param configurable) NTP tick adjustment (upscaled)	*/
 static s64			ntp_tick_adj;
 
+/* second value of the next pending leapsecond, or TIME64_MAX if no leap */
+static time64_t			ntp_next_leap_sec = TIME64_MAX;
+
 #ifdef CONFIG_NTP_PPS
 
 /*
@@ -350,6 +353,7 @@ void ntp_clear(void)
 	tick_length	= tick_length_base;
 	time_offset	= 0;
 
+	ntp_next_leap_sec = TIME64_MAX;
 	/* Clear PPS state variables */
 	pps_clear();
 }
@@ -360,6 +364,21 @@ u64 ntp_tick_length(void)
 	return tick_length;
 }
 
+/**
+ * ntp_get_next_leap - Returns the next leapsecond in CLOCK_REALTIME ktime_t
+ *
+ * Provides the time of the next leapsecond against CLOCK_REALTIME in
+ * a ktime_t format. Returns KTIME_MAX if no leapsecond is pending.
+ */
+ktime_t ntp_get_next_leap(void)
+{
+	ktime_t ret;
+
+	if ((time_state == TIME_INS) && (time_status & STA_INS))
+		return ktime_set(ntp_next_leap_sec, 0);
+	ret.tv64 = KTIME_MAX;
+	return ret;
+}
 
 /*
  * this routine handles the overflow of the microsecond field
@@ -383,15 +402,21 @@ int second_overflow(unsigned long secs)
 	 */
 	switch (time_state) {
 	case TIME_OK:
-		if (time_status & STA_INS)
+		if (time_status & STA_INS) {
 			time_state = TIME_INS;
-		else if (time_status & STA_DEL)
+			ntp_next_leap_sec = secs + SECS_PER_DAY -
+						(secs % SECS_PER_DAY);
+		} else if (time_status & STA_DEL) {
 			time_state = TIME_DEL;
+			ntp_next_leap_sec = secs + SECS_PER_DAY -
+						 ((secs+1) % SECS_PER_DAY);
+		}
 		break;
 	case TIME_INS:
-		if (!(time_status & STA_INS))
+		if (!(time_status & STA_INS)) {
+			ntp_next_leap_sec = TIME64_MAX;
 			time_state = TIME_OK;
-		else if (secs % SECS_PER_DAY == 0) {
+		} else if (secs % SECS_PER_DAY == 0) {
 			leap = -1;
 			time_state = TIME_OOP;
 			printk(KERN_NOTICE
@@ -399,19 +424,21 @@ int second_overflow(unsigned long secs)
 		}
 		break;
 	case TIME_DEL:
-		if (!(time_status & STA_DEL))
+		if (!(time_status & STA_DEL)) {
+			ntp_next_leap_sec = TIME64_MAX;
 			time_state = TIME_OK;
-		else if ((secs + 1) % SECS_PER_DAY == 0) {
+		} else if ((secs + 1) % SECS_PER_DAY == 0) {
 			leap = 1;
+			ntp_next_leap_sec = TIME64_MAX;
 			time_state = TIME_WAIT;
 			printk(KERN_NOTICE
 				"Clock: deleting leap second 23:59:59 UTC\n");
 		}
 		break;
 	case TIME_OOP:
+		ntp_next_leap_sec = TIME64_MAX;
 		time_state = TIME_WAIT;
 		break;
-
 	case TIME_WAIT:
 		if (!(time_status & (STA_INS | STA_DEL)))
 			time_state = TIME_OK;
@@ -548,6 +575,7 @@ static inline void process_adj_status(struct timex *txc, struct timespec64 *ts)
 	if ((time_status & STA_PLL) && !(txc->status & STA_PLL)) {
 		time_state = TIME_OK;
 		time_status = STA_UNSYNC;
+		ntp_next_leap_sec = TIME64_MAX;
 		/* restart PPS frequency calibration */
 		pps_reset_freq_interval();
 	}
diff --git a/kernel/time/ntp_internal.h b/kernel/time/ntp_internal.h
index bbd102a..6543050 100644
--- a/kernel/time/ntp_internal.h
+++ b/kernel/time/ntp_internal.h
@@ -5,6 +5,7 @@ extern void ntp_init(void);
 extern void ntp_clear(void);
 /* Returns how long ticks are at present, in ns / 2^NTP_SCALE_SHIFT. */
 extern u64 ntp_tick_length(void);
+extern ktime_t ntp_get_next_leap(void);
 extern int second_overflow(unsigned long secs);
 extern int ntp_validate_timex(struct timex *);
 extern int __do_adjtimex(struct timex *, struct timespec64 *, s32 *);
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 849b932..5d67ffb 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -540,6 +540,17 @@ int pvclock_gtod_unregister_notifier(struct notifier_block *nb)
 EXPORT_SYMBOL_GPL(pvclock_gtod_unregister_notifier);
 
 /*
+ * tk_update_leap_state - helper to update the next_leap_ktime
+ */
+static inline void tk_update_leap_state(struct timekeeper *tk)
+{
+	tk->next_leap_ktime = ntp_get_next_leap();
+	if (tk->next_leap_ktime.tv64 != KTIME_MAX)
+		/* Convert to monotonic time */
+		tk->next_leap_ktime = ktime_sub(tk->next_leap_ktime, tk->offs_real);
+}
+
+/*
  * Update the ktime_t based scalar nsec members of the timekeeper
  */
 static inline void tk_update_ktime_data(struct timekeeper *tk)
@@ -580,6 +591,7 @@ static void timekeeping_update(struct timekeeper *tk, unsigned int action)
 		ntp_clear();
 	}
 
+	tk_update_leap_state(tk);
 	tk_update_ktime_data(tk);
 
 	update_vsyscall(tk);
@@ -1956,15 +1968,22 @@ ktime_t ktime_get_update_offsets_now(unsigned int *cwsseq, ktime_t *offs_real,
 
 		base = tk->tkr_mono.base;
 		nsecs = timekeeping_get_ns(&tk->tkr_mono);
+		base = ktime_add_ns(base, nsecs);
+
 		if (*cwsseq != tk->clock_was_set_seq) {
 			*cwsseq = tk->clock_was_set_seq;
 			*offs_real = tk->offs_real;
 			*offs_boot = tk->offs_boot;
 			*offs_tai = tk->offs_tai;
 		}
+
+		/* Handle leapsecond insertion adjustments */
+		if (unlikely(base.tv64 >= tk->next_leap_ktime.tv64))
+			*offs_real = ktime_sub(tk->offs_real, ktime_set(1, 0));
+
 	} while (read_seqcount_retry(&tk_core.seq, seq));
 
-	return ktime_add_ns(base, nsecs);
+	return base;
 }
 
 /**
@@ -2006,6 +2025,8 @@ int do_adjtimex(struct timex *txc)
 		__timekeeping_set_tai_offset(tk, tai);
 		timekeeping_update(tk, TK_MIRROR | TK_CLOCK_WAS_SET);
 	}
+	tk_update_leap_state(tk);
+
 	write_seqcount_end(&tk_core.seq);
 	raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
 
-- 
1.9.1

[PATCH 4/5] ntp: Do leapsecond adjustment in adjtimex read path

[John Stultz]

Since the leapsecond is applied at tick-time, this
means there is a small window of time at the start
of a leap-second where we cross into the next second
before applying the leap.

This patch modified adjtimex so that the leap-second
is applied on the second edge. Providing more correct
leapsecond behavior.

This does make it so that adjtimex()'s returned time
values can be inconsistent with time values read from
gettimeofday() or clock_gettime(CLOCK_REALTIME,...)
for a brief period of one tick at the leapsecond.
However, those other interfaces do not provide the
TIME_OOP time_state return that adjtimex() provides, which
allows the leapsecond to be properly represented. They
instead only see a time discontinuity, and cannot tell the
first 23:59:59 from the repeated 23:59:59 leap second.

This seems like a reasonable tradeoff given clock_gettime()
/ gettimeofday() cannot properly represent a leapsecond,
and users likely care more about performance, while folks
who are using adjtimex() more likely care about leap-second
correctness.

Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Jan Kara <jack@suse.cz>
Cc: Jiri Bohac <jbohac@suse.cz>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 kernel/time/ntp.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 033743e..fb4d98c 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -740,6 +740,24 @@ int __do_adjtimex(struct timex *txc, struct timespec64 *ts, s32 *time_tai)
 	if (!(time_status & STA_NANO))
 		txc->time.tv_usec /= NSEC_PER_USEC;
 
+	/* Handle leapsec adjustments */
+	if (unlikely(ts->tv_sec >= ntp_next_leap_sec)) {
+		if ((time_state == TIME_INS) && (time_status & STA_INS)) {
+			result = TIME_OOP;
+			txc->tai++;
+			txc->time.tv_sec--;
+		}
+		if ((time_state == TIME_DEL) && (time_status & STA_DEL)) {
+			result = TIME_WAIT;
+			txc->tai--;
+			txc->time.tv_sec++;
+		}
+		if ((time_state == TIME_OOP) &&
+					(ts->tv_sec == ntp_next_leap_sec)) {
+			result = TIME_WAIT;
+		}
+	}
+
 	return result;
 }
 
-- 
1.9.1

[PATCH 5/5] selftests: timers: Add leap-second timer edge testing to leap-a-day.c

[John Stultz]

Prarit reported an issue w/ timers around the leapsecond, where
a timer set for Midnight UTC (00:00:00) might fire a second early
right before the leapsecond (23:59:60 - though it appears as a
repeated 23:59:59) is applied.

So I've updated the leap-a-day.c test to integrate a similar
test, where we set a timer and check if it triggers at the
right time, and if the ntp state transition is managed
properly.

Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Jan Kara <jack@suse.cz>
Cc: Jiri Bohac <jbohac@suse.cz>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Shuah Khan <shuahkh@osg.samsung.com>
Reported-by: Daniel Bristot de Oliveira <bristot@redhat.com>
Reported-by: Prarit Bhargava <prarit@redhat.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 tools/testing/selftests/timers/leap-a-day.c | 76 +++++++++++++++++++++++++++--
 1 file changed, 72 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/timers/leap-a-day.c b/tools/testing/selftests/timers/leap-a-day.c
index b8272e6..331c4f7 100644
--- a/tools/testing/selftests/timers/leap-a-day.c
+++ b/tools/testing/selftests/timers/leap-a-day.c
@@ -44,6 +44,7 @@
 #include <time.h>
 #include <sys/time.h>
 #include <sys/timex.h>
+#include <sys/errno.h>
 #include <string.h>
 #include <signal.h>
 #include <unistd.h>
@@ -63,6 +64,9 @@ static inline int ksft_exit_fail(void)
 #define NSEC_PER_SEC 1000000000ULL
 #define CLOCK_TAI 11
 
+time_t next_leap;
+int error_found;
+
 /* returns 1 if a <= b, 0 otherwise */
 static inline int in_order(struct timespec a, struct timespec b)
 {
@@ -134,6 +138,34 @@ void handler(int unused)
 	exit(0);
 }
 
+void sigalarm(int signo)
+{
+	struct timex tx;
+	char buf[26];
+	int ret;
+
+	tx.modes = 0;
+	ret = adjtimex(&tx);
+
+	ctime_r(&tx.time.tv_sec, buf);
+	buf[strlen(buf)-1] = 0; /*remove trailing\n */
+	printf("%s + %6ld us (%i)\t%s - TIMER FIRED\n",
+					buf,
+					tx.time.tv_usec,
+					tx.tai,
+					time_state_str(ret));
+
+	if (tx.time.tv_sec < next_leap) {
+		printf("Error: Early timer expiration!\n");
+		error_found = 1;
+	}
+	if (ret != TIME_WAIT) {
+		printf("Error: Incorrect NTP state?\n");
+		error_found = 1;
+	}
+}
+
+
 /* Test for known hrtimer failure */
 void test_hrtimer_failure(void)
 {
@@ -144,12 +176,19 @@ void test_hrtimer_failure(void)
 	clock_nanosleep(CLOCK_REALTIME, TIMER_ABSTIME, &target, NULL);
 	clock_gettime(CLOCK_REALTIME, &now);
 
-	if (!in_order(target, now))
+	if (!in_order(target, now)) {
 		printf("ERROR: hrtimer early expiration failure observed.\n");
+		error_found = 1;
+	}
 }
 
 int main(int argc, char **argv)
 {
+	timer_t tm1;
+	struct itimerspec its1;
+	struct sigevent se;
+	struct sigaction act;
+	int signum = SIGRTMAX;
 	int settime = 0;
 	int tai_time = 0;
 	int insert = 1;
@@ -191,6 +230,12 @@ int main(int argc, char **argv)
 	signal(SIGINT, handler);
 	signal(SIGKILL, handler);
 
+	/* Set up timer signal handler: */
+	sigfillset(&act.sa_mask);
+	act.sa_flags = 0;
+	act.sa_handler = sigalarm;
+	sigaction(signum, &act, NULL);
+
 	if (iterations < 0)
 		printf("This runs continuously. Press ctrl-c to stop\n");
 	else
@@ -201,7 +246,7 @@ int main(int argc, char **argv)
 		int ret;
 		struct timespec ts;
 		struct timex tx;
-		time_t now, next_leap;
+		time_t now;
 
 		/* Get the current time */
 		clock_gettime(CLOCK_REALTIME, &ts);
@@ -251,10 +296,27 @@ int main(int argc, char **argv)
 
 		printf("Scheduling leap second for %s", ctime(&next_leap));
 
+		/* Set up timer */
+		printf("Setting timer for %s", ctime(&next_leap));
+		memset(&se, 0, sizeof(se));
+		se.sigev_notify = SIGEV_SIGNAL;
+		se.sigev_signo = signum;
+		se.sigev_value.sival_int = 0;
+		if (timer_create(CLOCK_REALTIME, &se, &tm1) == -1) {
+			printf("Error: timer_create failed\n");
+			return ksft_exit_fail();
+		}
+		its1.it_value.tv_sec = next_leap;
+		its1.it_value.tv_nsec = 0;
+		its1.it_interval.tv_sec = 0;
+		its1.it_interval.tv_nsec = 0;
+		timer_settime(tm1, TIMER_ABSTIME, &its1, NULL);
+
 		/* Wake up 3 seconds before leap */
 		ts.tv_sec = next_leap - 3;
 		ts.tv_nsec = 0;
 
+
 		while (clock_nanosleep(CLOCK_REALTIME, TIMER_ABSTIME, &ts, NULL))
 			printf("Something woke us up, returning to sleep\n");
 
@@ -276,6 +338,7 @@ int main(int argc, char **argv)
 		while (now < next_leap + 2) {
 			char buf[26];
 			struct timespec tai;
+			int ret;
 
 			tx.modes = 0;
 			ret = adjtimex(&tx);
@@ -308,8 +371,13 @@ int main(int argc, char **argv)
 		/* Note if kernel has known hrtimer failure */
 		test_hrtimer_failure();
 
-		printf("Leap complete\n\n");
-
+		printf("Leap complete\n");
+		if (error_found) {
+			printf("Errors observed\n");
+			clear_time_state();
+			return ksft_exit_fail();
+		}
+		printf("\n");
 		if ((iterations != -1) && !(--iterations))
 			break;
 	}
-- 
1.9.1

Re: [PATCH 4/5] ntp: Do leapsecond adjustment in adjtimex read path

[Richard Cochran]

John,

The description is just awful.

On Thu, Jun 11, 2015 at 03:54:56PM -0700, John Stultz wrote:
> Since the leapsecond is applied at tick-time, this
> means there is a small window of time at the start
> of a leap-second where we cross into the next second
> before applying the leap.

First you say the leap second is applied at the tick, ...
 
> This patch modified adjtimex so that the leap-second
> is applied on the second edge. Providing more correct
> leapsecond behavior.

and then you say it is applied on the edge of the second.

Instead, this second paragraph should say:

   This patch modifies adjtimex to apply the leap second
   correction to the returned time value.  Callers of adjtimex
   will observe the leap second occuring exactly on the edge
   of the second.

> This does make it so that adjtimex()'s returned time
> values can be inconsistent with time values read from
> gettimeofday() or clock_gettime(CLOCK_REALTIME,...)
> for a brief period of one tick at the leapsecond.

How about this instead?

   As as a result, adjtimex()'s returned time values will be
   inconsistent with time values read from gettimeofday() or
   clock_gettime(CLOCK_REALTIME,...) for a brief period of one
   tick at the leapsecond.

Thanks,
Richard

[tip:timers/core] time: Move clock_was_set_seq update before updating shadow-timekeeper

[tip-bot for John Stultz]

Commit-ID:  d151832650ed98961a5650e73e85c349ad7839cb
Gitweb:     http://git.kernel.org/tip/d151832650ed98961a5650e73e85c349ad7839cb
Author:     John Stultz <john.stultz@linaro.org>
AuthorDate: Thu, 11 Jun 2015 15:54:53 -0700
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Fri, 12 Jun 2015 10:56:20 +0200

time: Move clock_was_set_seq update before updating shadow-timekeeper

It was reported that 868a3e915f7f5eba (hrtimer: Make offset
update smarter) was causing timer problems after suspend/resume.

The problem with that change is the modification to
clock_was_set_seq in timekeeping_update is done prior to
mirroring the time state to the shadow-timekeeper. Thus the
next time we do update_wall_time() the updated sequence is
overwritten by whats in the shadow copy.

This patch moves the shadow-timekeeper mirroring to the end
of the function, after all updates have been made, so all data
is kept in sync.

(This patch also affects the update_fast_timekeeper calls which
were also problematically done prior to the mirroring).

Reported-and-tested-by: Jeremiah Mahler <jmmahler@gmail.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
Cc: Preeti U Murthy <preeti@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Marcelo Tosatti <mtosatti@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Link: http://lkml.kernel.org/r/1434063297-28657-2-git-send-email-john.stultz@linaro.org
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/timekeeping.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 90ed5db..849b932 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -585,15 +585,19 @@ static void timekeeping_update(struct timekeeper *tk, unsigned int action)
 	update_vsyscall(tk);
 	update_pvclock_gtod(tk, action & TK_CLOCK_WAS_SET);
 
-	if (action & TK_MIRROR)
-		memcpy(&shadow_timekeeper, &tk_core.timekeeper,
-		       sizeof(tk_core.timekeeper));
-
 	update_fast_timekeeper(&tk->tkr_mono, &tk_fast_mono);
 	update_fast_timekeeper(&tk->tkr_raw,  &tk_fast_raw);
 
 	if (action & TK_CLOCK_WAS_SET)
 		tk->clock_was_set_seq++;
+	/*
+	 * The mirroring of the data to the shadow-timekeeper needs
+	 * to happen last here to ensure we don't over-write the
+	 * timekeeper structure on the next update with stale data
+	 */
+	if (action & TK_MIRROR)
+		memcpy(&shadow_timekeeper, &tk_core.timekeeper,
+		       sizeof(tk_core.timekeeper));
 }
 
 /**

[tip:timers/core] ntp: Introduce and use SECS_PER_DAY macro instead of 86400

[tip-bot for John Stultz]

Commit-ID:  90bf361ceae28dee50a584c3dd4c1a96178d982c
Gitweb:     http://git.kernel.org/tip/90bf361ceae28dee50a584c3dd4c1a96178d982c
Author:     John Stultz <john.stultz@linaro.org>
AuthorDate: Thu, 11 Jun 2015 15:54:54 -0700
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Fri, 12 Jun 2015 11:15:49 +0200

ntp: Introduce and use SECS_PER_DAY macro instead of 86400

Currently the leapsecond logic uses what looks like magic values.

Improve this by defining SECS_PER_DAY and using that macro
to make the logic more clear.

Signed-off-by: John Stultz <john.stultz@linaro.org>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Jan Kara <jack@suse.cz>
Cc: Jiri Bohac <jbohac@suse.cz>
Cc: Ingo Molnar <mingo@kernel.org>
Link: http://lkml.kernel.org/r/1434063297-28657-3-git-send-email-john.stultz@linaro.org
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/ntp.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 7a68100..7aa2161 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -35,6 +35,7 @@ unsigned long			tick_nsec;
 static u64			tick_length;
 static u64			tick_length_base;
 
+#define SECS_PER_DAY		86400
 #define MAX_TICKADJ		500LL		/* usecs */
 #define MAX_TICKADJ_SCALED \
 	(((MAX_TICKADJ * NSEC_PER_USEC) << NTP_SCALE_SHIFT) / NTP_INTERVAL_FREQ)
@@ -390,7 +391,7 @@ int second_overflow(unsigned long secs)
 	case TIME_INS:
 		if (!(time_status & STA_INS))
 			time_state = TIME_OK;
-		else if (secs % 86400 == 0) {
+		else if (secs % SECS_PER_DAY == 0) {
 			leap = -1;
 			time_state = TIME_OOP;
 			printk(KERN_NOTICE
@@ -400,7 +401,7 @@ int second_overflow(unsigned long secs)
 	case TIME_DEL:
 		if (!(time_status & STA_DEL))
 			time_state = TIME_OK;
-		else if ((secs + 1) % 86400 == 0) {
+		else if ((secs + 1) % SECS_PER_DAY == 0) {
 			leap = 1;
 			time_state = TIME_WAIT;
 			printk(KERN_NOTICE

[tip:timers/core] time: Prevent early expiry of hrtimers[ CLOCK_REALTIME] at the leap second edge

[tip-bot for John Stultz]

Commit-ID:  833f32d763028c1bb371c64f457788b933773b3e
Gitweb:     http://git.kernel.org/tip/833f32d763028c1bb371c64f457788b933773b3e
Author:     John Stultz <john.stultz@linaro.org>
AuthorDate: Thu, 11 Jun 2015 15:54:55 -0700
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Fri, 12 Jun 2015 11:15:49 +0200

time: Prevent early expiry of hrtimers[CLOCK_REALTIME] at the leap second edge

Currently, leapsecond adjustments are done at tick time. As a result,
the leapsecond was applied at the first timer tick *after* the
leapsecond (~1-10ms late depending on HZ), rather then exactly on the
second edge.

This was in part historical from back when we were always tick based,
but correcting this since has been avoided since it adds extra
conditional checks in the gettime fastpath, which has performance
overhead.

However, it was recently pointed out that ABS_TIME CLOCK_REALTIME
timers set for right after the leapsecond could fire a second early,
since some timers may be expired before we trigger the timekeeping
timer, which then applies the leapsecond.

This isn't quite as bad as it sounds, since behaviorally it is similar
to what is possible w/ ntpd made leapsecond adjustments done w/o using
the kernel discipline. Where due to latencies, timers may fire just
prior to the settimeofday call. (Also, one should note that all
applications using CLOCK_REALTIME timers should always be careful,
since they are prone to quirks from settimeofday() disturbances.)

However, the purpose of having the kernel do the leap adjustment is to
avoid such latencies, so I think this is worth fixing.

So in order to properly keep those timers from firing a second early,
this patch modifies the ntp and timekeeping logic so that we keep
enough state so that the update_base_offsets_now accessor, which
provides the hrtimer core the current time, can check and apply the
leapsecond adjustment on the second edge. This prevents the hrtimer
core from expiring timers too early.

This patch does not modify any other time read path, so no additional
overhead is incurred. However, this also means that the leap-second
continues to be applied at tick time for all other read-paths.

Apologies to Richard Cochran, who pushed for similar changes years
ago, which I resisted due to the concerns about the performance
overhead.

While I suspect this isn't extremely critical, folks who care about
strict leap-second correctness will likely want to watch
this. Potentially a -stable candidate eventually.

Originally-suggested-by: Richard Cochran <richardcochran@gmail.com>
Reported-by: Daniel Bristot de Oliveira <bristot@redhat.com>
Reported-by: Prarit Bhargava <prarit@redhat.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Jan Kara <jack@suse.cz>
Cc: Jiri Bohac <jbohac@suse.cz>
Cc: Shuah Khan <shuahkh@osg.samsung.com>
Cc: Ingo Molnar <mingo@kernel.org>
Link: http://lkml.kernel.org/r/1434063297-28657-4-git-send-email-john.stultz@linaro.org
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/time64.h              |  1 +
 include/linux/timekeeper_internal.h |  2 ++
 kernel/time/ntp.c                   | 42 ++++++++++++++++++++++++++++++-------
 kernel/time/ntp_internal.h          |  1 +
 kernel/time/timekeeping.c           | 23 +++++++++++++++++++-
 5 files changed, 61 insertions(+), 8 deletions(-)

diff --git a/include/linux/time64.h b/include/linux/time64.h
index 12d4e82..77b5df2 100644
--- a/include/linux/time64.h
+++ b/include/linux/time64.h
@@ -29,6 +29,7 @@ struct timespec64 {
 #define FSEC_PER_SEC	1000000000000000LL
 
 /* Located here for timespec[64]_valid_strict */
+#define TIME64_MAX			((s64)~((u64)1 << 63))
 #define KTIME_MAX			((s64)~((u64)1 << 63))
 #define KTIME_SEC_MAX			(KTIME_MAX / NSEC_PER_SEC)
 
diff --git a/include/linux/timekeeper_internal.h b/include/linux/timekeeper_internal.h
index e1f5a11..2524722 100644
--- a/include/linux/timekeeper_internal.h
+++ b/include/linux/timekeeper_internal.h
@@ -50,6 +50,7 @@ struct tk_read_base {
  * @offs_tai:		Offset clock monotonic -> clock tai
  * @tai_offset:		The current UTC to TAI offset in seconds
  * @clock_was_set_seq:	The sequence number of clock was set events
+ * @next_leap_ktime:	CLOCK_MONOTONIC time value of a pending leap-second
  * @raw_time:		Monotonic raw base time in timespec64 format
  * @cycle_interval:	Number of clock cycles in one NTP interval
  * @xtime_interval:	Number of clock shifted nano seconds in one NTP
@@ -90,6 +91,7 @@ struct timekeeper {
 	ktime_t			offs_tai;
 	s32			tai_offset;
 	unsigned int		clock_was_set_seq;
+	ktime_t			next_leap_ktime;
 	struct timespec64	raw_time;
 
 	/* The following members are for timekeeping internal use */
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 7aa2161..033743e 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -77,6 +77,9 @@ static long			time_adjust;
 /* constant (boot-param configurable) NTP tick adjustment (upscaled)	*/
 static s64			ntp_tick_adj;
 
+/* second value of the next pending leapsecond, or TIME64_MAX if no leap */
+static time64_t			ntp_next_leap_sec = TIME64_MAX;
+
 #ifdef CONFIG_NTP_PPS
 
 /*
@@ -350,6 +353,7 @@ void ntp_clear(void)
 	tick_length	= tick_length_base;
 	time_offset	= 0;
 
+	ntp_next_leap_sec = TIME64_MAX;
 	/* Clear PPS state variables */
 	pps_clear();
 }
@@ -360,6 +364,21 @@ u64 ntp_tick_length(void)
 	return tick_length;
 }
 
+/**
+ * ntp_get_next_leap - Returns the next leapsecond in CLOCK_REALTIME ktime_t
+ *
+ * Provides the time of the next leapsecond against CLOCK_REALTIME in
+ * a ktime_t format. Returns KTIME_MAX if no leapsecond is pending.
+ */
+ktime_t ntp_get_next_leap(void)
+{
+	ktime_t ret;
+
+	if ((time_state == TIME_INS) && (time_status & STA_INS))
+		return ktime_set(ntp_next_leap_sec, 0);
+	ret.tv64 = KTIME_MAX;
+	return ret;
+}
 
 /*
  * this routine handles the overflow of the microsecond field
@@ -383,15 +402,21 @@ int second_overflow(unsigned long secs)
 	 */
 	switch (time_state) {
 	case TIME_OK:
-		if (time_status & STA_INS)
+		if (time_status & STA_INS) {
 			time_state = TIME_INS;
-		else if (time_status & STA_DEL)
+			ntp_next_leap_sec = secs + SECS_PER_DAY -
+						(secs % SECS_PER_DAY);
+		} else if (time_status & STA_DEL) {
 			time_state = TIME_DEL;
+			ntp_next_leap_sec = secs + SECS_PER_DAY -
+						 ((secs+1) % SECS_PER_DAY);
+		}
 		break;
 	case TIME_INS:
-		if (!(time_status & STA_INS))
+		if (!(time_status & STA_INS)) {
+			ntp_next_leap_sec = TIME64_MAX;
 			time_state = TIME_OK;
-		else if (secs % SECS_PER_DAY == 0) {
+		} else if (secs % SECS_PER_DAY == 0) {
 			leap = -1;
 			time_state = TIME_OOP;
 			printk(KERN_NOTICE
@@ -399,19 +424,21 @@ int second_overflow(unsigned long secs)
 		}
 		break;
 	case TIME_DEL:
-		if (!(time_status & STA_DEL))
+		if (!(time_status & STA_DEL)) {
+			ntp_next_leap_sec = TIME64_MAX;
 			time_state = TIME_OK;
-		else if ((secs + 1) % SECS_PER_DAY == 0) {
+		} else if ((secs + 1) % SECS_PER_DAY == 0) {
 			leap = 1;
+			ntp_next_leap_sec = TIME64_MAX;
 			time_state = TIME_WAIT;
 			printk(KERN_NOTICE
 				"Clock: deleting leap second 23:59:59 UTC\n");
 		}
 		break;
 	case TIME_OOP:
+		ntp_next_leap_sec = TIME64_MAX;
 		time_state = TIME_WAIT;
 		break;
-
 	case TIME_WAIT:
 		if (!(time_status & (STA_INS | STA_DEL)))
 			time_state = TIME_OK;
@@ -548,6 +575,7 @@ static inline void process_adj_status(struct timex *txc, struct timespec64 *ts)
 	if ((time_status & STA_PLL) && !(txc->status & STA_PLL)) {
 		time_state = TIME_OK;
 		time_status = STA_UNSYNC;
+		ntp_next_leap_sec = TIME64_MAX;
 		/* restart PPS frequency calibration */
 		pps_reset_freq_interval();
 	}
diff --git a/kernel/time/ntp_internal.h b/kernel/time/ntp_internal.h
index bbd102a..6543050 100644
--- a/kernel/time/ntp_internal.h
+++ b/kernel/time/ntp_internal.h
@@ -5,6 +5,7 @@ extern void ntp_init(void);
 extern void ntp_clear(void);
 /* Returns how long ticks are at present, in ns / 2^NTP_SCALE_SHIFT. */
 extern u64 ntp_tick_length(void);
+extern ktime_t ntp_get_next_leap(void);
 extern int second_overflow(unsigned long secs);
 extern int ntp_validate_timex(struct timex *);
 extern int __do_adjtimex(struct timex *, struct timespec64 *, s32 *);
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 849b932..5d67ffb 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -540,6 +540,17 @@ int pvclock_gtod_unregister_notifier(struct notifier_block *nb)
 EXPORT_SYMBOL_GPL(pvclock_gtod_unregister_notifier);
 
 /*
+ * tk_update_leap_state - helper to update the next_leap_ktime
+ */
+static inline void tk_update_leap_state(struct timekeeper *tk)
+{
+	tk->next_leap_ktime = ntp_get_next_leap();
+	if (tk->next_leap_ktime.tv64 != KTIME_MAX)
+		/* Convert to monotonic time */
+		tk->next_leap_ktime = ktime_sub(tk->next_leap_ktime, tk->offs_real);
+}
+
+/*
  * Update the ktime_t based scalar nsec members of the timekeeper
  */
 static inline void tk_update_ktime_data(struct timekeeper *tk)
@@ -580,6 +591,7 @@ static void timekeeping_update(struct timekeeper *tk, unsigned int action)
 		ntp_clear();
 	}
 
+	tk_update_leap_state(tk);
 	tk_update_ktime_data(tk);
 
 	update_vsyscall(tk);
@@ -1956,15 +1968,22 @@ ktime_t ktime_get_update_offsets_now(unsigned int *cwsseq, ktime_t *offs_real,
 
 		base = tk->tkr_mono.base;
 		nsecs = timekeeping_get_ns(&tk->tkr_mono);
+		base = ktime_add_ns(base, nsecs);
+
 		if (*cwsseq != tk->clock_was_set_seq) {
 			*cwsseq = tk->clock_was_set_seq;
 			*offs_real = tk->offs_real;
 			*offs_boot = tk->offs_boot;
 			*offs_tai = tk->offs_tai;
 		}
+
+		/* Handle leapsecond insertion adjustments */
+		if (unlikely(base.tv64 >= tk->next_leap_ktime.tv64))
+			*offs_real = ktime_sub(tk->offs_real, ktime_set(1, 0));
+
 	} while (read_seqcount_retry(&tk_core.seq, seq));
 
-	return ktime_add_ns(base, nsecs);
+	return base;
 }
 
 /**
@@ -2006,6 +2025,8 @@ int do_adjtimex(struct timex *txc)
 		__timekeeping_set_tai_offset(tk, tai);
 		timekeeping_update(tk, TK_MIRROR | TK_CLOCK_WAS_SET);
 	}
+	tk_update_leap_state(tk);
+
 	write_seqcount_end(&tk_core.seq);
 	raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
 

[tip:timers/core] ntp: Do leapsecond adjustment in adjtimex read path

[tip-bot for John Stultz]

Commit-ID:  96efdcf2d080687e041b0353c604b708546689fd
Gitweb:     http://git.kernel.org/tip/96efdcf2d080687e041b0353c604b708546689fd
Author:     John Stultz <john.stultz@linaro.org>
AuthorDate: Thu, 11 Jun 2015 15:54:56 -0700
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Fri, 12 Jun 2015 11:15:49 +0200

ntp: Do leapsecond adjustment in adjtimex read path

Since the leapsecond is applied at tick-time, this means there is a
small window of time at the start of a leap-second where we cross into
the next second before applying the leap.

This patch modified adjtimex so that the leap-second is applied on the
second edge. Providing more correct leapsecond behavior.

This does make it so that adjtimex()'s returned time values can be
inconsistent with time values read from gettimeofday() or
clock_gettime(CLOCK_REALTIME,...)  for a brief period of one tick at
the leapsecond.  However, those other interfaces do not provide the
TIME_OOP time_state return that adjtimex() provides, which allows the
leapsecond to be properly represented. They instead only see a time
discontinuity, and cannot tell the first 23:59:59 from the repeated
23:59:59 leap second.

This seems like a reasonable tradeoff given clock_gettime() /
gettimeofday() cannot properly represent a leapsecond, and users
likely care more about performance, while folks who are using
adjtimex() more likely care about leap-second correctness.

Signed-off-by: John Stultz <john.stultz@linaro.org>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Jan Kara <jack@suse.cz>
Cc: Jiri Bohac <jbohac@suse.cz>
Cc: Ingo Molnar <mingo@kernel.org>
Link: http://lkml.kernel.org/r/1434063297-28657-5-git-send-email-john.stultz@linaro.org
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/ntp.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 033743e..fb4d98c 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -740,6 +740,24 @@ int __do_adjtimex(struct timex *txc, struct timespec64 *ts, s32 *time_tai)
 	if (!(time_status & STA_NANO))
 		txc->time.tv_usec /= NSEC_PER_USEC;
 
+	/* Handle leapsec adjustments */
+	if (unlikely(ts->tv_sec >= ntp_next_leap_sec)) {
+		if ((time_state == TIME_INS) && (time_status & STA_INS)) {
+			result = TIME_OOP;
+			txc->tai++;
+			txc->time.tv_sec--;
+		}
+		if ((time_state == TIME_DEL) && (time_status & STA_DEL)) {
+			result = TIME_WAIT;
+			txc->tai--;
+			txc->time.tv_sec++;
+		}
+		if ((time_state == TIME_OOP) &&
+					(ts->tv_sec == ntp_next_leap_sec)) {
+			result = TIME_WAIT;
+		}
+	}
+
 	return result;
 }
 

[tip:timers/core] selftests: timers: Add leap-second timer edge testing to leap-a-day.c

[tip-bot for John Stultz]

Commit-ID:  0c4a5fc95b1df42651a9b4c1f72d348b3d196ea0
Gitweb:     http://git.kernel.org/tip/0c4a5fc95b1df42651a9b4c1f72d348b3d196ea0
Author:     John Stultz <john.stultz@linaro.org>
AuthorDate: Thu, 11 Jun 2015 15:54:57 -0700
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Fri, 12 Jun 2015 11:15:50 +0200

selftests: timers: Add leap-second timer edge testing to leap-a-day.c

Prarit reported an issue w/ timers around the leapsecond, where a
timer set for Midnight UTC (00:00:00) might fire a second early right
before the leapsecond (23:59:60 - though it appears as a repeated
23:59:59) is applied.

So I've updated the leap-a-day.c test to integrate a similar test,
where we set a timer and check if it triggers at the right time, and
if the ntp state transition is managed properly.

Reported-by: Daniel Bristot de Oliveira <bristot@redhat.com>
Reported-by: Prarit Bhargava <prarit@redhat.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Jan Kara <jack@suse.cz>
Cc: Jiri Bohac <jbohac@suse.cz>
Cc: Shuah Khan <shuahkh@osg.samsung.com>
Cc: Ingo Molnar <mingo@kernel.org>
Link: http://lkml.kernel.org/r/1434063297-28657-6-git-send-email-john.stultz@linaro.org
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 tools/testing/selftests/timers/leap-a-day.c | 76 +++++++++++++++++++++++++++--
 1 file changed, 72 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/timers/leap-a-day.c b/tools/testing/selftests/timers/leap-a-day.c
index b8272e6..331c4f7 100644
--- a/tools/testing/selftests/timers/leap-a-day.c
+++ b/tools/testing/selftests/timers/leap-a-day.c
@@ -44,6 +44,7 @@
 #include <time.h>
 #include <sys/time.h>
 #include <sys/timex.h>
+#include <sys/errno.h>
 #include <string.h>
 #include <signal.h>
 #include <unistd.h>
@@ -63,6 +64,9 @@ static inline int ksft_exit_fail(void)
 #define NSEC_PER_SEC 1000000000ULL
 #define CLOCK_TAI 11
 
+time_t next_leap;
+int error_found;
+
 /* returns 1 if a <= b, 0 otherwise */
 static inline int in_order(struct timespec a, struct timespec b)
 {
@@ -134,6 +138,34 @@ void handler(int unused)
 	exit(0);
 }
 
+void sigalarm(int signo)
+{
+	struct timex tx;
+	char buf[26];
+	int ret;
+
+	tx.modes = 0;
+	ret = adjtimex(&tx);
+
+	ctime_r(&tx.time.tv_sec, buf);
+	buf[strlen(buf)-1] = 0; /*remove trailing\n */
+	printf("%s + %6ld us (%i)\t%s - TIMER FIRED\n",
+					buf,
+					tx.time.tv_usec,
+					tx.tai,
+					time_state_str(ret));
+
+	if (tx.time.tv_sec < next_leap) {
+		printf("Error: Early timer expiration!\n");
+		error_found = 1;
+	}
+	if (ret != TIME_WAIT) {
+		printf("Error: Incorrect NTP state?\n");
+		error_found = 1;
+	}
+}
+
+
 /* Test for known hrtimer failure */
 void test_hrtimer_failure(void)
 {
@@ -144,12 +176,19 @@ void test_hrtimer_failure(void)
 	clock_nanosleep(CLOCK_REALTIME, TIMER_ABSTIME, &target, NULL);
 	clock_gettime(CLOCK_REALTIME, &now);
 
-	if (!in_order(target, now))
+	if (!in_order(target, now)) {
 		printf("ERROR: hrtimer early expiration failure observed.\n");
+		error_found = 1;
+	}
 }
 
 int main(int argc, char **argv)
 {
+	timer_t tm1;
+	struct itimerspec its1;
+	struct sigevent se;
+	struct sigaction act;
+	int signum = SIGRTMAX;
 	int settime = 0;
 	int tai_time = 0;
 	int insert = 1;
@@ -191,6 +230,12 @@ int main(int argc, char **argv)
 	signal(SIGINT, handler);
 	signal(SIGKILL, handler);
 
+	/* Set up timer signal handler: */
+	sigfillset(&act.sa_mask);
+	act.sa_flags = 0;
+	act.sa_handler = sigalarm;
+	sigaction(signum, &act, NULL);
+
 	if (iterations < 0)
 		printf("This runs continuously. Press ctrl-c to stop\n");
 	else
@@ -201,7 +246,7 @@ int main(int argc, char **argv)
 		int ret;
 		struct timespec ts;
 		struct timex tx;
-		time_t now, next_leap;
+		time_t now;
 
 		/* Get the current time */
 		clock_gettime(CLOCK_REALTIME, &ts);
@@ -251,10 +296,27 @@ int main(int argc, char **argv)
 
 		printf("Scheduling leap second for %s", ctime(&next_leap));
 
+		/* Set up timer */
+		printf("Setting timer for %s", ctime(&next_leap));
+		memset(&se, 0, sizeof(se));
+		se.sigev_notify = SIGEV_SIGNAL;
+		se.sigev_signo = signum;
+		se.sigev_value.sival_int = 0;
+		if (timer_create(CLOCK_REALTIME, &se, &tm1) == -1) {
+			printf("Error: timer_create failed\n");
+			return ksft_exit_fail();
+		}
+		its1.it_value.tv_sec = next_leap;
+		its1.it_value.tv_nsec = 0;
+		its1.it_interval.tv_sec = 0;
+		its1.it_interval.tv_nsec = 0;
+		timer_settime(tm1, TIMER_ABSTIME, &its1, NULL);
+
 		/* Wake up 3 seconds before leap */
 		ts.tv_sec = next_leap - 3;
 		ts.tv_nsec = 0;
 
+
 		while (clock_nanosleep(CLOCK_REALTIME, TIMER_ABSTIME, &ts, NULL))
 			printf("Something woke us up, returning to sleep\n");
 
@@ -276,6 +338,7 @@ int main(int argc, char **argv)
 		while (now < next_leap + 2) {
 			char buf[26];
 			struct timespec tai;
+			int ret;
 
 			tx.modes = 0;
 			ret = adjtimex(&tx);
@@ -308,8 +371,13 @@ int main(int argc, char **argv)
 		/* Note if kernel has known hrtimer failure */
 		test_hrtimer_failure();
 
-		printf("Leap complete\n\n");
-
+		printf("Leap complete\n");
+		if (error_found) {
+			printf("Errors observed\n");
+			clear_time_state();
+			return ksft_exit_fail();
+		}
+		printf("\n");
 		if ((iterations != -1) && !(--iterations))
 			break;
 	}

Re: [PATCH 0/5 v2] Fixes for leapsecond expiring early ABS_TIME CLOCK_REALTIME timers

[Dave Jones]

On Thu, Jun 11, 2015 at 03:54:52PM -0700, John Stultz wrote:
 > So this is a second round at trying to address the issue, trying
 > to integrate feedback from Ingo and Thomas, trying to simplify
 > what I can. I've also split out the changes so each can be
 > more easily reviewed. Its still not tiny, but its simpler.
 > 
 > This series is against tip/timers/core, and the first patch isn't
 > strictly related but is a fix that is needed in tip/timers/core.
 > 
 > As Prarit reported here:
 > https://lkml.org/lkml/2015/5/27/458
 > 
 > Since the leapsecond is applied at timer tick time, and not
 > the actual second edge, ABS_TIME CLOCK_REALTIME timers set for
 > right after the leapsecond could fire a second early, since
 > some timers may be expired before we trigger the timekeeping
 > timer, which then applies the leapsecond.
 > 
 > Thus this patch series tries to address this issue, including
 > extending the leap-a-day test to catch this problem, as well
 > as other relevant fixups I found while working on the code.
 > 
 > This series has only had limited testing, so I wanted to send
 > it out for initial review and comment. Folks can grab this tree
 > via git for testing here:
 > https://git.linaro.org/people/john.stultz/linux.git dev/early-leap-timer

Any idea how far back this reaches ? Ie, which longterm stable releases
might be affected by this ?

(It really creeps me out that we're still changing this code so close
 to the next leap second event).

	Dave

Re: [PATCH 0/5 v2] Fixes for leapsecond expiring early ABS_TIME CLOCK_REALTIME timers

[Prarit Bhargava]

On 06/12/2015 10:52 AM, Dave Jones wrote:
> On Thu, Jun 11, 2015 at 03:54:52PM -0700, John Stultz wrote:
>  > So this is a second round at trying to address the issue, trying
>  > to integrate feedback from Ingo and Thomas, trying to simplify
>  > what I can. I've also split out the changes so each can be
>  > more easily reviewed. Its still not tiny, but its simpler.
>  > 
>  > This series is against tip/timers/core, and the first patch isn't
>  > strictly related but is a fix that is needed in tip/timers/core.
>  > 
>  > As Prarit reported here:
>  > https://lkml.org/lkml/2015/5/27/458
>  > 
>  > Since the leapsecond is applied at timer tick time, and not
>  > the actual second edge, ABS_TIME CLOCK_REALTIME timers set for
>  > right after the leapsecond could fire a second early, since
>  > some timers may be expired before we trigger the timekeeping
>  > timer, which then applies the leapsecond.
>  > 
>  > Thus this patch series tries to address this issue, including
>  > extending the leap-a-day test to catch this problem, as well
>  > as other relevant fixups I found while working on the code.
>  > 
>  > This series has only had limited testing, so I wanted to send
>  > it out for initial review and comment. Folks can grab this tree
>  > via git for testing here:
>  > https://git.linaro.org/people/john.stultz/linux.git dev/early-leap-timer
> 
> Any idea how far back this reaches ? Ie, which longterm stable releases
> might be affected by this ?
> 

Likely occurs in any kernel after NOHZ was introduced.

> (It really creeps me out that we're still changing this code so close
>  to the next leap second event).
> 

davej, I'm not taking this into RHEL because of the obvious stability concerns.
 I'm also very much against taking this into linux-stable for now until we can
get some testing done on it.

FWIW I'd rather live with the devil we know ...

I'm going to be testing (heavily) over the weekend.

P.

> 	Dave
> 

Re: [PATCH 0/5 v2] Fixes for leapsecond expiring early ABS_TIME CLOCK_REALTIME timers

[Dave Jones]

On Fri, Jun 12, 2015 at 10:55:57AM -0400, Prarit Bhargava wrote:
 
 > >  > This series has only had limited testing, so I wanted to send
 > >  > it out for initial review and comment. Folks can grab this tree
 > >  > via git for testing here:
 > >  > https://git.linaro.org/people/john.stultz/linux.git dev/early-leap-timer
 > > 
 > > Any idea how far back this reaches ? Ie, which longterm stable releases
 > > might be affected by this ?
 > 
 > Likely occurs in any kernel after NOHZ was introduced.

At least on 3.14, it seems to happen even if NOHZ is disabled.

# ./leap -i 5 -s
Setting time to speed up testing
Running for 5 iterations. Press ctrl-c to stop

Setting time to Sat Jun 13 23:59:50 2015
Scheduling leap second for Sun Jun 14 00:00:00 2015
Setting timer for Sun Jun 14 00:00:00 2015
Sat Jun 13 23:59:57 2015 +     55 us (1)	TIME_INS
Sat Jun 13 23:59:57 2015 + 500127 us (1)	TIME_INS
Sat Jun 13 23:59:58 2015 +    189 us (1)	TIME_INS
Sat Jun 13 23:59:58 2015 + 500250 us (1)	TIME_INS
Sat Jun 13 23:59:59 2015 +    312 us (1)	TIME_INS
Sat Jun 13 23:59:59 2015 + 500372 us (1)	TIME_INS
Sun Jun 14 00:00:00 2015 +      3 us (1)	TIME_INS - TIMER FIRED
Error: Incorrect NTP state?
Sun Jun 14 00:00:00 2015 +     12 us (1)	TIME_INS
Sat Jun 13 23:59:59 2015 + 500068 us (2)	TIME_OOP
Sun Jun 14 00:00:00 2015 +    130 us (2)	TIME_OOP
Sun Jun 14 00:00:00 2015 + 500190 us (2)	TIME_WAIT
Sun Jun 14 00:00:01 2015 +    256 us (2)	TIME_WAIT
Sun Jun 14 00:00:01 2015 + 500315 us (2)	TIME_WAIT
Sun Jun 14 00:00:02 2015 +    376 us (2)	TIME_WAIT
Leap complete
Errors observed



	Dave

Re: [PATCH 0/5 v2] Fixes for leapsecond expiring early ABS_TIME CLOCK_REALTIME timers

[Prarit Bhargava]

On 06/12/2015 10:59 AM, Dave Jones wrote:
> On Fri, Jun 12, 2015 at 10:55:57AM -0400, Prarit Bhargava wrote:
>  
>  > >  > This series has only had limited testing, so I wanted to send
>  > >  > it out for initial review and comment. Folks can grab this tree
>  > >  > via git for testing here:
>  > >  > https://git.linaro.org/people/john.stultz/linux.git dev/early-leap-timer
>  > > 
>  > > Any idea how far back this reaches ? Ie, which longterm stable releases
>  > > might be affected by this ?
>  > 
>  > Likely occurs in any kernel after NOHZ was introduced.
> 
> At least on 3.14, it seems to happen even if NOHZ is disabled.

Ouch. :(

I wonder if we've every synched the tick edge with the second edge.  Your test
implies the answer is "no".

P.

Re: [PATCH 0/5 v2] Fixes for leapsecond expiring early ABS_TIME CLOCK_REALTIME timers

[John Stultz]

On Fri, Jun 12, 2015 at 7:52 AM, Dave Jones <davej@codemonkey.org.uk> wrote:
> On Thu, Jun 11, 2015 at 03:54:52PM -0700, John Stultz wrote:
>  > So this is a second round at trying to address the issue, trying
>  > to integrate feedback from Ingo and Thomas, trying to simplify
>  > what I can. I've also split out the changes so each can be
>  > more easily reviewed. Its still not tiny, but its simpler.
>  >
>  > This series is against tip/timers/core, and the first patch isn't
>  > strictly related but is a fix that is needed in tip/timers/core.
>  >
>  > As Prarit reported here:
>  > https://lkml.org/lkml/2015/5/27/458
>  >
>  > Since the leapsecond is applied at timer tick time, and not
>  > the actual second edge, ABS_TIME CLOCK_REALTIME timers set for
>  > right after the leapsecond could fire a second early, since
>  > some timers may be expired before we trigger the timekeeping
>  > timer, which then applies the leapsecond.
>  >
>  > Thus this patch series tries to address this issue, including
>  > extending the leap-a-day test to catch this problem, as well
>  > as other relevant fixups I found while working on the code.
>  >
>  > This series has only had limited testing, so I wanted to send
>  > it out for initial review and comment. Folks can grab this tree
>  > via git for testing here:
>  > https://git.linaro.org/people/john.stultz/linux.git dev/early-leap-timer
>
> Any idea how far back this reaches ? Ie, which longterm stable releases
> might be affected by this ?

I suspect all the way back to where hrtimers were introduced and
possibly further.

> (It really creeps me out that we're still changing this code so close
>  to the next leap second event).

Yea. This isn't something I'm suggesting folks deploy for the end of
this month. It likely will be 4.3 material.

thanks
-john

Re: [PATCH 0/5 v2] Fixes for leapsecond expiring early ABS_TIME CLOCK_REALTIME timers

[John Stultz]

On Fri, Jun 12, 2015 at 10:58 AM, John Stultz <john.stultz@linaro.org> wrote:
> On Fri, Jun 12, 2015 at 7:52 AM, Dave Jones <davej@codemonkey.org.uk> wrote:
>> On Thu, Jun 11, 2015 at 03:54:52PM -0700, John Stultz wrote:
>>  > So this is a second round at trying to address the issue, trying
>>  > to integrate feedback from Ingo and Thomas, trying to simplify
>>  > what I can. I've also split out the changes so each can be
>>  > more easily reviewed. Its still not tiny, but its simpler.
>>  >
>>  > This series is against tip/timers/core, and the first patch isn't
>>  > strictly related but is a fix that is needed in tip/timers/core.
>>  >
>>  > As Prarit reported here:
>>  > https://lkml.org/lkml/2015/5/27/458
>>  >
>>  > Since the leapsecond is applied at timer tick time, and not
>>  > the actual second edge, ABS_TIME CLOCK_REALTIME timers set for
>>  > right after the leapsecond could fire a second early, since
>>  > some timers may be expired before we trigger the timekeeping
>>  > timer, which then applies the leapsecond.
>>  >
>>  > Thus this patch series tries to address this issue, including
>>  > extending the leap-a-day test to catch this problem, as well
>>  > as other relevant fixups I found while working on the code.
>>  >
>>  > This series has only had limited testing, so I wanted to send
>>  > it out for initial review and comment. Folks can grab this tree
>>  > via git for testing here:
>>  > https://git.linaro.org/people/john.stultz/linux.git dev/early-leap-timer
>>
>> Any idea how far back this reaches ? Ie, which longterm stable releases
>> might be affected by this ?
>
> I suspect all the way back to where hrtimers were introduced and
> possibly further.
>
>> (It really creeps me out that we're still changing this code so close
>>  to the next leap second event).
>
> Yea. This isn't something I'm suggesting folks deploy for the end of
> this month. It likely will be 4.3 material.

Well, noticing the -tip merge messages in my inbox, it may actually
make it for 4.2 (after all the push-back from last time, I expected
there would still be more), but still not something I'm wanting to
aggressively push to -stable.

thanks
-john

Re: [PATCH 0/5 v2] Fixes for leapsecond expiring early ABS_TIME CLOCK_REALTIME timers

[Thomas Gleixner]

On Fri, 12 Jun 2015, John Stultz wrote:
> Well, noticing the -tip merge messages in my inbox, it may actually
> make it for 4.2 (after all the push-back from last time, I expected
> there would still be more), but still not something I'm wanting to
> aggressively push to -stable.

Yes, I decided to take it now. It looks really sane and reasonable
simple. So it's better to get it exposed sooner than later.

Thanks,

	tglx

Re: [PATCH 0/5 v2] Fixes for leapsecond expiring early ABS_TIME CLOCK_REALTIME timers

[Ingo Molnar]

* Thomas Gleixner <tglx@linutronix.de> wrote:

> On Fri, 12 Jun 2015, John Stultz wrote:
> > Well, noticing the -tip merge messages in my inbox, it may actually
> > make it for 4.2 (after all the push-back from last time, I expected
> > there would still be more), but still not something I'm wanting to
> > aggressively push to -stable.
> 
> Yes, I decided to take it now. It looks really sane and reasonable
> simple. So it's better to get it exposed sooner than later.

Yes, and June 30 is drawing close, with the next leap second, giving us a natural 
testbed.

Obviously no production system should use this as around June 30 we'll be around 
-rc1 or so. But this way it should see some 'real world' testing.

Thanks,

	Ingo

Re: [PATCH 0/5 v2] Fixes for leapsecond expiring early ABS_TIME CLOCK_REALTIME timers

[Prarit Bhargava]

On 06/11/2015 06:54 PM, John Stultz wrote:
> So this is a second round at trying to address the issue, trying
> to integrate feedback from Ingo and Thomas, trying to simplify
> what I can. I've also split out the changes so each can be
> more easily reviewed. Its still not tiny, but its simpler.
> 
> This series is against tip/timers/core, and the first patch isn't
> strictly related but is a fix that is needed in tip/timers/core.
> 
> As Prarit reported here:
> https://lkml.org/lkml/2015/5/27/458
> 
> Since the leapsecond is applied at timer tick time, and not
> the actual second edge, ABS_TIME CLOCK_REALTIME timers set for
> right after the leapsecond could fire a second early, since
> some timers may be expired before we trigger the timekeeping
> timer, which then applies the leapsecond.
> 
> Thus this patch series tries to address this issue, including
> extending the leap-a-day test to catch this problem, as well
> as other relevant fixups I found while working on the code.
> 
> This series has only had limited testing, so I wanted to send
> it out for initial review and comment. Folks can grab this tree
> via git for testing here:
> https://git.linaro.org/people/john.stultz/linux.git dev/early-leap-timer
> 

I'm testing this on a few more large CPU count systems.  So far, so good ...

P.

> Thoughts and feedback would be appreciated!
> thanks
> -john
> 
> Cc: Prarit Bhargava <prarit@redhat.com>
> Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
> Cc: Richard Cochran <richardcochran@gmail.com>
> Cc: Jan Kara <jack@suse.cz>
> Cc: Jiri Bohac <jbohac@suse.cz>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Shuah Khan <shuahkh@osg.samsung.com>
> 
> 
> John Stultz (5):
>   time: Move clock_was_set_seq update to before we update the
>     shadow-timekeeper
>   ntp: Introduce and use SECS_PER_DAY macro instead of 86400
>   time: Do leapsecond adjustment to avoid early timer expirations
>   ntp: Do leapsecond adjustment in adjtimex read path
>   selftests: timers: Add leap-second timer edge testing to leap-a-day.c
> 
>  include/linux/time64.h                      |  1 +
>  include/linux/timekeeper_internal.h         |  2 +
>  kernel/time/ntp.c                           | 61 ++++++++++++++++++++---
>  kernel/time/ntp_internal.h                  |  1 +
>  kernel/time/timekeeping.c                   | 35 +++++++++++--
>  tools/testing/selftests/timers/leap-a-day.c | 76 +++++++++++++++++++++++++++--
>  6 files changed, 160 insertions(+), 16 deletions(-)
> 

Re: [PATCH 0/5 v2] Fixes for leapsecond expiring early ABS_TIME CLOCK_REALTIME timers

[Prarit Bhargava]

On 06/15/2015 09:10 AM, Prarit Bhargava wrote:
> 
> 
> On 06/11/2015 06:54 PM, John Stultz wrote:
>> So this is a second round at trying to address the issue, trying
>> to integrate feedback from Ingo and Thomas, trying to simplify
>> what I can. I've also split out the changes so each can be
>> more easily reviewed. Its still not tiny, but its simpler.
>>
>> This series is against tip/timers/core, and the first patch isn't
>> strictly related but is a fix that is needed in tip/timers/core.
>>
>> As Prarit reported here:
>> https://lkml.org/lkml/2015/5/27/458
>>
>> Since the leapsecond is applied at timer tick time, and not
>> the actual second edge, ABS_TIME CLOCK_REALTIME timers set for
>> right after the leapsecond could fire a second early, since
>> some timers may be expired before we trigger the timekeeping
>> timer, which then applies the leapsecond.
>>
>> Thus this patch series tries to address this issue, including
>> extending the leap-a-day test to catch this problem, as well
>> as other relevant fixups I found while working on the code.
>>
>> This series has only had limited testing, so I wanted to send
>> it out for initial review and comment. Folks can grab this tree
>> via git for testing here:
>> https://git.linaro.org/people/john.stultz/linux.git dev/early-leap-timer
>>
> 
> I'm testing this on a few more large CPU count systems.  So far, so good ...
> 

John, unfortunately I'm seeing failures across a lot of the large systems
dev/early-leap-timer as of this AM

[root@intel-chiefriver-04 ~]# systemctl status chronyd
chronyd.service - NTP client/server
   Loaded: loaded (/usr/lib/systemd/system/chronyd.service; enabled)
   Active: inactive (dead) since Mon 2015-06-15 09:25:33 EDT; 5s ago
  Process: 731 ExecStartPost=/usr/libexec/chrony-helper add-dhclient-servers
(code=exited, status=0/SUCCESS)
  Process: 713 ExecStart=/usr/sbin/chronyd -u chrony $OPTIONS (code=exited,
status=0/SUCCESS)
 Main PID: 724 (code=exited, status=0/SUCCESS)
   CGroup: /system.slice/chronyd.service

Jun 15 09:25:06 intel-chiefriver-04.khw.lab.eng.bos.redhat.com chronyd[724]: ...
Jun 15 09:25:06 intel-chiefriver-04.khw.lab.eng.bos.redhat.com chronyd[724]: ...
Jun 15 09:25:06 intel-chiefriver-04.khw.lab.eng.bos.redhat.com chronyd[724]: ...
Jun 15 09:25:06 intel-chiefriver-04.khw.lab.eng.bos.redhat.com chronyd[724]: ...
Jun 15 09:25:06 intel-chiefriver-04.khw.lab.eng.bos.redhat.com systemd[1]: St...
Jun 15 09:25:19 intel-chiefriver-04.khw.lab.eng.bos.redhat.com chronyd[724]: ...
Jun 15 09:25:19 intel-chiefriver-04.khw.lab.eng.bos.redhat.com chronyd[724]: ...
Jun 15 09:25:33 intel-chiefriver-04.khw.lab.eng.bos.redhat.com systemd[1]: St...
Jun 15 09:25:33 intel-chiefriver-04.khw.lab.eng.bos.redhat.com chronyd[724]: ...
Jun 15 09:25:33 intel-chiefriver-04.khw.lab.eng.bos.redhat.com systemd[1]: St...
Hint: Some lines were ellipsized, use -l to show in full.
[root@intel-chiefriver-04 ~]# ./leap-a-day -s
Setting time to speed up testing
This runs continuously. Press ctrl-c to stop

Setting time to Mon Jun 15 19:59:50 2015
Scheduling leap second for Mon Jun 15 20:00:00 2015
Setting timer for Mon Jun 15 20:00:00 2015
Mon Jun 15 19:59:57 2015 +  18177 us (0)        TIME_INS
Mon Jun 15 19:59:57 2015 + 526181 us (0)        TIME_INS
Mon Jun 15 19:59:58 2015 +  37418 us (0)        TIME_INS
Mon Jun 15 19:59:58 2015 + 546474 us (0)        TIME_INS
Mon Jun 15 19:59:59 2015 +  55724 us (0)        TIME_INS
Mon Jun 15 19:59:59 2015 + 566970 us (0)        TIME_INS
Mon Jun 15 19:59:59 2015 +   6013 us (1)        TIME_OOP - TIMER FIRED
Error: Early timer expiration!
Error: Incorrect NTP state?
Mon Jun 15 19:59:59 2015 +   6040 us (1)        TIME_OOP
Mon Jun 15 19:59:59 2015 + 517240 us (1)        TIME_OOP
Mon Jun 15 20:00:00 2015 +  28504 us (1)        TIME_WAIT
Mon Jun 15 20:00:00 2015 + 539745 us (1)        TIME_WAIT
Mon Jun 15 20:00:01 2015 +  50978 us (1)        TIME_WAIT
Mon Jun 15 20:00:01 2015 + 560360 us (1)        TIME_WAIT
Mon Jun 15 20:00:02 2015 +  69952 us (1)        TIME_WAIT
ERROR: hrtimer early expiration failure observed.
Leap complete
Errors observed

P.

Re: [PATCH 0/5 v2] Fixes for leapsecond expiring early ABS_TIME CLOCK_REALTIME timers

[John Stultz]

[-- Attachment #1: Type: text/plain, Size: 1843 bytes --]

On Mon, Jun 15, 2015 at 6:46 AM, Prarit Bhargava <prarit@redhat.com> wrote:
> On 06/15/2015 09:10 AM, Prarit Bhargava wrote:
>> I'm testing this on a few more large CPU count systems.  So far, so good ...
>>
>
> John, unfortunately I'm seeing failures across a lot of the large systems
> dev/early-leap-timer as of this AM
>
[snip]
> [root@intel-chiefriver-04 ~]# ./leap-a-day -s
> Setting time to speed up testing
> This runs continuously. Press ctrl-c to stop
>
> Setting time to Mon Jun 15 19:59:50 2015
> Scheduling leap second for Mon Jun 15 20:00:00 2015
> Setting timer for Mon Jun 15 20:00:00 2015
> Mon Jun 15 19:59:57 2015 +  18177 us (0)        TIME_INS
> Mon Jun 15 19:59:57 2015 + 526181 us (0)        TIME_INS
> Mon Jun 15 19:59:58 2015 +  37418 us (0)        TIME_INS
> Mon Jun 15 19:59:58 2015 + 546474 us (0)        TIME_INS
> Mon Jun 15 19:59:59 2015 +  55724 us (0)        TIME_INS
> Mon Jun 15 19:59:59 2015 + 566970 us (0)        TIME_INS
> Mon Jun 15 19:59:59 2015 +   6013 us (1)        TIME_OOP - TIMER FIRED
> Error: Early timer expiration!
> Error: Incorrect NTP state?
> Mon Jun 15 19:59:59 2015 +   6040 us (1)        TIME_OOP
> Mon Jun 15 19:59:59 2015 + 517240 us (1)        TIME_OOP
> Mon Jun 15 20:00:00 2015 +  28504 us (1)        TIME_WAIT
> Mon Jun 15 20:00:00 2015 + 539745 us (1)        TIME_WAIT
> Mon Jun 15 20:00:01 2015 +  50978 us (1)        TIME_WAIT
> Mon Jun 15 20:00:01 2015 + 560360 us (1)        TIME_WAIT
> Mon Jun 15 20:00:02 2015 +  69952 us (1)        TIME_WAIT

Hrm.. I'm unable so far to reproduce this.

Attached is a simple trace debug patch, would you mind running the
test, and when the failure occurs send me the output from
cat /sys/kernel/debug/tracing/trace (along with the test output for
the failure).

(Do make sure you have tracing enabled in your kernel config)

thanks
-john

[-- Attachment #2: tmp.patch --]
[-- Type: text/x-patch, Size: 1209 bytes --]

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 5d67ffb..b613a27 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -545,9 +545,13 @@ EXPORT_SYMBOL_GPL(pvclock_gtod_unregister_notifier);
 static inline void tk_update_leap_state(struct timekeeper *tk)
 {
 	tk->next_leap_ktime = ntp_get_next_leap();
-	if (tk->next_leap_ktime.tv64 != KTIME_MAX)
+	if (tk->next_leap_ktime.tv64 != KTIME_MAX) {
 		/* Convert to monotonic time */
+		trace_printk("Setting leap %lld (from %lld - %lld)\n",
+					ktime_sub(tk->next_leap_ktime, tk->offs_real).tv64,
+					tk->next_leap_ktime.tv64, tk->offs_real.tv64);
 		tk->next_leap_ktime = ktime_sub(tk->next_leap_ktime, tk->offs_real);
+	}
 }
 
 /*
@@ -1978,8 +1982,10 @@ ktime_t ktime_get_update_offsets_now(unsigned int *cwsseq, ktime_t *offs_real,
 		}
 
 		/* Handle leapsecond insertion adjustments */
-		if (unlikely(base.tv64 >= tk->next_leap_ktime.tv64))
+		if (unlikely(base.tv64 >= tk->next_leap_ktime.tv64)) {
+			trace_printk("Inserting leap %lld >= %lld\n", base.tv64, tk->next_leap_ktime.tv64);
 			*offs_real = ktime_sub(tk->offs_real, ktime_set(1, 0));
+		}
 
 	} while (read_seqcount_retry(&tk_core.seq, seq));