[PATCH 0/8] NTP updates

[PATCH 0/8] NTP updates

[zippel]

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

-- 

[PATCH 1/8] cleanup ntp.c

[zippel]

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

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 <zippel@linux-m68k.org>

---
 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;
+	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)

-- 

[PATCH 2/8] NTP4 user space bits update

[zippel]

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

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 <zippel@linux-m68k.org>

---
 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 <asm/param.h>
 
+#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();
 

-- 

[PATCH 3/8] increase time_freq resolution

[zippel]

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

This changes time_freq to a 64bit value and makes it static (the only
outside user had no real need to modify it).
Intermediate values were already 64bit, so the change isn't that big,
but it saves a little in shifts by replacing SHIFT_NSEC with
TICK_LENGTH_SHIFT. PPM_SCALE is then used to convert between user space
and kernel space representation.

Signed-off-by: Roman Zippel <zippel@linux-m68k.org>

---
 arch/powerpc/kernel/time.c |    2 --
 include/linux/timex.h      |   11 ++++++-----
 kernel/time/ntp.c          |   30 +++++++++++++++---------------
 3 files changed, 21 insertions(+), 22 deletions(-)

Index: linux-2.6/arch/powerpc/kernel/time.c
===================================================================
--- linux-2.6.orig/arch/powerpc/kernel/time.c	2008-03-10 14:36:42.000000000 +0100
+++ linux-2.6/arch/powerpc/kernel/time.c	2008-03-10 19:50:43.000000000 +0100
@@ -1007,8 +1007,6 @@ void __init time_init(void)
 	vdso_data->stamp_xsec = (u64) xtime.tv_sec * XSEC_PER_SEC;
 	vdso_data->tb_to_xs = tb_to_xs;
 
-	time_freq = 0;
-
 	write_sequnlock_irqrestore(&xtime_lock, flags);
 
 	/* Register the clocksource, if we're not running on iSeries */
Index: linux-2.6/include/linux/timex.h
===================================================================
--- linux-2.6.orig/include/linux/timex.h	2008-03-10 14:46:52.000000000 +0100
+++ linux-2.6/include/linux/timex.h	2008-03-10 19:50:43.000000000 +0100
@@ -84,11 +84,14 @@
  */
 #define SHIFT_UPDATE (SHIFT_HZ + 1) /* time offset scale (shift) */
 #define SHIFT_USEC 16		/* frequency offset scale (shift) */
-#define SHIFT_NSEC 12		/* kernel frequency offset scale */
+#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 MAXFREQ (512L << SHIFT_USEC)  /* max frequency error (ppm) */
-#define MAXFREQ_NSEC (512000L << SHIFT_NSEC) /* max frequency error (ppb) */
+#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 */
@@ -208,8 +211,6 @@ extern int time_status;		/* clock synchr
 extern long time_maxerror;	/* maximum error */
 extern long time_esterror;	/* estimated error */
 
-extern long time_freq;		/* frequency offset (scaled ppm) */
-
 extern long time_adjust;	/* The amount of adjtime left */
 
 extern void ntp_clear(void);
Index: linux-2.6/kernel/time/ntp.c
===================================================================
--- linux-2.6.orig/kernel/time/ntp.c	2008-03-10 14:46:52.000000000 +0100
+++ linux-2.6/kernel/time/ntp.c	2008-03-10 19:50:43.000000000 +0100
@@ -39,7 +39,7 @@ static s64 time_offset;			/* time adjust
 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)		*/
-long time_freq;				/* frequency offset (scaled ppm)*/
+static s64 time_freq;			/* frequency offset (scaled ns/s)*/
 static long time_reftime;		/* time at last adjustment (s)	*/
 long time_adjust;
 static long ntp_tick_adj;
@@ -49,7 +49,7 @@ static void ntp_update_frequency(void)
 	u64 second_length = (u64)(tick_usec * NSEC_PER_USEC * USER_HZ)
 				<< TICK_LENGTH_SHIFT;
 	second_length += (s64)ntp_tick_adj << TICK_LENGTH_SHIFT;
-	second_length += (s64)time_freq << (TICK_LENGTH_SHIFT - SHIFT_NSEC);
+	second_length += time_freq;
 
 	tick_length_base = second_length;
 
@@ -86,16 +86,16 @@ static void ntp_update_offset(long offse
 	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);
+	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 << (SHIFT_NSEC - SHIFT_FLL), mtemp);
+		freq_adj += div_s64(time_offset << (TICK_LENGTH_SHIFT - 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);
+	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;
 }
@@ -131,7 +131,7 @@ void second_overflow(void)
 	long time_adj;
 
 	/* Bump the maxerror field */
-	time_maxerror += MAXFREQ >> SHIFT_USEC;
+	time_maxerror += MAXFREQ / NSEC_PER_USEC;
 	if (time_maxerror > NTP_PHASE_LIMIT) {
 		time_maxerror = NTP_PHASE_LIMIT;
 		time_status |= STA_UNSYNC;
@@ -323,10 +323,9 @@ int do_adjtimex(struct timex *txc)
 			time_status &= ~STA_NANO;
 
 		if (txc->modes & ADJ_FREQUENCY) {
-			time_freq = min(txc->freq, MAXFREQ);
-			time_freq = min(time_freq, -MAXFREQ);
-			time_freq = ((s64)time_freq * NSEC_PER_USEC)
-					>> (SHIFT_USEC - SHIFT_NSEC);
+			time_freq = (s64)txc->freq * PPM_SCALE;
+			time_freq = min(time_freq, MAXFREQ_SCALED);
+			time_freq = max(time_freq, -MAXFREQ_SCALED);
 		}
 
 		if (txc->modes & ADJ_MAXERROR)
@@ -369,14 +368,15 @@ int do_adjtimex(struct timex *txc)
 		if (!(time_status & STA_NANO))
 			txc->offset /= NSEC_PER_USEC;
 	}
-	txc->freq	   = (time_freq / NSEC_PER_USEC) <<
-				(SHIFT_USEC - SHIFT_NSEC);
+	txc->freq	   = shift_right((s32)(time_freq >> PPM_SCALE_INV_SHIFT) *
+					 (s64)PPM_SCALE_INV,
+					 TICK_LENGTH_SHIFT);
 	txc->maxerror	   = time_maxerror;
 	txc->esterror	   = time_esterror;
 	txc->status	   = time_status;
 	txc->constant	   = time_constant;
 	txc->precision	   = 1;
-	txc->tolerance	   = MAXFREQ;
+	txc->tolerance	   = MAXFREQ_SCALED / PPM_SCALE;
 	txc->tick	   = tick_usec;
 
 	/* PPS is not implemented, so these are zero */

-- 

[PATCH 4/8] increase time_offset resolution

[zippel]

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

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 <zippel@linux-m68k.org>

---
 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;
 	}

-- 

[PATCH 5/8] support for TAI

[zippel]

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

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 <zippel@linux-m68k.org>

---
 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;

-- 

[PATCH 6/8] Rename TICK_LENGTH_SHIFT to NTP_SCALE_SHIFT

[zippel]

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

As TICK_LENGTH_SHIFT is used for more than just the tick length, the
name isn't quite approriate anymore, so this renames it to
NTP_SCALE_SHIFT.

Signed-off-by: Roman Zippel <zippel@linux-m68k.org>

---
 arch/powerpc/kernel/time.c |    2 +-
 include/linux/timex.h      |    8 ++++----
 kernel/time/ntp.c          |   20 ++++++++++----------
 kernel/time/timekeeping.c  |   10 +++++-----
 4 files changed, 20 insertions(+), 20 deletions(-)

Index: linux-2.6/arch/powerpc/kernel/time.c
===================================================================
--- linux-2.6.orig/arch/powerpc/kernel/time.c	2008-03-10 14:46:52.000000000 +0100
+++ linux-2.6/arch/powerpc/kernel/time.c	2008-03-10 14:57:30.000000000 +0100
@@ -149,7 +149,7 @@ EXPORT_SYMBOL(tb_ticks_per_sec);	/* for 
 u64 tb_to_xs;
 unsigned tb_to_us;
 
-#define TICKLEN_SCALE	TICK_LENGTH_SHIFT
+#define TICKLEN_SCALE	NTP_SCALE_SHIFT
 u64 last_tick_len;	/* units are ns / 2^TICKLEN_SCALE */
 u64 ticklen_to_xs;	/* 0.64 fraction */
 
Index: linux-2.6/include/linux/timex.h
===================================================================
--- linux-2.6.orig/include/linux/timex.h	2008-03-10 14:56:07.000000000 +0100
+++ linux-2.6/include/linux/timex.h	2008-03-10 19:50:34.000000000 +0100
@@ -79,14 +79,14 @@
  * offset and maximum frequency tolerance.
  */
 #define SHIFT_USEC 16		/* frequency offset scale (shift) */
-#define PPM_SCALE (NSEC_PER_USEC << (TICK_LENGTH_SHIFT - SHIFT_USEC))
+#define PPM_SCALE (NSEC_PER_USEC << (NTP_SCALE_SHIFT - SHIFT_USEC))
 #define PPM_SCALE_INV_SHIFT 20
-#define PPM_SCALE_INV ((1ll << (PPM_SCALE_INV_SHIFT + TICK_LENGTH_SHIFT)) / \
+#define PPM_SCALE_INV ((1ll << (PPM_SCALE_INV_SHIFT + NTP_SCALE_SHIFT)) / \
 		       PPM_SCALE + 1)
 
 #define MAXPHASE 500000000l	/* max phase error (ns) */
 #define MAXFREQ 500000		/* max frequency error (ns/s) */
-#define MAXFREQ_SCALED ((s64)MAXFREQ << TICK_LENGTH_SHIFT)
+#define MAXFREQ_SCALED ((s64)MAXFREQ << NTP_SCALE_SHIFT)
 #define MINSEC 256		/* min interval between updates (s) */
 #define MAXSEC 2048		/* max interval between updates (s) */
 #define NTP_PHASE_LIMIT ((MAXPHASE / NSEC_PER_USEC) << 5) /* beyond max. dispersion */
@@ -228,7 +228,7 @@ static inline int ntp_synced(void)
 	__x < 0 ? -(-__x >> __s) : __x >> __s;	\
 })
 
-#define TICK_LENGTH_SHIFT	32
+#define NTP_SCALE_SHIFT		32
 
 #ifdef CONFIG_NO_HZ
 #define NTP_INTERVAL_FREQ  (2)
Index: linux-2.6/kernel/time/ntp.c
===================================================================
--- linux-2.6.orig/kernel/time/ntp.c	2008-03-10 14:56:07.000000000 +0100
+++ linux-2.6/kernel/time/ntp.c	2008-03-10 19:50:34.000000000 +0100
@@ -27,7 +27,7 @@ static u64 tick_length, tick_length_base
 
 #define MAX_TICKADJ		500		/* microsecs */
 #define MAX_TICKADJ_SCALED	(((u64)(MAX_TICKADJ * NSEC_PER_USEC) << \
-				  TICK_LENGTH_SHIFT) / NTP_INTERVAL_FREQ)
+				  NTP_SCALE_SHIFT) / NTP_INTERVAL_FREQ)
 
 /*
  * phase-lock loop variables
@@ -48,13 +48,13 @@ static long ntp_tick_adj;
 static void ntp_update_frequency(void)
 {
 	u64 second_length = (u64)(tick_usec * NSEC_PER_USEC * USER_HZ)
-				<< TICK_LENGTH_SHIFT;
-	second_length += (s64)ntp_tick_adj << TICK_LENGTH_SHIFT;
+				<< NTP_SCALE_SHIFT;
+	second_length += (s64)ntp_tick_adj << NTP_SCALE_SHIFT;
 	second_length += time_freq;
 
 	tick_length_base = second_length;
 
-	tick_nsec = div_u64(second_length, HZ) >> TICK_LENGTH_SHIFT;
+	tick_nsec = div_u64(second_length, HZ) >> NTP_SCALE_SHIFT;
 	tick_length_base = div_u64(tick_length_base, NTP_INTERVAL_FREQ);
 }
 
@@ -86,10 +86,10 @@ static void ntp_update_offset(long offse
 	time_reftime = xtime.tv_sec;
 
 	freq_adj = (s64)offset * mtemp;
-	freq_adj <<= TICK_LENGTH_SHIFT - 2 * (SHIFT_PLL + 2 + time_constant);
+	freq_adj <<= NTP_SCALE_SHIFT - 2 * (SHIFT_PLL + 2 + time_constant);
 	time_status &= ~STA_MODE;
 	if (mtemp >= MINSEC && (time_status & STA_FLL || mtemp > MAXSEC)) {
-		freq_adj += div_s64((s64)offset << (TICK_LENGTH_SHIFT - SHIFT_FLL),
+		freq_adj += div_s64((s64)offset << (NTP_SCALE_SHIFT - SHIFT_FLL),
 				    mtemp);
 		time_status |= STA_MODE;
 	}
@@ -97,7 +97,7 @@ static void ntp_update_offset(long offse
 	freq_adj = min(freq_adj, MAXFREQ_SCALED);
 	time_freq = max(freq_adj, -MAXFREQ_SCALED);
 
-	time_offset = div_s64((s64)offset << TICK_LENGTH_SHIFT, NTP_INTERVAL_FREQ);
+	time_offset = div_s64((s64)offset << NTP_SCALE_SHIFT, NTP_INTERVAL_FREQ);
 }
 
 /**
@@ -197,7 +197,7 @@ void second_overflow(void)
 			tick_length -= MAX_TICKADJ_SCALED;
 		} else {
 			tick_length += (s64)(time_adjust * NSEC_PER_USEC /
-					NTP_INTERVAL_FREQ) << TICK_LENGTH_SHIFT;
+					NTP_INTERVAL_FREQ) << NTP_SCALE_SHIFT;
 			time_adjust = 0;
 		}
 	}
@@ -369,13 +369,13 @@ int do_adjtimex(struct timex *txc)
 		txc->offset = save_adjust;
 	else {
 		txc->offset = shift_right(time_offset * NTP_INTERVAL_FREQ,
-					  TICK_LENGTH_SHIFT);
+					  NTP_SCALE_SHIFT);
 		if (!(time_status & STA_NANO))
 			txc->offset /= NSEC_PER_USEC;
 	}
 	txc->freq	   = shift_right((s32)(time_freq >> PPM_SCALE_INV_SHIFT) *
 					 (s64)PPM_SCALE_INV,
-					 TICK_LENGTH_SHIFT);
+					 NTP_SCALE_SHIFT);
 	txc->maxerror	   = time_maxerror;
 	txc->esterror	   = time_esterror;
 	txc->status	   = time_status;
Index: linux-2.6/kernel/time/timekeeping.c
===================================================================
--- linux-2.6.orig/kernel/time/timekeeping.c	2008-03-10 14:37:04.000000000 +0100
+++ linux-2.6/kernel/time/timekeeping.c	2008-03-10 19:50:34.000000000 +0100
@@ -365,7 +365,7 @@ static __always_inline int clocksource_b
 	 * here.  This is tuned so that an error of about 1 msec is adjusted
 	 * within about 1 sec (or 2^20 nsec in 2^SHIFT_HZ ticks).
 	 */
-	error2 = clock->error >> (TICK_LENGTH_SHIFT + 22 - 2 * SHIFT_HZ);
+	error2 = clock->error >> (NTP_SCALE_SHIFT + 22 - 2 * SHIFT_HZ);
 	error2 = abs(error2);
 	for (look_ahead = 0; error2 > 0; look_ahead++)
 		error2 >>= 2;
@@ -375,7 +375,7 @@ static __always_inline int clocksource_b
 	 * remove the single look ahead already included in the error.
 	 */
 	tick_error = current_tick_length() >>
-		(TICK_LENGTH_SHIFT - clock->shift + 1);
+		(NTP_SCALE_SHIFT - clock->shift + 1);
 	tick_error -= clock->xtime_interval >> 1;
 	error = ((error - tick_error) >> look_ahead) + tick_error;
 
@@ -406,7 +406,7 @@ static void clocksource_adjust(s64 offse
 	s64 error, interval = clock->cycle_interval;
 	int adj;
 
-	error = clock->error >> (TICK_LENGTH_SHIFT - clock->shift - 1);
+	error = clock->error >> (NTP_SCALE_SHIFT - clock->shift - 1);
 	if (error > interval) {
 		error >>= 2;
 		if (likely(error <= interval))
@@ -428,7 +428,7 @@ static void clocksource_adjust(s64 offse
 	clock->xtime_interval += interval;
 	clock->xtime_nsec -= offset;
 	clock->error -= (interval - offset) <<
-			(TICK_LENGTH_SHIFT - clock->shift);
+			(NTP_SCALE_SHIFT - clock->shift);
 }
 
 /**
@@ -468,7 +468,7 @@ void update_wall_time(void)
 
 		/* accumulate error between NTP and clock interval */
 		clock->error += current_tick_length();
-		clock->error -= clock->xtime_interval << (TICK_LENGTH_SHIFT - clock->shift);
+		clock->error -= clock->xtime_interval << (NTP_SCALE_SHIFT - clock->shift);
 	}
 
 	/* correct the clock when NTP error is too big */

-- 

[PATCH 7/8] Remove current_tick_length()

[zippel]

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

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 <zippel@linux-m68k.org>

---
 include/linux/timex.h     |    4 ++--
 kernel/time/ntp.c         |   16 ++--------------
 kernel/time/timekeeping.c |    5 ++---
 3 files changed, 6 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
@@ -234,8 +234,8 @@ static inline int ntp_synced(void)
 #endif
 #define NTP_INTERVAL_LENGTH (NSEC_PER_SEC/NTP_INTERVAL_FREQ)
 
-/* Returns how long ticks are at present, in ns / 2^(SHIFT_SCALE-10). */
-extern u64 current_tick_length(void);
+/* Returns how long ticks are at present, in ns / 2^NTP_SCALE_SHIFT. */
+extern u64 tick_length;
 
 extern void second_overflow(void);
 extern void update_ntp_one_tick(void);
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
@@ -23,7 +23,8 @@
  */
 unsigned long tick_usec = TICK_USEC; 		/* USER_HZ period (usec) */
 unsigned long tick_nsec;			/* ACTHZ period (nsec) */
-static u64 tick_length, tick_length_base;
+u64 tick_length;
+static u64 tick_length_base;
 
 #define MAX_TICKADJ		500		/* microsecs */
 #define MAX_TICKADJ_SCALED	(((u64)(MAX_TICKADJ * NSEC_PER_USEC) << \
@@ -203,19 +204,6 @@ void second_overflow(void)
 	}
 }
 
-/*
- * Return how long ticks are at the moment, that is, how much time
- * update_wall_time_one_tick will add to xtime next time we call it
- * (assuming no calls to do_adjtimex in the meantime).
- * The return value is in fixed-point nanoseconds shifted by the
- * specified number of bits to the right of the binary point.
- * This function has no side-effects.
- */
-u64 current_tick_length(void)
-{
-	return tick_length;
-}
-
 #ifdef CONFIG_GENERIC_CMOS_UPDATE
 
 /* Disable the cmos update - used by virtualization and embedded */
Index: linux-2.6-mm/kernel/time/timekeeping.c
===================================================================
--- linux-2.6-mm.orig/kernel/time/timekeeping.c
+++ linux-2.6-mm/kernel/time/timekeeping.c
@@ -374,8 +374,7 @@ static __always_inline int clocksource_b
 	 * Now calculate the error in (1 << look_ahead) ticks, but first
 	 * remove the single look ahead already included in the error.
 	 */
-	tick_error = current_tick_length() >>
-		(NTP_SCALE_SHIFT - clock->shift + 1);
+	tick_error = tick_length >> (NTP_SCALE_SHIFT - clock->shift + 1);
 	tick_error -= clock->xtime_interval >> 1;
 	error = ((error - tick_error) >> look_ahead) + tick_error;
 
@@ -467,7 +466,7 @@ void update_wall_time(void)
 		}
 
 		/* accumulate error between NTP and clock interval */
-		clock->error += current_tick_length();
+		clock->error += tick_length;
 		clock->error -= clock->xtime_interval << (NTP_SCALE_SHIFT - clock->shift);
 	}
 

-- 

[PATCH 8/8] handle leap second via timer

[zippel]

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

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 <zippel@linux-m68k.org>

---
 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 <linux/hrtimer.h>
 #include <linux/capability.h>
 #include <linux/math64.h>
+#include <linux/clocksource.h>
 #include <asm/timex.h>
 
 /*
@@ -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::
  */

-- 

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

[john stultz]

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 <zippel@linux-m68k.org>

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);
> +

<snip>

> 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

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

[john stultz]

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 <zippel@linux-m68k.org>

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)
> 

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

[john stultz]

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 <zippel@linux-m68k.org>

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 <asm/param.h>
> 
> +#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;

Re: [PATCH 5/8] support for TAI

[john stultz]

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 <zippel@linux-m68k.org>
> 

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;
> 

Re: [PATCH 0/8] NTP updates

[john stultz]

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

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

[john stultz]

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 <zippel@linux-m68k.org>

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

Re: [PATCH 0/8] NTP updates

[Roman Zippel]

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

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

[Roman Zippel]

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

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

[Roman Zippel]

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

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

[Roman Zippel]

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

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

[Roman Zippel]

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 <zippel@linux-m68k.org>
> 
> 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

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

[john stultz]

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 <zippel@linux-m68k.org>
> > 
> > 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

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

[Roman Zippel]

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

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

[Ray Lee]

On Fri, Mar 14, 2008 at 9:18 PM, Roman Zippel <zippel@linux-m68k.org> 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.

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

[Roman Zippel]

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

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

[Andrew Morton]

On Sat, 15 Mar 2008 04:18:39 +0100 (CET)
Roman Zippel <zippel@linux-m68k.org> wrote:

> 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.
> 

It sounds like a few updates are in the pipeline, but I merged this series
as-is into -mm.

I'll normally push ntp changes through Thomas's git-hrt tree, however this
patch series has dependencies upon at least

introduce-explicit-signed-unsigned-64bit-divide.patch
convert-a-few-do_div-user.patch
rename-div64_64-to-div64_u64.patch
rename-div64_64-to-div64_u64-mm.patch
remove-div_long_long_rem.patch

so they can't go into git-hrt immediately.

The idealised algorithm is

- 2.6.26 opens
- Thomas merges git-hrt
- I merge the above patches
- I then send these ntp patches to Thomas
- He merges them into git-hrt
- He does another git-hrt -> Linus merge

And that's all OK, but various lags might cause us to miss the merge
window, as I merge the -mm stuff last, and some git-tree maintainers are
dawdlers.

So if Thomas wants to ack these I can merge them directly late in the
2.6.26 merge window.

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

[john stultz]

On Sat, 2008-03-15 at 18:14 +0100, Roman Zippel wrote:
> 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.

Well, don't mistake me for being fanatical about it. Having the values
be static is cleaner, but if its a real performance issue, then clearly
performance wins.

I do like Ray's suggestion, and think using the inline'd function is
preferred to the raw variable, as it better establishes through use if
nothing else, the read-only nature of the value outside of ntp.

thanks
-john

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

[Thomas Gleixner]

On Mon, 17 Mar 2008, Andrew Morton wrote:
> It sounds like a few updates are in the pipeline, but I merged this series
> as-is into -mm.
> 
> I'll normally push ntp changes through Thomas's git-hrt tree, however this
> patch series has dependencies upon at least
> 
> introduce-explicit-signed-unsigned-64bit-divide.patch
> convert-a-few-do_div-user.patch
> rename-div64_64-to-div64_u64.patch
> rename-div64_64-to-div64_u64-mm.patch
> remove-div_long_long_rem.patch
> 
> so they can't go into git-hrt immediately.
> 
> The idealised algorithm is
> 
> - 2.6.26 opens
> - Thomas merges git-hrt
> - I merge the above patches
> - I then send these ntp patches to Thomas
> - He merges them into git-hrt
> - He does another git-hrt -> Linus merge
> 
> And that's all OK, but various lags might cause us to miss the merge
> window, as I merge the -mm stuff last, and some git-tree maintainers are
> dawdlers.

:)

> So if Thomas wants to ack these I can merge them directly late in the
> 2.6.26 merge window.

Fine for me either way.

Thanks,

	tglx

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

[Roman Zippel]

Hi,

On Tuesday 18. March 2008, john stultz wrote:

> On Sat, 2008-03-15 at 18:14 +0100, Roman Zippel wrote:
> > 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.
>
> Well, don't mistake me for being fanatical about it. Having the values
> be static is cleaner, but if its a real performance issue, then clearly
> performance wins.

There are two aspects, such functions tend to generate slightly larger and 
slower code.

> I do like Ray's suggestion, and think using the inline'd function is
> preferred to the raw variable, as it better establishes through use if
> nothing else, the read-only nature of the value outside of ntp.

In other languages one would use private or protected for this, but we don't 
have this. I'm not too fond of an inline function, as it would mark it as 
some kind of public API, which it isn't. It's just an internal value used by 
the timekeeping code, which happens to be needed by a few source files. If 
you want to make a little more private, it would be better to move it to 
header under kernel/time/.

bye, Roman