From: Mayank Rungta <mrungta@google.com>

Currently, arch_touch_nmi_watchdog() causes an early return that
skips updating hrtimer_interrupts_saved. This leads to stale
comparisons and delayed lockup detection.

I found this issue because in our system the serial console is fairly
chatty. For example, the 8250 console driver frequently calls
touch_nmi_watchdog() via console_write(). If a CPU locks up after a
timer interrupt but before next watchdog check, we see the following
sequence:

  * watchdog_hardlockup_check() saves counter (e.g., 1000)
  * Timer runs and updates the counter (1001)
  * touch_nmi_watchdog() is called
  * CPU locks up
  * 10s pass: check() notices touch, returns early, skips update
  * 10s pass: check() saves counter (1001)
  * 10s pass: check() finally detects lockup

This delays detection to 30 seconds. With this fix, we detect the
lockup in 20 seconds.

Reviewed-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Mayank Rungta <mrungta@google.com>
---
 kernel/watchdog.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index 4c5b47495745..431c540bd035 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -159,21 +159,28 @@ void watchdog_hardlockup_touch_cpu(unsigned int cpu)
 	per_cpu(watchdog_hardlockup_touched, cpu) = true;
 }
 
-static bool is_hardlockup(unsigned int cpu)
+static void watchdog_hardlockup_update(unsigned int cpu)
 {
 	int hrint = atomic_read(&per_cpu(hrtimer_interrupts, cpu));
 
-	if (per_cpu(hrtimer_interrupts_saved, cpu) == hrint)
-		return true;
-
 	/*
 	 * NOTE: we don't need any fancy atomic_t or READ_ONCE/WRITE_ONCE
 	 * for hrtimer_interrupts_saved. hrtimer_interrupts_saved is
 	 * written/read by a single CPU.
 	 */
 	per_cpu(hrtimer_interrupts_saved, cpu) = hrint;
+}
+
+static bool is_hardlockup(unsigned int cpu)
+{
+	int hrint = atomic_read(&per_cpu(hrtimer_interrupts, cpu));
+
+	if (per_cpu(hrtimer_interrupts_saved, cpu) != hrint) {
+		watchdog_hardlockup_update(cpu);
+		return false;
+	}
 
-	return false;
+	return true;
 }
 
 static void watchdog_hardlockup_kick(void)
@@ -191,6 +198,7 @@ void watchdog_hardlockup_check(unsigned int cpu, struct pt_regs *regs)
 	unsigned long flags;
 
 	if (per_cpu(watchdog_hardlockup_touched, cpu)) {
+		watchdog_hardlockup_update(cpu);
 		per_cpu(watchdog_hardlockup_touched, cpu) = false;
 		return;
 	}

-- 
2.53.0.851.ga537e3e6e9-goog

On Thu 2026-03-12 16:22:03, Mayank Rungta via B4 Relay wrote:
> From: Mayank Rungta <mrungta@google.com>
> 
> Currently, arch_touch_nmi_watchdog() causes an early return that
> skips updating hrtimer_interrupts_saved. This leads to stale
> comparisons and delayed lockup detection.
> 
> I found this issue because in our system the serial console is fairly
> chatty. For example, the 8250 console driver frequently calls
> touch_nmi_watchdog() via console_write(). If a CPU locks up after a
> timer interrupt but before next watchdog check, we see the following
> sequence:
> 
>   * watchdog_hardlockup_check() saves counter (e.g., 1000)
>   * Timer runs and updates the counter (1001)
>   * touch_nmi_watchdog() is called
>   * CPU locks up
>   * 10s pass: check() notices touch, returns early, skips update
>   * 10s pass: check() saves counter (1001)
>   * 10s pass: check() finally detects lockup
> 
> This delays detection to 30 seconds. With this fix, we detect the
> lockup in 20 seconds.
> 
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> Signed-off-by: Mayank Rungta <mrungta@google.com>

I agree with Doug's analyze and it looks good to me:

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr

Hi,

On Thu, Mar 12, 2026 at 4:22 PM Mayank Rungta via B4 Relay
<devnull+mrungta.google.com@kernel.org> wrote:
>
> From: Mayank Rungta <mrungta@google.com>
>
> Currently, arch_touch_nmi_watchdog() causes an early return that
> skips updating hrtimer_interrupts_saved. This leads to stale
> comparisons and delayed lockup detection.
>
> I found this issue because in our system the serial console is fairly
> chatty. For example, the 8250 console driver frequently calls
> touch_nmi_watchdog() via console_write(). If a CPU locks up after a
> timer interrupt but before next watchdog check, we see the following
> sequence:
>
>   * watchdog_hardlockup_check() saves counter (e.g., 1000)
>   * Timer runs and updates the counter (1001)
>   * touch_nmi_watchdog() is called
>   * CPU locks up
>   * 10s pass: check() notices touch, returns early, skips update
>   * 10s pass: check() saves counter (1001)
>   * 10s pass: check() finally detects lockup
>
> This delays detection to 30 seconds. With this fix, we detect the
> lockup in 20 seconds.
>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> Signed-off-by: Mayank Rungta <mrungta@google.com>
> ---
>  kernel/watchdog.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/watchdog.c b/kernel/watchdog.c
> index 4c5b47495745..431c540bd035 100644
> --- a/kernel/watchdog.c
> +++ b/kernel/watchdog.c
> @@ -159,21 +159,28 @@ void watchdog_hardlockup_touch_cpu(unsigned int cpu)
>         per_cpu(watchdog_hardlockup_touched, cpu) = true;
>  }
>
> -static bool is_hardlockup(unsigned int cpu)
> +static void watchdog_hardlockup_update(unsigned int cpu)
>  {
>         int hrint = atomic_read(&per_cpu(hrtimer_interrupts, cpu));
>
> -       if (per_cpu(hrtimer_interrupts_saved, cpu) == hrint)
> -               return true;
> -
>         /*
>          * NOTE: we don't need any fancy atomic_t or READ_ONCE/WRITE_ONCE
>          * for hrtimer_interrupts_saved. hrtimer_interrupts_saved is
>          * written/read by a single CPU.
>          */
>         per_cpu(hrtimer_interrupts_saved, cpu) = hrint;
> +}
> +
> +static bool is_hardlockup(unsigned int cpu)
> +{
> +       int hrint = atomic_read(&per_cpu(hrtimer_interrupts, cpu));
> +
> +       if (per_cpu(hrtimer_interrupts_saved, cpu) != hrint) {
> +               watchdog_hardlockup_update(cpu);
> +               return false;
> +       }
>
> -       return false;
> +       return true;
>  }
>
>  static void watchdog_hardlockup_kick(void)
> @@ -191,6 +198,7 @@ void watchdog_hardlockup_check(unsigned int cpu, struct pt_regs *regs)
>         unsigned long flags;
>
>         if (per_cpu(watchdog_hardlockup_touched, cpu)) {
> +               watchdog_hardlockup_update(cpu);

In the new solution, we read `hrtimer_interrupts twice instead of
once. That means that (potentially) those two reads could give us back
different values. I spent time thinking about whether this is a
problem, and I don't think it is.

The first time we read `hrtimer_interrupts`, we only care about
whether the value is the same as the saved value. If it is the same,
we won't read `hrtimer_interrupts` again anyway. If it isn't the same,
then we will read it agian. ...but that's OK. All we cared about was
whether it was the same as the (old) saved value. The second time we
read `hrtimer_interrupts` it could only have become more different (by
getting incremented again).

That's a longwinded way of saying:

Reviewed-by: Douglas Anderson <dianders@chromium.org>