mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] clocksource: Use the clocksource under test for the skew check timeout
@ 2026-11-02 21:45 Mani Tofigh via B4 Relay
  2026-11-03 20:20 ` Thomas Gleixner
  0 siblings, 1 reply; 3+ messages in thread
From: Mani Tofigh via B4 Relay @ 2026-11-02 21:45 UTC (permalink / raw)
  To: Thomas Gleixner, John Stultz
  Cc: Stephen Boyd, Miroslav Lichvar, Srikanth Aithal, Borislav Petkov,
	K Prateek Nayak, linux-kernel, tz2294, stable, Mani Tofigh

From: Mani Tofigh <mt4022@columbia.edu>

watchdog_check_skew() waits for the remote CPU with interrupts disabled
and gives up after a timeout measured with ktime_get_raw_fast_ns().
While the clocksource is still jiffies during boot (or with
clocksource=jiffies) that timeout can't expire, because only the boot
CPU's tick advances jiffies and that's the CPU spinning with interrupts
disabled.

The boot then hangs when the remote CPU can't answer, eg in the
stop_machine() of the switch away from jiffies:

  CPU0                                  CPU1
  multi_cpu_stop()                      multi_cpu_stop()
                                          local_irq_disable()
                                          /* waits for CPU0 */
    <timer interrupt>
      clocksource_watchdog()
        /* sends CSD to CPU1 */
        watchdog_check_skew()
          /* waits for CPU1 forever */

Nothing gets printed.  With the hard lockup detector on in a KVM guest:

  [    1.554193] watchdog: CPU0: Watchdog detected hard LOCKUP on cpu 0
  [    1.554193] CPU: 0 UID: 0 PID: 18 Comm: migration/0 Not tainted 7.3.0-17087-gd865f2677c44 #1 PREEMPT
  [    1.554193] Stopper: multi_cpu_stop+0x0/0x310 <- __stop_cpus.constprop.0+0xdd/0x150
  [    1.554193] RIP: 0010:watchdog_check_skew+0x12d/0x5a0
  [    1.554193] Call Trace:
  [    1.554193]  <IRQ>
  [    1.554193]  clocksource_watchdog+0x761/0xc90
  [    1.554193]  call_timer_fn+0x154/0x470
  [    1.554193]  __run_timer_base.part.0+0x5d0/0x910
  [    1.554193]  run_timer_softirq+0xc2/0x280
  [    1.554193]  handle_softirqs+0x1f9/0x840
  [    1.554193]  __irq_exit_rcu+0x157/0x210
  [    1.554193]  irq_exit_rcu+0x9/0x20
  [    1.554193]  sysvec_apic_timer_interrupt+0x6b/0x80
  [    1.554193]  </IRQ>
  [    1.554193]  <TASK>
  [    1.554193]  asm_sysvec_apic_timer_interrupt+0x1a/0x20
  [    1.554193] RIP: 0010:multi_cpu_stop+0xf7/0x310
  ...
  [    1.554193]  cpu_stopper_thread+0x1fa/0x3f0
  [    1.554193]  smpboot_thread_fn+0x2fb/0x8a0
  [    1.554193]  kthread+0x300/0x3e0
  [    1.554193]  ret_from_fork+0x4f0/0x6f0
  [    1.554193]  ret_from_fork_asm+0x1a/0x30
  [    1.554193]  </TASK>

Measure the timeout with the clocksource under test instead, which
keeps counting with interrupts disabled.  With the fix, the watchdog
gives up after the timeout and the boot goes on.  A 2-vCPU guest on one
host core hung in 4 of 150 boots without it, and it didn't hang at all
in 400 tests with the fix applied.

This bug was identified and confirmed with the help of a deterministic
hypervisor currently under development.  The C reproducer below however
hits it within about 80s on a kernel booted with clocksource=jiffies.

Fixes: 763aacf86f1b ("clocksource: Rewrite watchdog code completely")
Cc: stable@vger.kernel.org
Signed-off-by: Mani Tofigh <mt4022@columbia.edu>
---
Srikanth reported silent early boot hangs of large KVM guests that he
bisected to the same commit [1].  I think it's the same bug but it was
never confirmed.  Srikanth, would you mind giving this a try on your
setup?

[1] https://lore.kernel.org/all/e4a41d43-4d60-4132-aa2d-70e038a3e9dd@amd.com/

Reproducer: https://github.com/manitofigh/files/blob/main/linux-repros/wdboot.c
---
 kernel/time/clocksource.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
index 58c0ca11cdc5..fdce67f7c3d1 100644
--- a/kernel/time/clocksource.c
+++ b/kernel/time/clocksource.c
@@ -305,11 +305,16 @@ static bool watchdog_wait_seq(struct watchdog_cpu_data *wd, u64 start, int seq)
 		if (READ_ONCE(wd->result) != WD_SUCCESS)
 			return false;
 
-		/* Prevent endless loops if the other CPU does not react. */
+		/*
+		 * Prevent endless loops if the other CPU does not react.  The
+		 * timeout uses the clocksource under test because the timekeeping
+		 * clocksource can be jiffies, which doesn't advance while this CPU
+		 * waits with interrupts disabled.
+		 */
 		if (cnt == 5000) {
-			u64 nsecs = ktime_get_raw_fast_ns();
+			struct clocksource *cs = wd->cs;
 
-			if (nsecs - start >=wd->timeout_ns) {
+			if (cycles_to_nsec_safe(cs, start, cs->read(cs)) >= wd->timeout_ns) {
 				watchdog_set_result(wd, WD_CPU_TIMEOUT);
 				return false;
 			}
@@ -322,12 +327,13 @@ static bool watchdog_wait_seq(struct watchdog_cpu_data *wd, u64 start, int seq)
 
 static void watchdog_check_skew(struct watchdog_cpu_data *wd, int index)
 {
-	u64 prev, now, delta, start = ktime_get_raw_fast_ns();
+	u64 prev, now, delta, start;
 	int local = index, remote = (index + 1) & 0x1;
 	struct clocksource *cs = wd->cs;
 
 	/* Set the local timestamp so that the first iteration works correctly */
-	wd->cpu_ts[local] = cs->read(cs);
+	start = cs->read(cs);
+	wd->cpu_ts[local] = start;
 
 	/* Signal arrival */
 	atomic_inc(&wd->seq);

---
base-commit: f322d7f7ae56bce728ba03a387e35380bc4ad939
change-id: 20261102-clocksource-skew-timeout-f2a9e5fb3f75

Best regards,
--  
Mani Tofigh <mt4022@columbia.edu>



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

* Re: [PATCH] clocksource: Use the clocksource under test for the skew check timeout
  2026-11-02 21:45 [PATCH] clocksource: Use the clocksource under test for the skew check timeout Mani Tofigh via B4 Relay
@ 2026-11-03 20:20 ` Thomas Gleixner
  2026-11-04 10:38   ` Aithal, Srikanth
  0 siblings, 1 reply; 3+ messages in thread
From: Thomas Gleixner @ 2026-11-03 20:20 UTC (permalink / raw)
  To: Mani Tofigh via B4 Relay, John Stultz
  Cc: Stephen Boyd, Miroslav Lichvar, Srikanth Aithal, Borislav Petkov,
	K Prateek Nayak, linux-kernel, tz2294, stable, Mani Tofigh

On Mon, Nov 02 2026 at 16:45, Mani Tofigh via wrote:
> watchdog_check_skew() waits for the remote CPU with interrupts disabled
> and gives up after a timeout measured with ktime_get_raw_fast_ns().
> While the clocksource is still jiffies during boot (or with
> clocksource=jiffies) that timeout can't expire, because only the boot
> CPU's tick advances jiffies and that's the CPU spinning with interrupts
> disabled.
>
> The boot then hangs when the remote CPU can't answer, eg in the
> stop_machine() of the switch away from jiffies:
>
>   CPU0                                  CPU1
>   multi_cpu_stop()                      multi_cpu_stop()
>                                           local_irq_disable()
>                                           /* waits for CPU0 */
>     <timer interrupt>
>       clocksource_watchdog()
>         /* sends CSD to CPU1 */
>         watchdog_check_skew()
>           /* waits for CPU1 forever */
>
> Nothing gets printed.  With the hard lockup detector on in a KVM guest:

The above explanation is enough. No need for this backtrace.

> Measure the timeout with the clocksource under test instead, which
> keeps counting with interrupts disabled.  With the fix, the watchdog
> gives up after the timeout and the boot goes on.  A 2-vCPU guest on one
> host core hung in 4 of 150 boots without it, and it didn't hang at all
> in 400 tests with the fix applied.
>
> This bug was identified and confirmed with the help of a deterministic
> hypervisor currently under development.  The C reproducer below however
> hits it within about 80s on a kernel booted with clocksource=jiffies.
>
> Fixes: 763aacf86f1b ("clocksource: Rewrite watchdog code completely")
> Cc: stable@vger.kernel.org
> Signed-off-by: Mani Tofigh <mt4022@columbia.edu>
> ---
> Srikanth reported silent early boot hangs of large KVM guests that he
> bisected to the same commit [1].  I think it's the same bug but it was
> never confirmed.  Srikanth, would you mind giving this a try on your
> setup?

Yes. I gave him a similar patch which simply uses sched_clock()
instead. That solves the problem, but I was reluctant to post it due to
the lack of explanation why this ends up in a situation which does not
allow to make progress.

So your reproducer does:

  while true; do echo 0 >/sys/devices/system/cpu/cpu1/online; echo 1 >/sys/devices/system/cpu/cpu1/online; done

right?

Unfortunately it does not reproduce here within a reasonable amount of
time (still going strong after one hour...).

But yes, your explanation makes sense and triggering it with the hotplug
code is obviously a matter of probability.

Though one thing I thought about before looking at your mail while
reasoning over the possible causes of Srikanth's report and the possible
consequences is that such a timeout might in the worst case result in a
stale CPU state which is silently ignored. The consequence is that the
watchdog wont run on a CPU with such state ever again.

So something like the below delta patch on top of your fix (or the
sched_clock()) variant might be appropriate.

Can you and Srikanth please run that on top to validate that we don't
end up with that?

Thanks,

        tglx
---
--- a/kernel/time/clocksource.c
+++ b/kernel/time/clocksource.c
@@ -250,6 +250,7 @@ enum wd_result {
 	WD_FREQ_RESET,
 	WD_FREQ_SKEWED,
 	WD_CPU_TIMEOUT,
+	WD_CPU_STALLED,
 	WD_CPU_SKEWED,
 };
 
@@ -398,7 +399,7 @@ static void __watchdog_check_cpu_skew(st
 
 	wd = per_cpu_ptr(&watchdog_cpu_data, cpu);
 	if (atomic_read(&wd->remote_inprogress) || wd_csd_locked(wd)) {
-		watchdog_data.result = WD_CPU_TIMEOUT;
+		watchdog_data.result = WD_CPU_STALLED;
 		return;
 	}
 
@@ -590,11 +591,16 @@ static void watchdog_print_freq_skew(str
 	pr_warn("Clocksource %20s interval: %16lluns\n", cs->name, watchdog_data.cs_delta);
 }
 
-static void watchdog_handle_remote_timeout(struct clocksource *cs)
+static void watchdog_print_remote_timeout(struct clocksource *cs)
 {
 	pr_info_once("Watchdog remote CPU %u read timed out\n", watchdog_data.curr_cpu);
 }
 
+static void watchdog_print_remote_stall(struct clocksource *cs)
+{
+	pr_warn("Watchdog remote CPU %u stalled\n", watchdog_data.curr_cpu);
+}
+
 static void watchdog_print_remote_skew(struct clocksource *cs)
 {
 	pr_warn("Marking clocksource %s unstable due to inter CPU skew\n", cs->name);
@@ -635,9 +641,13 @@ static void watchdog_check_result(struct
 
 	case WD_CPU_TIMEOUT:
 		/* Remote check timed out. Try again next cycle. */
-		watchdog_handle_remote_timeout(cs);
+		watchdog_print_remote_timeout(cs);
 		return;
 
+	case WD_CPU_STALLED:
+		watchdog_print_remote_stall(cs);
+		break;
+
 	case WD_CPU_SKEWED:
 		watchdog_print_remote_skew(cs);
 		break;


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

* Re: [PATCH] clocksource: Use the clocksource under test for the skew check timeout
  2026-11-03 20:20 ` Thomas Gleixner
@ 2026-11-04 10:38   ` Aithal, Srikanth
  0 siblings, 0 replies; 3+ messages in thread
From: Aithal, Srikanth @ 2026-11-04 10:38 UTC (permalink / raw)
  To: Thomas Gleixner, Mani Tofigh via B4 Relay, John Stultz
  Cc: Stephen Boyd, Miroslav Lichvar, Borislav Petkov, K Prateek Nayak,
	linux-kernel, tz2294, stable, Mani Tofigh


On 11/4/2026 1:50 AM, Thomas Gleixner wrote:
> On Mon, Nov 02 2026 at 16:45, Mani Tofigh via wrote:
>> watchdog_check_skew() waits for the remote CPU with interrupts disabled
>> and gives up after a timeout measured with ktime_get_raw_fast_ns().
>> While the clocksource is still jiffies during boot (or with
>> clocksource=jiffies) that timeout can't expire, because only the boot
>> CPU's tick advances jiffies and that's the CPU spinning with interrupts
>> disabled.
>>
>> The boot then hangs when the remote CPU can't answer, eg in the
>> stop_machine() of the switch away from jiffies:
>>
>>    CPU0                                  CPU1
>>    multi_cpu_stop()                      multi_cpu_stop()
>>                                            local_irq_disable()
>>                                            /* waits for CPU0 */
>>      <timer interrupt>
>>        clocksource_watchdog()
>>          /* sends CSD to CPU1 */
>>          watchdog_check_skew()
>>            /* waits for CPU1 forever */
>>
>> Nothing gets printed.  With the hard lockup detector on in a KVM guest:
> The above explanation is enough. No need for this backtrace.
>
>> Measure the timeout with the clocksource under test instead, which
>> keeps counting with interrupts disabled.  With the fix, the watchdog
>> gives up after the timeout and the boot goes on.  A 2-vCPU guest on one
>> host core hung in 4 of 150 boots without it, and it didn't hang at all
>> in 400 tests with the fix applied.
>>
>> This bug was identified and confirmed with the help of a deterministic
>> hypervisor currently under development.  The C reproducer below however
>> hits it within about 80s on a kernel booted with clocksource=jiffies.
>>
>> Fixes: 763aacf86f1b ("clocksource: Rewrite watchdog code completely")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Mani Tofigh <mt4022@columbia.edu>
>> ---
>> Srikanth reported silent early boot hangs of large KVM guests that he
>> bisected to the same commit [1].  I think it's the same bug but it was
>> never confirmed.  Srikanth, would you mind giving this a try on your
>> setup?
> Yes. I gave him a similar patch which simply uses sched_clock()
> instead. That solves the problem, but I was reluctant to post it due to
> the lack of explanation why this ends up in a situation which does not
> allow to make progress.
>
> So your reproducer does:
>
>    while true; do echo 0 >/sys/devices/system/cpu/cpu1/online; echo 1 >/sys/devices/system/cpu/cpu1/online; done
>
> right?
>
> Unfortunately it does not reproduce here within a reasonable amount of
> time (still going strong after one hour...).
>
> But yes, your explanation makes sense and triggering it with the hotplug
> code is obviously a matter of probability.
>
> Though one thing I thought about before looking at your mail while
> reasoning over the possible causes of Srikanth's report and the possible
> consequences is that such a timeout might in the worst case result in a
> stale CPU state which is silently ignored. The consequence is that the
> watchdog wont run on a CPU with such state ever again.
>
> So something like the below delta patch on top of your fix (or the
> sched_clock()) variant might be appropriate.
>
> Can you and Srikanth please run that on top to validate that we don't
> end up with that?
>
> Thanks,
>
>          tglx
> ---
> --- a/kernel/time/clocksource.c
> +++ b/kernel/time/clocksource.c
> @@ -250,6 +250,7 @@ enum wd_result {
>   	WD_FREQ_RESET,
>   	WD_FREQ_SKEWED,
>   	WD_CPU_TIMEOUT,
> +	WD_CPU_STALLED,
>   	WD_CPU_SKEWED,
>   };
>   
> @@ -398,7 +399,7 @@ static void __watchdog_check_cpu_skew(st
>   
>   	wd = per_cpu_ptr(&watchdog_cpu_data, cpu);
>   	if (atomic_read(&wd->remote_inprogress) || wd_csd_locked(wd)) {
> -		watchdog_data.result = WD_CPU_TIMEOUT;
> +		watchdog_data.result = WD_CPU_STALLED;
>   		return;
>   	}
>   
> @@ -590,11 +591,16 @@ static void watchdog_print_freq_skew(str
>   	pr_warn("Clocksource %20s interval: %16lluns\n", cs->name, watchdog_data.cs_delta);
>   }
>   
> -static void watchdog_handle_remote_timeout(struct clocksource *cs)
> +static void watchdog_print_remote_timeout(struct clocksource *cs)
>   {
>   	pr_info_once("Watchdog remote CPU %u read timed out\n", watchdog_data.curr_cpu);
>   }
>   
> +static void watchdog_print_remote_stall(struct clocksource *cs)
> +{
> +	pr_warn("Watchdog remote CPU %u stalled\n", watchdog_data.curr_cpu);
> +}
> +
>   static void watchdog_print_remote_skew(struct clocksource *cs)
>   {
>   	pr_warn("Marking clocksource %s unstable due to inter CPU skew\n", cs->name);
> @@ -635,9 +641,13 @@ static void watchdog_check_result(struct
>   
>   	case WD_CPU_TIMEOUT:
>   		/* Remote check timed out. Try again next cycle. */
> -		watchdog_handle_remote_timeout(cs);
> +		watchdog_print_remote_timeout(cs);
>   		return;
>   
> +	case WD_CPU_STALLED:
> +		watchdog_print_remote_stall(cs);
> +		break;
> +
>   	case WD_CPU_SKEWED:
>   		watchdog_print_remote_skew(cs);
>   		break;


Tested on 6628e7d4c6b25 / v7.4-rc1:

1. Mani's skew-timeout change + your WD_CPU_STALLED delta
2. Mani's patch alone (without the STALLED delta)
Both passed on my large-KVM recreation setup (5× smp=255 SVM guests, 
~2500 reboot stress).

Without these patches, the issue is still recreatable on 6628e7d4c6b25 / 
v7.4-rc1.



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

end of thread, other threads:[~2026-11-04 10:38 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-02 21:45 [PATCH] clocksource: Use the clocksource under test for the skew check timeout Mani Tofigh via B4 Relay
2026-11-03 20:20 ` Thomas Gleixner
2026-11-04 10:38   ` Aithal, Srikanth

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®