All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Luis Claudio R. Goncalves" <lclaudio@uudg.org>
To: linux-rt-users@vger.kernel.org
Cc: Clark Williams <williams@redhat.com>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: [Patch 1/2] backport of x86: add rdtsc barrier to TSC sync check
Date: Tue, 7 Apr 2009 11:33:54 -0300	[thread overview]
Message-ID: <20090407143354.GD1833@unix.sh> (raw)
In-Reply-To: <20090407143141.GC1833@unix.sh>

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

| commit 93ce99e849433ede4ce8b410b749dc0cad1100b2
| Author: Venki Pallipadi <venkatesh.pallipadi@intel.com>
| 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@intel.com>
|     Signed-off-by: Ingo Molnar <mingo@elte.hu>

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

---
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 ]


  reply	other threads:[~2009-04-07 14:34 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-04-07 14:31 [Patch 0/2] TSC: fix incorrectly marked unstable TSC clock Luis Claudio R. Goncalves
2009-04-07 14:33 ` Luis Claudio R. Goncalves [this message]
2009-04-07 14:36   ` [Patch 2/2] TSC: several enhancements on callers of mark_tsc_unstable() Luis Claudio R. Goncalves

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090407143354.GD1833@unix.sh \
    --to=lclaudio@uudg.org \
    --cc=linux-rt-users@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=williams@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.