[Patch 1/2] backport of x86: add rdtsc barrier to TSC sync check

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This changeset is mainly a backport of upstream commit
93ce99e849433ede4ce8b410b749dc0cad1100b2 and its required
infrastructure.

| commit 93ce99e849433ede4ce8b410b749dc0cad1100b2
| Author: Venki Pallipadi <venkatesh.pallipadi@xxxxxxxxx>
| Date:   Mon Nov 17 14:43:58 2008 -0800
| 
|     x86: add rdtsc barrier to TSC sync check
|     
|     Impact: fix incorrectly marked unstable TSC clock
|     
|     Patch (commit 0d12cdd "sched: improve sched_clock() performance") has
|     a regression on one of the test systems here.
|     
|     With the patch, I see:
|     
|      checking TSC synchronization [CPU#0 -> CPU#1]:
|      Measured 28 cycles TSC warp between CPUs, turning off TSC clock.
|      Marking TSC unstable due to check_tsc_sync_source failed
|     
|     Whereas, without the patch syncs pass fine on all CPUs:
|     
|      checking TSC synchronization [CPU#0 -> CPU#1]: passed.
|     
|     Due to this, TSC is marked unstable, when it is not actually unstable.
|     This is because syncs in check_tsc_wrap() goes away due to this commit.
|     
|     As per the discussion on this thread, correct way to fix this is to add
|     explicit syncs as below?
|     
|     Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi@xxxxxxxxx>
|     Signed-off-by: Ingo Molnar <mingo@xxxxxxx>

Signed-off-by: Luis Claudio R. Goncalves <lgoncalv@xxxxxxxxxx>

---
diff --git a/arch/x86/kernel/tsc_sync.c b/arch/x86/kernel/tsc_sync.c
index 4f08c7b..f5fd23c 100644
--- a/arch/x86/kernel/tsc_sync.c
+++ b/arch/x86/kernel/tsc_sync.c
@@ -46,7 +46,9 @@ static __cpuinit void check_tsc_warp(void)
 	cycles_t start, now, prev, end;
 	int i;
 
-	start = get_cycles_sync();
+	rdtsc_barrier();
+	start = get_cycles();
+	rdtsc_barrier();
 	/*
 	 * The measurement runs for 20 msecs:
 	 */
@@ -61,18 +63,20 @@ static __cpuinit void check_tsc_warp(void)
 		 */
 		__raw_spin_lock(&sync_lock);
 		prev = last_tsc;
-		now = get_cycles_sync();
+		rdtsc_barrier();
+		now = get_cycles();
+		rdtsc_barrier();
 		last_tsc = now;
 		__raw_spin_unlock(&sync_lock);
 
 		/*
 		 * Be nice every now and then (and also check whether
-		 * measurement is done [we also insert a 100 million
+		 * measurement is done [we also insert a 10 million
 		 * loops safety exit, so we dont lock up in case the
 		 * TSC readout is totally broken]):
 		 */
 		if (unlikely(!(i & 7))) {
-			if (now > end || i > 100000000)
+			if (now > end || i > 10000000)
 				break;
 			cpu_relax();
 			touch_nmi_watchdog();
@@ -87,8 +91,10 @@ static __cpuinit void check_tsc_warp(void)
 			nr_warps++;
 			__raw_spin_unlock(&sync_lock);
 		}
-
 	}
+	if (!(now-start))
+		printk(KERN_WARNING "Warning: zero tsc calibration delta:"
+			" %lld [max: %lld]\n", now-start, end-start);
 }
 
 /*
@@ -97,7 +103,6 @@ static __cpuinit void check_tsc_warp(void)
  */
 void __cpuinit check_tsc_sync_source(int cpu)
 {
-	unsigned long flags;
 	int cpus = 2;
 
 	/*
@@ -118,11 +123,8 @@ void __cpuinit check_tsc_sync_source(int cpu)
 	/*
 	 * Wait for the target to arrive:
 	 */
-	local_save_flags(flags);
-	local_irq_enable();
 	while (atomic_read(&start_count) != cpus-1)
 		cpu_relax();
-	local_irq_restore(flags);
 	/*
 	 * Trigger the target to continue into the measurement too:
 	 */
@@ -133,24 +135,24 @@ void __cpuinit check_tsc_sync_source(int cpu)
 	while (atomic_read(&stop_count) != cpus-1)
 		cpu_relax();
 
-	/*
-	 * Reset it - just in case we boot another CPU later:
-	 */
-	atomic_set(&start_count, 0);
-
 	if (nr_warps) {
 		printk("\n");
 		printk(KERN_WARNING "Measured %Ld cycles TSC warp between CPUs,"
 				    " turning off TSC clock.\n", max_warp);
 		mark_tsc_unstable("check_tsc_sync_source failed");
-		nr_warps = 0;
-		max_warp = 0;
-		last_tsc = 0;
 	} else {
 		printk(" passed.\n");
 	}
 
 	/*
+	 * Reset it - just in case we boot another CPU later:
+	 */
+	atomic_set(&start_count, 0);
+	nr_warps = 0;
+	max_warp = 0;
+	last_tsc = 0;
+
+	/*
 	 * Let the target continue with the bootup:
 	 */
 	atomic_inc(&stop_count);
diff --git a/include/asm-x86/cpufeature_32.h b/include/asm-x86/cpufeature_32.h
index f17e688..49f14b4 100644
--- a/include/asm-x86/cpufeature_32.h
+++ b/include/asm-x86/cpufeature_32.h
@@ -82,6 +82,8 @@
 /* 14 free */
 #define X86_FEATURE_SYNC_RDTSC	(3*32+15)  /* RDTSC synchronizes the CPU */
 #define X86_FEATURE_REP_GOOD   (3*32+16) /* rep microcode works well on this CPU */
+#define X86_FEATURE_MFENCE_RDTSC (3*32+17) /* "" Mfence synchronizes RDTSC */
+#define X86_FEATURE_LFENCE_RDTSC (3*32+18) /* "" Lfence synchronizes RDTSC */
 
 /* Intel-defined CPU features, CPUID level 0x00000001 (ecx), word 4 */
 #define X86_FEATURE_XMM3	(4*32+ 0) /* Streaming SIMD Extensions-3 */
diff --git a/include/asm-x86/processor.h b/include/asm-x86/processor.h
index 46e1c04..e1dc8fc 100644
--- a/include/asm-x86/processor.h
+++ b/include/asm-x86/processor.h
@@ -1,5 +1,23 @@
+#ifndef __ASM_PROCESSOR_H
+#define __ASM_PROCESSOR_H
+
 #ifdef CONFIG_X86_32
 # include "processor_32.h"
 #else
 # include "processor_64.h"
 #endif
+
+/*
+ * Stop RDTSC speculation. This is needed when you need to use RDTSC
+ * (or get_cycles or vread that possibly accesses the TSC) in a defined
+ * code region.
+ *
+ * (Could use an alternative three way for this if there was one.)
+ */
+static inline void rdtsc_barrier(void)
+{
+	alternative(ASM_NOP3, "mfence", X86_FEATURE_MFENCE_RDTSC);
+	alternative(ASM_NOP3, "lfence", X86_FEATURE_LFENCE_RDTSC);
+}
+
+#endif /* __ASM_PROCESSOR_H */
-- 
[ Luis Claudio R. Goncalves             Red Hat  -  Realtime Team ]
[ Fingerprint: 4FDD B8C4 3C59 34BD 8BE9  2696 7203 D980 A448 C8F8 ]

--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [RT Stable]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]

  Powered by Linux