[PATCH 0/8] NTP updates - Kernel

This is a discussion on [PATCH 0/8] NTP updates - Kernel ; Hi, These patches clean up the NTP code a little and add the remaining user space bits from the ntp nanokernel, so that it's e.g. now possible to enable the nanosecond resolution, which should be useful to keep time in ...

+ Reply to Thread
Page 1 of 2 1 2 LastLast
Results 1 to 20 of 24

Thread: [PATCH 0/8] NTP updates

  1. [PATCH 0/8] NTP updates

    Hi,

    These patches clean up the NTP code a little and add the remaining user
    space bits from the ntp nanokernel, so that it's e.g. now possible to
    enable the nanosecond resolution, which should be useful to keep time in
    a local network in sync (but it also needs an updated glibc/ntpd first).

    I tried my best to also update the various user space APIs (e.g. compat,
    vsyscall), but it would be great if someone could look over it, in case
    I forgot something.

    bye, Roman

    --

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. [PATCH 2/8] NTP4 user space bits update

    This adds a few more things from the ntp nanokernel related to user
    space. It's now possible to select the resolution used of some values
    via STA_NANO and the kernel reports in which mode it works (pll/fll).

    If some values for adjtimex() are outside the acceptable range, they are
    now simply normalized instead of letting the syscall fail.
    I removed MOD_CLKA/MOD_CLKB as the mapping didn't really makes any
    sense, the kernel doesn't support setting the clock.

    Signed-off-by: Roman Zippel

    ---
    include/linux/timex.h | 12 ++++--
    kernel/time/ntp.c | 91 +++++++++++++++++++++++++-------------------------
    2 files changed, 56 insertions(+), 47 deletions(-)

    Index: linux-2.6/include/linux/timex.h
    ================================================== =================
    --- linux-2.6.orig/include/linux/timex.h 2008-03-13 10:42:20.000000000 +0100
    +++ linux-2.6/include/linux/timex.h 2008-03-13 10:48:59.000000000 +0100
    @@ -58,6 +58,8 @@

    #include

    +#define NTP_API 4 /* NTP API version */
    +
    /*
    * SHIFT_KG and SHIFT_KF establish the damping of the PLL and are chosen
    * for a slightly underdamped convergence characteristic. SHIFT_KH
    @@ -135,6 +137,8 @@ struct timex {
    #define ADJ_ESTERROR 0x0008 /* estimated time error */
    #define ADJ_STATUS 0x0010 /* clock status */
    #define ADJ_TIMECONST 0x0020 /* pll time constant */
    +#define ADJ_MICRO 0x1000 /* select microsecond resolution */
    +#define ADJ_NANO 0x2000 /* select nanosecond resolution */
    #define ADJ_TICK 0x4000 /* tick value */
    #define ADJ_OFFSET_SINGLESHOT 0x8001 /* old-fashioned adjtime */
    #define ADJ_OFFSET_SS_READ 0xa001 /* read-only adjtime */
    @@ -146,8 +150,6 @@ struct timex {
    #define MOD_ESTERROR ADJ_ESTERROR
    #define MOD_STATUS ADJ_STATUS
    #define MOD_TIMECONST ADJ_TIMECONST
    -#define MOD_CLKB ADJ_TICK
    -#define MOD_CLKA ADJ_OFFSET_SINGLESHOT /* 0x8000 in original */


    /*
    @@ -169,9 +171,13 @@ struct timex {
    #define STA_PPSERROR 0x0800 /* PPS signal calibration error (ro) */

    #define STA_CLOCKERR 0x1000 /* clock hardware fault (ro) */
    +#define STA_NANO 0x2000 /* resolution (0 = us, 1 = ns) (ro) */
    +#define STA_MODE 0x4000 /* mode (0 = PLL, 1 = FLL) (ro) */
    +#define STA_CLK 0x8000 /* clock source (0 = A, 1 = B) (ro) */

    +/* read-only bits */
    #define STA_RONLY (STA_PPSSIGNAL | STA_PPSJITTER | STA_PPSWANDER | \
    - STA_PPSERROR | STA_CLOCKERR) /* read-only bits */
    + STA_PPSERROR | STA_CLOCKERR | STA_NANO | STA_MODE | STA_CLK)

    /*
    * Clock states (time_state)
    Index: linux-2.6/kernel/time/ntp.c
    ================================================== =================
    --- linux-2.6.orig/kernel/time/ntp.c 2008-03-13 10:42:20.000000000 +0100
    +++ linux-2.6/kernel/time/ntp.c 2008-03-13 10:47:49.000000000 +0100
    @@ -65,7 +65,9 @@ static void ntp_update_offset(long offse
    if (!(time_status & STA_PLL))
    return;

    - time_offset = offset * NSEC_PER_USEC;
    + time_offset = offset;
    + if (!(time_status & STA_NANO))
    + time_offset *= NSEC_PER_USEC;

    /*
    * Scale the phase adjustment and
    @@ -86,8 +88,11 @@ static void ntp_update_offset(long offse
    freq_adj = time_offset * mtemp;
    freq_adj = shift_right(freq_adj, time_constant * 2 +
    (SHIFT_PLL + 2) * 2 - SHIFT_NSEC);
    - if (mtemp >= MINSEC && (time_status & STA_FLL || mtemp > MAXSEC))
    + time_status &= ~STA_MODE;
    + if (mtemp >= MINSEC && (time_status & STA_FLL || mtemp > MAXSEC)) {
    freq_adj += div_s64(time_offset << (SHIFT_NSEC - SHIFT_FLL), mtemp);
    + time_status |= STA_MODE;
    + }
    freq_adj += time_freq;
    freq_adj = min(freq_adj, (s64)MAXFREQ_NSEC);
    time_freq = max(freq_adj, (s64)-MAXFREQ_NSEC);
    @@ -272,6 +277,7 @@ static inline void notify_cmos_timer(voi
    */
    int do_adjtimex(struct timex *txc)
    {
    + struct timespec ts;
    long save_adjust;
    int result;

    @@ -282,17 +288,11 @@ int do_adjtimex(struct timex *txc)
    /* Now we validate the data before disabling interrupts */

    if ((txc->modes & ADJ_OFFSET_SINGLESHOT) == ADJ_OFFSET_SINGLESHOT) {
    - /* singleshot must not be used with any other mode bits */
    - if (txc->modes != ADJ_OFFSET_SINGLESHOT &&
    - txc->modes != ADJ_OFFSET_SS_READ)
    + /* singleshot must not be used with any other mode bits */
    + if (txc->modes & ~ADJ_OFFSET_SS_READ)
    return -EINVAL;
    }

    - if (txc->modes != ADJ_OFFSET_SINGLESHOT && (txc->modes & ADJ_OFFSET))
    - /* adjustment Offset limited to +- .512 seconds */
    - if (txc->offset <= - MAXPHASE || txc->offset >= MAXPHASE )
    - return -EINVAL;
    -
    /* if the quartz is off by more than 10% something is VERY wrong ! */
    if (txc->modes & ADJ_TICK)
    if (txc->tick < 900000/USER_HZ ||
    @@ -300,51 +300,46 @@ int do_adjtimex(struct timex *txc)
    return -EINVAL;

    write_seqlock_irq(&xtime_lock);
    - result = time_state; /* mostly `TIME_OK' */

    /* Save for later - semantics of adjtime is to return old value */
    save_adjust = time_adjust;

    -#if 0 /* STA_CLOCKERR is never set yet */
    - time_status &= ~STA_CLOCKERR; /* reset STA_CLOCKERR */
    -#endif
    /* If there are input parameters, then process them */
    if (txc->modes) {
    - if (txc->modes & ADJ_STATUS) /* only set allowed bits */
    - time_status = (txc->status & ~STA_RONLY) |
    - (time_status & STA_RONLY);
    + if (txc->modes & ADJ_STATUS) {
    + if ((time_status & STA_PLL) &&
    + !(txc->status & STA_PLL)) {
    + time_state = TIME_OK;
    + time_status = STA_UNSYNC;
    + }
    + /* only set allowed bits */
    + time_status &= STA_RONLY;
    + time_status |= txc->status & ~STA_RONLY;
    + }
    +
    + if (txc->modes & ADJ_NANO)
    + time_status |= STA_NANO;
    + if (txc->modes & ADJ_MICRO)
    + time_status &= ~STA_NANO;

    if (txc->modes & ADJ_FREQUENCY) {
    - if (txc->freq > MAXFREQ || txc->freq < -MAXFREQ) {
    - result = -EINVAL;
    - goto leave;
    - }
    - time_freq = ((s64)txc->freq * NSEC_PER_USEC)
    + time_freq = min(txc->freq, MAXFREQ);
    + time_freq = min(time_freq, -MAXFREQ);
    + time_freq = ((s64)time_freq * NSEC_PER_USEC)
    >> (SHIFT_USEC - SHIFT_NSEC);

    }

    - if (txc->modes & ADJ_MAXERROR) {
    - if (txc->maxerror < 0 || txc->maxerror >= NTP_PHASE_LIMIT) {
    - result = -EINVAL;
    - goto leave;
    - }
    + if (txc->modes & ADJ_MAXERROR)
    time_maxerror = txc->maxerror;
    - }
    -
    - if (txc->modes & ADJ_ESTERROR) {
    - if (txc->esterror < 0 || txc->esterror >= NTP_PHASE_LIMIT) {
    - result = -EINVAL;
    - goto leave;
    - }
    + if (txc->modes & ADJ_ESTERROR)
    time_esterror = txc->esterror;
    - }

    if (txc->modes & ADJ_TIMECONST) {
    - if (txc->constant < 0) { /* NTP v4 uses values > 6 */
    - result = -EINVAL;
    - goto leave;
    - }
    - time_constant = min(txc->constant + 4, (long)MAXTC);
    + time_constant = txc->constant;
    + if (!(time_status & STA_NANO))
    + time_constant += 4;
    + time_constant = min(time_constant, (long)MAXTC);
    + time_constant = max(time_constant, 0l);
    }

    if (txc->modes & ADJ_OFFSET) {
    @@ -360,16 +355,20 @@ int do_adjtimex(struct timex *txc)
    if (txc->modes & (ADJ_TICK|ADJ_FREQUENCY|ADJ_OFFSET))
    ntp_update_frequency();
    }
    -leave:
    +
    + result = time_state; /* mostly `TIME_OK' */
    if (time_status & (STA_UNSYNC|STA_CLOCKERR))
    result = TIME_ERROR;

    if ((txc->modes == ADJ_OFFSET_SINGLESHOT) ||
    (txc->modes == ADJ_OFFSET_SS_READ))
    txc->offset = save_adjust;
    - else
    + else {
    txc->offset = ((long)shift_right(time_offset, SHIFT_UPDATE)) *
    - NTP_INTERVAL_FREQ / 1000;
    + NTP_INTERVAL_FREQ;
    + if (!(time_status & STA_NANO))
    + txc->offset /= NSEC_PER_USEC;
    + }
    txc->freq = (time_freq / NSEC_PER_USEC) <<
    (SHIFT_USEC - SHIFT_NSEC);
    txc->maxerror = time_maxerror;
    @@ -391,7 +390,11 @@ leave:
    txc->stbcnt = 0;
    write_sequnlock_irq(&xtime_lock);

    - do_gettimeofday(&txc->time);
    + getnstimeofday(&ts);
    + txc->time.tv_sec = ts.tv_sec;
    + txc->time.tv_usec = ts.tv_nsec;
    + if (!(time_status & STA_NANO))
    + txc->time.tv_usec /= NSEC_PER_USEC;

    notify_cmos_timer();


    --

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. [PATCH 4/8] increase time_offset resolution

    time_offset is already a 64bit value but its resolution barely used, so
    this makes better use of it by replacing SHIFT_UPDATE with
    TICK_LENGTH_SHIFT.
    Side note: the SHIFT_HZ in SHIFT_UPDATE was incorrect for CONFIG_NO_HZ
    and the primary reason for changing time_offset to 64bit to avoid the
    overflow.

    Signed-off-by: Roman Zippel

    ---
    include/linux/timex.h | 9 ++-------
    kernel/time/ntp.c | 23 +++++++++++------------
    2 files changed, 13 insertions(+), 19 deletions(-)

    Index: linux-2.6-mm/include/linux/timex.h
    ================================================== =================
    --- linux-2.6-mm.orig/include/linux/timex.h
    +++ linux-2.6-mm/include/linux/timex.h
    @@ -74,27 +74,22 @@
    #define MAXTC 10 /* maximum time constant (shift) */

    /*
    - * The SHIFT_UPDATE define establishes the decimal point of the
    - * time_offset variable which represents the current offset with
    - * respect to standard time.
    - *
    * SHIFT_USEC defines the scaling (shift) of the time_freq and
    * time_tolerance variables, which represent the current frequency
    * offset and maximum frequency tolerance.
    */
    -#define SHIFT_UPDATE (SHIFT_HZ + 1) /* time offset scale (shift) */
    #define SHIFT_USEC 16 /* frequency offset scale (shift) */
    #define PPM_SCALE (NSEC_PER_USEC << (TICK_LENGTH_SHIFT - SHIFT_USEC))
    #define PPM_SCALE_INV_SHIFT 20
    #define PPM_SCALE_INV ((1ll << (PPM_SCALE_INV_SHIFT + TICK_LENGTH_SHIFT)) / \
    PPM_SCALE + 1)

    -#define MAXPHASE 512000L /* max phase error (us) */
    +#define MAXPHASE 500000000l /* max phase error (ns) */
    #define MAXFREQ 500000 /* max frequency error (ns/s) */
    #define MAXFREQ_SCALED ((s64)MAXFREQ << TICK_LENGTH_SHIFT)
    #define MINSEC 256 /* min interval between updates (s) */
    #define MAXSEC 2048 /* max interval between updates (s) */
    -#define NTP_PHASE_LIMIT (MAXPHASE << 5) /* beyond max. dispersion */
    +#define NTP_PHASE_LIMIT ((MAXPHASE / NSEC_PER_USEC) << 5) /* beyond max. dispersion */

    /*
    * syscall interface - used (mainly by NTP daemon)
    Index: linux-2.6-mm/kernel/time/ntp.c
    ================================================== =================
    --- linux-2.6-mm.orig/kernel/time/ntp.c
    +++ linux-2.6-mm/kernel/time/ntp.c
    @@ -65,16 +65,15 @@ static void ntp_update_offset(long offse
    if (!(time_status & STA_PLL))
    return;

    - time_offset = offset;
    if (!(time_status & STA_NANO))
    - time_offset *= NSEC_PER_USEC;
    + offset *= NSEC_PER_USEC;

    /*
    * Scale the phase adjustment and
    * clamp to the operating range.
    */
    - time_offset = min(time_offset, (s64)MAXPHASE * NSEC_PER_USEC);
    - time_offset = max(time_offset, (s64)-MAXPHASE * NSEC_PER_USEC);
    + offset = min(offset, MAXPHASE);
    + offset = max(offset, -MAXPHASE);

    /*
    * Select how the frequency is to be controlled
    @@ -85,19 +84,19 @@ static void ntp_update_offset(long offse
    mtemp = xtime.tv_sec - time_reftime;
    time_reftime = xtime.tv_sec;

    - freq_adj = time_offset * mtemp;
    + freq_adj = (s64)offset * mtemp;
    freq_adj <<= TICK_LENGTH_SHIFT - 2 * (SHIFT_PLL + 2 + time_constant);
    time_status &= ~STA_MODE;
    if (mtemp >= MINSEC && (time_status & STA_FLL || mtemp > MAXSEC)) {
    - freq_adj += div_s64(time_offset << (TICK_LENGTH_SHIFT - SHIFT_FLL),
    + freq_adj += div_s64((s64)offset << (TICK_LENGTH_SHIFT - SHIFT_FLL),
    mtemp);
    time_status |= STA_MODE;
    }
    freq_adj += time_freq;
    freq_adj = min(freq_adj, MAXFREQ_SCALED);
    time_freq = max(freq_adj, -MAXFREQ_SCALED);
    - time_offset = div_s64(time_offset, NTP_INTERVAL_FREQ);
    - time_offset <<= SHIFT_UPDATE;
    +
    + time_offset = div_s64((s64)offset << TICK_LENGTH_SHIFT, NTP_INTERVAL_FREQ);
    }

    /**
    @@ -128,7 +127,7 @@ void ntp_clear(void)
    */
    void second_overflow(void)
    {
    - long time_adj;
    + s64 time_adj;

    /* Bump the maxerror field */
    time_maxerror += MAXFREQ / NSEC_PER_USEC;
    @@ -184,7 +183,7 @@ void second_overflow(void)
    tick_length = tick_length_base;
    time_adj = shift_right(time_offset, SHIFT_PLL + time_constant);
    time_offset -= time_adj;
    - tick_length += (s64)time_adj << (TICK_LENGTH_SHIFT - SHIFT_UPDATE);
    + tick_length += time_adj;

    if (unlikely(time_adjust)) {
    if (time_adjust > MAX_TICKADJ) {
    @@ -363,8 +362,8 @@ int do_adjtimex(struct timex *txc)
    (txc->modes == ADJ_OFFSET_SS_READ))
    txc->offset = save_adjust;
    else {
    - txc->offset = ((long)shift_right(time_offset, SHIFT_UPDATE)) *
    - NTP_INTERVAL_FREQ;
    + txc->offset = shift_right(time_offset * NTP_INTERVAL_FREQ,
    + TICK_LENGTH_SHIFT);
    if (!(time_status & STA_NANO))
    txc->offset /= NSEC_PER_USEC;
    }

    --

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. [PATCH 5/8] support for TAI

    This adds support for setting the TAI value (International Atomic Time).
    The value is reported back to userspace via timex (as we don't have a
    ntp_gettime() syscall).

    Signed-off-by: Roman Zippel

    ---
    include/linux/compat.h | 3 ++-
    include/linux/timex.h | 4 +++-
    kernel/compat.c | 3 ++-
    kernel/time/ntp.c | 7 +++++++
    4 files changed, 14 insertions(+), 3 deletions(-)

    Index: linux-2.6-mm/include/linux/compat.h
    ================================================== =================
    --- linux-2.6-mm.orig/include/linux/compat.h
    +++ linux-2.6-mm/include/linux/compat.h
    @@ -65,10 +65,11 @@ struct compat_timex {
    compat_long_t calcnt;
    compat_long_t errcnt;
    compat_long_t stbcnt;
    + compat_int_t tai;

    compat_int_t :32; compat_int_t :32; compat_int_t :32; compat_int_t :32;
    compat_int_t :32; compat_int_t :32; compat_int_t :32; compat_int_t :32;
    - compat_int_t :32; compat_int_t :32; compat_int_t :32; compat_int_t :32;
    + compat_int_t :32; compat_int_t :32; compat_int_t :32;
    };

    #define _COMPAT_NSIG_WORDS (_COMPAT_NSIG / _COMPAT_NSIG_BPW)
    Index: linux-2.6-mm/include/linux/timex.h
    ================================================== =================
    --- linux-2.6-mm.orig/include/linux/timex.h
    +++ linux-2.6-mm/include/linux/timex.h
    @@ -116,9 +116,11 @@ struct timex {
    long errcnt; /* calibration errors (ro) */
    long stbcnt; /* stability limit exceeded (ro) */

    + int tai; /* TAI offset (ro) */
    +
    int :32; int :32; int :32; int :32;
    int :32; int :32; int :32; int :32;
    - int :32; int :32; int :32; int :32;
    + int :32; int :32; int :32;
    };

    /*
    @@ -135,6 +135,7 @@ struct timex {
    #define ADJ_ESTERROR 0x0008 /* estimated time error */
    #define ADJ_STATUS 0x0010 /* clock status */
    #define ADJ_TIMECONST 0x0020 /* pll time constant */
    +#define ADJ_TAI 0x0080 /* set TAI offset */
    #define ADJ_MICRO 0x1000 /* select microsecond resolution */
    #define ADJ_NANO 0x2000 /* select nanosecond resolution */
    #define ADJ_TICK 0x4000 /* tick value */
    Index: linux-2.6-mm/kernel/compat.c
    ================================================== =================
    --- linux-2.6-mm.orig/kernel/compat.c
    +++ linux-2.6-mm/kernel/compat.c
    @@ -955,7 +955,8 @@ asmlinkage long compat_sys_adjtimex(stru
    __put_user(txc.jitcnt, &utp->jitcnt) ||
    __put_user(txc.calcnt, &utp->calcnt) ||
    __put_user(txc.errcnt, &utp->errcnt) ||
    - __put_user(txc.stbcnt, &utp->stbcnt))
    + __put_user(txc.stbcnt, &utp->stbcnt) ||
    + __put_user(txc.tai, &utp->tai))
    ret = -EFAULT;

    return ret;
    Index: linux-2.6-mm/kernel/time/ntp.c
    ================================================== =================
    --- linux-2.6-mm.orig/kernel/time/ntp.c
    +++ linux-2.6-mm/kernel/time/ntp.c
    @@ -35,6 +35,7 @@ static u64 tick_length, tick_length_base
    /* TIME_ERROR prevents overwriting the CMOS clock */
    static int time_state = TIME_OK; /* clock synchronization status */
    int time_status = STA_UNSYNC; /* clock status bits */
    +static long time_tai; /* TAI offset (s) */
    static s64 time_offset; /* time adjustment (ns) */
    static long time_constant = 2; /* pll time constant */
    long time_maxerror = NTP_PHASE_LIMIT; /* maximum error (us) */
    @@ -162,6 +163,7 @@ void second_overflow(void)
    case TIME_DEL:
    if ((xtime.tv_sec + 1) % 86400 == 0) {
    xtime.tv_sec++;
    + time_tai--;
    wall_to_monotonic.tv_sec--;
    time_state = TIME_WAIT;
    printk(KERN_NOTICE "Clock: deleting leap second "
    @@ -169,6 +171,7 @@ void second_overflow(void)
    }
    break;
    case TIME_OOP:
    + time_tai++;
    time_state = TIME_WAIT;
    break;
    case TIME_WAIT:
    @@ -340,6 +343,9 @@ int do_adjtimex(struct timex *txc)
    time_constant = max(time_constant, 0l);
    }

    + if (txc->modes & ADJ_TAI && txc->constant > 0)
    + time_tai = txc->constant;
    +
    if (txc->modes & ADJ_OFFSET) {
    if (txc->modes == ADJ_OFFSET_SINGLESHOT)
    /* adjtime() is independent from ntp_adjtime() */
    @@ -375,6 +381,7 @@ int do_adjtimex(struct timex *txc)
    txc->precision = 1;
    txc->tolerance = MAXFREQ_SCALED / PPM_SCALE;
    txc->tick = tick_usec;
    + txc->tai = time_tai;

    /* PPS is not implemented, so these are zero */
    txc->ppsfreq = 0;

    --

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. [PATCH 8/8] handle leap second via timer

    Remove the leap second handling from second_overflow(), which doesn't
    has to check for it every second anymore. With CONFIG_NO_HZ this also
    makes sure the leap second is handled close to the full second.
    Additionally this makes it possible to abort a leap second properly
    by resetting the STA_INS/STA_DEL status bits.

    Signed-off-by: Roman Zippel

    ---
    include/linux/clocksource.h | 2
    include/linux/timex.h | 1
    kernel/time/ntp.c | 133 +++++++++++++++++++++++++++++---------------
    kernel/time/timekeeping.c | 4 -
    4 files changed, 95 insertions(+), 45 deletions(-)

    Index: linux-2.6/include/linux/timex.h
    ================================================== =================
    --- linux-2.6.orig/include/linux/timex.h 2008-03-13 10:32:07.000000000 +0100
    +++ linux-2.6/include/linux/timex.h 2008-03-13 10:33:24.000000000 +0100
    @@ -210,6 +210,7 @@ extern long time_esterror; /* estimated

    extern long time_adjust; /* The amount of adjtime left */

    +extern void ntp_init(void);
    extern void ntp_clear(void);

    /**
    Index: linux-2.6/kernel/time/ntp.c
    ================================================== =================
    --- linux-2.6.orig/kernel/time/ntp.c 2008-03-13 10:32:07.000000000 +0100
    +++ linux-2.6/kernel/time/ntp.c 2008-03-13 10:33:24.000000000 +0100
    @@ -16,6 +16,7 @@
    #include
    #include
    #include
    +#include
    #include

    /*
    @@ -26,6 +27,8 @@ unsigned long tick_nsec; /* ACTHZ peri
    u64 tick_length;
    static u64 tick_length_base;

    +static struct hrtimer leap_timer;
    +
    #define MAX_TICKADJ 500 /* microsecs */
    #define MAX_TICKADJ_SCALED (((u64)(MAX_TICKADJ * NSEC_PER_USEC) << \
    NTP_SCALE_SHIFT) / NTP_INTERVAL_FREQ)
    @@ -120,64 +123,70 @@ void ntp_clear(void)
    }

    /*
    - * this routine handles the overflow of the microsecond field
    - *
    - * The tricky bits of code to handle the accurate clock support
    - * were provided by Dave Mills (Mills@UDEL.EDU) of NTP fame.
    - * They were originally developed for SUN and DEC kernels.
    - * All the kudos should go to Dave for this stuff.
    + * 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.
    */
    -void second_overflow(void)
    +static enum hrtimer_restart ntp_leap_second(struct hrtimer *timer)
    {
    - s64 time_adj;
    + enum hrtimer_restart res = HRTIMER_NORESTART;

    - /* Bump the maxerror field */
    - time_maxerror += MAXFREQ / NSEC_PER_USEC;
    - if (time_maxerror > NTP_PHASE_LIMIT) {
    - time_maxerror = NTP_PHASE_LIMIT;
    - time_status |= STA_UNSYNC;
    - }
    + write_seqlock_irq(&xtime_lock);

    - /*
    - * 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. The microtime()
    - * routine or external clock driver will insure that reported time is
    - * always monotonic. The ugly divides should be replaced.
    - */
    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 (xtime.tv_sec % 86400 == 0) {
    - xtime.tv_sec--;
    - wall_to_monotonic.tv_sec++;
    - time_state = TIME_OOP;
    - printk(KERN_NOTICE "Clock: inserting leap second "
    - "23:59:60 UTC\n");
    - }
    + xtime.tv_sec--;
    + wall_to_monotonic.tv_sec++;
    + time_state = TIME_OOP;
    + printk(KERN_NOTICE "Clock: "
    + "inserting leap second 23:59:60 UTC\n");
    + leap_timer.expires = ktime_add_ns(leap_timer.expires,
    + NSEC_PER_SEC);
    + res = HRTIMER_RESTART;
    break;
    case TIME_DEL:
    - if ((xtime.tv_sec + 1) % 86400 == 0) {
    - xtime.tv_sec++;
    - time_tai--;
    - wall_to_monotonic.tv_sec--;
    - time_state = TIME_WAIT;
    - printk(KERN_NOTICE "Clock: deleting leap second "
    - "23:59:59 UTC\n");
    - }
    + xtime.tv_sec++;
    + time_tai--;
    + wall_to_monotonic.tv_sec--;
    + time_state = TIME_WAIT;
    + printk(KERN_NOTICE "Clock: "
    + "deleting leap second 23:59:59 UTC\n");
    break;
    case TIME_OOP:
    time_tai++;
    time_state = TIME_WAIT;
    - break;
    + /* fall through */
    case TIME_WAIT:
    if (!(time_status & (STA_INS | STA_DEL)))
    time_state = TIME_OK;
    + break;
    + }
    + update_vsyscall(&xtime, clock);
    +
    + write_sequnlock_irq(&xtime_lock);
    +
    + return res;
    +}
    +
    +/*
    + * this routine handles the overflow of the microsecond field
    + *
    + * The tricky bits of code to handle the accurate clock support
    + * were provided by Dave Mills (Mills@UDEL.EDU) of NTP fame.
    + * They were originally developed for SUN and DEC kernels.
    + * All the kudos should go to Dave for this stuff.
    + */
    +void second_overflow(void)
    +{
    + s64 time_adj;
    +
    + /* Bump the maxerror field */
    + time_maxerror += MAXFREQ / NSEC_PER_USEC;
    + if (time_maxerror > NTP_PHASE_LIMIT) {
    + time_maxerror = NTP_PHASE_LIMIT;
    + time_status |= STA_UNSYNC;
    }

    /*
    @@ -268,7 +277,7 @@ static inline void notify_cmos_timer(voi
    int do_adjtimex(struct timex *txc)
    {
    struct timespec ts;
    - long save_adjust;
    + long save_adjust, sec;
    int result;

    /* In order to modify anything, you gotta be super-user! */
    @@ -289,6 +298,10 @@ int do_adjtimex(struct timex *txc)
    txc->tick > 1100000/USER_HZ)
    return -EINVAL;

    + if (time_state != TIME_OK && txc->modes & ADJ_STATUS)
    + hrtimer_cancel(&leap_timer);
    + getnstimeofday(&ts);
    +
    write_seqlock_irq(&xtime_lock);

    /* Save for later - semantics of adjtime is to return old value */
    @@ -305,6 +318,34 @@ int do_adjtimex(struct timex *txc)
    /* only set allowed bits */
    time_status &= STA_RONLY;
    time_status |= txc->status & ~STA_RONLY;
    +
    + switch (time_state) {
    + case TIME_OK:
    + start_timer:
    + sec = ts.tv_sec;
    + if (time_status & STA_INS) {
    + time_state = TIME_INS;
    + sec += 86400 - sec % 86400;
    + hrtimer_start(&leap_timer, ktime_set(sec, 0), HRTIMER_MODE_ABS);
    + } else if (time_status & STA_DEL) {
    + time_state = TIME_DEL;
    + sec += 86400 - (sec + 1) % 86400;
    + hrtimer_start(&leap_timer, ktime_set(sec, 0), HRTIMER_MODE_ABS);
    + }
    + break;
    + case TIME_INS:
    + case TIME_DEL:
    + time_state = TIME_OK;
    + goto start_timer;
    + break;
    + case TIME_WAIT:
    + if (!(time_status & (STA_INS | STA_DEL)))
    + time_state = TIME_OK;
    + break;
    + case TIME_OOP:
    + hrtimer_restart(&leap_timer);
    + break;
    + }
    }

    if (txc->modes & ADJ_NANO)
    @@ -384,7 +425,6 @@ int do_adjtimex(struct timex *txc)
    txc->stbcnt = 0;
    write_sequnlock_irq(&xtime_lock);

    - getnstimeofday(&ts);
    txc->time.tv_sec = ts.tv_sec;
    txc->time.tv_usec = ts.tv_nsec;
    if (!(time_status & STA_NANO))
    @@ -402,3 +442,10 @@ static int __init ntp_tick_adj_setup(cha
    }

    __setup("ntp_tick_adj=", ntp_tick_adj_setup);
    +
    +void __init ntp_init(void)
    +{
    + ntp_clear();
    + hrtimer_init(&leap_timer, CLOCK_REALTIME, HRTIMER_MODE_ABS);
    + leap_timer.function = ntp_leap_second;
    +}
    Index: linux-2.6/kernel/time/timekeeping.c
    ================================================== =================
    --- linux-2.6.orig/kernel/time/timekeeping.c 2008-03-13 10:32:07.000000000 +0100
    +++ linux-2.6/kernel/time/timekeeping.c 2008-03-13 10:33:24.000000000 +0100
    @@ -53,7 +53,7 @@ void update_xtime_cache(u64 nsec)
    timespec_add_ns(&xtime_cache, nsec);
    }

    -static struct clocksource *clock; /* pointer to current clocksource */
    +struct clocksource *clock;


    #ifdef CONFIG_GENERIC_TIME
    @@ -241,7 +241,7 @@ void __init timekeeping_init(void)

    write_seqlock_irqsave(&xtime_lock, flags);

    - ntp_clear();
    + ntp_init();

    clock = clocksource_get_next();
    clocksource_calculate_interval(clock, NTP_INTERVAL_LENGTH);
    Index: linux-2.6/include/linux/clocksource.h
    ================================================== =================
    --- linux-2.6.orig/include/linux/clocksource.h 2008-03-10 19:50:33.000000000 +0100
    +++ linux-2.6/include/linux/clocksource.h 2008-03-13 10:33:24.000000000 +0100
    @@ -93,6 +93,8 @@ struct clocksource {
    #endif
    };

    +extern struct clocksource *clock; /* current clocksource */
    +
    /*
    * Clock source flags bits::
    */

    --

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  6. Re: [PATCH 8/8] handle leap second via timer


    On Fri, 2008-03-14 at 19:40 +0100, zippel@linux-m68k.org wrote:
    > plain text document attachment (leap_sec)
    > Remove the leap second handling from second_overflow(), which doesn't
    > has to check for it every second anymore. With CONFIG_NO_HZ this also
    > makes sure the leap second is handled close to the full second.
    > Additionally this makes it possible to abort a leap second properly
    > by resetting the STA_INS/STA_DEL status bits.
    >
    > Signed-off-by: Roman Zippel


    Looks interesting. Small comment below.


    > -void second_overflow(void)
    > +static enum hrtimer_restart ntp_leap_second(struct hrtimer *timer)
    > {
    > - s64 time_adj;
    > + enum hrtimer_restart res = HRTIMER_NORESTART;
    >
    > - /* Bump the maxerror field */
    > - time_maxerror += MAXFREQ / NSEC_PER_USEC;
    > - if (time_maxerror > NTP_PHASE_LIMIT) {
    > - time_maxerror = NTP_PHASE_LIMIT;
    > - time_status |= STA_UNSYNC;
    > - }
    > + write_seqlock_irq(&xtime_lock);
    >
    > - /*
    > - * 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. The microtime()
    > - * routine or external clock driver will insure that reported time is
    > - * always monotonic. The ugly divides should be replaced.
    > - */
    > 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 (xtime.tv_sec % 86400 == 0) {
    > - xtime.tv_sec--;
    > - wall_to_monotonic.tv_sec++;
    > - time_state = TIME_OOP;
    > - printk(KERN_NOTICE "Clock: inserting leap second "
    > - "23:59:60 UTC\n");
    > - }
    > + xtime.tv_sec--;
    > + wall_to_monotonic.tv_sec++;
    > + time_state = TIME_OOP;
    > + printk(KERN_NOTICE "Clock: "
    > + "inserting leap second 23:59:60 UTC\n");
    > + leap_timer.expires = ktime_add_ns(leap_timer.expires,
    > + NSEC_PER_SEC);
    > + res = HRTIMER_RESTART;
    > break;
    > case TIME_DEL:
    > - if ((xtime.tv_sec + 1) % 86400 == 0) {
    > - xtime.tv_sec++;
    > - time_tai--;
    > - wall_to_monotonic.tv_sec--;
    > - time_state = TIME_WAIT;
    > - printk(KERN_NOTICE "Clock: deleting leap second "
    > - "23:59:59 UTC\n");
    > - }
    > + xtime.tv_sec++;
    > + time_tai--;
    > + wall_to_monotonic.tv_sec--;
    > + time_state = TIME_WAIT;
    > + printk(KERN_NOTICE "Clock: "
    > + "deleting leap second 23:59:59 UTC\n");
    > break;
    > case TIME_OOP:
    > time_tai++;
    > time_state = TIME_WAIT;
    > - break;
    > + /* fall through */
    > case TIME_WAIT:
    > if (!(time_status & (STA_INS | STA_DEL)))
    > time_state = TIME_OK;
    > + break;
    > + }
    > + update_vsyscall(&xtime, clock);
    > +




    > Index: linux-2.6/include/linux/clocksource.h
    > ================================================== =================
    > --- linux-2.6.orig/include/linux/clocksource.h 2008-03-10 19:50:33.000000000 +0100
    > +++ linux-2.6/include/linux/clocksource.h 2008-03-13 10:33:24.000000000 +0100
    > @@ -93,6 +93,8 @@ struct clocksource {
    > #endif
    > };
    >
    > +extern struct clocksource *clock; /* current clocksource */
    > +
    > /*
    > * Clock source flags bits::
    > */


    Instead of exporting the clocksource making it global, could you use a
    timekeeping_insert/remove_second() style interface?

    That would help remove the xtime/wall_to_monotonic references in ntp.c
    as well, cleaning up things nicely.

    thanks
    -john


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  7. Re: [PATCH 2/8] NTP4 user space bits update


    On Fri, 2008-03-14 at 19:40 +0100, zippel@linux-m68k.org wrote:
    > plain text document attachment (ntp4_user)
    > This adds a few more things from the ntp nanokernel related to user
    > space. It's now possible to select the resolution used of some values
    > via STA_NANO and the kernel reports in which mode it works (pll/fll).
    >
    > If some values for adjtimex() are outside the acceptable range, they are
    > now simply normalized instead of letting the syscall fail.
    > I removed MOD_CLKA/MOD_CLKB as the mapping didn't really makes any
    > sense, the kernel doesn't support setting the clock.
    >
    > Signed-off-by: Roman Zippel


    I've not looked up the new NTP4-ims, but few comments below.

    > ---
    > include/linux/timex.h | 12 ++++--
    > kernel/time/ntp.c | 91 +++++++++++++++++++++++++-------------------------
    > 2 files changed, 56 insertions(+), 47 deletions(-)
    >
    > Index: linux-2.6/include/linux/timex.h
    > ================================================== =================
    > --- linux-2.6.orig/include/linux/timex.h 2008-03-13 10:42:20.000000000 +0100
    > +++ linux-2.6/include/linux/timex.h 2008-03-13 10:48:59.000000000 +0100
    > @@ -58,6 +58,8 @@
    >
    > #include
    >
    > +#define NTP_API 4 /* NTP API version */
    > +
    > /*
    > * SHIFT_KG and SHIFT_KF establish the damping of the PLL and are chosen
    > * for a slightly underdamped convergence characteristic. SHIFT_KH
    > @@ -135,6 +137,8 @@ struct timex {
    > #define ADJ_ESTERROR 0x0008 /* estimated time error */
    > #define ADJ_STATUS 0x0010 /* clock status */
    > #define ADJ_TIMECONST 0x0020 /* pll time constant */
    > +#define ADJ_MICRO 0x1000 /* select microsecond resolution */
    > +#define ADJ_NANO 0x2000 /* select nanosecond resolution */
    > #define ADJ_TICK 0x4000 /* tick value */
    > #define ADJ_OFFSET_SINGLESHOT 0x8001 /* old-fashioned adjtime */
    > #define ADJ_OFFSET_SS_READ 0xa001 /* read-only adjtime */
    > @@ -146,8 +150,6 @@ struct timex {
    > #define MOD_ESTERROR ADJ_ESTERROR
    > #define MOD_STATUS ADJ_STATUS
    > #define MOD_TIMECONST ADJ_TIMECONST
    > -#define MOD_CLKB ADJ_TICK
    > -#define MOD_CLKA ADJ_OFFSET_SINGLESHOT /* 0x8000 in original */
    >
    >
    > /*
    > @@ -169,9 +171,13 @@ struct timex {
    > #define STA_PPSERROR 0x0800 /* PPS signal calibration error (ro) */
    >
    > #define STA_CLOCKERR 0x1000 /* clock hardware fault (ro) */
    > +#define STA_NANO 0x2000 /* resolution (0 = us, 1 = ns) (ro) */
    > +#define STA_MODE 0x4000 /* mode (0 = PLL, 1 = FLL) (ro) */
    > +#define STA_CLK 0x8000 /* clock source (0 = A, 1 = B) (ro) */
    >
    > +/* read-only bits */
    > #define STA_RONLY (STA_PPSSIGNAL | STA_PPSJITTER | STA_PPSWANDER | \
    > - STA_PPSERROR | STA_CLOCKERR) /* read-only bits */
    > + STA_PPSERROR | STA_CLOCKERR | STA_NANO | STA_MODE | STA_CLK)
    >
    > /*
    > * Clock states (time_state)
    > Index: linux-2.6/kernel/time/ntp.c
    > ================================================== =================
    > --- linux-2.6.orig/kernel/time/ntp.c 2008-03-13 10:42:20.000000000 +0100
    > +++ linux-2.6/kernel/time/ntp.c 2008-03-13 10:47:49.000000000 +0100
    > @@ -65,7 +65,9 @@ static void ntp_update_offset(long offse
    > if (!(time_status & STA_PLL))
    > return;
    >
    > - time_offset = offset * NSEC_PER_USEC;
    > + time_offset = offset;
    > + if (!(time_status & STA_NANO))
    > + time_offset *= NSEC_PER_USEC;
    >
    > /*
    > * Scale the phase adjustment and
    > @@ -86,8 +88,11 @@ static void ntp_update_offset(long offse
    > freq_adj = time_offset * mtemp;
    > freq_adj = shift_right(freq_adj, time_constant * 2 +
    > (SHIFT_PLL + 2) * 2 - SHIFT_NSEC);
    > - if (mtemp >= MINSEC && (time_status & STA_FLL || mtemp > MAXSEC))
    > + time_status &= ~STA_MODE;
    > + if (mtemp >= MINSEC && (time_status & STA_FLL || mtemp > MAXSEC)) {
    > freq_adj += div_s64(time_offset << (SHIFT_NSEC - SHIFT_FLL), mtemp);
    > + time_status |= STA_MODE;
    > + }
    > freq_adj += time_freq;
    > freq_adj = min(freq_adj, (s64)MAXFREQ_NSEC);
    > time_freq = max(freq_adj, (s64)-MAXFREQ_NSEC);
    > @@ -272,6 +277,7 @@ static inline void notify_cmos_timer(voi
    > */
    > int do_adjtimex(struct timex *txc)
    > {
    > + struct timespec ts;
    > long save_adjust;
    > int result;
    >
    > @@ -282,17 +288,11 @@ int do_adjtimex(struct timex *txc)
    > /* Now we validate the data before disabling interrupts */
    >
    > if ((txc->modes & ADJ_OFFSET_SINGLESHOT) == ADJ_OFFSET_SINGLESHOT) {
    > - /* singleshot must not be used with any other mode bits */
    > - if (txc->modes != ADJ_OFFSET_SINGLESHOT &&
    > - txc->modes != ADJ_OFFSET_SS_READ)
    > + /* singleshot must not be used with any other mode bits */
    > + if (txc->modes & ~ADJ_OFFSET_SS_READ)
    > return -EINVAL;


    This could probably go in the cleanup patch, no?


    > }
    >
    > - if (txc->modes != ADJ_OFFSET_SINGLESHOT && (txc->modes & ADJ_OFFSET))
    > - /* adjustment Offset limited to +- .512 seconds */
    > - if (txc->offset <= - MAXPHASE || txc->offset >= MAXPHASE )
    > - return -EINVAL;
    > -
    > /* if the quartz is off by more than 10% something is VERY wrong ! */
    > if (txc->modes & ADJ_TICK)
    > if (txc->tick < 900000/USER_HZ ||
    > @@ -300,51 +300,46 @@ int do_adjtimex(struct timex *txc)
    > return -EINVAL;
    >
    > write_seqlock_irq(&xtime_lock);
    > - result = time_state; /* mostly `TIME_OK' */
    >
    > /* Save for later - semantics of adjtime is to return old value */
    > save_adjust = time_adjust;
    >
    > -#if 0 /* STA_CLOCKERR is never set yet */
    > - time_status &= ~STA_CLOCKERR; /* reset STA_CLOCKERR */
    > -#endif


    This as well is just cleanup.


    > /* If there are input parameters, then process them */
    > if (txc->modes) {
    > - if (txc->modes & ADJ_STATUS) /* only set allowed bits */
    > - time_status = (txc->status & ~STA_RONLY) |
    > - (time_status & STA_RONLY);
    > + if (txc->modes & ADJ_STATUS) {
    > + if ((time_status & STA_PLL) &&
    > + !(txc->status & STA_PLL)) {
    > + time_state = TIME_OK;
    > + time_status = STA_UNSYNC;
    > + }
    > + /* only set allowed bits */
    > + time_status &= STA_RONLY;
    > + time_status |= txc->status & ~STA_RONLY;
    > + }


    More cleanup.


    > + if (txc->modes & ADJ_NANO)
    > + time_status |= STA_NANO;
    > + if (txc->modes & ADJ_MICRO)
    > + time_status &= ~STA_NANO;
    >
    > if (txc->modes & ADJ_FREQUENCY) {
    > - if (txc->freq > MAXFREQ || txc->freq < -MAXFREQ) {
    > - result = -EINVAL;
    > - goto leave;
    > - }
    > - time_freq = ((s64)txc->freq * NSEC_PER_USEC)
    > + time_freq = min(txc->freq, MAXFREQ);
    > + time_freq = min(time_freq, -MAXFREQ);
    > + time_freq = ((s64)time_freq * NSEC_PER_USEC)
    > >> (SHIFT_USEC - SHIFT_NSEC);
    > }
    >
    > - if (txc->modes & ADJ_MAXERROR) {
    > - if (txc->maxerror < 0 || txc->maxerror >= NTP_PHASE_LIMIT) {
    > - result = -EINVAL;
    > - goto leave;
    > - }
    > + if (txc->modes & ADJ_MAXERROR)
    > time_maxerror = txc->maxerror;
    > - }
    > -
    > - if (txc->modes & ADJ_ESTERROR) {
    > - if (txc->esterror < 0 || txc->esterror >= NTP_PHASE_LIMIT) {
    > - result = -EINVAL;
    > - goto leave;
    > - }
    > + if (txc->modes & ADJ_ESTERROR)
    > time_esterror = txc->esterror;
    > - }
    >
    > if (txc->modes & ADJ_TIMECONST) {
    > - if (txc->constant < 0) { /* NTP v4 uses values > 6 */
    > - result = -EINVAL;
    > - goto leave;
    > - }
    > - time_constant = min(txc->constant + 4, (long)MAXTC);
    > + time_constant = txc->constant;
    > + if (!(time_status & STA_NANO))
    > + time_constant += 4;
    > + time_constant = min(time_constant, (long)MAXTC);
    > + time_constant = max(time_constant, 0l);
    > }
    >
    > if (txc->modes & ADJ_OFFSET) {
    > @@ -360,16 +355,20 @@ int do_adjtimex(struct timex *txc)
    > if (txc->modes & (ADJ_TICK|ADJ_FREQUENCY|ADJ_OFFSET))
    > ntp_update_frequency();
    > }
    > -leave:
    > +
    > + result = time_state; /* mostly `TIME_OK' */
    > if (time_status & (STA_UNSYNC|STA_CLOCKERR))
    > result = TIME_ERROR;
    >
    > if ((txc->modes == ADJ_OFFSET_SINGLESHOT) ||
    > (txc->modes == ADJ_OFFSET_SS_READ))
    > txc->offset = save_adjust;
    > - else
    > + else {
    > txc->offset = ((long)shift_right(time_offset, SHIFT_UPDATE)) *
    > - NTP_INTERVAL_FREQ / 1000;
    > + NTP_INTERVAL_FREQ;
    > + if (!(time_status & STA_NANO))
    > + txc->offset /= NSEC_PER_USEC;
    > + }
    > txc->freq = (time_freq / NSEC_PER_USEC) <<
    > (SHIFT_USEC - SHIFT_NSEC);
    > txc->maxerror = time_maxerror;
    > @@ -391,7 +390,11 @@ leave:
    > txc->stbcnt = 0;
    > write_sequnlock_irq(&xtime_lock);
    >
    > - do_gettimeofday(&txc->time);
    > + getnstimeofday(&ts);
    > + txc->time.tv_sec = ts.tv_sec;
    > + txc->time.tv_usec = ts.tv_nsec;
    > + if (!(time_status & STA_NANO))
    > + txc->time.tv_usec /= NSEC_PER_USEC;




    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  8. Re: [PATCH 1/8] cleanup ntp.c


    On Fri, 2008-03-14 at 19:40 +0100, zippel@linux-m68k.org wrote:
    > plain text document attachment (update_offset)
    > This is mostly a style cleanup of ntp.c and extracts part of do_adjtimex
    > as ntp_update_offset(). Otherwise the functionality is still the same as
    > before.
    >
    > Signed-off-by: Roman Zippel


    Ah crud. Ingo sent very similar changes to me a few weeks ago, and I've
    been too pinched for time to test and send them out.

    I'll spend some time today to layer his fixes as well as my own cleanups
    on top.

    Minor nits below:

    Otherwise looks good (although I've not tested it).

    -john


    > ---
    > kernel/time/ntp.c | 171 ++++++++++++++++++++++++++++--------------------------
    > 1 file changed, 90 insertions(+), 81 deletions(-)
    >
    > Index: linux-2.6-mm/kernel/time/ntp.c
    > ================================================== =================
    > --- linux-2.6-mm.orig/kernel/time/ntp.c
    > +++ linux-2.6-mm/kernel/time/ntp.c
    > @@ -35,7 +35,7 @@ static u64 tick_length, tick_length_base
    > /* TIME_ERROR prevents overwriting the CMOS clock */
    > static int time_state = TIME_OK; /* clock synchronization status */
    > int time_status = STA_UNSYNC; /* clock status bits */
    > -static s64 time_offset; /* time adjustment (ns) */
    > +static s64 time_offset; /* time adjustment (ns) */
    > static long time_constant = 2; /* pll time constant */
    > long time_maxerror = NTP_PHASE_LIMIT; /* maximum error (us) */
    > long time_esterror = NTP_PHASE_LIMIT; /* estimated error (us) */
    > @@ -57,6 +57,44 @@ static void ntp_update_frequency(void)
    > tick_length_base = div_u64(tick_length_base, NTP_INTERVAL_FREQ);
    > }
    >
    > +static void ntp_update_offset(long offset)
    > +{
    > + long mtemp;


    Could we change this to a more descriptive name?

    Maybe last_update_interval?

    > + s64 freq_adj;
    > +
    > + if (!(time_status & STA_PLL))
    > + return;
    > +
    > + time_offset = offset * NSEC_PER_USEC;
    > +
    > + /*
    > + * Scale the phase adjustment and
    > + * clamp to the operating range.
    > + */
    > + time_offset = min(time_offset, (s64)MAXPHASE * NSEC_PER_USEC);
    > + time_offset = max(time_offset, (s64)-MAXPHASE * NSEC_PER_USEC);
    > +
    > + /*
    > + * Select how the frequency is to be controlled
    > + * and in which mode (PLL or FLL).
    > + */
    > + if (time_status & STA_FREQHOLD || time_reftime == 0)
    > + time_reftime = xtime.tv_sec;
    > + mtemp = xtime.tv_sec - time_reftime;
    > + time_reftime = xtime.tv_sec;
    > +
    > + freq_adj = time_offset * mtemp;
    > + freq_adj = shift_right(freq_adj, time_constant * 2 +
    > + (SHIFT_PLL + 2) * 2 - SHIFT_NSEC);
    > + if (mtemp >= MINSEC && (time_status & STA_FLL || mtemp > MAXSEC))
    > + freq_adj += div_s64(time_offset << (SHIFT_NSEC - SHIFT_FLL), mtemp);
    > + freq_adj += time_freq;
    > + freq_adj = min(freq_adj, (s64)MAXFREQ_NSEC);
    > + time_freq = max(freq_adj, (s64)-MAXFREQ_NSEC);
    > + time_offset = div_s64(time_offset, NTP_INTERVAL_FREQ);
    > + time_offset <<= SHIFT_UPDATE;
    > +}
    > +
    > /**
    > * ntp_clear - Clears the NTP state variables
    > *
    > @@ -131,7 +169,7 @@ void second_overflow(void)
    > break;
    > case TIME_WAIT:
    > if (!(time_status & (STA_INS | STA_DEL)))
    > - time_state = TIME_OK;
    > + time_state = TIME_OK;
    > }
    >
    > /*
    > @@ -234,8 +272,7 @@ static inline void notify_cmos_timer(voi
    > */
    > int do_adjtimex(struct timex *txc)
    > {
    > - long mtemp, save_adjust;
    > - s64 freq_adj;
    > + long save_adjust;
    > int result;
    >
    > /* In order to modify anything, you gotta be super-user! */
    > @@ -272,94 +309,63 @@ int do_adjtimex(struct timex *txc)
    > time_status &= ~STA_CLOCKERR; /* reset STA_CLOCKERR */
    > #endif
    > /* If there are input parameters, then process them */
    > - if (txc->modes)
    > - {
    > - if (txc->modes & ADJ_STATUS) /* only set allowed bits */
    > - time_status = (txc->status & ~STA_RONLY) |
    > - (time_status & STA_RONLY);
    > -
    > - if (txc->modes & ADJ_FREQUENCY) { /* p. 22 */
    > - if (txc->freq > MAXFREQ || txc->freq < -MAXFREQ) {
    > - result = -EINVAL;
    > - goto leave;
    > + if (txc->modes) {
    > + if (txc->modes & ADJ_STATUS) /* only set allowed bits */
    > + time_status = (txc->status & ~STA_RONLY) |
    > + (time_status & STA_RONLY);
    > +
    > + if (txc->modes & ADJ_FREQUENCY) {
    > + if (txc->freq > MAXFREQ || txc->freq < -MAXFREQ) {
    > + result = -EINVAL;
    > + goto leave;
    > + }
    > + time_freq = ((s64)txc->freq * NSEC_PER_USEC)
    > + >> (SHIFT_USEC - SHIFT_NSEC);
    > }
    > - time_freq = ((s64)txc->freq * NSEC_PER_USEC)
    > - >> (SHIFT_USEC - SHIFT_NSEC);
    > - }
    > -
    > - if (txc->modes & ADJ_MAXERROR) {
    > - if (txc->maxerror < 0 || txc->maxerror >= NTP_PHASE_LIMIT) {
    > - result = -EINVAL;
    > - goto leave;
    > +
    > + if (txc->modes & ADJ_MAXERROR) {
    > + if (txc->maxerror < 0 || txc->maxerror >= NTP_PHASE_LIMIT) {
    > + result = -EINVAL;
    > + goto leave;
    > + }
    > + time_maxerror = txc->maxerror;
    > }
    > - time_maxerror = txc->maxerror;
    > - }
    >
    > - if (txc->modes & ADJ_ESTERROR) {
    > - if (txc->esterror < 0 || txc->esterror >= NTP_PHASE_LIMIT) {
    > - result = -EINVAL;
    > - goto leave;
    > + if (txc->modes & ADJ_ESTERROR) {
    > + if (txc->esterror < 0 || txc->esterror >= NTP_PHASE_LIMIT) {
    > + result = -EINVAL;
    > + goto leave;
    > + }
    > + time_esterror = txc->esterror;
    > }
    > - time_esterror = txc->esterror;
    > - }
    >
    > - if (txc->modes & ADJ_TIMECONST) { /* p. 24 */
    > - if (txc->constant < 0) { /* NTP v4 uses values > 6 */
    > - result = -EINVAL;
    > - goto leave;
    > + if (txc->modes & ADJ_TIMECONST) {
    > + if (txc->constant < 0) { /* NTP v4 uses values > 6 */
    > + result = -EINVAL;
    > + goto leave;
    > + }
    > + time_constant = min(txc->constant + 4, (long)MAXTC);
    > }
    > - time_constant = min(txc->constant + 4, (long)MAXTC);
    > - }
    >
    > - if (txc->modes & ADJ_OFFSET) { /* values checked earlier */
    > - if (txc->modes == ADJ_OFFSET_SINGLESHOT) {
    > - /* adjtime() is independent from ntp_adjtime() */
    > - time_adjust = txc->offset;
    > + if (txc->modes & ADJ_OFFSET) {
    > + if (txc->modes == ADJ_OFFSET_SINGLESHOT)
    > + /* adjtime() is independent from ntp_adjtime() */
    > + time_adjust = txc->offset;
    > + else
    > + ntp_update_offset(txc->offset);
    > }
    > - else if (time_status & STA_PLL) {
    > - time_offset = txc->offset * NSEC_PER_USEC;
    > + if (txc->modes & ADJ_TICK)
    > + tick_usec = txc->tick;
    >
    > - /*
    > - * Scale the phase adjustment and
    > - * clamp to the operating range.
    > - */
    > - time_offset = min(time_offset, (s64)MAXPHASE * NSEC_PER_USEC);
    > - time_offset = max(time_offset, (s64)-MAXPHASE * NSEC_PER_USEC);
    > -
    > - /*
    > - * Select whether the frequency is to be controlled
    > - * and in which mode (PLL or FLL). Clamp to the operating
    > - * range. Ugly multiply/divide should be replaced someday.
    > - */
    > -
    > - if (time_status & STA_FREQHOLD || time_reftime == 0)
    > - time_reftime = xtime.tv_sec;
    > - mtemp = xtime.tv_sec - time_reftime;
    > - time_reftime = xtime.tv_sec;
    > -
    > - freq_adj = time_offset * mtemp;
    > - freq_adj = shift_right(freq_adj, time_constant * 2 +
    > - (SHIFT_PLL + 2) * 2 - SHIFT_NSEC);
    > - if (mtemp >= MINSEC && (time_status & STA_FLL || mtemp > MAXSEC))
    > - freq_adj += div_s64(time_offset << (SHIFT_NSEC - SHIFT_FLL), mtemp);
    > - freq_adj += time_freq;
    > - freq_adj = min(freq_adj, (s64)MAXFREQ_NSEC);
    > - time_freq = max(freq_adj, (s64)-MAXFREQ_NSEC);
    > - time_offset = div_s64(time_offset, NTP_INTERVAL_FREQ);
    > - time_offset <<= SHIFT_UPDATE;
    > - } /* STA_PLL */
    > - } /* txc->modes & ADJ_OFFSET */
    > - if (txc->modes & ADJ_TICK)
    > - tick_usec = txc->tick;
    > -
    > - if (txc->modes & (ADJ_TICK|ADJ_FREQUENCY|ADJ_OFFSET))
    > - ntp_update_frequency();
    > - } /* txc->modes */
    > -leave: if ((time_status & (STA_UNSYNC|STA_CLOCKERR)) != 0)
    > + if (txc->modes & (ADJ_TICK|ADJ_FREQUENCY|ADJ_OFFSET))
    > + ntp_update_frequency();
    > + }
    > +leave:
    > + if (time_status & (STA_UNSYNC|STA_CLOCKERR))
    > result = TIME_ERROR;
    >
    > if ((txc->modes == ADJ_OFFSET_SINGLESHOT) ||
    > - (txc->modes == ADJ_OFFSET_SS_READ))
    > + (txc->modes == ADJ_OFFSET_SS_READ))
    > txc->offset = save_adjust;
    > else
    > txc->offset = ((long)shift_right(time_offset, SHIFT_UPDATE)) *
    > @@ -384,9 +390,12 @@ leave: if ((time_status & (STA_UNSYNC|ST
    > txc->errcnt = 0;
    > txc->stbcnt = 0;
    > write_sequnlock_irq(&xtime_lock);
    > +
    > do_gettimeofday(&txc->time);
    > +
    > notify_cmos_timer();
    > - return(result);
    > +
    > + return result;
    > }
    >
    > static int __init ntp_tick_adj_setup(char *str)
    >


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  9. Re: [PATCH 5/8] support for TAI


    On Fri, 2008-03-14 at 19:40 +0100, zippel@linux-m68k.org wrote:
    > plain text document attachment (time_tai)
    > This adds support for setting the TAI value (International Atomic Time).
    > The value is reported back to userspace via timex (as we don't have a
    > ntp_gettime() syscall).
    >
    > Signed-off-by: Roman Zippel
    >


    Very cool. I just was talking with someone who was looking for a TAI
    interface.

    Minor nit below.

    -john


    > Index: linux-2.6-mm/kernel/time/ntp.c
    > ================================================== =================
    > --- linux-2.6-mm.orig/kernel/time/ntp.c
    > +++ linux-2.6-mm/kernel/time/ntp.c
    > @@ -35,6 +35,7 @@ static u64 tick_length, tick_length_base
    > /* TIME_ERROR prevents overwriting the CMOS clock */
    > static int time_state = TIME_OK; /* clock synchronization status */
    > int time_status = STA_UNSYNC; /* clock status bits */
    > +static long time_tai; /* TAI offset (s) */


    I think timekeeping.c would be the better place for time_tai, keeping it
    closer to xtime and wall_to_monotonic's definitions.


    > static s64 time_offset; /* time adjustment (ns) */
    > static long time_constant = 2; /* pll time constant */
    > long time_maxerror = NTP_PHASE_LIMIT; /* maximum error (us) */
    > @@ -162,6 +163,7 @@ void second_overflow(void)
    > case TIME_DEL:
    > if ((xtime.tv_sec + 1) % 86400 == 0) {
    > xtime.tv_sec++;
    > + time_tai--;
    > wall_to_monotonic.tv_sec--;
    > time_state = TIME_WAIT;
    > printk(KERN_NOTICE "Clock: deleting leap second "
    > @@ -169,6 +171,7 @@ void second_overflow(void)
    > }
    > break;
    > case TIME_OOP:
    > + time_tai++;
    > time_state = TIME_WAIT;
    > break;
    > case TIME_WAIT:



    These manipulations could also be done in the
    timekeeping_insert/remove_second interface I suggested earlier.


    > @@ -340,6 +343,9 @@ int do_adjtimex(struct timex *txc)
    > time_constant = max(time_constant, 0l);
    > }
    >
    > + if (txc->modes & ADJ_TAI && txc->constant > 0)
    > + time_tai = txc->constant;
    > +
    > if (txc->modes & ADJ_OFFSET) {
    > if (txc->modes == ADJ_OFFSET_SINGLESHOT)
    > /* adjtime() is independent from ntp_adjtime() */
    > @@ -375,6 +381,7 @@ int do_adjtimex(struct timex *txc)
    > txc->precision = 1;
    > txc->tolerance = MAXFREQ_SCALED / PPM_SCALE;
    > txc->tick = tick_usec;
    > + txc->tai = time_tai;
    >
    > /* PPS is not implemented, so these are zero */
    > txc->ppsfreq = 0;
    >


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  10. Re: [PATCH 0/8] NTP updates


    On Fri, 2008-03-14 at 19:40 +0100, zippel@linux-m68k.org wrote:
    > Hi,
    >
    > These patches clean up the NTP code a little and add the remaining user
    > space bits from the ntp nanokernel, so that it's e.g. now possible to
    > enable the nanosecond resolution, which should be useful to keep time in
    > a local network in sync (but it also needs an updated glibc/ntpd first).
    >
    > I tried my best to also update the various user space APIs (e.g. compat,
    > vsyscall), but it would be great if someone could look over it, in case
    > I forgot something.


    Hey Roman,

    BTW, These don't seem to apply to the current -git tree, or Andrew's
    -mm. What should I be testing them against?

    thanks
    -john

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  11. Re: [PATCH 7/8] Remove current_tick_length()


    On Fri, 2008-03-14 at 19:40 +0100, zippel@linux-m68k.org wrote:
    > plain text document attachment (tick_length)
    > current_tick_length used to do a little more, but now it just returns
    > tick_length, which we can also access directly at the few places, where
    > it's needed.
    >
    > Signed-off-by: Roman Zippel


    Hrm. I'm not sure I like using a global variable instead of using an
    accessor function. At least with the accessor function folks couldn't
    just tweak the value outside ntp as easily.

    Is there any additional rational for this change?

    thanks
    -john


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  12. Re: [PATCH 0/8] NTP updates

    Hi,

    On Fri, 14 Mar 2008, john stultz wrote:

    > BTW, These don't seem to apply to the current -git tree, or Andrew's
    > -mm. What should I be testing them against?


    Sorry, I forgot to mentioned that it depends on the div64 patches I
    recently posted.

    bye, Roman
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  13. Re: [PATCH 8/8] handle leap second via timer

    Hi,

    On Fri, 14 Mar 2008, john stultz wrote:

    > Instead of exporting the clocksource making it global, could you use a
    > timekeeping_insert/remove_second() style interface?


    Sounds good.

    bye, Roman
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  14. Re: [PATCH 1/8] cleanup ntp.c

    Hi,

    On Fri, 14 Mar 2008, john stultz wrote:

    > Ah crud. Ingo sent very similar changes to me a few weeks ago, and I've
    > been too pinched for time to test and send them out.
    >
    > I'll spend some time today to layer his fixes as well as my own cleanups
    > on top.


    What fixes?

    > > +static void ntp_update_offset(long offset)
    > > +{
    > > + long mtemp;

    >
    > Could we change this to a more descriptive name?
    >
    > Maybe last_update_interval?


    It's a local variable, so IMO temp works just fine, it's not that
    difficult to figure out how it's used (e.g. it's like using loop_index
    instead of i).

    bye, Roman
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  15. Re: [PATCH 7/8] Remove current_tick_length()

    Hi,

    On Fri, 14 Mar 2008, john stultz wrote:

    > On Fri, 2008-03-14 at 19:40 +0100, zippel@linux-m68k.org wrote:
    > > plain text document attachment (tick_length)
    > > current_tick_length used to do a little more, but now it just returns
    > > tick_length, which we can also access directly at the few places, where
    > > it's needed.
    > >
    > > Signed-off-by: Roman Zippel

    >
    > Hrm. I'm not sure I like using a global variable instead of using an
    > accessor function. At least with the accessor function folks couldn't
    > just tweak the value outside ntp as easily.


    Why would someone do something silly like this?

    > Is there any additional rational for this change?


    Useless bloat?

    bye, Roman
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  16. Re: [PATCH 2/8] NTP4 user space bits update

    Hi,

    On Fri, 14 Mar 2008, john stultz wrote:

    > > - /* singleshot must not be used with any other mode bits */
    > > - if (txc->modes != ADJ_OFFSET_SINGLESHOT &&
    > > - txc->modes != ADJ_OFFSET_SS_READ)
    > > + /* singleshot must not be used with any other mode bits */
    > > + if (txc->modes & ~ADJ_OFFSET_SS_READ)
    > > return -EINVAL;

    >
    > This could probably go in the cleanup patch, no?


    It does more than simple code indentation and refactoring, so I didn't
    want to include it there.

    > > -#if 0 /* STA_CLOCKERR is never set yet */
    > > - time_status &= ~STA_CLOCKERR; /* reset STA_CLOCKERR */
    > > -#endif

    >
    > This as well is just cleanup.


    Debatable, it refers to user space APIs, so I included it here.

    > > + if (txc->modes & ADJ_STATUS) {
    > > + if ((time_status & STA_PLL) &&
    > > + !(txc->status & STA_PLL)) {
    > > + time_state = TIME_OK;
    > > + time_status = STA_UNSYNC;
    > > + }
    > > + /* only set allowed bits */
    > > + time_status &= STA_RONLY;
    > > + time_status |= txc->status & ~STA_RONLY;
    > > + }

    >
    > More cleanup.


    Not really.

    bye, Roman
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  17. Re: [PATCH 7/8] Remove current_tick_length()


    On Sat, 2008-03-15 at 04:32 +0100, Roman Zippel wrote:
    > Hi,
    >
    > On Fri, 14 Mar 2008, john stultz wrote:
    >
    > > On Fri, 2008-03-14 at 19:40 +0100, zippel@linux-m68k.org wrote:
    > > > plain text document attachment (tick_length)
    > > > current_tick_length used to do a little more, but now it just returns
    > > > tick_length, which we can also access directly at the few places, where
    > > > it's needed.
    > > >
    > > > Signed-off-by: Roman Zippel

    > >
    > > Hrm. I'm not sure I like using a global variable instead of using an
    > > accessor function. At least with the accessor function folks couldn't
    > > just tweak the value outside ntp as easily.

    >
    > Why would someone do something silly like this?


    Its not a huge deal, but we've seen globally scoped timekeeping
    variables misused either accidentally or intentionally. Awhile back ppc
    was corrupting time_offset by using it for a timezone offset value.

    So I think its a valid maintainability issue.

    > > Is there any additional rational for this change?

    >
    > Useless bloat?


    I agree its a trade off. But do you have performance numbers to make the
    maintainability trade off worth it? (I admit, it is used in the
    timer_interrupt, so it may very well be worth it, but we might want to
    check first).

    thanks
    -john


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  18. Re: [PATCH 7/8] Remove current_tick_length()

    Hi,

    On Fri, 14 Mar 2008, john stultz wrote:

    > > Why would someone do something silly like this?

    >
    > Its not a huge deal, but we've seen globally scoped timekeeping
    > variables misused either accidentally or intentionally. Awhile back ppc
    > was corrupting time_offset by using it for a timezone offset value.
    >
    > So I think its a valid maintainability issue.


    ppc used to do a whole lot more than that, so I don't think that counts as
    a valid example.
    tick_length is overwritten at every timer and modifying it would have
    barely any effect compared to other values which are already exported.

    > > > Is there any additional rational for this change?

    > >
    > > Useless bloat?

    >
    > I agree its a trade off. But do you have performance numbers to make the
    > maintainability trade off worth it? (I admit, it is used in the
    > timer_interrupt, so it may very well be worth it, but we might want to
    > check first).


    It depends on the architecture, but it's effects regularly executed code
    and every byte counts.

    bye, Roman
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  19. Re: [PATCH 7/8] Remove current_tick_length()

    On Fri, Mar 14, 2008 at 9:18 PM, Roman Zippel wrote:
    > > > > Is there any additional rational for this change?
    > > >
    > > > Useless bloat?

    > >
    > > I agree its a trade off. But do you have performance numbers to make the
    > > maintainability trade off worth it? (I admit, it is used in the
    > > timer_interrupt, so it may very well be worth it, but we might want to
    > > check first).

    >
    > It depends on the architecture, but it's effects regularly executed code
    > and every byte counts.


    Then make the original function an inline. With -O2 it should compile
    to exactly the same thing.
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  20. Re: [PATCH 7/8] Remove current_tick_length()

    Hi,

    On Sat, 15 Mar 2008, Ray Lee wrote:

    > Then make the original function an inline. With -O2 it should compile
    > to exactly the same thing.


    This would also defeat John's intention of keeping the value static.

    bye, Roman
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread
Page 1 of 2 1 2 LastLast