Deadly bug in fsys_gettimeofday

Deadly bug in fsys_gettimeofday

[Chen, Kenneth W]

The order of reading ar.itc is incorrect with respect to all other variables
in fsys_gettimeofday.  All variables involved in time interpolation need to be
protected by the sequence lock xtime_lock for consistency check.  However, the
first time that fsys_gettimeofday take a time stamp, it read the count outside
the seq lock and leading to the following time leap into the future scenario:

fsys_gettimeofday:
	read ar.itc

		.... <went out for lunch > ....

	read_seqbegin
		read all other variables (last_nsec_offset, jiffies, xtime)
		do time interpolation - elapsed_cycles become a large negative
					      number and converting it to nsec overflows
						and make it a large positive number
	read_seqretry success
	updates last_nsec_offset (which is several seconds into the future).

All subsequent gettiemofday will be wrong as well because monotonic property
kicks in.  It only self correct until that future time actually arrives. This
bug can be trivially demonstrated. I can post a test case if anyone is interested.


Here is a patch to fix the bug. (moved one instruction to make some room).


--- 1.1/arch/ia64/kernel/fsys.S	Sun May  9 19:33:22 2004
+++ edited/arch/ia64/kernel/fsys.S	Fri Jul  9 13:08:17 2004
@@ -165,7 +165,6 @@
 	add r9=TI_FLAGS+IA64_TASK_SIZE,r16
 	addl r3=THIS_CPU(cpu_info),r0

-	mov.m r31=ar.itc		// put time stamp into r31 (ITC) = now		(35 cyc)
 #ifdef CONFIG_SMP
 	movl r10=__per_cpu_offset
 	movl r2=sal_platform_features
@@ -240,12 +239,13 @@
 	;;

 	ldf8 f8=[r21]			// f8 now contains itm_next
+	mov.m r31=ar.itc		// put time stamp into r31 (ITC) = now
 	sub r28=r29, r28, 1		// r28 now contains "-(lost + 1)"
-	tbit.nz p9, p10=r23, 0		// p9 <- is_odd(r23), p10 <- is_even(r23)
 	;;

 	ld8 r2=[r19]			// r2 = sec = xtime.tv_sec
 	ld8 r29=[r20]			// r29 = nsec = xtime.tv_nsec
+	tbit.nz p9, p10=r23, 0		// p9 <- is_odd(r23), p10 <- is_even(r23)

 	setf.sig f6=r28			// f6 <- -(lost + 1)				(6 cyc)
 	;;
@@ -260,7 +260,6 @@
 	nop 0
 	;;

-	mov r31=ar.itc			// re-read ITC in case we .retry		(35 cyc)
 	xma.l f8ñ1, f8, f12	// f8 (elapsed_cycles) <- (-1*last_tick + now) = (now - last_tick)
 	nop 0
 	;;

RE: Deadly bug in fsys_gettimeofday

[Chen, Kenneth W]

Chen, Kenneth W wrote on Friday, July 09, 2004 2:03 PM
>
> The order of reading ar.itc is incorrect with respect to all other variables
> in fsys_gettimeofday.  All variables involved in time interpolation need to be
> protected by the sequence lock xtime_lock for consistency check.  However, the
> first time that fsys_gettimeofday take a time stamp, it read the count outside
> the seq lock and leading to the following time leap into the future scenario:

Sorry, keep on forgetting the signed off trailer.

Signed-off-by: Ken Chen <kenneth.w.chen@intel.com>
Signed-off-by: Asit Mallick <asit.k.mallick@intel.com>