Re: [PATCH RFC V2 5/6] time: move leap second management into timekeeping core
From: John Stultz
Date: Mon May 21 2012 - 14:18:51 EST
On 05/18/2012 07:09 AM, Richard Cochran wrote:
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<john.stultz@xxxxxxxxxx> ]
[ 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<richardcochran@xxxxxxxxx>
---
include/linux/time.h | 1 +
kernel/time/ntp.c | 86 +++++++++++++-------------------------------
kernel/time/timekeeping.c | 54 ++++++++++++++++++++++++----
3 files changed, 73 insertions(+), 68 deletions(-)
diff --git a/include/linux/time.h b/include/linux/time.h
index 179f4d6..b6034b0 100644
--- a/include/linux/time.h
+++ b/include/linux/time.h
@@ -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 *);
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index d4d48b0..53c3227 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -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;
@@ -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.
{
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]
- getnstimeofday(&ts);
+ result = timekeeping_gettod_status(&ts,&orig_tai);
+ time_tai = orig_tai;
+
+ if (txc->modes& ADJ_STATUS) {
+ /*
+ * Check for new leap second commands.
+ */
+ 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
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))
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index eab03fb..fdd1a48 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -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;
}
+static void timekeeper_overflow(void)
Mind renaming this to timekeeper_second_overflow, just so readers don't
mix it up with clocksource related overflows.
thanks
-john
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/