NTP patches

PATCH In review [email protected] 4 participants 18 messages Aug 10, 2006 – Aug 20, 2006 Timekeeping, clocksource core, NTP, alarmtimer

lore

Share & tools

lore

[email protected]

author

2006-08-10 00:15 UTC #0 lore raw

cover NTP patches

Hi,

Here is my current version of the NTP patches.
They precalculate as much possible and get rid of a lot of rather crude compensation code. The tick length is now a much simpler value, updated once a second, which greatly reduces the dependency on HZ.
I rebased the patches against current -mm + John's ntp.c patch.

bye, Roman

[email protected]

author

2006-08-10 00:16 UTC #1 lore raw

PATCH 1/9 add ntp_update_frequency

This introduces ntp_update_frequency() and deinlines ntp_clear() (as it's not performance critical).
ntp_update_frequency() calculates the base tick length using tick_usec and adds a base adjustment, in case the frequency doesn't divide evenly by HZ.

Signed-off-by Roman Zippel <[email protected]>

---
 include/linux/timex.h |   14 ++------------
 kernel/time/ntp.c     |   50 ++++++++++++++++++++++++++++++++++++++++++++------
 kernel/timer.c        |   11 ++++-------
 3 files changed, 50 insertions(+), 25 deletions(-)

include/linux/timex.h +2 ~~-12~~

Index: linux-2.6-mm/include/linux/timex.h
===================================================================
@@ -218,18 +218,8 @@ extern long time_reftime;	/* time at las
 extern long time_adjust;	/* The amount of adjtime left */
 extern long time_next_adjust;	/* Value for time_adjust at next tick */
 
-/**
- * ntp_clear - Clears the NTP state variables
- *
- * Must be called while holding a write on the xtime_lock
- */
-static inline void ntp_clear(void)
-{
-	time_adjust = 0;		/* stop active adjtime() */
-	time_status |= STA_UNSYNC;
-	time_maxerror = NTP_PHASE_LIMIT;
-	time_esterror = NTP_PHASE_LIMIT;
-}
+extern void ntp_clear(void);
+extern void ntp_update_frequency(void);
 
 /**
  * ntp_synced - Returns 1 if the NTP status is not UNSYNC

kernel/time/ntp.c +44 -6

86 lines not on this page · raw message

kernel/timer.c +4 -7

Index: linux-2.6-mm/kernel/timer.c
===================================================================
@@ -568,12 +568,6 @@ found:
 
 /******************************************************************/
 
-/*
- * Timekeeping variables
- */
-unsigned long tick_usec = TICK_USEC; 		/* USER_HZ period (usec) */
-unsigned long tick_nsec = TICK_NSEC;		/* ACTHZ period (nsec) */
-
 /* 
  * The current time 
  * wall_to_monotonic is what we need to add to xtime (or xtime corrected

@@ -763,10 +757,13 @@ void __init timekeeping_init(void)
 	unsigned long flags;
 
 	write_seqlock_irqsave(&xtime_lock, flags);
+
+	ntp_clear();
+
 	clock = clocksource_get_next();
 	clocksource_calculate_interval(clock, tick_nsec);
 	clock->cycle_last = clocksource_read(clock);
-	ntp_clear();
+
 	write_sequnlock_irqrestore(&xtime_lock, flags);
 }

[email protected]

author

2006-08-10 00:14 UTC #2 lore raw

PATCH 2/9 add time_adj to tick length

In reply to [email protected]

This makes time_adj local to second_overflow() and integrates it into the tick length instead of adding it everytime.

Signed-off-by Roman Zippel <[email protected]>

---
 kernel/time/ntp.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

kernel/time/ntp.c +2 -4

Index: linux-2.6-mm/kernel/time/ntp.c
===================================================================
@@ -45,7 +45,6 @@ long time_maxerror = NTP_PHASE_LIMIT;	/*
 long time_esterror = NTP_PHASE_LIMIT;	/* estimated error (us)		*/
 long time_freq = (((NSEC_PER_SEC + HZ/2) % HZ - HZ/2) << SHIFT_USEC) / NSEC_PER_USEC;
 					/* frequency offset (scaled ppm)*/
-static long time_adj;			/* tick adjust (scaled 1 / HZ)	*/
 long time_reftime;			/* time at last adjustment (s)	*/
 long time_adjust;
 long time_next_adjust;

@@ -90,7 +89,7 @@ void ntp_update_frequency(void)
  */
 void second_overflow(void)
 {
-	long ltemp;
+	long ltemp, time_adj;
 
 	/* Bump the maxerror field */
 	time_maxerror += time_tolerance >> SHIFT_USEC;

@@ -195,6 +194,7 @@ void second_overflow(void)
 	time_adj += shift_right(time_adj, 6) + shift_right(time_adj, 7);
 #endif
 	tick_length = tick_length_base;
+	tick_length += (s64)time_adj << (TICK_LENGTH_SHIFT - (SHIFT_SCALE - 10));
 }
 
 /*

@@ -251,11 +251,9 @@ u64 current_tick_length(void)
 	u64 ret;
 
 	/* calculate the finest interval NTP will allow.
-	 *    ie: nanosecond value shifted by (SHIFT_SCALE - 10)
 	 */
 	ret = tick_length;
 	ret += (u64)(adjtime_adjustment() * 1000) << TICK_LENGTH_SHIFT;
-	ret += (s64)time_adj << (TICK_LENGTH_SHIFT - (SHIFT_SCALE - 10));
 
 	return ret;
 }

[email protected]

author

2006-08-10 00:14 UTC #3 lore raw

PATCH 3/9 add time_freq to tick length

In reply to [email protected]

This adds the frequency part to ntp_update_frequency().

Signed-off-by Roman Zippel <[email protected]>

---
 kernel/time/ntp.c |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

kernel/time/ntp.c +3 -5

Index: linux-2.6-mm/kernel/time/ntp.c
===================================================================
@@ -43,8 +43,7 @@ long time_tolerance = MAXFREQ;		/* frequ
 long time_precision = 1;		/* clock precision (us)		*/
 long time_maxerror = NTP_PHASE_LIMIT;	/* maximum error (us)		*/
 long time_esterror = NTP_PHASE_LIMIT;	/* estimated error (us)		*/
-long time_freq = (((NSEC_PER_SEC + HZ/2) % HZ - HZ/2) << SHIFT_USEC) / NSEC_PER_USEC;
-					/* frequency offset (scaled ppm)*/
+long time_freq;				/* frequency offset (scaled ppm)*/
 long time_reftime;			/* time at last adjustment (s)	*/
 long time_adjust;
 long time_next_adjust;

@@ -73,6 +72,7 @@ void ntp_update_frequency(void)
 {
 	tick_length_base = (u64)(tick_usec * NSEC_PER_USEC * USER_HZ) << TICK_LENGTH_SHIFT;
 	tick_length_base += (s64)CLOCK_TICK_ADJUST << TICK_LENGTH_SHIFT;
+	tick_length_base += ((s64)time_freq * NSEC_PER_USEC) << (TICK_LENGTH_SHIFT - SHIFT_USEC);
 
 	do_div(tick_length_base, HZ);

@@ -169,8 +169,6 @@ void second_overflow(void)
 	 * Compute the frequency estimate and additional phase adjustment due
 	 * to frequency error for the next second.
 	 */
-	ltemp = time_freq;
-	time_adj += shift_right(ltemp,(SHIFT_USEC + SHIFT_HZ - SHIFT_SCALE));
 
 #if HZ == 100
 	/*

@@ -395,7 +393,7 @@ int do_adjtimex(struct timex *txc)
 	    if (txc->modes & ADJ_TICK)
 		tick_usec = txc->tick;
 
-	    if (txc->modes & ADJ_TICK)
+	    if (txc->modes & (ADJ_TICK|ADJ_FREQUENCY|ADJ_OFFSET))
 		    ntp_update_frequency();
 	} /* txc->modes */
 leave:	if ((time_status & (STA_UNSYNC|STA_CLOCKERR)) != 0)

[email protected]

author

2006-08-10 00:17 UTC #4 lore raw

PATCH 4/9 prescale time_offset

In reply to [email protected]

This converts time_offset into a scaled per tick value. This avoids now completely the crude compensation in second_overflow().

Signed-off-by Roman Zippel <[email protected]>

---
 include/linux/timex.h |    2 -
 kernel/time/ntp.c     |   64 ++++++++++++--------------------------------------
 2 files changed, 17 insertions(+), 49 deletions(-)

include/linux/timex.h +1 -1

Index: linux-2.6-mm/include/linux/timex.h
===================================================================
@@ -90,7 +90,7 @@
  * FINENSEC is 1 ns in SHIFT_UPDATE units of the time_phase variable.
  */
 #define SHIFT_SCALE 22		/* phase scale (shift) */
-#define SHIFT_UPDATE (SHIFT_KG + MAXTC) /* time offset scale (shift) */
+#define SHIFT_UPDATE (SHIFT_HZ + 1) /* time offset scale (shift) */
 #define SHIFT_USEC 16		/* frequency offset scale (shift) */
 #define FINENSEC (1L << (SHIFT_SCALE - 10)) /* ~1 ns in phase units */

kernel/time/ntp.c +16 ~~-48~~

113 lines not on this page · raw message

[email protected]

author

2006-08-10 00:17 UTC #5 lore raw

PATCH 5/9 add time_adjust to tick length

In reply to [email protected]

This folds update_ntp_one_tick() into second_overflow() and adds time_adjust to the tick length, this makes time_next_adjust unnecessary. This slightly changes the adjtime() behaviour, instead of applying it to the next tick, it's applied to the next second.

Signed-off-by Roman Zippel <[email protected]>

---
 include/linux/timex.h |    1 
 kernel/time/ntp.c     |   71 ++++++++++++--------------------------------------
 kernel/timer.c        |    2 -
 3 files changed, 18 insertions(+), 56 deletions(-)

include/linux/timex.h +0 -1

Index: linux-2.6-mm/include/linux/timex.h
===================================================================
@@ -216,7 +216,6 @@ extern long time_freq;		/* frequency off
 extern long time_reftime;	/* time at last adjustment (s) */
 
 extern long time_adjust;	/* The amount of adjtime left */
-extern long time_next_adjust;	/* Value for time_adjust at next tick */
 
 extern void ntp_clear(void);
 extern void ntp_update_frequency(void);

kernel/time/ntp.c +18 ~~-53~~

108 lines not on this page · raw message

kernel/timer.c +0 -2

Index: linux-2.6-mm/kernel/timer.c
===================================================================
@@ -943,8 +943,6 @@ static void update_wall_time(void)
 		/* interpolator bits */
 		time_interpolator_update(clock->xtime_interval
 						>> clock->shift);
-		/* increment the NTP state machine */
-		update_ntp_one_tick();
 
 		/* accumulate error between NTP and clock interval */
 		clock->error += current_tick_length();

Atsushi Nemoto

2006-08-20 15:31 UTC #6 lore raw

Re: [NTP 5/9] add time_adjust to tick length

Quoted 4 lines · On Thu, 10 Aug 2006 02:01:51 +0200, [email protected] wrote:

This folds update_ntp_one_tick() into second_overflow() and adds time_adjust to the tick length, this makes time_next_adjust unnecessary. This slightly changes the adjtime() behaviour, instead of applying it to the next tick, it's applied to the next second.

...

kernel/time/ntp.c unsigned long tick_usec = TICK_USEC; / · line 32

Quoted 2 lines

kernel/time/ntp.c +0 -2

-/* Don't completely fail for HZ > 500.  */
-int tickadj = 500/HZ ? : 1;		/* microsecs */

The tickadj is used by cris, frv, m32r, m68k, mips and sparc. This patch would break build on those platforms.

I have not looked at this patch closely yet. Just a report.
---
Atsushi Nemoto

Roman Zippel

author

2006-08-20 17:25 UTC #7 lore raw

Re: [NTP 5/9] add time_adjust to tick length

Hi,

Quoted 5 lines · Atsushi Nemoto

Quoted 4 lines · On Thu, 10 Aug 2006 02:01:51 +0200, [email protected] wrote:
This folds update_ntp_one_tick() into second_overflow() and adds time_adjust to the tick length, this makes time_next_adjust unnecessary. This slightly changes the adjtime() behaviour, instead of applying it to the next tick, it's applied to the next second.

Quoted 8 lines

...
kernel/time/ntp.c unsigned long tick_usec = TICK_USEC; / · line 32
Quoted 2 lines
kernel/time/ntp.c +0 -2
-/* Don't completely fail for HZ > 500.  */
-int tickadj = 500/HZ ? : 1;		/* microsecs */
The tickadj is used by cris, frv, m32r, m68k, mips and sparc. This patch would break build on those platforms.
I have not looked at this patch closely yet. Just a report.

Oops, I indeed missed that. I searched for if anyone would change that value, but later forgot about the other users.
A simple solution would be to move this (constant) value to a header and another rather simple solution would be to remove it completely, as it's rather bogus anyway. These users check time_adjust, which is unused in a NTP controlled system. The correct value to check would be the tick length, but I'm sure it's really worth the trouble.

bye, Roman

[email protected]

author

2006-08-10 00:15 UTC #8 lore raw

PATCH 6/9 remove time_tolerance

In reply to [email protected]

time_tolerance isn't changed at all in the kernel, so simply remove it, this simplifies the next patch, as it avoids a number of conversions.

Signed-off-by Roman Zippel <[email protected]>

---
 include/linux/timex.h |    1 -
 kernel/time/ntp.c     |    9 ++++-----
 2 files changed, 4 insertions(+), 6 deletions(-)

include/linux/timex.h +0 -1

Index: linux-2.6-mm/include/linux/timex.h
===================================================================
@@ -207,7 +207,6 @@ extern int time_state;		/* clock status 
 extern int time_status;		/* clock synchronization status bits */
 extern long time_offset;	/* time adjustment (us) */
 extern long time_constant;	/* pll time constant */
-extern long time_tolerance;	/* frequency tolerance (ppm) */
 extern long time_precision;	/* clock precision (us) */
 extern long time_maxerror;	/* maximum error */
 extern long time_esterror;	/* estimated error */

kernel/time/ntp.c +4 -5

Index: linux-2.6-mm/kernel/time/ntp.c
===================================================================
@@ -40,7 +40,6 @@ int time_state = TIME_OK;		/* clock sync
 int time_status = STA_UNSYNC;		/* clock status bits		*/
 long time_offset;			/* time adjustment (ns)		*/
 long time_constant = 2;			/* pll time constant		*/
-long time_tolerance = MAXFREQ;		/* frequency tolerance (ppm)	*/
 long time_precision = 1;		/* clock precision (us)		*/
 long time_maxerror = NTP_PHASE_LIMIT;	/* maximum error (us)		*/
 long time_esterror = NTP_PHASE_LIMIT;	/* estimated error (us)		*/

@@ -93,7 +92,7 @@ void second_overflow(void)
 	long time_adj;
 
 	/* Bump the maxerror field */
-	time_maxerror += time_tolerance >> SHIFT_USEC;
+	time_maxerror += MAXFREQ >> SHIFT_USEC;
 	if (time_maxerror > NTP_PHASE_LIMIT) {
 		time_maxerror = NTP_PHASE_LIMIT;
 		time_status |= STA_UNSYNC;

@@ -319,8 +318,8 @@ int do_adjtimex(struct timex *txc)
 			} else /* calibration interval too long (p. 12) */
 				result = TIME_ERROR;
 		    }
-		    time_freq = min(time_freq, time_tolerance);
-		    time_freq = max(time_freq, -time_tolerance);
+		    time_freq = min(time_freq, MAXFREQ);
+		    time_freq = max(time_freq, -MAXFREQ);
 		    time_offset = (time_offset * NSEC_PER_USEC / HZ) << SHIFT_UPDATE;
 		} /* STA_PLL */
 	    } /* txc->modes & ADJ_OFFSET */

@@ -343,7 +342,7 @@ leave:	if ((time_status & (STA_UNSYNC|ST
 	txc->status	   = time_status;
 	txc->constant	   = time_constant;
 	txc->precision	   = time_precision;
-	txc->tolerance	   = time_tolerance;
+	txc->tolerance	   = MAXFREQ;
 	txc->tick	   = tick_usec;
 
 	/* PPS is not implemented, so these are zero */

[email protected]

author

2006-08-10 00:16 UTC #9 lore raw

PATCH 7/9 convert time_freq to nsec value

In reply to [email protected]

This converts time_freq to a scaled nsec value and adds around 6bit of extra resolution. This pushes the time_freq to its 32bit limits so the calculatons have to be done with 64bit.

Signed-off-by Roman Zippel <[email protected]>

---
 include/linux/timex.h |    2 ++
 kernel/time/ntp.c     |   36 ++++++++++++++++++++++--------------
 2 files changed, 24 insertions(+), 14 deletions(-)

include/linux/timex.h +2 -0

Index: linux-2.6-mm/include/linux/timex.h
===================================================================
@@ -92,10 +92,12 @@
 #define SHIFT_SCALE 22		/* phase scale (shift) */
 #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 FINENSEC (1L << (SHIFT_SCALE - 10)) /* ~1 ns in phase units */
 
 #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 MINSEC 16L              /* min interval between updates (s) */
 #define MAXSEC 1200L            /* max interval between updates (s) */
 #define	NTP_PHASE_LIMIT	(MAXPHASE << 5)	/* beyond max. dispersion */

kernel/time/ntp.c +22 ~~-14~~

Index: linux-2.6-mm/kernel/time/ntp.c
===================================================================
@@ -72,7 +72,7 @@ void ntp_update_frequency(void)
 {
 	tick_length_base = (u64)(tick_usec * NSEC_PER_USEC * USER_HZ) << TICK_LENGTH_SHIFT;
 	tick_length_base += (s64)CLOCK_TICK_ADJUST << TICK_LENGTH_SHIFT;
-	tick_length_base += ((s64)time_freq * NSEC_PER_USEC) << (TICK_LENGTH_SHIFT - SHIFT_USEC);
+	tick_length_base += (s64)time_freq << (TICK_LENGTH_SHIFT - SHIFT_NSEC);
 
 	do_div(tick_length_base, HZ);

@@ -206,6 +206,7 @@ void __attribute__ ((weak)) notify_arch_
 int do_adjtimex(struct timex *txc)
 {
 	long ltemp, mtemp, save_adjust;
+	s64 freq_adj;
 	int result;
 
 	/* In order to modify anything, you gotta be super-user! */

@@ -251,7 +252,7 @@ int do_adjtimex(struct timex *txc)
 		    result = -EINVAL;
 		    goto leave;
 		}
-		time_freq = txc->freq;
+		time_freq = ((s64)txc->freq * NSEC_PER_USEC) >> (SHIFT_USEC - SHIFT_NSEC);
 	    }
 
 	    if (txc->modes & ADJ_MAXERROR) {

@@ -284,14 +285,14 @@ int do_adjtimex(struct timex *txc)
 		    time_adjust = txc->offset;
 		}
 		else if (time_status & STA_PLL) {
-		    ltemp = txc->offset;
+		    ltemp = txc->offset * NSEC_PER_USEC;
 
 		    /*
 		     * Scale the phase adjustment and
 		     * clamp to the operating range.
 		     */
-		    time_offset = min(ltemp, MAXPHASE);
-		    time_offset = max(time_offset, -MAXPHASE);
+		    time_offset = min(ltemp, MAXPHASE * NSEC_PER_USEC);
+		    time_offset = max(time_offset, -MAXPHASE * NSEC_PER_USEC);
 
 		    /*
 		     * Select whether the frequency is to be controlled

@@ -303,24 +304,31 @@ int do_adjtimex(struct timex *txc)
 		        time_reftime = xtime.tv_sec;
 		    mtemp = xtime.tv_sec - time_reftime;
 		    time_reftime = xtime.tv_sec;
+		    freq_adj = 0;
 		    if (time_status & STA_FLL) {
 		        if (mtemp >= MINSEC) {
-			    ltemp = ((time_offset << 12) / mtemp) << (SHIFT_USEC - 12);
-			    time_freq += shift_right(ltemp, SHIFT_KH);
+			    freq_adj = (s64)time_offset << (SHIFT_NSEC - SHIFT_KH);
+			    if (time_offset < 0) {
+				freq_adj = -freq_adj;
+				do_div(freq_adj, mtemp);
+				freq_adj = -freq_adj;
+			    } else
+				do_div(freq_adj, mtemp);
 			} else /* calibration interval too short (p. 12) */
 				result = TIME_ERROR;
 		    } else {	/* PLL mode */
 		        if (mtemp < MAXSEC) {
-			    ltemp *= mtemp;
-			    time_freq += shift_right(ltemp,(time_constant +
+			    freq_adj = (s64)ltemp * mtemp;
+			    freq_adj = shift_right(freq_adj,(time_constant +
 						       time_constant +
-						       SHIFT_KF - SHIFT_USEC));
+						       SHIFT_KF - SHIFT_NSEC));
 			} else /* calibration interval too long (p. 12) */
 				result = TIME_ERROR;
 		    }
-		    time_freq = min(time_freq, MAXFREQ);
-		    time_freq = max(time_freq, -MAXFREQ);
-		    time_offset = (time_offset * NSEC_PER_USEC / HZ) << SHIFT_UPDATE;
+		    freq_adj += time_freq;
+		    freq_adj = min(freq_adj, (s64)MAXFREQ_NSEC);
+		    time_freq = max(freq_adj, (s64)-MAXFREQ_NSEC);
+		    time_offset = (time_offset / HZ) << SHIFT_UPDATE;
 		} /* STA_PLL */
 	    } /* txc->modes & ADJ_OFFSET */
 	    if (txc->modes & ADJ_TICK)

@@ -336,7 +344,7 @@ leave:	if ((time_status & (STA_UNSYNC|ST
 	    txc->offset	   = save_adjust;
 	else
 	    txc->offset    = shift_right(time_offset, SHIFT_UPDATE) * HZ / 1000;
-	txc->freq	   = time_freq;
+	txc->freq	   = (time_freq / NSEC_PER_USEC) << (SHIFT_USEC - SHIFT_NSEC);
 	txc->maxerror	   = time_maxerror;
 	txc->esterror	   = time_esterror;
 	txc->status	   = time_status;

[email protected]

author

2006-08-10 00:16 UTC #10 lore raw

PATCH 8/9 convert to the NTP4 reference model

In reply to [email protected]

This converts the kernel ntp model into a model which matches the nanokernel reference implementations. The previous patches already increased the resolution and precision of the computations, so that this conversion becomes quite simple.

Signed-off-by Roman Zippel <[email protected]>

---
 include/linux/timex.h |   11 ++++------
 kernel/time/ntp.c     |   51 ++++++++++++++++++--------------------------------
 2 files changed, 24 insertions(+), 38 deletions(-)

include/linux/timex.h +5 -6

Index: linux-2.6-mm/include/linux/timex.h
===================================================================
@@ -70,10 +70,9 @@
  * zero to MAXTC, the PLL will converge in 15 minutes to 16 hours,
  * respectively.
  */
-#define SHIFT_KG 6		/* phase factor (shift) */
-#define SHIFT_KF 16		/* PLL frequency factor (shift) */
-#define SHIFT_KH 2		/* FLL frequency factor (shift) */
-#define MAXTC 6			/* maximum time constant (shift) */
+#define SHIFT_PLL	4	/* PLL frequency factor (shift) */
+#define SHIFT_FLL	2	/* FLL frequency factor (shift) */
+#define MAXTC		10	/* maximum time constant (shift) */
 
 /*
  * The SHIFT_SCALE define establishes the decimal point of the time_phase

@@ -98,8 +97,8 @@
 #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 MINSEC 16L              /* min interval between updates (s) */
-#define MAXSEC 1200L            /* max interval between updates (s) */
+#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 */
 
 /*

kernel/time/ntp.c +19 ~~-32~~

77 lines not on this page · raw message

Andrew Morton

2006-08-10 18:52 UTC #11 lore raw

Re: [NTP 8/9] convert to the NTP4 reference model

Quoted 2 lines · On Thu, 10 Aug 2006 02:01:54 +0200 [email protected] wrote:

This converts the kernel ntp model into a model which matches the nanokernel reference implementations.

For the ntp ignorami amongst us... what is a "nanokernel reference implementation" and why do we want one?

Thanks.

Roman Zippel

author

2006-08-10 19:14 UTC #12 lore raw

Re: [NTP 8/9] convert to the NTP4 reference model

Hi,

Quoted 5 lines · Andrew Morton

Quoted 2 lines
This converts the kernel ntp model into a model which matches the nanokernel reference implementations.
For the ntp ignorami amongst us... what is a "nanokernel reference implementation" and why do we want one?

It's the behaviour the current ntp daemon expects, the ntp documentation has more information and a link to the package (e.g. under Debian at /usr/share/doc/ntp-doc/html/kern.html).

bye, Roman

Andrew Morton

2006-08-10 19:29 UTC #13 lore raw

Re: [NTP 8/9] convert to the NTP4 reference model

Quoted 7 lines · On Thu, 10 Aug 2006 21:14:00 +0200 (CEST) Roman Zippel <[email protected]>…

Hi,
Quoted 3 lines · Andrew Morton
Quoted 2 lines
This converts the kernel ntp model into a model which matches the nanokernel reference implementations.

Quoted 7 lines

Quoted 2 lines
For the ntp ignorami amongst us... what is a "nanokernel reference implementation" and why do we want one?
It's the behaviour the current ntp daemon expects, the ntp documentation has more information and a link to the package (e.g. under Debian at /usr/share/doc/ntp-doc/html/kern.html).

So... the current kernel is behaving in a manner other than that which the NTP daemon expects? Does this cause any problems?

I'm trying to work out what reason we might have for merging this patch.

Roman Zippel

author

2006-08-10 20:02 UTC #14 lore raw

Re: [NTP 8/9] convert to the NTP4 reference model

Hi,

Quoted 7 lines · Andrew Morton

Quoted 4 lines
It's the behaviour the current ntp daemon expects, the ntp documentation has more information and a link to the package (e.g. under Debian at /usr/share/doc/ntp-doc/html/kern.html).
So... the current kernel is behaving in a manner other than that which the NTP daemon expects? Does this cause any problems?

It's not drastically different, so for normal internet usage there is no big difference.
http://www.ntp.org/ntpfaq/NTP-s-compat.htm has a bit more information on this topic.

Quoted 1 line

I'm trying to work out what reason we might have for merging this patch.

It also allows us to sanely readd the PPS bits, where the changes were more significant.

bye, Roman

[email protected]

author

2006-08-10 00:17 UTC #15 lore raw

PATCH 9/9 cleanup defines and comments

In reply to [email protected]

Remove a few unused defines and remove obsolete information from comments.

Signed-off-by Roman Zippel <[email protected]>

---
 include/asm-frv/timex.h    |    5 -----
 include/asm-m32r/timex.h   |    3 ---
 include/asm-sh64/timex.h   |    3 ---
 include/asm-xtensa/timex.h |    3 ---
 include/linux/timex.h      |   13 +++----------
 5 files changed, 3 insertions(+), 24 deletions(-)

include/asm-frv/timex.h +0 -5

index 2aa562f..a89bdde 100644
@@ -6,11 +6,6 @@ #define _ASM_TIMEX_H
 #define CLOCK_TICK_RATE		1193180 /* Underlying HZ */
 #define CLOCK_TICK_FACTOR	20	/* Factor of both 1000000 and CLOCK_TICK_RATE */
 
-#define FINETUNE							\
-((((((long)LATCH * HZ - CLOCK_TICK_RATE) << SHIFT_HZ) *			\
-   (1000000/CLOCK_TICK_FACTOR) / (CLOCK_TICK_RATE/CLOCK_TICK_FACTOR))	\
-  << (SHIFT_SCALE-SHIFT_HZ)) / HZ)
-
 typedef unsigned long cycles_t;
 
 static inline cycles_t get_cycles(void)

include/asm-m32r/timex.h +0 -3

index e89bfd1..019441c 100644
@@ -12,9 +12,6 @@ #define _ASM_M32R_TIMEX_H
 
 #define CLOCK_TICK_RATE	(CONFIG_BUS_CLOCK / CONFIG_TIMER_DIVIDE)
 #define CLOCK_TICK_FACTOR	20	/* Factor of both 1000000 and CLOCK_TICK_RATE */
-#define FINETUNE ((((((long)LATCH * HZ - CLOCK_TICK_RATE) << SHIFT_HZ) * \
-	(1000000/CLOCK_TICK_FACTOR) / (CLOCK_TICK_RATE/CLOCK_TICK_FACTOR)) \
-		<< (SHIFT_SCALE-SHIFT_HZ)) / HZ)
 
 #ifdef __KERNEL__
 /*

include/asm-sh64/timex.h +0 -3

index af0b792..163e2b6 100644
@@ -17,9 +17,6 @@ #define __ASM_SH64_TIMEX_H
 
 #define CLOCK_TICK_RATE	1193180 /* Underlying HZ */
 #define CLOCK_TICK_FACTOR	20	/* Factor of both 1000000 and CLOCK_TICK_RATE */
-#define FINETUNE ((((((long)LATCH * HZ - CLOCK_TICK_RATE) << SHIFT_HZ) * \
-	(1000000/CLOCK_TICK_FACTOR) / (CLOCK_TICK_RATE/CLOCK_TICK_FACTOR)) \
-		<< (SHIFT_SCALE-SHIFT_HZ)) / HZ)
 
 typedef unsigned long cycles_t;

include/asm-xtensa/timex.h +0 -3

index d14a375..c7b705e 100644
@@ -31,9 +31,6 @@ #define LINUX_TIMER_MASK        (1L << L
 
 #define CLOCK_TICK_RATE 	1193180	/* (everyone is using this value) */
 #define CLOCK_TICK_FACTOR       20 /* Factor of both 10^6 and CLOCK_TICK_RATE */
-#define FINETUNE ((((((long)LATCH * HZ - CLOCK_TICK_RATE) << SHIFT_HZ) * \
-	(1000000/CLOCK_TICK_FACTOR) / (CLOCK_TICK_RATE/CLOCK_TICK_FACTOR)) \
-		<< (SHIFT_SCALE-SHIFT_HZ)) / HZ)
 
 #ifdef CONFIG_XTENSA_CALIBRATE_CCOUNT
 extern unsigned long ccount_per_jiffy;

include/linux/timex.h +3 ~~-10~~

index 9879e3c..0b2e1b5 100644
@@ -75,24 +75,17 @@ #define SHIFT_FLL	2	/* FLL frequency fac
 #define MAXTC		10	/* maximum time constant (shift) */
 
 /*
- * The SHIFT_SCALE define establishes the decimal point of the time_phase
- * variable which serves as an extension to the low-order bits of the
- * system clock variable. The SHIFT_UPDATE define establishes the decimal
- * point of the time_offset variable which represents the current offset
- * with respect to standard time. The FINENSEC define represents 1 nsec in
- * scaled units.
+ * 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.
- *
- * FINENSEC is 1 ns in SHIFT_UPDATE units of the time_phase variable.
  */
-#define SHIFT_SCALE 22		/* phase scale (shift) */
 #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 FINENSEC (1L << (SHIFT_SCALE - 10)) /* ~1 ns in phase units */
 
 #define MAXPHASE 512000L        /* max phase error (us) */
 #define MAXFREQ (512L << SHIFT_USEC)  /* max frequency error (ppm) */

Signature · 3 lines

1.4.1

--

john stultz

2006-08-16 23:08 UTC #16 lore raw

In reply to [email protected]

Quoted 5 lines · On Thu, 2006-08-10 at 02:01 +0200, [email protected] wrote:

Here is my current version of the NTP patches.
They precalculate as much possible and get rid of a lot of rather crude compensation code. The tick length is now a much simpler value, updated once a second, which greatly reduces the dependency on HZ.
I rebased the patches against current -mm + John's ntp.c patch.

Hey Roman,
How much real-world testing have you done with these patches? I've been
running w/ this set of patches for a few days and I've been noticing my system is having difficulties synching up w/ the NTP server.

I haven't been logging anything, so its currently uncertain data, but normally I've seen NTP sync the time within 1-2ms in just an hour or so, however since this morning (~6 hours ago) I'm seeing it still 10ms off.

I'm going to let it run for the rest of the day then try to bisect the patches to see where things went wrong. I'll let you know as soon as I find anything.

thanks
-john

Roman Zippel

author

2006-08-17 10:38 UTC #17 lore raw

Hi,

Quoted 3 lines · john stultz

How much real-world testing have you done with these patches? I've been
running w/ this set of patches for a few days and I've been noticing my system is having difficulties synching up w/ the NTP server.

I tested it on a few machines of course.

Quoted 7 lines

I haven't been logging anything, so its currently uncertain data, but normally I've seen NTP sync the time within 1-2ms in just an hour or so, however since this morning (~6 hours ago) I'm seeing it still 10ms off.
I'm going to let it run for the rest of the day then try to bisect the patches to see where things went wrong. I'll let you know as soon as I find anything.

I would need the loopstats over a few days to really say something about this. It may also depend on the stability of your remote NTP serer. The new code adjusts a little a slower, but should be overall a little more stable.
The ntpd does basically "if (!pll_nano) ntv.constant = sys_poll - 4;" to adjust for the difference in behaviour of the two models, but this causes rather fast adjustments. In the NTP4 patch I undo this adjustment for the new model.

Quoted 3 lines

I'm going to let it run for the rest of the day then try to bisect the patches to see where things went wrong. I'll let you know as soon as I find anything.

Well, upto the NTP4 patch the behaviour should be mostly unchanged.

bye, Roman