Fix leap seconds and add tai clock

The function, timekeeping_leap_insert, was removed in commit 6b43ae8a619d17c4935c3320d2ef9e92bdeed05d

Signed-off-by Richard Cochran<[email protected]>

This patch removes some leftover cruft in the NTP code.

Signed-off-by Richard Cochran<[email protected]>

This patch introduces time keeping variables to track the next mini-epoch between the UTC and TAI timescales. A leap second occurs one second before a mini-epoch. When no leap second is pending, then the epoch is set to the far future, LONG_MAX.

This code will become useful later on for providing correct time surrounding a leap second.

Signed-off-by Richard Cochran<[email protected]>

---
  kernel/time/timekeeping.c |   14 ++++++++++++++
  1 files changed, 14 insertions(+), 0 deletions(-)

kernel/time/timekeeping.c +0 -0

index d66b213..ac04de4 100644

Quoted 21 lines · Richard Cochran

This patch introduces time keeping variables to track the next mini-epoch between the UTC and TAI timescales. A leap second occurs one second before a mini-epoch. When no leap second is pending, then the epoch is set to the far future, LONG_MAX.

This code will become useful later on for providing correct time surrounding a leap second.

Signed-off-by Richard Cochran<[email protected]>

---
 kernel/time/timekeeping.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

kernel/time/timekeeping.c +0 -0

index d66b213..ac04de4 100644
@@ -70,6 +70,19 @@ struct timekeeper {
 	/* The raw monotonic time for the CLOCK_MONOTONIC_RAW posix clock. */
 	struct timespec raw_time;
 

kernel/time/timekeeping.c struct timekeeper { · line 76

Quoted 4 lines

kernel/time/timekeeping.c +4 -0

+	/* The current TAI - UTC offset */
+	time_t tai_offset;
+	/* The UTC time of the next leap second epoch */
+	time_t utc_epoch;

How about leap_utc_epoch just to be more clear?

kernel/time/timekeeping.c struct timekeeper { · line 85

Quoted 10 lines

kernel/time/timekeeping.c +9 -0

+	/* Tracks where we stand with regard to leap the second epoch. */
+	enum {
+		LEAP_IDLE,
+		LEAP_INS_PENDING,
+		LEAP_INS_DONE,
+		LEAP_DEL_PENDING,
+		LEAP_DEL_DONE,
+	} leap_state;
+

For continuity, would it make more sense for these to named closer to the NTP time_state values, or maybe reworked to make use of them? Not sure if its worth having separate state machines in the timekeeping code and the ntp code, but maybe I'm not seeing a necessary detail here?

Quoted 9 lines · John Stultz

Quoted 8 lines · Richard Cochran

kernel/time/timekeeping.c +8 -0

+	/* Tracks where we stand with regard to leap the second epoch. */
+	enum {
+		LEAP_IDLE,
+		LEAP_INS_PENDING,
+		LEAP_INS_DONE,
+		LEAP_DEL_PENDING,
+		LEAP_DEL_DONE,
+	} leap_state;

I don't think I am explaining this very well. I will try again to make it clear using a table or something later on.

Quoted 10 lines · Richard Cochran

Quoted 9 lines · John Stultz

Quoted 8 lines · Richard Cochran

kernel/time/timekeeping.c +8 -0

+	/* Tracks where we stand with regard to leap the second epoch. */
+	enum {
+		LEAP_IDLE,
+		LEAP_INS_PENDING,
+		LEAP_INS_DONE,
+		LEAP_DEL_PENDING,
+		LEAP_DEL_DONE,
+	} leap_state;

...

Quoted 2 lines

I don't think I am explaining this very well. I will try again to make it clear using a table or something later on.

The following table illustrates what happens around a (fictitious) leap second. Imagine a new epoch will occur at UTC time value 10 and the new TAI - UTC offset will be 2 seconds. The columns of the table show the values of the relevant time variables.

U:     UTC time
CODE:  NTP time code
T:     TAI - UTC offset
P:     pending (explained below)

U CODE T P
--------------------
1 INS 1 1 leap second sheduled
--------------------
2 INS 1 1
--------------------

        ...
  --------------------
    8   INS     1   1
  --------------------
    9   INS     1   1
  --------------------
| 10   OOP     1   1  leap second, 1st tick
|~~~~~~~~~~~~~~~~~~~
|  9           2   0  leap second, 2nd and subsequent ticks

--------------------
10 WAIT 2 0 new epoch
--------------------
11 WAIT 2 0

Quoted 40 lines · Richard Cochran

Quoted 10 lines · Richard Cochran

Quoted 9 lines · John Stultz

Quoted 8 lines · Richard Cochran

kernel/time/timekeeping.c +8 -0

+	/* Tracks where we stand with regard to leap the second epoch. */
+	enum {
+		LEAP_IDLE,
+		LEAP_INS_PENDING,
+		LEAP_INS_DONE,
+		LEAP_DEL_PENDING,
+		LEAP_DEL_DONE,
+	} leap_state;

...

Quoted 2 lines

I don't think I am explaining this very well. I will try again to make it clear using a table or something later on.

The following table illustrates what happens around a (fictitious) leap second. Imagine a new epoch will occur at UTC time value 10 and the new TAI - UTC offset will be 2 seconds. The columns of the table show the values of the relevant time variables.

U:     UTC time
CODE:  NTP time code
T:     TAI - UTC offset
P:     pending (explained below)

U CODE T P
--------------------
1 INS 1 1 leap second sheduled
--------------------
2 INS 1 1
--------------------

       ...
 --------------------
   8   INS     1   1
 --------------------
   9   INS     1   1
 --------------------
| 10   OOP     1   1  leap second, 1st tick
|~~~~~~~~~~~~~~~~~~~
|  9           2   0  leap second, 2nd and subsequent ticks

Quoted 4 lines

--------------------
10 WAIT 2 0 new epoch
--------------------
11 WAIT 2 0

Not sure I'm still following.

It seems currently we have:

U CODE T
----------------
9 INS 1
----------------
10 INS 1 pre tick, post leap second edge (this is the technically incorrect interval)
~~~~~~~~~~~~~~~~
9 OOP 2 post tick, post leap second edge
----------------
10 WAIT 2 new epoch

If you're trying to correct the pre-tick, post leap second edge, the above provides all you need.

In the adjtimex code, all you have to do is:

if (unlikely(CODE == INS&& U == 10))

	/*note, we're not modifying state here, just returning corrected local values*/

	return (U-1, OOP, T+1);

return (U,CODE, T);

Since when the tick triggers, we'll move the CODE state appropriately.

Or am I still missing something?

Quoted 18 lines · John Stultz

It seems currently we have:

U CODE T
----------------
9 INS 1
----------------
10 INS 1 pre tick, post leap second edge (this is the technically incorrect interval)
~~~~~~~~~~~~~~~~
9 OOP 2 post tick, post leap second edge
----------------
10 WAIT 2 new epoch

If you're trying to correct the pre-tick, post leap second edge, the above provides all you need.

In the adjtimex code, all you have to do is:

Quoted 7 lines

if (unlikely(CODE == INS&& U == 10))

	/*note, we're not modifying state here, just returning corrected local values*/

	return (U-1, OOP, T+1);

return (U,CODE, T);

Okay, if you want it that way, then you will have to add the other cases. For example:

	switch (code) {
	case INS:
		if (U == epoch) {
			U--;
			T++;
			code = OOP;
		}
		break;
	case OOP:
		if (U == epoch) {

			code = WAIT;
		}
		break;
	case DEL:
		if (U == epoch - 1) {
			U++;
			T--;
			code = WAIT;
		}
		break;
	default:
		break;
	}
	return (U, code, T);

This is beginning to look a lot like the code in my patch. However, your approach is somewhat simpler, because it assumes the tick will never miss a second overflow.

Quoted 3 lines

Since when the tick triggers, we'll move the CODE state appropriately.

Or am I still missing something?

Considering the tickless options, is it safe to assume that the CODE state update will never be an entire second too late? If so, then I'll rework the patch as above. If not, then I think the patch I posted already handles all the cases.

Quoted 13 lines · Richard Cochran

Okay, if you want it that way, then you will have to add the other cases. For example:

	switch (code) {
	case INS:
		if (U == epoch) {
			U--;
			T++;
			code = OOP;
		}
		break;
	case OOP:
		if (U == epoch) {

epoch + 1 here, right?

Quoted 15 lines

			code = WAIT;
		}
		break;
	case DEL:
		if (U == epoch - 1) {
			U++;
			T--;
			code = WAIT;
		}
		break;
	default:
		break;
	}
	return (U, code, T);

Quoted 3 lines

This is beginning to look a lot like the code in my patch. However, your approach is somewhat simpler, because it assumes the tick will never miss a second overflow.

I'm a little unclear on the above, because it looks like you're modifying the state from the reader.

I still don't think it matters. If we know the when next leap second is supposed to be, if the time_state is INS, then we can still handle things without extra state.

if (unlikely(CODE == INS&& U == next_leap))

	return (U-1, OOP, T+1);

if (unlikely(CODE == INS&& U == next_leap + 1))

	return (U-1, WAIT, T+1);

if (unlikely(CODE == DEL&& U == next_leap - 1))

	return (U+1, WAIT, T-1);

So even if we somehow sleep for two seconds over the leap second, and then an application hits the read critical section before the timer interrupt comes in the update the state, we can still provide correct state transition in the reader.

Thus the only additional state you might need over what we already have is the next_leap value.

Quoted 14 lines · John Stultz

Quoted 13 lines · Richard Cochran

Okay, if you want it that way, then you will have to add the other cases. For example:

	switch (code) {
	case INS:
		if (U == epoch) {
			U--;
			T++;
			code = OOP;
		}
		break;
	case OOP:
		if (U == epoch) {

Quoted 1 line

epoch + 1 here, right?

No, I really mean epoch (not the leap second value, but the value when the new TAI offset comes into effect).

Quoted 15 lines

Quoted 15 lines

			code = WAIT;
		}
		break;
	case DEL:
		if (U == epoch - 1) {
			U++;
			T--;
			code = WAIT;
		}
		break;
	default:
		break;
	}
	return (U, code, T);

Quoted 5 lines

Quoted 3 lines

This is beginning to look a lot like the code in my patch. However, your approach is somewhat simpler, because it assumes the tick will never miss a second overflow.

I'm a little unclear on the above, because it looks like you're modifying the state from the reader.

Sorry about that. Here is a more exact pseudo code:

	switch (time_state) {
	case INS:
		if (U == epoch) {
			U--;
			T++;
			result_code = OOP;
		}
		break;
	case OOP:
		if (U == epoch) {
			result_code = WAIT;
		}
		break;
	case DEL:
		if (U == epoch - 1) {
			U++;

			T--;
			result_code = WAIT;
		}
		break;
	default:
		break;
	}
	return (U, result_code, T);

Quoted 7 lines

I still don't think it matters. If we know the when next leap second is supposed to be, if the time_state is INS, then we can still handle things without extra state.

if (unlikely(CODE == INS&& U == next_leap))

	return (U-1, OOP, T+1);

Quoted 2 lines

if (unlikely(CODE == INS&& U == next_leap + 1))

	return (U-1, WAIT, T+1);

And what if (U> next_leap + 1)?

In that case, you must also return WAIT. Are you going to add a test for every second beyond 'next_leap'? I don't think so.

Quoted 8 lines

if (unlikely(CODE == DEL&& U == next_leap - 1))

	return (U+1, WAIT, T-1);

So even if we somehow sleep for two seconds over the leap second, and then an application hits the read critical section before the timer interrupt comes in the update the state, we can still provide correct state transition in the reader.

No, I think what you wrote above is not correct.

Quoted 2 lines

Thus the only additional state you might need over what we already have is the next_leap value.

Again, you will need two things.

1. the epoch threshold value (not the leap second value)

2. whether the new epoch has been applied yet, or not

So I've avoided the term epoch just to try not to confuse things with the unix epoch, that's why I've used next_leap, etc. Even so, I'm not sure you've made clear the subtlety of the difference.

I believe the internal time_state (along with the next leap second) already provides this.

From the reader's perspective:

Not applied: (INS&& U< leap): return (INS, U)
Applied: (INS&& U == leap): return (OOP, U-1)
Finished applied: ((INS||OOP)&& U>= (leap+1)): return (WAIT,U-1)
Delete: (DEL&& U>= (leap-1)): return (WAIT,U+1)

Again, no state change is done by the reader, so we don't have to keep track of application state or not.
Then when the tick comes in, it will move the state machine appropriately.

Sorry working this out is so difficult. If we don't come to consensus soon, I'll try to find some time to implement what I'm suggesting so you aren't up against my unclear hand-waving. :)

BTW you can use the program I have been using to test this at

git://github.com/richardcochran/leap.git

Quoted 3 lines · Richard Cochran

BTW you can use the program I have been using to test this at

git://github.com/richardcochran/leap.git

That program exposes another leap second bug, too, I think. It reads the time via adjtimex in a tight loop, optionally sleeping using

	clock_nanosleep(CLOCK_MONOTONIC, 0, &ts, NULL);

The program does not wake from this call during a leap second. It is my expectation that CLOCK_MONOTONIC should always work. Why doesn't it?

Quoted 3 lines · Richard Cochran

BTW you can use the program I have been using to test this at

git://github.com/richardcochran/leap.git

That program exposes another leap second bug, too, I think. It reads the time via adjtimex in a tight loop, optionally sleeping using

	clock_nanosleep(CLOCK_MONOTONIC, 0,&ts, NULL);

The program does not wake from this call during a leap second. It is my expectation that CLOCK_MONOTONIC should always work. Why doesn't it?

Quoted 9 lines · Richard Cochran

Quoted 3 lines · Richard Cochran

BTW you can use the program I have been using to test this at

git://github.com/richardcochran/leap.git

That program exposes another leap second bug, too, I think. It reads the time via adjtimex in a tight loop, optionally sleeping using

    clock_nanosleep(CLOCK_MONOTONIC, 0,&ts, NULL);

Quoted 3 lines

The program does not wake from this call during a leap second. It is my expectation that CLOCK_MONOTONIC should always work. Why doesn't it?

Sorry for being slow here, just got a chance to look at this.

Does the following patch solve this issue for you?

kernel/time/timekeeping.c static cycle_t logarithmic_accumulation(cycle_t offset, int shift) · line 965

Quoted 9 lines · John Stultz

kernel/time/timekeeping.c +1 -0

index 6e46cac..81c76a9 100644
@@ -962,6 +962,7 @@ static cycle_t logarithmic_accumulation(cycle_t offset, int shift)
  		timekeeper.xtime.tv_sec++;
  		leap = second_overflow(timekeeper.xtime.tv_sec);
  		timekeeper.xtime.tv_sec += leap;
+		timekeeper.wall_to_monotonic.tv_sec -= leap;

Don't you need this in update_wall_time() too?

BTW I suggest refactoring this code (two almost identical if{} bodies) into a shared helper function.

Quoted 11 lines · Richard Cochran

kernel/time/timekeeping.c static cycle_t logarithmic_accumulation(cycle_t offset, int shift) · line 965

Quoted 9 lines · John Stultz

kernel/time/timekeeping.c +1 -0

index 6e46cac..81c76a9 100644
@@ -962,6 +962,7 @@ static cycle_t logarithmic_accumulation(cycle_t offset, int shift)
 		timekeeper.xtime.tv_sec++;
 		leap = second_overflow(timekeeper.xtime.tv_sec);
 		timekeeper.xtime.tv_sec += leap;
+		timekeeper.wall_to_monotonic.tv_sec -= leap;

Don't you need this in update_wall_time() too?

Yep. Good point.

Quoted 12 lines · John Stultz

Quoted 11 lines · Richard Cochran

kernel/time/timekeeping.c static cycle_t logarithmic_accumulation(cycle_t offset, int shift) · line 965

Quoted 9 lines · John Stultz

kernel/time/timekeeping.c +1 -0

index 6e46cac..81c76a9 100644
@@ -962,6 +962,7 @@ static cycle_t logarithmic_accumulation(cycle_t offset, int shift)
  		timekeeper.xtime.tv_sec++;
  		leap = second_overflow(timekeeper.xtime.tv_sec);
  		timekeeper.xtime.tv_sec += leap;
+		timekeeper.wall_to_monotonic.tv_sec -= leap;

Don't you need this in update_wall_time() too?

Quoted 1 line

Yep. Good point.

Okay, so I can confirm that this fixes the CLOCK_MONOTONIC timer issue during a leap second.

Quoted 13 lines · Richard Cochran

Quoted 12 lines · John Stultz

Quoted 11 lines · Richard Cochran

kernel/time/timekeeping.c static cycle_t logarithmic_accumulation(cycle_t offset, int shift) · line 965

Quoted 9 lines · John Stultz

kernel/time/timekeeping.c +1 -0

index 6e46cac..81c76a9 100644
@@ -962,6 +962,7 @@ static cycle_t logarithmic_accumulation(cycle_t offset, int shift)
 		timekeeper.xtime.tv_sec++;
 		leap = second_overflow(timekeeper.xtime.tv_sec);
 		timekeeper.xtime.tv_sec += leap;
+		timekeeper.wall_to_monotonic.tv_sec -= leap;

Don't you need this in update_wall_time() too?

Quoted 3 lines

Quoted 1 line

Yep. Good point.

Okay, so I can confirm that this fixes the CLOCK_MONOTONIC timer issue during a leap second.

Thanks! Is it ok if I add your Tested-by: to the patch?

Thus the only additional state you might need over what we already have is the next_leap value.

This patch introduces time keeping variables to track the next mini-epoch between the UTC and TAI timescales. A leap second occurs one second before a mini-epoch. When no leap second is pending, then the epoch is set to the far future, LONG_MAX.

This code will become useful later on for providing correct time surrounding a leap second.

Signed-off-by Richard Cochran<[email protected]>

---
  kernel/time/timekeeping.c |   14 ++++++++++++++
  1 files changed, 14 insertions(+), 0 deletions(-)

kernel/time/timekeeping.c +3 -0

index d66b213..ac04de4 100644
@@ -70,6 +70,19 @@ struct timekeeper {
  	/* The raw monotonic time for the CLOCK_MONOTONIC_RAW posix clock. */
  	struct timespec raw_time;
 
+	/* The current TAI - UTC offset */
+	time_t tai_offset;
+	/* The UTC time of the next leap second epoch */

Quoted 21 lines · Richard Cochran

This patch introduces time keeping variables to track the next mini-epoch between the UTC and TAI timescales. A leap second occurs one second before a mini-epoch. When no leap second is pending, then the epoch is set to the far future, LONG_MAX.

This code will become useful later on for providing correct time surrounding a leap second.

Signed-off-by Richard Cochran<[email protected]>

---
 kernel/time/timekeeping.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

kernel/time/timekeeping.c +0 -0

index d66b213..ac04de4 100644
@@ -70,6 +70,19 @@ struct timekeeper {
 	/* The raw monotonic time for the CLOCK_MONOTONIC_RAW posix clock. */
 	struct timespec raw_time;
 

kernel/time/timekeeping.c struct timekeeper { · line 85

Quoted 13 lines

kernel/time/timekeeping.c +13 -0

+	/* The current TAI - UTC offset */
+	time_t tai_offset;
+	/* The UTC time of the next leap second epoch */
+	time_t utc_epoch;
+	/* Tracks where we stand with regard to leap the second epoch. */
+	enum {
+		LEAP_IDLE,
+		LEAP_INS_PENDING,
+		LEAP_INS_DONE,
+		LEAP_DEL_PENDING,
+		LEAP_DEL_DONE,
+	} leap_state;
+

One other thing, I'd rather you break this patch set up logically as:
1) Move tai offset management into timeekeeping core

2) Move leap_state managment into timekeeping core.

Instead of:
1) Add unused values to timekeeper for tai and leapstate
2) Convert NTP code to use both these new values

This patch adds a new private leap second interface for use by the NTP code. In addition to methods for starting and ending leap seconds, the interface provides a way to get the correct UTC time of day even during a leap second.

Signed-off-by Richard Cochran<[email protected]>

---
  kernel/time/leap-seconds.h |   21 +++++++
  kernel/time/timekeeping.c  |  129 ++++++++++++++++++++++++++++++++++++++++++++
  2 files changed, 150 insertions(+), 0 deletions(-)

create mode 100644 kernel/time/leap-seconds.h

kernel/time/leap-seconds.h new file +11 -0

new file mode 100644
index 0000000..3dea7b0
@@ -0,0 +1,21 @@
+/*
+ * linux/kernel/time/leap-seconds.h
+ *
+ * Functional interface to the timekeeper code,
+ * for use by the NTP code.
+ *
+ */
+#ifndef __LINUX_KERNEL_TIME_LEAP_SECONDS_H
+#define __LINUX_KERNEL_TIME_LEAP_SECONDS_H
+
+#include<linux/time.h>

kernel/time/leap-seconds.h line 21

Quoted 10 lines · Richard Cochran

kernel/time/leap-seconds.h new file +10 -0

+
+int timekeeping_gettod_status(struct timespec *ts, time_t *offset);
+
+void timekeeping_delete_leap_second(void);
+
+void timekeeping_finish_leap_second(void);
+
+void timekeeping_insert_leap_second(void);
+
+#endif

Why not just add these to time.h?

Quoted 12 lines · John Stultz

kernel/time/leap-seconds.h line 21

Quoted 10 lines · Richard Cochran

kernel/time/leap-seconds.h new file +10 -0

+
+int timekeeping_gettod_status(struct timespec *ts, time_t *offset);
+
+void timekeeping_delete_leap_second(void);
+
+void timekeeping_finish_leap_second(void);
+
+void timekeeping_insert_leap_second(void);
+
+#endif

Why not just add these to time.h?

This is a private interface only for ntp.c, not for the whole rest of the kernel via time.h.

BTW this highlights the very icky incestuous relationship between ntp.c and timekeeper.c. Probably there should be a comment documenting the (unspoken) locking sequence for ntp_lock and timekeeper.lock.

On 05/21/2012 12:18 PM, Richard Cochran wrote:
Hrm. I prefer to keep things fairly flat (even having time.h and timex.h bugs me somewhat). But having such a separation could be useful, but maybe at a slightly more coarse level. Something like timekeeping-internal.h and time.h, splitting all the general accessors away from the non-general.

The locking order is pretty straight forward: timekeeper.lock ->
ntp_lock. This only gets messy when you require timekeeping data
from the ntp context, but usually we provide the required data via the caller. But better documentation is always welcome.

Quoted 4 lines · John Stultz

The locking order is pretty straight forward: timekeeper.lock ->
ntp_lock. This only gets messy when you require timekeeping data
from the ntp context, but usually we provide the required data via the caller. But better documentation is always welcome.

The icky part is the fact that ntp would need access to timekeeper state while holding ntp_lock.

This patch refactors the timekeeping and ntp code in order to improve leap second handling and to provide a TAI based clock. The change has a number of aspects.

* remove the NTP time_status variable

Instead, compute this value on demand in adjtimex.

* move TAI managment into timekeeping core from ntp

Currently NTP manages the TAI offset. Since there's plans for a CLOCK_TAI clockid, push the TAI management into the timekeeping core.

[ Original idea from: John Stultz<[email protected]> ] [ Changed by RC: ]

- replace u32 with time_t
- fixup second call site of second_overflow()

* replace modulus with integer test and schedule leap second

On the day of a leap second, the NTP code performs a division on every tick in order to know when to leap. This patch replaces the division with an integer comparison, making the code faster and easier to understand.

Signed-off-by Richard Cochran<[email protected]>

---
  include/linux/time.h      |    1 +
  kernel/time/ntp.c         |   86 +++++++++++++-------------------------------
  kernel/time/timekeeping.c |   54 ++++++++++++++++++++++++----
  3 files changed, 73 insertions(+), 68 deletions(-)

include/linux/time.h +1 -0

index 179f4d6..b6034b0 100644
@@ -168,6 +168,7 @@ extern struct timespec timespec_trunc(struct timespec t, unsigned gran);
  extern int timekeeping_valid_for_hres(void);
  extern u64 timekeeping_max_deferment(void);
  extern int timekeeping_inject_offset(struct timespec *ts);
+extern void timekeeping_set_tai_offset(time_t tai_offset);
 
  struct tms;
  extern void do_sys_times(struct tms *);

kernel/time/ntp.c +2 -10

index d4d48b0..53c3227 100644
@@ -16,6 +16,7 @@
  #include<linux/mm.h>
  #include<linux/module.h>
 
+#include "leap-seconds.h"
  #include "tick-internal.h"
 
  /*

@@ -24,6 +25,7 @@
 
  DEFINE_SPINLOCK(ntp_lock);
 
+#define STA_LEAP		(STA_INS | STA_DEL)
 
  /* USER_HZ period (usecs): */
  unsigned long			tick_usec = TICK_USEC;

@@ -42,19 +44,9 @@ static u64			tick_length_base;
   * phase-lock loop variables
   */
 
-/*
- * clock synchronization status
- *
- * (TIME_ERROR prevents overwriting the CMOS clock)
- */
-static int			time_state = TIME_OK;
-
  /* clock status bits:							*/
  static int			time_status = STA_UNSYNC;
 
-/* TAI offset (secs):							*/
-static long			time_tai;
-
  /* time adjustment (nsecs):						*/
  static s64			time_offset;
 

kernel/time/ntp.c +1 -43

  {
  	s64 delta;
-	int leap = 0;
  	unsigned long flags;
 
  	spin_lock_irqsave(&ntp_lock, flags);
 
-	/*
-	 * Leap second processing. If in leap-insert state at the end of the
-	 * day, the system clock is set back one second; if in leap-delete
-	 * state, the system clock is set ahead one second.
-	 */
-	switch (time_state) {
-	case TIME_OK:
-		if (time_status&  STA_INS)
-			time_state = TIME_INS;
-		else if (time_status&  STA_DEL)
-			time_state = TIME_DEL;
-		break;
-	case TIME_INS:
-		if (secs % 86400 == 0) {
-			leap = -1;
-			time_state = TIME_OOP;
-			time_tai++;
-			printk(KERN_NOTICE
-				"Clock: inserting leap second 23:59:60 UTC\n");
-		}
-		break;
-	case TIME_DEL:
-		if ((secs + 1) % 86400 == 0) {
-			leap = 1;
-			time_tai--;
-			time_state = TIME_WAIT;
-			printk(KERN_NOTICE
-				"Clock: deleting leap second 23:59:59 UTC\n");
-		}
-		break;
-	case TIME_OOP:
-		time_state = TIME_WAIT;
-		break;
-
-	case TIME_WAIT:
-		if (!(time_status&  (STA_INS | STA_DEL)))
-			time_state = TIME_OK;
-		break;
-	}
-
-
  	/* Bump the maxerror field */
  	time_maxerror += MAXFREQ / NSEC_PER_USEC;
  	if (time_maxerror>  NTP_PHASE_LIMIT) {

@@ -478,7 +427,7 @@ int second_overflow(unsigned long secs)
  out:
  	spin_unlock_irqrestore(&ntp_lock, flags);
 
-	return leap;
+	return 0;
  }

kernel/time/ntp.c +6 -1

-	getnstimeofday(&ts);
+	result = timekeeping_gettod_status(&ts,&orig_tai);
+	time_tai = orig_tai;
+
+	if (txc->modes&  ADJ_STATUS) {
+		/*
+		 * Check for new leap second commands.

  	spin_lock_irq(&ntp_lock);

kernel/time/ntp.c +4 -2

@@ -673,7 +637,7 @@ int do_adjtimex(struct timex *txc)
 
  		/* If there are input parameters, then process them: */
  		if (txc->modes)
-			process_adjtimex_modes(txc);
+			process_adjtimex_modes(txc,&time_tai);
 
  		txc->offset = shift_right(time_offset * NTP_INTERVAL_FREQ,
  				  NTP_SCALE_SHIFT);

@@ -681,7 +645,6 @@ int do_adjtimex(struct timex *txc)
  			txc->offset /= NSEC_PER_USEC;
  	}
 
-	result = time_state;	/* mostly `TIME_OK' */
  	/* check for errors */
  	if (is_error_status(time_status))
  		result = TIME_ERROR;

@@ -702,6 +665,9 @@ int do_adjtimex(struct timex *txc)
 
  	spin_unlock_irq(&ntp_lock);
 
+	if (time_tai != orig_tai&&  result == TIME_OK)
+		timekeeping_set_tai_offset(time_tai);
+
  	txc->time.tv_sec = ts.tv_sec;
  	txc->time.tv_usec = ts.tv_nsec;
  	if (!(time_status&  STA_NANO))

kernel/time/timekeeping.c +12 -0

index eab03fb..fdd1a48 100644
@@ -444,6 +444,18 @@ int timekeeping_inject_offset(struct timespec *ts)
  EXPORT_SYMBOL(timekeeping_inject_offset);
 
  /**
+ * timekeeping_set_tai_offset - Sets the current TAI offset from UTC
+ */
+void timekeeping_set_tai_offset(time_t tai_offset)
+{
+	unsigned long flags;
+
+	write_seqlock_irqsave(&timekeeper.lock, flags);
+	timekeeper.tai_offset = tai_offset;
+	write_sequnlock_irqrestore(&timekeeper.lock, flags);
+}
+
+/**
   * change_clocksource - Swaps clocksources if a new one is available
   *
   * Accumulates current time interval and initializes new clocksource

Quoted 86 lines · Richard Cochran

This patch refactors the timekeeping and ntp code in order to improve leap second handling and to provide a TAI based clock. The change has a number of aspects.

* remove the NTP time_status variable

Instead, compute this value on demand in adjtimex.

* move TAI managment into timekeeping core from ntp

Currently NTP manages the TAI offset. Since there's plans for a CLOCK_TAI clockid, push the TAI management into the timekeeping core.

[ Original idea from: John Stultz<[email protected]> ] [ Changed by RC: ]

- replace u32 with time_t
- fixup second call site of second_overflow()

* replace modulus with integer test and schedule leap second

On the day of a leap second, the NTP code performs a division on every tick in order to know when to leap. This patch replaces the division with an integer comparison, making the code faster and easier to understand.

Signed-off-by Richard Cochran<[email protected]>

---
 include/linux/time.h      |    1 +
 kernel/time/ntp.c         |   86 +++++++++++++-------------------------------
 kernel/time/timekeeping.c |   54 ++++++++++++++++++++++++----
 3 files changed, 73 insertions(+), 68 deletions(-)

include/linux/time.h +1 -0

index 179f4d6..b6034b0 100644
@@ -168,6 +168,7 @@ extern struct timespec timespec_trunc(struct timespec t, unsigned gran);
 extern int timekeeping_valid_for_hres(void);
 extern u64 timekeeping_max_deferment(void);
 extern int timekeeping_inject_offset(struct timespec *ts);
+extern void timekeeping_set_tai_offset(time_t tai_offset);
 
 struct tms;
 extern void do_sys_times(struct tms *);

kernel/time/ntp.c +2 -10

index d4d48b0..53c3227 100644
@@ -16,6 +16,7 @@
 #include<linux/mm.h>
 #include<linux/module.h>
 
+#include "leap-seconds.h"
 #include "tick-internal.h"
 
 /*

@@ -24,6 +25,7 @@
 
 DEFINE_SPINLOCK(ntp_lock);
 
+#define STA_LEAP		(STA_INS | STA_DEL)
 
 /* USER_HZ period (usecs): */
 unsigned long			tick_usec = TICK_USEC;

@@ -42,19 +44,9 @@ static u64			tick_length_base;
  * phase-lock loop variables
  */
 
-/*
- * clock synchronization status
- *
- * (TIME_ERROR prevents overwriting the CMOS clock)
- */
-static int			time_state = TIME_OK;
-
 /* clock status bits:							*/
 static int			time_status = STA_UNSYNC;
 
-/* TAI offset (secs):							*/
-static long			time_tai;
-
 /* time adjustment (nsecs):						*/
 static s64			time_offset;
 

kernel/time/ntp.c u64 ntp_tick_length(void) · line 382

Quoted 7 lines

kernel/time/ntp.c +0 -1

@@ -386,57 +378,14 @@ u64 ntp_tick_length(void)
  * They were originally developed for SUN and DEC kernels.
  * All the kudos should go to Dave for this stuff.
  *
- * Also handles leap second processing, and returns leap offset
  */
 int second_overflow(unsigned long secs)

Might be good to rename second_overflow() to ntp_second_overflow() and drop passing the time_t as its now unused.

Quoted 58 lines

kernel/time/ntp.c +1 -43

 {
 	s64 delta;
-	int leap = 0;
 	unsigned long flags;
 
 	spin_lock_irqsave(&ntp_lock, flags);
 
-	/*
-	 * Leap second processing. If in leap-insert state at the end of the
-	 * day, the system clock is set back one second; if in leap-delete
-	 * state, the system clock is set ahead one second.
-	 */
-	switch (time_state) {
-	case TIME_OK:
-		if (time_status&  STA_INS)
-			time_state = TIME_INS;
-		else if (time_status&  STA_DEL)
-			time_state = TIME_DEL;
-		break;
-	case TIME_INS:
-		if (secs % 86400 == 0) {
-			leap = -1;
-			time_state = TIME_OOP;
-			time_tai++;
-			printk(KERN_NOTICE
-				"Clock: inserting leap second 23:59:60 UTC\n");
-		}
-		break;
-	case TIME_DEL:
-		if ((secs + 1) % 86400 == 0) {
-			leap = 1;
-			time_tai--;
-			time_state = TIME_WAIT;
-			printk(KERN_NOTICE
-				"Clock: deleting leap second 23:59:59 UTC\n");
-		}
-		break;
-	case TIME_OOP:
-		time_state = TIME_WAIT;
-		break;
-
-	case TIME_WAIT:
-		if (!(time_status&  (STA_INS | STA_DEL)))
-			time_state = TIME_OK;
-		break;
-	}
-
-
 	/* Bump the maxerror field */
 	time_maxerror += MAXFREQ / NSEC_PER_USEC;
 	if (time_maxerror>  NTP_PHASE_LIMIT) {

@@ -478,7 +427,7 @@ int second_overflow(unsigned long secs)
 out:
 	spin_unlock_irqrestore(&ntp_lock, flags);
 
-	return leap;
+	return 0;
 }

[snip]

Quoted 7 lines

kernel/time/ntp.c +6 -1

-	getnstimeofday(&ts);
+	result = timekeeping_gettod_status(&ts,&orig_tai);
+	time_tai = orig_tai;
+
+	if (txc->modes&  ADJ_STATUS) {
+		/*
+		 * Check for new leap second commands.

kernel/time/ntp.c int do_adjtimex(struct timex *txc) · line 623

Quoted 10 lines

kernel/time/ntp.c +10 -0

+		 */
+		if (!(time_status&  STA_INS)&&  (txc->status&  STA_INS))
+			timekeeping_insert_leap_second();
+
+		else if (!(time_status&  STA_DEL)&&  (txc->status&  STA_DEL))
+			timekeeping_delete_leap_second();
+
+		else if ((time_status&  STA_LEAP)&&  !(txc->status&  STA_LEAP))
+			timekeeping_finish_leap_second();
+	}

Doing this early doesn't run into any trouble with the tcx->modes rules, right?

I'm just worried about changes in behavior if modes = ADJ_ADJTIME|ADJ_STATUS

Quoted 58 lines

kernel/time/ntp.c +4 -2

 	spin_lock_irq(&ntp_lock);

@@ -673,7 +637,7 @@ int do_adjtimex(struct timex *txc)
 
 		/* If there are input parameters, then process them: */
 		if (txc->modes)
-			process_adjtimex_modes(txc);
+			process_adjtimex_modes(txc,&time_tai);
 
 		txc->offset = shift_right(time_offset * NTP_INTERVAL_FREQ,
 				  NTP_SCALE_SHIFT);

@@ -681,7 +645,6 @@ int do_adjtimex(struct timex *txc)
 			txc->offset /= NSEC_PER_USEC;
 	}
 
-	result = time_state;	/* mostly `TIME_OK' */
 	/* check for errors */
 	if (is_error_status(time_status))
 		result = TIME_ERROR;

@@ -702,6 +665,9 @@ int do_adjtimex(struct timex *txc)
 
 	spin_unlock_irq(&ntp_lock);
 
+	if (time_tai != orig_tai&&  result == TIME_OK)
+		timekeeping_set_tai_offset(time_tai);
+
 	txc->time.tv_sec = ts.tv_sec;
 	txc->time.tv_usec = ts.tv_nsec;
 	if (!(time_status&  STA_NANO))

kernel/time/timekeeping.c +12 -0

index eab03fb..fdd1a48 100644
@@ -444,6 +444,18 @@ int timekeeping_inject_offset(struct timespec *ts)
 EXPORT_SYMBOL(timekeeping_inject_offset);
 
 /**
+ * timekeeping_set_tai_offset - Sets the current TAI offset from UTC
+ */
+void timekeeping_set_tai_offset(time_t tai_offset)
+{
+	unsigned long flags;
+
+	write_seqlock_irqsave(&timekeeper.lock, flags);
+	timekeeper.tai_offset = tai_offset;
+	write_sequnlock_irqrestore(&timekeeper.lock, flags);
+}
+
+/**
  * change_clocksource - Swaps clocksources if a new one is available
  *
  * Accumulates current time interval and initializes new clocksource

@@ -950,6 +962,38 @@ static void timekeeping_adjust(s64 offset)
 				timekeeper.ntp_error_shift;
 }
 

kernel/time/timekeeping.c static void timekeeping_adjust(s64 offset) · line 965

Quoted 1 line

kernel/time/timekeeping.c +1 -0

+static void timekeeper_overflow(void)

Mind renaming this to timekeeper_second_overflow, just so readers don't mix it up with clocksource related overflows.