Skip to content

Commit 6d90215

Browse files
Lai Jiangshanhtejun
authored andcommitted
workqueue: Don't rely on wq->rescuer to stop rescuer
The commit1 def98c8 ("workqueue: Fix spurious sanity check failures in destroy_workqueue()") tries to fix spurious sanity check failures by stopping send_mayday() via setting wq->rescuer to NULL. But it fails to stop the pwq->mayday_node requeuing in the rescuer, and the commit2 e66b39a ("workqueue: Fix pwq ref leak in rescuer_thread()") fixes it by checking wq->rescuer which is the result of commit1. Both commits together really fix spurious sanity check failures caused by the rescuer, but they both use a convoluted method by relying on wq->rescuer state rather than the real count of work items. Actually __WQ_DESTROYING and drain_workqueue() together already stop send_mayday() by draining all the work items and ensuring no new work item requeuing. And the more proper fix to stop the pwq->mayday_node requeuing in the rescuer is from commit3 4f3f4cf ("workqueue: avoid unneeded requeuing the pwq in rescuer thread") and renders the checking of wq->rescuer in commit2 unnecessary. So __WQ_DESTROYING, drain_workqueue() and commit3 together fix spurious sanity check failures introduced by the rescuer. Just remove the convoluted code of using wq->rescuer. Signed-off-by: Lai Jiangshan <[email protected]> Signed-off-by: Tejun Heo <[email protected]>
1 parent 7b05c90 commit 6d90215

File tree

1 file changed

+5
-12
lines changed

1 file changed

+5
-12
lines changed

kernel/workqueue.c

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3539,10 +3539,9 @@ static int rescuer_thread(void *__rescuer)
35393539
if (pwq->nr_active && need_to_create_worker(pool)) {
35403540
raw_spin_lock(&wq_mayday_lock);
35413541
/*
3542-
* Queue iff we aren't racing destruction
3543-
* and somebody else hasn't queued it already.
3542+
* Queue iff somebody else hasn't queued it already.
35443543
*/
3545-
if (wq->rescuer && list_empty(&pwq->mayday_node)) {
3544+
if (list_empty(&pwq->mayday_node)) {
35463545
get_pwq(pwq);
35473546
list_add_tail(&pwq->mayday_node, &wq->maydays);
35483547
}
@@ -5905,16 +5904,10 @@ void destroy_workqueue(struct workqueue_struct *wq)
59055904

59065905
/* kill rescuer, if sanity checks fail, leave it w/o rescuer */
59075906
if (wq->rescuer) {
5908-
struct worker *rescuer = wq->rescuer;
5909-
5910-
/* this prevents new queueing */
5911-
raw_spin_lock_irq(&wq_mayday_lock);
5912-
wq->rescuer = NULL;
5913-
raw_spin_unlock_irq(&wq_mayday_lock);
5914-
59155907
/* rescuer will empty maydays list before exiting */
5916-
kthread_stop(rescuer->task);
5917-
kfree(rescuer);
5908+
kthread_stop(wq->rescuer->task);
5909+
kfree(wq->rescuer);
5910+
wq->rescuer = NULL;
59185911
}
59195912

59205913
/*

0 commit comments

Comments
 (0)