Re: [PATCH_TAKE_2] now < last_tick problem

[Ian Wienand <ianw@gelato.unsw.edu.au>]

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

On Fri, Oct 10, 2003 at 10:52:21AM +1000, Ian Wienand wrote:
> Suggested patch attached; note the fsyscall implementation does not
> appear to have this problem.

Sorry to reply to myself, but I noticed when using httperf I still got
one or two messages (literally, it was very much reduced); the first
patch misses the fact there is also a race between 'lost' being
calculated and the timer interrupt possibly catching jiffies up to
wall_jiffies.  I also added a comment as I get easily confused.

The attached patch lets me run httperf repeatedly with none of these
warnings, though there still might be a better way to do it.

-i



[-- Attachment #2: time.c.take2.diff --]
[-- Type: text/plain, Size: 1444 bytes --]

===== arch/ia64/kernel/time.c 1.35 vs edited =====
--- 1.35/arch/ia64/kernel/time.c	Wed Oct  8 12:53:38 2003
+++ edited/arch/ia64/kernel/time.c	Fri Oct 10 14:08:15 2003
@@ -71,11 +71,32 @@
 unsigned long
 itc_get_offset (void)
 {
-	unsigned long elapsed_cycles, lost = jiffies - wall_jiffies;
-	unsigned long now = ia64_get_itc(), last_tick;
+	unsigned long elapsed_cycles;
+	unsigned long now, last_tick;
 
+	/* 
+	 * itm_next is the next timer tick
+	 * itm_delta is the time between timer ticks
+	 * wall_jiffies are timer ticks the timer interrupt hasn't 
+	 * added to jiffies yet.
+	 *
+	 *    itm_delta      itm_delta
+	 * |--------------|---------------|
+	 * jiffies        wall_jiffies    itm_next
+	 *
+	 * (wall_jiffies - jiffies)*itm_delta = ITC ticks between jiffies and wall_jiffies
+	 * itm_next - itm_delta = ITC at wall_jiffies
+	 * last_tick = ITC at wall_jiffies - ITC ticks between jiffies and wall_jiffies 
+	 * elapsed ITC ticks since jiffies updated = ITC now - last_tick
+	 */
 	last_tick = (cpu_data(TIME_KEEPER_ID)->itm_next
-		     - (lost + 1)*cpu_data(TIME_KEEPER_ID)->itm_delta);
+		     - (jiffies - wall_jiffies + 1)*cpu_data(TIME_KEEPER_ID)->itm_delta);
+	
+	/*
+	 * get now after last_tick to avoid race condition where
+	 * itm_next might be updated.
+	 */
+	now = ia64_get_itc();
 
 	if (unlikely((long) (now - last_tick) < 0)) {
 		printk(KERN_ERR "CPU %d: now < last_tick (now=0x%lx,last_tick=0x%lx)!\n",