Skip to content

Commit e398978

Browse files
pmladekhtejun
authored andcommitted
workqueue: Better describe stall check
Try to be more explicit why the workqueue watchdog does not take pool->lock by default. Spin locks are full memory barriers which delay anything. Obviously, they would primary delay operations on the related worker pools. Explain why it is enough to prevent the false positive by re-checking the timestamp under the pool->lock. Finally, make it clear what would be the alternative solution in __queue_work() which is a hotter path. Signed-off-by: Petr Mladek <pmladek@suse.com> Acked-by: Song Liu <song@kernel.org> Signed-off-by: Tejun Heo <tj@kernel.org>
1 parent c7f27a8 commit e398978

1 file changed

Lines changed: 8 additions & 7 deletions

File tree

kernel/workqueue.c

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7702,13 +7702,14 @@ static void wq_watchdog_timer_fn(struct timer_list *unused)
77027702
/*
77037703
* Did we stall?
77047704
*
7705-
* Do a lockless check first. On weakly ordered
7706-
* architectures, the lockless check can observe a
7707-
* reordering between worklist insert_work() and
7708-
* last_progress_ts update from __queue_work(). Since
7709-
* __queue_work() is a much hotter path than the timer
7710-
* function, we handle false positive here by reading
7711-
* last_progress_ts again with pool->lock held.
7705+
* Do a lockless check first to do not disturb the system.
7706+
*
7707+
* Prevent false positives by double checking the timestamp
7708+
* under pool->lock. The lock makes sure that the check reads
7709+
* an updated pool->last_progress_ts when this CPU saw
7710+
* an already updated pool->worklist above. It seems better
7711+
* than adding another barrier into __queue_work() which
7712+
* is a hotter path.
77127713
*/
77137714
if (time_after(now, ts + thresh)) {
77147715
scoped_guard(raw_spinlock_irqsave, &pool->lock) {

0 commit comments

Comments
 (0)