workqueue: Don't rely on wq->rescuer to stop rescuer

The commit1 def98c84b6cd ("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 e66b39af00f4 ("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 4f3f4cf388f8 ("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 <jiangshan.ljs@antgroup.com>
Signed-off-by: Tejun Heo <tj@kernel.org>

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 656715d216b49958619ea981542a21cfcbc5fbc4..253311af47c6d3f09810103b96d0e238a2cacec9 100644 (file)
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -3539,10 +3539,9 @@ static int rescuer_thread(void *__rescuer)
                        if (pwq->nr_active && need_to_create_worker(pool)) {
                                raw_spin_lock(&wq_mayday_lock);
                                /*
-                                * Queue iff we aren't racing destruction
-                                * and somebody else hasn't queued it already.
+                                * Queue iff somebody else hasn't queued it already.
                                 */
-                               if (wq->rescuer && list_empty(&pwq->mayday_node)) {
+                               if (list_empty(&pwq->mayday_node)) {
                                        get_pwq(pwq);
                                        list_add_tail(&pwq->mayday_node, &wq->maydays);
                                }
@@ -5905,16 +5904,10 @@ void destroy_workqueue(struct workqueue_struct *wq)
 
        /* kill rescuer, if sanity checks fail, leave it w/o rescuer */
        if (wq->rescuer) {
-               struct worker *rescuer = wq->rescuer;
-
-               /* this prevents new queueing */
-               raw_spin_lock_irq(&wq_mayday_lock);
-               wq->rescuer = NULL;
-               raw_spin_unlock_irq(&wq_mayday_lock);
-
                /* rescuer will empty maydays list before exiting */
-               kthread_stop(rescuer->task);
-               kfree(rescuer);
+               kthread_stop(wq->rescuer->task);
+               kfree(wq->rescuer);
+               wq->rescuer = NULL;
        }
 
        /*