Commit 89e28ce6 authored by Wang Qing's avatar Wang Qing Committed by Tejun Heo

workqueue/watchdog: Make unbound workqueues aware of touch_softlockup_watchdog()

84;0;0c84;0;0c
There are two workqueue-specific watchdog timestamps:

    + @wq_watchdog_touched_cpu (per-CPU) updated by
      touch_softlockup_watchdog()

    + @wq_watchdog_touched (global) updated by
      touch_all_softlockup_watchdogs()

watchdog_timer_fn() checks only the global @wq_watchdog_touched for
unbound workqueues. As a result, unbound workqueues are not aware
of touch_softlockup_watchdog(). The watchdog might report a stall
even when the unbound workqueues are blocked by a known slow code.

Solution:
touch_softlockup_watchdog() must touch also the global @wq_watchdog_touched
timestamp.

The global timestamp can no longer be used for bound workqueues because
it is now updated from all CPUs. Instead, bound workqueues have to check
only @wq_watchdog_touched_cpu and these timestamps have to be updated for
all CPUs in touch_all_softlockup_watchdogs().

Beware:
The change might cause the opposite problem. An unbound workqueue
might get blocked on CPU A because of a real softlockup. The workqueue
watchdog would miss it when the timestamp got touched on CPU B.

It is acceptable because softlockups are detected by softlockup
watchdog. The workqueue watchdog is there to detect stalls where
a work never finishes, for example, because of dependencies of works
queued into the same workqueue.

V3:
- Modify the commit message clearly according to Petr's suggestion.
Signed-off-by: default avatarWang Qing <wangqing@vivo.com>
Signed-off-by: default avatarTejun Heo <tj@kernel.org>
parent 0687c66b
...@@ -278,9 +278,10 @@ void touch_all_softlockup_watchdogs(void) ...@@ -278,9 +278,10 @@ void touch_all_softlockup_watchdogs(void)
* update as well, the only side effect might be a cycle delay for * update as well, the only side effect might be a cycle delay for
* the softlockup check. * the softlockup check.
*/ */
for_each_cpu(cpu, &watchdog_allowed_mask) for_each_cpu(cpu, &watchdog_allowed_mask) {
per_cpu(watchdog_touch_ts, cpu) = SOFTLOCKUP_RESET; per_cpu(watchdog_touch_ts, cpu) = SOFTLOCKUP_RESET;
wq_watchdog_touch(-1); wq_watchdog_touch(cpu);
}
} }
void touch_softlockup_watchdog_sync(void) void touch_softlockup_watchdog_sync(void)
......
...@@ -5787,22 +5787,17 @@ static void wq_watchdog_timer_fn(struct timer_list *unused) ...@@ -5787,22 +5787,17 @@ static void wq_watchdog_timer_fn(struct timer_list *unused)
continue; continue;
/* get the latest of pool and touched timestamps */ /* get the latest of pool and touched timestamps */
if (pool->cpu >= 0)
touched = READ_ONCE(per_cpu(wq_watchdog_touched_cpu, pool->cpu));
else
touched = READ_ONCE(wq_watchdog_touched);
pool_ts = READ_ONCE(pool->watchdog_ts); pool_ts = READ_ONCE(pool->watchdog_ts);
touched = READ_ONCE(wq_watchdog_touched);
if (time_after(pool_ts, touched)) if (time_after(pool_ts, touched))
ts = pool_ts; ts = pool_ts;
else else
ts = touched; ts = touched;
if (pool->cpu >= 0) {
unsigned long cpu_touched =
READ_ONCE(per_cpu(wq_watchdog_touched_cpu,
pool->cpu));
if (time_after(cpu_touched, ts))
ts = cpu_touched;
}
/* did we stall? */ /* did we stall? */
if (time_after(jiffies, ts + thresh)) { if (time_after(jiffies, ts + thresh)) {
lockup_detected = true; lockup_detected = true;
...@@ -5826,8 +5821,8 @@ notrace void wq_watchdog_touch(int cpu) ...@@ -5826,8 +5821,8 @@ notrace void wq_watchdog_touch(int cpu)
{ {
if (cpu >= 0) if (cpu >= 0)
per_cpu(wq_watchdog_touched_cpu, cpu) = jiffies; per_cpu(wq_watchdog_touched_cpu, cpu) = jiffies;
else
wq_watchdog_touched = jiffies; wq_watchdog_touched = jiffies;
} }
static void wq_watchdog_set_thresh(unsigned long thresh) static void wq_watchdog_set_thresh(unsigned long thresh)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment