Commit 6d90215d authored by Lai Jiangshan's avatar Lai Jiangshan Committed by Tejun Heo
Browse files

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



The commit1 def98c84 ("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 e66b39af ("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 4f3f4cf3 ("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: default avatarLai Jiangshan <jiangshan.ljs@antgroup.com>
Signed-off-by: default avatarTejun Heo <tj@kernel.org>
parent 7b05c90b

kernel/workqueue.c

+5 −12
Original line number Diff line number Diff line
@@ -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;

	}



	/*