Re: [PATCH 1/3] Updated dynamic tick patches - Fix lost tick

[Thomas Schlichter]

Hi Srivatsa, 

on LKML I did see your patch trying to increase the accuracy of tme pmtmr by 
directly converting the PM-timer-ticks to jiffies. I think this is a good 
idea but as you already recognized, it is not completely correct...

There are at least these issues:
1. "offset_last" corresponds to the time when the last recognized
   jiffyoccoured. So "delta" always corresponds to the time from the last
   recognized jiffy to _now_. "monotonic_base" is increased by delta, so after
   the first run it will correspond to _now_. But the next time the
   offset-time between the last recognized jiffy and the last _now_ is added
   _again_. So the monotonic clock ist too fast...
2. "offset_last is modified outside the "monotonic_lock", what is not allowed.

I fixed these issues by using most of the old code, but simply changed "delta" 
and "offset_delay" to always contain PM-timer-ticks and compute the lost 
jiffies directly using PMTMR_TICKS_PER_JIFFY.

I tested the attached patch during the last night and it sems to work...

Best regards
  Thomas Schlichter
--- linux-2.6.13/arch/i386/kernel/timers/timer_pm.c.orig	2005-08-31 23:34:11.000000000 +0200
+++ linux-2.6.13/arch/i386/kernel/timers/timer_pm.c	2005-09-01 00:08:20.000000000 +0200
@@ -28,6 +28,7 @@
 #define PMTMR_TICKS_PER_SEC 3579545
 #define PMTMR_EXPECTED_RATE \
   ((CALIBRATE_LATCH * (PMTMR_TICKS_PER_SEC >> 10)) / (CLOCK_TICK_RATE>>10))
+#define PMTMR_TICKS_PER_JIFFY (PMTMR_TICKS_PER_SEC / HZ)
 
 
 /* The I/O port the PMTMR resides at.
@@ -128,6 +129,11 @@ pm_good:
 		return -ENODEV;
 
 	init_cpu_khz();
+
+	printk ("Using %u PM timer ticks per jiffy \n", PMTMR_TICKS_PER_JIFFY);
+
+	offset_tick = read_pmtmr();
+
 	return 0;
 }
 
@@ -151,7 +157,6 @@ static inline u32 cyc2us(u32 cycles)
 static void mark_offset_pmtmr(void)
 {
 	u32 lost, delta, last_offset;
-	static int first_run = 1;
 	last_offset = offset_tick;
 
 	write_seqlock(&monotonic_lock);
@@ -161,29 +166,23 @@ static void mark_offset_pmtmr(void)
 	/* calculate tick interval */
 	delta = (offset_tick - last_offset) & ACPI_PM_MASK;
 
-	/* convert to usecs */
-	delta = cyc2us(delta);
-
 	/* update the monotonic base value */
-	monotonic_base += delta * NSEC_PER_USEC;
+	monotonic_base += cyc2us(delta) * NSEC_PER_USEC;
 	write_sequnlock(&monotonic_lock);
 
 	/* convert to ticks */
 	delta += offset_delay;
-	lost = delta / (USEC_PER_SEC / HZ);
-	offset_delay = delta % (USEC_PER_SEC / HZ);
+	lost = delta / PMTMR_TICKS_PER_JIFFY;
+	offset_delay = delta % PMTMR_TICKS_PER_JIFFY;
 
 
 	/* compensate for lost ticks */
 	if (lost >= 2)
 		jiffies_64 += lost - 1;
 
-	/* don't calculate delay for first run,
-	   or if we've got less then a tick */
-	if (first_run || (lost < 1)) {
-		first_run = 0;
+	/* don't calculate delay if we've got less then a tick */
+	if (lost < 1)
 		offset_delay = 0;
-	}
 }
 
 
@@ -233,9 +232,9 @@ static unsigned long get_offset_pmtmr(vo
 
 	offset = offset_tick;
 	now = read_pmtmr();
-	delta = (now - offset)&ACPI_PM_MASK;
+	delta = (now - offset) & ACPI_PM_MASK;
 
-	return (unsigned long) offset_delay + cyc2us(delta);
+	return (unsigned long) cyc2us(delta + offset_delay);
 }

Attachment: pgp00000.pgp
Description: signature