[PATCH] x86/tsc: Fix incorrect enabling of __use_tsc static_key

* [PATCH] x86/tsc: Fix incorrect enabling of __use_tsc static_key
@ 2019-12-03 20:40 Waiman Long
  2019-12-03 20:54 ` Pavel Tatashin
  0 siblings, 1 reply; 3+ messages in thread
From: Waiman Long @ 2019-12-03 20:40 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin, x86
  Cc: linux-kernel, Peter Zijlstra, Juri Lelli, Pavel Tatashin,
	Dou Liyang, Waiman Long

After applying the commit 4763f03d3d18 ("x86/tsc: Use TSC as sched clock
early") and the commit 608008a45798 ("x86/tsc: Consolidate init code"),
some x86 systems boot up with the following warnings:

[    0.000000] tsc: Fast TSC calibration using PIT
[    0.000000] tsc: Detected 2599.853 MHz processor
[    0.000000] ------------[ cut here ]------------
[    0.000000] static_key_enable_cpuslocked(): static key
'__use_tsc+0x0/0x10' used before call to jump_label_init()
[    0.000000] WARNING: CPU: 0 PID: 0 at kernel/jump_label.c:132 static_key_enable_cpuslocked+0x7b/0x80
[    0.000000] Modules linked in:
[    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.18.0-154.el8.x86_64 #1
[    0.000000] Hardware name: Dell Inc. PowerEdge R730/072T6D, BIOS 2.4.3 01/17/2017
[    0.000000] RIP: 0010:static_key_enable_cpuslocked+0x7b/0x80
  :
[    0.000000] Call Trace:
[    0.000000]  ? static_key_enable+0x16/0x20
[    0.000000]  ? setup_arch+0x43f/0xf68
[    0.000000]  ? printk+0x58/0x6f
[    0.000000]  ? start_kernel+0x63/0x55b
[    0.000000]  ? load_ucode_bsp+0xfb/0x12e
[    0.000000]  ? secondary_startup_64+0xb7/0xc0
[    0.000000] ---[ end trace fc2166797a50a8e0 ]---
  :
[ 1781.404905] INFO: NMI handler (nmi_cpu_backtrace_handler) took too long to run: 1.000 msecs
[ 1781.409905] INFO: NMI handler (nmi_cpu_backtrace_handler) took too long to run: 1.000 msecs
[ 1781.412905] INFO: NMI handler (nmi_cpu_backtrace_handler) took too long to run: 1.000 msecs
[ 1781.578905] INFO: NMI handler (nmi_cpu_backtrace_handler) took too long to run: 1.000 msecs
[ 1781.973905] INFO: NMI handler (nmi_cpu_backtrace_handler) took too long to run: 1.000 msecs
  :

In this particular case,

  setup_arch() => tsc_early_init()
               => tsc_enable_sched_clock()
               => static_branch_enable()

However, jump_label_init() is called after setup_arch(). Before the
2 commits listed above, static_branch_enable() was only called in
tsc_init() which is after jump_label_init().

Since sched_clock() is used by the NMI handler to compute the elapsed
time and a 1000 HZ jiffies has a granularity of 1ms, the NMI warning
will be printed whenever the jiffies changes while in the middle of the
NMI handler. On systems that generate GHES NMI frequently, the constant
spewing of warning messages will significantly impact the performance
and usability of those systems.

To fix this problem, the static_branch_enable() is now taken out of
tsc_enable_sched_clock() and put back into tsc_init() like before. That
effective reverts commit 4763f03d3d18 ("x86/tsc: Use TSC as sched
clock early").

Fixes: 4763f03d3d18 ("x86/tsc: Use TSC as sched clock early")
Fixes: 608008a45798 ("x86/tsc: Consolidate init code")
Signed-off-by: Waiman Long <longman@redhat.com>
---
 arch/x86/kernel/tsc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 7e322e2daaf5..1e3a72040399 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -1444,12 +1444,11 @@ static unsigned long __init get_loops_per_jiffy(void)
 	return lpj;
 }
 
-static void __init tsc_enable_sched_clock(void)
+static void __init tsc_init_sched_clock(void)
 {
 	/* Sanitize TSC ADJUST before cyc2ns gets initialized */
 	tsc_store_and_check_tsc_adjust(true);
 	cyc2ns_init_boot_cpu();
-	static_branch_enable(&__use_tsc);
 }
 
 void __init tsc_early_init(void)
@@ -1463,7 +1462,7 @@ void __init tsc_early_init(void)
 		return;
 	loops_per_jiffy = get_loops_per_jiffy();
 
-	tsc_enable_sched_clock();
+	tsc_init_sched_clock();
 }
 
 void __init tsc_init(void)
@@ -1487,10 +1486,11 @@ void __init tsc_init(void)
 			setup_clear_cpu_cap(X86_FEATURE_TSC_DEADLINE_TIMER);
 			return;
 		}
-		tsc_enable_sched_clock();
+		tsc_init_sched_clock();
 	}
 
 	cyc2ns_init_secondary_cpus();
+	static_branch_enable(&__use_tsc);
 
 	if (!no_sched_irq_time)
 		enable_sched_clock_irqtime();
-- 
2.18.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread