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.
Showing the changes to kernel (1 of 1 file). Show all
Unified · Split
| @@ -159,21 +159,28 @@ void watchdog_hardlockup_touch_cpu(unsigned int cpu) · 158 unmodified lines | |||
| 159 | per_cpu(watchdog_hardlockup_touched, cpu) = true; | 159 | per_cpu(watchdog_hardlockup_touched, cpu) = true; |
| 160 | } | 160 | } |
| 161 | 161 | ||
| 162 | static bool is_hardlockup(unsigned int cpu) | 162 | static void watchdog_hardlockup_update(unsigned int cpu) |
| 163 | { | 163 | { |
| 164 | int hrint = atomic_read(&per_cpu(hrtimer_interrupts, cpu)); | 164 | int hrint = atomic_read(&per_cpu(hrtimer_interrupts, cpu)); |
| 165 | 165 | ||
| 166 | if (per_cpu(hrtimer_interrupts_saved, cpu) == hrint) | ||
| 167 | return true; | ||
| 168 | |||
| 169 | /* | 166 | /* |
| 170 | * NOTE: we don't need any fancy atomic_t or READ_ONCE/WRITE_ONCE | 167 | * NOTE: we don't need any fancy atomic_t or READ_ONCE/WRITE_ONCE |
| 171 | * for hrtimer_interrupts_saved. hrtimer_interrupts_saved is | 168 | * for hrtimer_interrupts_saved. hrtimer_interrupts_saved is |
| 172 | * written/read by a single CPU. | 169 | * written/read by a single CPU. |
| 173 | */ | 170 | */ |
| 174 | per_cpu(hrtimer_interrupts_saved, cpu) = hrint; | 171 | per_cpu(hrtimer_interrupts_saved, cpu) = hrint; |
| 172 | } | ||
| 173 | |||
| 174 | static bool is_hardlockup(unsigned int cpu) | ||
| 175 | { | ||
| 176 | int hrint = atomic_read(&per_cpu(hrtimer_interrupts, cpu)); | ||
| 177 | |||
| 178 | if (per_cpu(hrtimer_interrupts_saved, cpu) != hrint) { | ||
| 179 | watchdog_hardlockup_update(cpu); | ||
| 180 | return false; | ||
| 181 | } | ||
| 175 | 182 | ||
| 176 | return false; | 183 | return true; |
| 177 | } | 184 | } |
| 178 | 185 | ||
| 179 | static void watchdog_hardlockup_kick(void) | 186 | static void watchdog_hardlockup_kick(void) |
| @@ -191,6 +198,7 @@ void watchdog_hardlockup_check(unsigned int cpu, struct pt_regs *regs) · 11 unmodified lines | |||
| 191 | unsigned long flags; | 198 | unsigned long flags; |
| 192 | 199 | ||
| 193 | if (per_cpu(watchdog_hardlockup_touched, cpu)) { | 200 | if (per_cpu(watchdog_hardlockup_touched, cpu)) { |
| 201 | watchdog_hardlockup_update(cpu); | ||
| 194 | per_cpu(watchdog_hardlockup_touched, cpu) = false; | 202 | per_cpu(watchdog_hardlockup_touched, cpu) = false; |
| 195 | return; | 203 | return; |
| 196 | } | 204 | } |