| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 21 May 2012 11:18:12 -0700
From: John Stultz <john.stultz@...aro.org>
To: Richard Cochran <richardcochran@...il.com>
CC: linux-kernel@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH RFC V2 5/6] time: move leap second management into time
keeping core
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@...aro.org> ]
> [ 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@...il.com>
> ---
> 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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists