Linux MIPS Architecture development
 help / color / mirror / Atom feed
* [PATCH] gettimeofday jumps backwards then forwards
@ 2005-07-20 14:43 Dave Johnson
  2006-01-16 16:00 ` Martin Michlmayr
  0 siblings, 1 reply; 5+ messages in thread
From: Dave Johnson @ 2005-07-20 14:43 UTC (permalink / raw)
  To: linux-mips


Below are 2 fixes I made to 2.6.12 to do with time jumping around
as reported by gettimeofday().  One is SB1250 specific and one appears
generic.

The symptom is revealed by running multile copies (1 per cpu) of a
simple test program that calls gettimeofday() as fast as possible
looking for time to go backwards.

When a jump is detected the program outputs a few samples before and
after each jump:

value               delta
1121781527.912525:      1
1121781527.912525:      0
1121781527.912526:      1
1121781527.912526:      0
1121781527.912527:      1
1121781527.912527:      0
1121781527.912527:      0
1121781527.912527:      0
1121781527.911528:   -999
1121781527.911529:      1
1121781527.911530:      1
1121781527.912532:   1002
1121781527.912533:      1
1121781527.912533:      0
1121781527.912534:      1
1121781527.912534:      0
1121781527.912535:      1
1121781527.912536:      1

value               delta
1121781545.635524:      1
1121781545.635524:      0
1121781545.635525:      1
1121781545.635525:      0
1121781545.635526:      1
1121781545.635526:      0
1121781545.635527:      1
1121781545.635527:      0
1121781545.634527:  -1000
1121781545.635527:   1000
1121781545.635528:      1
1121781545.635529:      1
1121781545.635529:      0
1121781545.635530:      1
1121781545.635530:      0
1121781545.635531:      1
1121781545.635531:      0
1121781545.635532:      1
1121781545.635533:      1

Time jumps backwards 1msec then forwards 1msec a few usec
later.  Usually lasts < 2us but I've seen it as long as 5us if the
system is under load.

--

First problem I found is that sb1250_gettimeoffset() simply reads the
current cpu 0 timer remaining value, however once this counter reaches
0 and the interrupt is raised, it immediately resets and begins to
count down again.

If sb1250_gettimeoffset() is called on cpu 1 via do_gettimeofday()
after the timer has reset but prior to cpu 0 processing the interrupt
and taking write_seqlock() in timer_interrupt() it will return a
full value (or close to it) causing time to jump backwards 1ms. Once
cpu 0 handles the interrupt and timer_interrupt() gets far enough
along it will jump forward 1ms.

To fix this problem I implemented mips_hpt_*() on sb1250 using a spare
timer unrelated to the existing periodic interrupt timers. It runs at
1Mhz with a full 23bit counter.  This eliminated the custom
do_gettimeoffset() for sb1250 and allowed use of the generic
fixed_rate_gettimeoffset() using mips_hpt_*() and timerhi/timerlo.

--

The second problem is that more of timer_interrupt() needs to be
protected by xtime_lock:

* do_timer() expects the arch-specific handler to take the lock as it
  modifies jiffies[_64] and xtime.
* writing timerhi/lo in timer_interrupt() will mess up
  fixed_rate_gettimeoffset() which reads timerhi/lo.

--

With both changes do_gettimeofday() works correctly on both cpu 0 and
cpu 1.

--

Other changes/cleanups:

The existing sb1250 periodic timers were slow by 999ppm (given a
perfect 100mhz reference).  The timers need to be loaded with 1 less
than the desired interval not the interval itself.

M_SCD_TIMER_INIT and M_SCD_TIMER_CNT had the wrong field width (should
be 23 bits not 20 bits)

-- 
Dave Johnson
Starent Networks

============

diff -Nru a/arch/mips/kernel/time.c b/arch/mips/kernel/time.c
--- a/arch/mips/kernel/time.c	2005-07-20 10:25:22 -04:00
+++ b/arch/mips/kernel/time.c	2005-07-20 10:25:22 -04:00
@@ -424,6 +424,8 @@
 	unsigned long j;
 	unsigned int count;
 
+	write_seqlock(&xtime_lock);
+
 	count = mips_hpt_read();
 	mips_timer_ack();
 
@@ -441,7 +443,6 @@
 	 * CMOS clock accordingly every ~11 minutes. rtc_set_time() has to be
 	 * called as close as possible to 500 ms before the new second starts.
 	 */
-	write_seqlock(&xtime_lock);
 	if ((time_status & STA_UNSYNC) == 0 &&
 	    xtime.tv_sec > last_rtc_update + 660 &&
 	    (xtime.tv_nsec / 1000) >= 500000 - ((unsigned) TICK_SIZE) / 2 &&
@@ -453,7 +454,6 @@
 			last_rtc_update = xtime.tv_sec - 600;
 		}
 	}
-	write_sequnlock(&xtime_lock);
 
 	/*
 	 * If jiffies has overflown in this timer_interrupt, we must
@@ -495,6 +495,8 @@
 			break;
 		}
 	}
+
+	write_sequnlock(&xtime_lock);
 
 	/*
 	 * In UP mode, we call local_timer_interrupt() to do profiling
diff -Nru a/arch/mips/sibyte/sb1250/time.c b/arch/mips/sibyte/sb1250/time.c
--- a/arch/mips/sibyte/sb1250/time.c	2005-07-20 10:25:22 -04:00
+++ b/arch/mips/sibyte/sb1250/time.c	2005-07-20 10:25:22 -04:00
@@ -47,23 +47,51 @@
 #define IMR_IP3_VAL	K_INT_MAP_I1
 #define IMR_IP4_VAL	K_INT_MAP_I2
 
+#define SB1250_HPT_NUM		3
+#define SB1250_HPT_VALUE	M_SCD_TIMER_CNT /* max value */
+#define SB1250_HPT_SHIFT	((sizeof(unsigned int)*8)-V_SCD_TIMER_WIDTH)
+
+
 extern int sb1250_steal_irq(int irq);
 
+static unsigned int sb1250_hpt_read(void);
+static void sb1250_hpt_init(unsigned int);
+
+static unsigned int hpt_offset;
+
+void __init sb1250_hpt_setup(void)
+{
+	int cpu = smp_processor_id();
+
+	if (!cpu) {
+		/* Setup hpt using timer #3 but do not enable irq for it */
+		__raw_writeq(0, IOADDR(A_SCD_TIMER_REGISTER(SB1250_HPT_NUM, R_SCD_TIMER_CFG)));
+		__raw_writeq(SB1250_HPT_VALUE,
+			     IOADDR(A_SCD_TIMER_REGISTER(SB1250_HPT_NUM, R_SCD_TIMER_INIT)));
+		__raw_writeq(M_SCD_TIMER_ENABLE | M_SCD_TIMER_MODE_CONTINUOUS,
+			     IOADDR(A_SCD_TIMER_REGISTER(SB1250_HPT_NUM, R_SCD_TIMER_CFG)));
+
+		/*
+		 * we need to fill 32 bits, so just use the upper 23 bits and pretend
+		 * the timer is going 512Mhz instead of 1Mhz
+		 */
+		mips_hpt_frequency = V_SCD_TIMER_FREQ << SB1250_HPT_SHIFT;
+		mips_hpt_init = sb1250_hpt_init;
+		mips_hpt_read = sb1250_hpt_read;
+	}
+}
+
+
 void sb1250_time_init(void)
 {
 	int cpu = smp_processor_id();
 	int irq = K_INT_TIMER_0+cpu;
 
-	/* Only have 4 general purpose timers */
-	if (cpu > 3) {
+	/* Only have 4 general purpose timers, and we use last one as hpt */
+	if (cpu > 2) {
 		BUG();
 	}
 
-	if (!cpu) {
-		/* Use our own gettimeoffset() routine */
-		do_gettimeoffset = sb1250_gettimeoffset;
-	}
-
 	sb1250_mask_irq(cpu, irq);
 
 	/* Map the timer interrupt to ip[4] of this cpu */
@@ -75,10 +103,10 @@
 	/* Disable the timer and set up the count */
 	__raw_writeq(0, IOADDR(A_SCD_TIMER_REGISTER(cpu, R_SCD_TIMER_CFG)));
 #ifdef CONFIG_SIMULATION
-	__raw_writeq(50000 / HZ,
+	__raw_writeq((50000 / HZ) - 1,
 		     IOADDR(A_SCD_TIMER_REGISTER(cpu, R_SCD_TIMER_INIT)));
 #else
-	__raw_writeq(1000000 / HZ,
+	__raw_writeq((V_SCD_TIMER_FREQ / HZ) - 1,
 		     IOADDR(A_SCD_TIMER_REGISTER(cpu, R_SCD_TIMER_INIT)));
 #endif
 
@@ -104,7 +132,7 @@
 	int cpu = smp_processor_id();
 	int irq = K_INT_TIMER_0 + cpu;
 
-	/* Reset the timer */
+	/* ACK interrupt */
 	____raw_writeq(M_SCD_TIMER_ENABLE | M_SCD_TIMER_MODE_CONTINUOUS,
 		       IOADDR(A_SCD_TIMER_REGISTER(cpu, R_SCD_TIMER_CFG)));
 
@@ -122,15 +150,26 @@
 }
 
 /*
- * We use our own do_gettimeoffset() instead of the generic one,
- * because the generic one does not work for SMP case.
- * In addition, since we use general timer 0 for system time,
- * we can get accurate intra-jiffy offset without calibration.
+ * The HPT is free running from SB1250_HPT_VALUE down to 0 then starts over
+ * again. There's no easy way to set to a specific value so store init value
+ * in hpt_offset and subtract each time.
+ *
+ * Note: Timer isn't full 32bits so shift it into the upper part making
+ *       it appear to run at a higher frequency.
  */
-unsigned long sb1250_gettimeoffset(void)
+static unsigned int sb1250_hpt_read(void)
 {
-	unsigned long count =
-		__raw_readq(IOADDR(A_SCD_TIMER_REGISTER(0, R_SCD_TIMER_CNT)));
+	unsigned int count;
+
+	count = G_SCD_TIMER_CNT(__raw_readq(IOADDR(A_SCD_TIMER_REGISTER(SB1250_HPT_NUM, R_SCD_TIMER_CNT))));
 
-	return 1000000/HZ - count;
- }
+	count = (SB1250_HPT_VALUE - count) << SB1250_HPT_SHIFT;
+
+	return count - hpt_offset;
+}
+
+static void sb1250_hpt_init(unsigned int count)
+{
+	hpt_offset = count;
+	return;
+}
diff -Nru a/arch/mips/sibyte/swarm/setup.c b/arch/mips/sibyte/swarm/setup.c
--- a/arch/mips/sibyte/swarm/setup.c	2005-07-20 10:25:22 -04:00
+++ b/arch/mips/sibyte/swarm/setup.c	2005-07-20 10:25:22 -04:00
@@ -64,6 +64,12 @@
 	return "SiByte " SIBYTE_BOARD_NAME;
 }
 
+void __init swarm_time_init(void)
+{
+	/* Setup HPT */
+	sb1250_hpt_setup();
+}
+
 void __init swarm_timer_setup(struct irqaction *irq)
 {
         /*
@@ -96,6 +102,7 @@
 
 	panic_timeout = 5;  /* For debug.  */
 
+	board_time_init = swarm_time_init;
 	board_timer_setup = swarm_timer_setup;
 	board_be_handler = swarm_be_handler;
 
diff -Nru a/include/asm-mips/sibyte/sb1250.h b/include/asm-mips/sibyte/sb1250.h
--- a/include/asm-mips/sibyte/sb1250.h	2005-07-20 10:25:22 -04:00
+++ b/include/asm-mips/sibyte/sb1250.h	2005-07-20 10:25:22 -04:00
@@ -41,8 +41,8 @@
 extern unsigned int periph_rev;
 extern unsigned int zbbus_mhz;
 
+extern void sb1250_hpt_setup(void);
 extern void sb1250_time_init(void);
-extern unsigned long sb1250_gettimeoffset(void);
 extern void sb1250_mask_irq(int cpu, int irq);
 extern void sb1250_unmask_irq(int cpu, int irq);
 extern void sb1250_smp_finish(void);
diff -Nru a/include/asm-mips/sibyte/sb1250_scd.h b/include/asm-mips/sibyte/sb1250_scd.h
--- a/include/asm-mips/sibyte/sb1250_scd.h	2005-07-20 10:25:22 -04:00
+++ b/include/asm-mips/sibyte/sb1250_scd.h	2005-07-20 10:25:22 -04:00
@@ -307,14 +307,15 @@
  */
 
 #define V_SCD_TIMER_FREQ            1000000
+#define V_SCD_TIMER_WIDTH           23
 
 #define S_SCD_TIMER_INIT            0
-#define M_SCD_TIMER_INIT            _SB_MAKEMASK(20,S_SCD_TIMER_INIT)
+#define M_SCD_TIMER_INIT            _SB_MAKEMASK(V_SCD_TIMER_WIDTH,S_SCD_TIMER_INIT)
 #define V_SCD_TIMER_INIT(x)         _SB_MAKEVALUE(x,S_SCD_TIMER_INIT)
 #define G_SCD_TIMER_INIT(x)         _SB_GETVALUE(x,S_SCD_TIMER_INIT,M_SCD_TIMER_INIT)
 
 #define S_SCD_TIMER_CNT             0
-#define M_SCD_TIMER_CNT             _SB_MAKEMASK(20,S_SCD_TIMER_CNT)
+#define M_SCD_TIMER_CNT             _SB_MAKEMASK(V_SCD_TIMER_WIDTH,S_SCD_TIMER_CNT)
 #define V_SCD_TIMER_CNT(x)         _SB_MAKEVALUE(x,S_SCD_TIMER_CNT)
 #define G_SCD_TIMER_CNT(x)         _SB_GETVALUE(x,S_SCD_TIMER_CNT,M_SCD_TIMER_CNT)
 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] gettimeofday jumps backwards then forwards
  2005-07-20 14:43 [PATCH] gettimeofday jumps backwards then forwards Dave Johnson
@ 2006-01-16 16:00 ` Martin Michlmayr
  2006-01-16 16:35   ` Dave Johnson
  0 siblings, 1 reply; 5+ messages in thread
From: Martin Michlmayr @ 2006-01-16 16:00 UTC (permalink / raw)
  To: Dave Johnson; +Cc: linux-mips

* Dave Johnson <djohnson+linuxmips@sw.starentnetworks.com> [2005-07-20 10:43]:
> Below are 2 fixes I made to 2.6.12 to do with time jumping around
> as reported by gettimeofday().  One is SB1250 specific and one appears
> generic.
> 
> The symptom is revealed by running multile copies (1 per cpu) of a
> simple test program that calls gettimeofday() as fast as possible
> looking for time to go backwards.
> 
> When a jump is detected the program outputs a few samples before and
> after each jump:

Does anyone have comments regarding this patch?

> value               delta
> 1121781527.912525:      1
> 1121781527.912525:      0
> 1121781527.912526:      1
> 1121781527.912526:      0
> 1121781527.912527:      1
> 1121781527.912527:      0
> 1121781527.912527:      0
> 1121781527.912527:      0
> 1121781527.911528:   -999
> 1121781527.911529:      1
> 1121781527.911530:      1
> 1121781527.912532:   1002
> 1121781527.912533:      1
> 1121781527.912533:      0
> 1121781527.912534:      1
> 1121781527.912534:      0
> 1121781527.912535:      1
> 1121781527.912536:      1
> 
> value               delta
> 1121781545.635524:      1
> 1121781545.635524:      0
> 1121781545.635525:      1
> 1121781545.635525:      0
> 1121781545.635526:      1
> 1121781545.635526:      0
> 1121781545.635527:      1
> 1121781545.635527:      0
> 1121781545.634527:  -1000
> 1121781545.635527:   1000
> 1121781545.635528:      1
> 1121781545.635529:      1
> 1121781545.635529:      0
> 1121781545.635530:      1
> 1121781545.635530:      0
> 1121781545.635531:      1
> 1121781545.635531:      0
> 1121781545.635532:      1
> 1121781545.635533:      1
> 
> Time jumps backwards 1msec then forwards 1msec a few usec
> later.  Usually lasts < 2us but I've seen it as long as 5us if the
> system is under load.
> 
> --
> 
> First problem I found is that sb1250_gettimeoffset() simply reads the
> current cpu 0 timer remaining value, however once this counter reaches
> 0 and the interrupt is raised, it immediately resets and begins to
> count down again.
> 
> If sb1250_gettimeoffset() is called on cpu 1 via do_gettimeofday()
> after the timer has reset but prior to cpu 0 processing the interrupt
> and taking write_seqlock() in timer_interrupt() it will return a
> full value (or close to it) causing time to jump backwards 1ms. Once
> cpu 0 handles the interrupt and timer_interrupt() gets far enough
> along it will jump forward 1ms.
> 
> To fix this problem I implemented mips_hpt_*() on sb1250 using a spare
> timer unrelated to the existing periodic interrupt timers. It runs at
> 1Mhz with a full 23bit counter.  This eliminated the custom
> do_gettimeoffset() for sb1250 and allowed use of the generic
> fixed_rate_gettimeoffset() using mips_hpt_*() and timerhi/timerlo.
> 
> --
> 
> The second problem is that more of timer_interrupt() needs to be
> protected by xtime_lock:
> 
> * do_timer() expects the arch-specific handler to take the lock as it
>   modifies jiffies[_64] and xtime.
> * writing timerhi/lo in timer_interrupt() will mess up
>   fixed_rate_gettimeoffset() which reads timerhi/lo.
> 
> --
> 
> With both changes do_gettimeofday() works correctly on both cpu 0 and
> cpu 1.
> 
> --
> 
> Other changes/cleanups:
> 
> The existing sb1250 periodic timers were slow by 999ppm (given a
> perfect 100mhz reference).  The timers need to be loaded with 1 less
> than the desired interval not the interval itself.
> 
> M_SCD_TIMER_INIT and M_SCD_TIMER_CNT had the wrong field width (should
> be 23 bits not 20 bits)
> 
> -- 
> Dave Johnson
> Starent Networks
> 
> ============
> 
> diff -Nru a/arch/mips/kernel/time.c b/arch/mips/kernel/time.c
> --- a/arch/mips/kernel/time.c	2005-07-20 10:25:22 -04:00
> +++ b/arch/mips/kernel/time.c	2005-07-20 10:25:22 -04:00
> @@ -424,6 +424,8 @@
>  	unsigned long j;
>  	unsigned int count;
>  
> +	write_seqlock(&xtime_lock);
> +
>  	count = mips_hpt_read();
>  	mips_timer_ack();
>  
> @@ -441,7 +443,6 @@
>  	 * CMOS clock accordingly every ~11 minutes. rtc_set_time() has to be
>  	 * called as close as possible to 500 ms before the new second starts.
>  	 */
> -	write_seqlock(&xtime_lock);
>  	if ((time_status & STA_UNSYNC) == 0 &&
>  	    xtime.tv_sec > last_rtc_update + 660 &&
>  	    (xtime.tv_nsec / 1000) >= 500000 - ((unsigned) TICK_SIZE) / 2 &&
> @@ -453,7 +454,6 @@
>  			last_rtc_update = xtime.tv_sec - 600;
>  		}
>  	}
> -	write_sequnlock(&xtime_lock);
>  
>  	/*
>  	 * If jiffies has overflown in this timer_interrupt, we must
> @@ -495,6 +495,8 @@
>  			break;
>  		}
>  	}
> +
> +	write_sequnlock(&xtime_lock);
>  
>  	/*
>  	 * In UP mode, we call local_timer_interrupt() to do profiling
> diff -Nru a/arch/mips/sibyte/sb1250/time.c b/arch/mips/sibyte/sb1250/time.c
> --- a/arch/mips/sibyte/sb1250/time.c	2005-07-20 10:25:22 -04:00
> +++ b/arch/mips/sibyte/sb1250/time.c	2005-07-20 10:25:22 -04:00
> @@ -47,23 +47,51 @@
>  #define IMR_IP3_VAL	K_INT_MAP_I1
>  #define IMR_IP4_VAL	K_INT_MAP_I2
>  
> +#define SB1250_HPT_NUM		3
> +#define SB1250_HPT_VALUE	M_SCD_TIMER_CNT /* max value */
> +#define SB1250_HPT_SHIFT	((sizeof(unsigned int)*8)-V_SCD_TIMER_WIDTH)
> +
> +
>  extern int sb1250_steal_irq(int irq);
>  
> +static unsigned int sb1250_hpt_read(void);
> +static void sb1250_hpt_init(unsigned int);
> +
> +static unsigned int hpt_offset;
> +
> +void __init sb1250_hpt_setup(void)
> +{
> +	int cpu = smp_processor_id();
> +
> +	if (!cpu) {
> +		/* Setup hpt using timer #3 but do not enable irq for it */
> +		__raw_writeq(0, IOADDR(A_SCD_TIMER_REGISTER(SB1250_HPT_NUM, R_SCD_TIMER_CFG)));
> +		__raw_writeq(SB1250_HPT_VALUE,
> +			     IOADDR(A_SCD_TIMER_REGISTER(SB1250_HPT_NUM, R_SCD_TIMER_INIT)));
> +		__raw_writeq(M_SCD_TIMER_ENABLE | M_SCD_TIMER_MODE_CONTINUOUS,
> +			     IOADDR(A_SCD_TIMER_REGISTER(SB1250_HPT_NUM, R_SCD_TIMER_CFG)));
> +
> +		/*
> +		 * we need to fill 32 bits, so just use the upper 23 bits and pretend
> +		 * the timer is going 512Mhz instead of 1Mhz
> +		 */
> +		mips_hpt_frequency = V_SCD_TIMER_FREQ << SB1250_HPT_SHIFT;
> +		mips_hpt_init = sb1250_hpt_init;
> +		mips_hpt_read = sb1250_hpt_read;
> +	}
> +}
> +
> +
>  void sb1250_time_init(void)
>  {
>  	int cpu = smp_processor_id();
>  	int irq = K_INT_TIMER_0+cpu;
>  
> -	/* Only have 4 general purpose timers */
> -	if (cpu > 3) {
> +	/* Only have 4 general purpose timers, and we use last one as hpt */
> +	if (cpu > 2) {
>  		BUG();
>  	}
>  
> -	if (!cpu) {
> -		/* Use our own gettimeoffset() routine */
> -		do_gettimeoffset = sb1250_gettimeoffset;
> -	}
> -
>  	sb1250_mask_irq(cpu, irq);
>  
>  	/* Map the timer interrupt to ip[4] of this cpu */
> @@ -75,10 +103,10 @@
>  	/* Disable the timer and set up the count */
>  	__raw_writeq(0, IOADDR(A_SCD_TIMER_REGISTER(cpu, R_SCD_TIMER_CFG)));
>  #ifdef CONFIG_SIMULATION
> -	__raw_writeq(50000 / HZ,
> +	__raw_writeq((50000 / HZ) - 1,
>  		     IOADDR(A_SCD_TIMER_REGISTER(cpu, R_SCD_TIMER_INIT)));
>  #else
> -	__raw_writeq(1000000 / HZ,
> +	__raw_writeq((V_SCD_TIMER_FREQ / HZ) - 1,
>  		     IOADDR(A_SCD_TIMER_REGISTER(cpu, R_SCD_TIMER_INIT)));
>  #endif
>  
> @@ -104,7 +132,7 @@
>  	int cpu = smp_processor_id();
>  	int irq = K_INT_TIMER_0 + cpu;
>  
> -	/* Reset the timer */
> +	/* ACK interrupt */
>  	____raw_writeq(M_SCD_TIMER_ENABLE | M_SCD_TIMER_MODE_CONTINUOUS,
>  		       IOADDR(A_SCD_TIMER_REGISTER(cpu, R_SCD_TIMER_CFG)));
>  
> @@ -122,15 +150,26 @@
>  }
>  
>  /*
> - * We use our own do_gettimeoffset() instead of the generic one,
> - * because the generic one does not work for SMP case.
> - * In addition, since we use general timer 0 for system time,
> - * we can get accurate intra-jiffy offset without calibration.
> + * The HPT is free running from SB1250_HPT_VALUE down to 0 then starts over
> + * again. There's no easy way to set to a specific value so store init value
> + * in hpt_offset and subtract each time.
> + *
> + * Note: Timer isn't full 32bits so shift it into the upper part making
> + *       it appear to run at a higher frequency.
>   */
> -unsigned long sb1250_gettimeoffset(void)
> +static unsigned int sb1250_hpt_read(void)
>  {
> -	unsigned long count =
> -		__raw_readq(IOADDR(A_SCD_TIMER_REGISTER(0, R_SCD_TIMER_CNT)));
> +	unsigned int count;
> +
> +	count = G_SCD_TIMER_CNT(__raw_readq(IOADDR(A_SCD_TIMER_REGISTER(SB1250_HPT_NUM, R_SCD_TIMER_CNT))));
>  
> -	return 1000000/HZ - count;
> - }
> +	count = (SB1250_HPT_VALUE - count) << SB1250_HPT_SHIFT;
> +
> +	return count - hpt_offset;
> +}
> +
> +static void sb1250_hpt_init(unsigned int count)
> +{
> +	hpt_offset = count;
> +	return;
> +}
> diff -Nru a/arch/mips/sibyte/swarm/setup.c b/arch/mips/sibyte/swarm/setup.c
> --- a/arch/mips/sibyte/swarm/setup.c	2005-07-20 10:25:22 -04:00
> +++ b/arch/mips/sibyte/swarm/setup.c	2005-07-20 10:25:22 -04:00
> @@ -64,6 +64,12 @@
>  	return "SiByte " SIBYTE_BOARD_NAME;
>  }
>  
> +void __init swarm_time_init(void)
> +{
> +	/* Setup HPT */
> +	sb1250_hpt_setup();
> +}
> +
>  void __init swarm_timer_setup(struct irqaction *irq)
>  {
>          /*
> @@ -96,6 +102,7 @@
>  
>  	panic_timeout = 5;  /* For debug.  */
>  
> +	board_time_init = swarm_time_init;
>  	board_timer_setup = swarm_timer_setup;
>  	board_be_handler = swarm_be_handler;
>  
> diff -Nru a/include/asm-mips/sibyte/sb1250.h b/include/asm-mips/sibyte/sb1250.h
> --- a/include/asm-mips/sibyte/sb1250.h	2005-07-20 10:25:22 -04:00
> +++ b/include/asm-mips/sibyte/sb1250.h	2005-07-20 10:25:22 -04:00
> @@ -41,8 +41,8 @@
>  extern unsigned int periph_rev;
>  extern unsigned int zbbus_mhz;
>  
> +extern void sb1250_hpt_setup(void);
>  extern void sb1250_time_init(void);
> -extern unsigned long sb1250_gettimeoffset(void);
>  extern void sb1250_mask_irq(int cpu, int irq);
>  extern void sb1250_unmask_irq(int cpu, int irq);
>  extern void sb1250_smp_finish(void);
> diff -Nru a/include/asm-mips/sibyte/sb1250_scd.h b/include/asm-mips/sibyte/sb1250_scd.h
> --- a/include/asm-mips/sibyte/sb1250_scd.h	2005-07-20 10:25:22 -04:00
> +++ b/include/asm-mips/sibyte/sb1250_scd.h	2005-07-20 10:25:22 -04:00
> @@ -307,14 +307,15 @@
>   */
>  
>  #define V_SCD_TIMER_FREQ            1000000
> +#define V_SCD_TIMER_WIDTH           23
>  
>  #define S_SCD_TIMER_INIT            0
> -#define M_SCD_TIMER_INIT            _SB_MAKEMASK(20,S_SCD_TIMER_INIT)
> +#define M_SCD_TIMER_INIT            _SB_MAKEMASK(V_SCD_TIMER_WIDTH,S_SCD_TIMER_INIT)
>  #define V_SCD_TIMER_INIT(x)         _SB_MAKEVALUE(x,S_SCD_TIMER_INIT)
>  #define G_SCD_TIMER_INIT(x)         _SB_GETVALUE(x,S_SCD_TIMER_INIT,M_SCD_TIMER_INIT)
>  
>  #define S_SCD_TIMER_CNT             0
> -#define M_SCD_TIMER_CNT             _SB_MAKEMASK(20,S_SCD_TIMER_CNT)
> +#define M_SCD_TIMER_CNT             _SB_MAKEMASK(V_SCD_TIMER_WIDTH,S_SCD_TIMER_CNT)
>  #define V_SCD_TIMER_CNT(x)         _SB_MAKEVALUE(x,S_SCD_TIMER_CNT)
>  #define G_SCD_TIMER_CNT(x)         _SB_GETVALUE(x,S_SCD_TIMER_CNT,M_SCD_TIMER_CNT)

-- 
Martin Michlmayr
http://www.cyrius.com/

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] gettimeofday jumps backwards then forwards
  2006-01-16 16:00 ` Martin Michlmayr
@ 2006-01-16 16:35   ` Dave Johnson
  2006-01-16 18:40     ` [PATCH] fix lost ticks on SB1250 [WAS: Re: [PATCH] gettimeofday jumps backwards then forwards] Dave Johnson
  0 siblings, 1 reply; 5+ messages in thread
From: Dave Johnson @ 2006-01-16 16:35 UTC (permalink / raw)
  To: Martin Michlmayr, linux-mips

Martin Michlmayr writes:
> * Dave Johnson <djohnson+linuxmips@sw.starentnetworks.com> [2005-07-20 10:43]:
> > Below are 2 fixes I made to 2.6.12 to do with time jumping around
> > as reported by gettimeofday().  One is SB1250 specific and one appears
> > generic.
> > 
> > The symptom is revealed by running multile copies (1 per cpu) of a
> > simple test program that calls gettimeofday() as fast as possible
> > looking for time to go backwards.
> > 
> > When a jump is detected the program outputs a few samples before and
> > after each jump:
> 
> Does anyone have comments regarding this patch?

In addition to this problem I found significant lost ticks under load
as there is no checking for lost ticks. I'll create a clean patch with
just that fix in a bit.


-- 
Dave Johnson
Starent Networks

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] fix lost ticks on SB1250 [WAS: Re: [PATCH] gettimeofday jumps backwards then forwards]
  2006-01-16 16:35   ` Dave Johnson
@ 2006-01-16 18:40     ` Dave Johnson
  2006-03-01 12:45       ` [PATCH] fix lost ticks on SB1250 Martin Michlmayr
  0 siblings, 1 reply; 5+ messages in thread
From: Dave Johnson @ 2006-01-16 18:40 UTC (permalink / raw)
  To: Martin Michlmayr, linux-mips

Dave Johnson writes:
> In addition to this problem I found significant lost ticks under load
> as there is no checking for lost ticks. I'll create a clean patch with
> just that fix in a bit.

Below patch fixes 2 issues and must be applied after my previous
gettimeofday patch because it uses the hpt introduced in that patch.

1) Lost timer ticks were not being noticed and counted for sb1250 causing
   time to slow down under heavy load.
2) The 2 periodic timers (one on each core of the sb1250) had a random phase
   when compared to each other.  In certain phases this will lead to artificial
   high cpu usage and load according to process accounting. (see comment in
   code below for more info).  This would occur on about 1 out of 50 cpu boots.

Patch is against 2.6.12 with some other patches unfortunatly (only the gettimeofday
one should matter as far as merging)

Signed-off-by: Dave Johnson <djohnson+linuxmips@sw.starentnetworks.com>

diff -Nru a/arch/mips/dec/time.c b/arch/mips/dec/time.c
--- a/arch/mips/dec/time.c	2006-01-16 13:15:10 -05:00
+++ b/arch/mips/dec/time.c	2006-01-16 13:15:10 -05:00
@@ -151,7 +151,7 @@
 	return (CMOS_READ(RTC_REG_C) & RTC_PF) != 0;
 }
 
-static void dec_timer_ack(void)
+static void dec_timer_ack(unsigned int count)
 {
 	CMOS_READ(RTC_REG_C);			/* Ack the RTC interrupt.  */
 }
diff -Nru a/arch/mips/kernel/time.c b/arch/mips/kernel/time.c
--- a/arch/mips/kernel/time.c	2006-01-16 13:15:08 -05:00
+++ b/arch/mips/kernel/time.c	2006-01-16 13:15:09 -05:00
@@ -88,7 +88,7 @@
 /*
  * Null timer ack for systems not needing one (e.g. i8254).
  */
-static void null_timer_ack(void) { /* nothing */ }
+static void null_timer_ack(unsigned int count) { /* nothing */ }
 
 /*
  * Null high precision timer functions for systems lacking one.
@@ -104,16 +104,13 @@
 /*
  * Timer ack for an R4k-compatible timer of a known frequency.
  */
-static void c0_timer_ack(void)
+static void c0_timer_ack(unsigned int count)
 {
-	unsigned int count;
-
 	/* Ack this timer interrupt and set the next one.  */
 	expirelo += cycles_per_jiffy;
 	write_c0_compare(expirelo);
 
 	/* Check to see if we have missed any timer interrupts.  */
-	count = read_c0_count();
 	if ((count - expirelo) < 0x7fffffff) {
 		/* missed_timer_count++; */
 		expirelo = count + cycles_per_jiffy;
@@ -146,7 +143,7 @@
 }
 
 int (*mips_timer_state)(void);
-void (*mips_timer_ack)(void);
+void (*mips_timer_ack)(unsigned int);
 unsigned int (*mips_hpt_read)(void);
 void (*mips_hpt_init)(unsigned int);
 
@@ -427,7 +424,7 @@
 	write_seqlock(&xtime_lock);
 
 	count = mips_hpt_read();
-	mips_timer_ack();
+	mips_timer_ack(count);
 
 	/* Update timerhi/timerlo for intra-jiffy calibration. */
 	timerhi += count < timerlo;			/* Wrap around */
diff -Nru a/arch/mips/sibyte/sb1250/time.c b/arch/mips/sibyte/sb1250/time.c
--- a/arch/mips/sibyte/sb1250/time.c	2006-01-16 13:15:09 -05:00
+++ b/arch/mips/sibyte/sb1250/time.c	2006-01-16 13:15:10 -05:00
@@ -54,6 +54,9 @@
 
 extern int sb1250_steal_irq(int irq);
 
+static void sb1250_timer_ack(unsigned int);
+
+static unsigned int sb1250_hpt_read_no_offset(void);
 static unsigned int sb1250_hpt_read(void);
 static void sb1250_hpt_init(unsigned int);
 
@@ -99,20 +102,10 @@
 		     IOADDR(A_IMR_REGISTER(cpu, R_IMR_INTERRUPT_MAP_BASE) +
 			    (irq << 3)));
 
-	/* the general purpose timer ticks at 1 Mhz independent if the rest of the system */
-	/* Disable the timer and set up the count */
-	__raw_writeq(0, IOADDR(A_SCD_TIMER_REGISTER(cpu, R_SCD_TIMER_CFG)));
-#ifdef CONFIG_SIMULATION
-	__raw_writeq((50000 / HZ) - 1,
-		     IOADDR(A_SCD_TIMER_REGISTER(cpu, R_SCD_TIMER_INIT)));
-#else
-	__raw_writeq((V_SCD_TIMER_FREQ / HZ) - 1,
-		     IOADDR(A_SCD_TIMER_REGISTER(cpu, R_SCD_TIMER_INIT)));
-#endif
+	/* this will setup the interrupt for the first time */
+	sb1250_timer_ack(-1);
 
-	/* Set the timer running */
-	__raw_writeq(M_SCD_TIMER_ENABLE | M_SCD_TIMER_MODE_CONTINUOUS,
-		     IOADDR(A_SCD_TIMER_REGISTER(cpu, R_SCD_TIMER_CFG)));
+	mips_timer_ack = sb1250_timer_ack;
 
 	sb1250_unmask_irq(cpu, irq);
 	sb1250_steal_irq(irq);
@@ -126,21 +119,66 @@
 	 */
 }
 
+
+static void sb1250_timer_ack(unsigned int count) {
+	static unsigned int expected[NR_CPUS];
+	int newval = (V_SCD_TIMER_FREQ / HZ) << SB1250_HPT_SHIFT;
+	int cpu = smp_processor_id();
+
+	/* first time through? setup expected */
+	if (unlikely(count == -1)) {
+		expected[cpu] = count = sb1250_hpt_read_no_offset();
+
+		/*
+		 * We want the 2 cores to have their timers in sync instead of at a 
+		 * random phase.  Once started the phase doesn't change until the
+		 * cpu is rebooted.  With perticular phasing artificially inflated
+		 * cpu usage values and load can be reported to userspace that
+		 * remain until the cpu is rebooted.
+		 */
+		expected[cpu] -= count % newval;
+	}
+	else if (cpu == 0)
+		count += hpt_offset;
+
+	/* did we miss any ticks? */
+	if (unlikely((cpu == 0) && ((count - expected[cpu]) > newval)))
+		jiffies_64 += (count - expected[cpu]) / newval;
+
+	/* Now, how much time is left in current slice? */
+	newval -= (count - expected[cpu]) % newval;
+
+	expected[cpu] = count + newval;
+	newval >>= SB1250_HPT_SHIFT;
+
+	/* wait at least 1us before next IRQ */
+	if (unlikely(newval < 2))
+		newval = 2;
+
+	/* Disable the timer and set up the new count */
+	__raw_writeq(0, IOADDR(A_SCD_TIMER_REGISTER(cpu, R_SCD_TIMER_CFG)));
+	__raw_writeq(newval - 1,
+		     IOADDR(A_SCD_TIMER_REGISTER(cpu, R_SCD_TIMER_INIT)));
+	/* Set the timer running */
+	__raw_writeq(M_SCD_TIMER_ENABLE,
+		     IOADDR(A_SCD_TIMER_REGISTER(cpu, R_SCD_TIMER_CFG)));
+}
+
+
 void sb1250_timer_interrupt(struct pt_regs *regs)
 {
 	int cpu = smp_processor_id();
 	int irq = K_INT_TIMER_0 + cpu;
 
-	/* ACK interrupt */
-	____raw_writeq(M_SCD_TIMER_ENABLE | M_SCD_TIMER_MODE_CONTINUOUS,
-		       IOADDR(A_SCD_TIMER_REGISTER(cpu, R_SCD_TIMER_CFG)));
-
 	/*
 	 * CPU 0 handles the global timer interrupt job
 	 */
 	if (cpu == 0) {
 		ll_timer_interrupt(irq, regs);
 	}
+	else {
+		sb1250_timer_ack(sb1250_hpt_read_no_offset());
+	}
 
 	/*
 	 * every CPU should do profiling and process accouting
@@ -156,7 +194,7 @@
  * Note: Timer isn't full 32bits so shift it into the upper part making
  *       it appear to run at a higher frequency.
  */
-static unsigned int sb1250_hpt_read(void)
+static unsigned int sb1250_hpt_read_no_offset(void)
 {
 	unsigned int count;
 
@@ -164,7 +202,12 @@
 
 	count = (SB1250_HPT_VALUE - count) << SB1250_HPT_SHIFT;
 
-	return count - hpt_offset;
+	return count;
+}
+
+static unsigned int sb1250_hpt_read(void)
+{
+	return sb1250_hpt_read_no_offset() - hpt_offset;
 }
 
 static void sb1250_hpt_init(unsigned int count)
diff -Nru a/include/asm-mips/sibyte/sb1250_scd.h b/include/asm-mips/sibyte/sb1250_scd.h
--- a/include/asm-mips/sibyte/sb1250_scd.h	2006-01-16 13:15:10 -05:00
+++ b/include/asm-mips/sibyte/sb1250_scd.h	2006-01-16 13:15:10 -05:00
@@ -306,7 +306,11 @@
  * Timer Registers (Table 4-11) (Table 4-12) (Table 4-13)
  */
 
+#ifdef CONFIG_SIMULATION
+#define V_SCD_TIMER_FREQ            50000
+#else
 #define V_SCD_TIMER_FREQ            1000000
+#endif
 #define V_SCD_TIMER_WIDTH           23
 
 #define S_SCD_TIMER_INIT            0
diff -Nru a/include/asm-mips/time.h b/include/asm-mips/time.h
--- a/include/asm-mips/time.h	2006-01-16 13:15:05 -05:00
+++ b/include/asm-mips/time.h	2006-01-16 13:15:06 -05:00
@@ -38,7 +38,7 @@
  * mips_timer_ack may be NULL if the interrupt is self-recoverable.
  */
 extern int (*mips_timer_state)(void);
-extern void (*mips_timer_ack)(void);
+extern void (*mips_timer_ack)(unsigned int);
 
 /*
  * High precision timer functions.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] fix lost ticks on SB1250
  2006-01-16 18:40     ` [PATCH] fix lost ticks on SB1250 [WAS: Re: [PATCH] gettimeofday jumps backwards then forwards] Dave Johnson
@ 2006-03-01 12:45       ` Martin Michlmayr
  0 siblings, 0 replies; 5+ messages in thread
From: Martin Michlmayr @ 2006-03-01 12:45 UTC (permalink / raw)
  To: Dave Johnson; +Cc: linux-mips

* Dave Johnson <djohnson+linuxmips@sw.starentnetworks.com> [2006-01-16 13:40]:
> Patch is against 2.6.12 with some other patches unfortunatly (only
> the gettimeofday one should matter as far as merging)

Do you think you can update the patch for 2.6.16 and clean it up for
submission?

-- 
Martin Michlmayr
http://www.cyrius.com/

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2006-03-01 12:38 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-07-20 14:43 [PATCH] gettimeofday jumps backwards then forwards Dave Johnson
2006-01-16 16:00 ` Martin Michlmayr
2006-01-16 16:35   ` Dave Johnson
2006-01-16 18:40     ` [PATCH] fix lost ticks on SB1250 [WAS: Re: [PATCH] gettimeofday jumps backwards then forwards] Dave Johnson
2006-03-01 12:45       ` [PATCH] fix lost ticks on SB1250 Martin Michlmayr

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox