Skip to content

Commit bd94d86

Browse files
committed
x86/tsc: Defer marking TSC unstable to a worker
Tetsuo reported the following lockdep splat when the TSC synchronization fails during CPU hotplug: tsc: Marking TSC unstable due to check_tsc_sync_source failed WARNING: inconsistent lock state inconsistent {IN-HARDIRQ-W} -> {HARDIRQ-ON-W} usage. ffffffff8cfa1c78 (watchdog_lock){?.-.}-{2:2}, at: clocksource_watchdog+0x23/0x5a0 {IN-HARDIRQ-W} state was registered at: _raw_spin_lock_irqsave+0x3f/0x60 clocksource_mark_unstable+0x1b/0x90 mark_tsc_unstable+0x41/0x50 check_tsc_sync_source+0x14f/0x180 sysvec_call_function_single+0x69/0x90 Possible unsafe locking scenario: lock(watchdog_lock); <Interrupt> lock(watchdog_lock); stack backtrace: _raw_spin_lock+0x30/0x40 clocksource_watchdog+0x23/0x5a0 run_timer_softirq+0x2a/0x50 sysvec_apic_timer_interrupt+0x6e/0x90 The reason is the recent conversion of the TSC synchronization function during CPU hotplug on the control CPU to a SMP function call. In case that the synchronization with the upcoming CPU fails, the TSC has to be marked unstable via clocksource_mark_unstable(). clocksource_mark_unstable() acquires 'watchdog_lock', but that lock is taken with interrupts enabled in the watchdog timer callback to minimize interrupt disabled time. That's obviously a possible deadlock scenario, Before that change the synchronization function was invoked in thread context so this could not happen. As it is not crucical whether the unstable marking happens slightly delayed, defer the call to a worker thread which avoids the lock context problem. Fixes: 9d349d4 ("x86/smpboot: Make TSC synchronization function call based") Reported-by: Tetsuo Handa <[email protected]> Signed-off-by: Thomas Gleixner <[email protected]> Tested-by: Tetsuo Handa <[email protected]> Cc: [email protected] Link: https://lore.kernel.org/r/87zg064ceg.ffs@tglx
1 parent 128b0c9 commit bd94d86

File tree

1 file changed

+9
-1
lines changed

1 file changed

+9
-1
lines changed

arch/x86/kernel/tsc_sync.c

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
* ( The serial nature of the boot logic and the CPU hotplug lock
1616
* protects against more than 2 CPUs entering this code. )
1717
*/
18+
#include <linux/workqueue.h>
1819
#include <linux/topology.h>
1920
#include <linux/spinlock.h>
2021
#include <linux/kernel.h>
@@ -342,6 +343,13 @@ static inline unsigned int loop_timeout(int cpu)
342343
return (cpumask_weight(topology_core_cpumask(cpu)) > 1) ? 2 : 20;
343344
}
344345

346+
static void tsc_sync_mark_tsc_unstable(struct work_struct *work)
347+
{
348+
mark_tsc_unstable("check_tsc_sync_source failed");
349+
}
350+
351+
static DECLARE_WORK(tsc_sync_work, tsc_sync_mark_tsc_unstable);
352+
345353
/*
346354
* The freshly booted CPU initiates this via an async SMP function call.
347355
*/
@@ -395,7 +403,7 @@ static void check_tsc_sync_source(void *__cpu)
395403
"turning off TSC clock.\n", max_warp);
396404
if (random_warps)
397405
pr_warn("TSC warped randomly between CPUs\n");
398-
mark_tsc_unstable("check_tsc_sync_source failed");
406+
schedule_work(&tsc_sync_work);
399407
}
400408

401409
/*

0 commit comments

Comments
 (0)