linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Lai Jiangshan <laijs@linux.alibaba.com>
To: linux-kernel@vger.kernel.org
Cc: Lai Jiangshan <laijs@linux.alibaba.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	Tejun Heo <tj@kernel.org>, Lai Jiangshan <jiangshanlai@gmail.com>
Subject: [PATCH 1/2] workqueue: pin the pool while it is managing
Date: Thu, 28 May 2020 03:06:55 +0000	[thread overview]
Message-ID: <20200528030657.1690-1-laijs@linux.alibaba.com> (raw)
In-Reply-To: <20200527194633.1660952-2-bigeasy@linutronix.de>

So that put_unbound_pool() can ensure all workers in idle,
no unfinished manager. And it doens't need to wait any manager
and can go to delete all the idle workers straight away.

Also removes manager waitqueue, because it is unneeded and as
Sebastian Andrzej Siewior said:

  The workqueue code has it's internal spinlock (pool::lock) and also
  implicit spinlock usage in the wq_manager waitqueue. These spinlocks
  are converted to 'sleeping' spinlocks on a RT-kernel.

  Workqueue functions can be invoked from contexts which are truly atomic
  even on a PREEMPT_RT enabled kernel. Taking sleeping locks from such
  contexts is forbidden.

  pool::lock can be converted to a raw spinlock as the lock held times
  are short. But the workqueue manager waitqueue is handled inside of
  pool::lock held regions which again violates the lock nesting rules
  of raw and regular spinlocks.

Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Link: https://lore.kernel.org/lkml/20200527194633.1660952-2-bigeasy@linutronix.de
Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com>
---
 kernel/workqueue.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 891ccad5f271..fde10a5dba82 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -301,7 +301,6 @@ static struct workqueue_attrs *wq_update_unbound_numa_attrs_buf;
 static DEFINE_MUTEX(wq_pool_mutex);	/* protects pools and workqueues list */
 static DEFINE_MUTEX(wq_pool_attach_mutex); /* protects worker attach/detach */
 static DEFINE_SPINLOCK(wq_mayday_lock);	/* protects wq->maydays list */
-static DECLARE_WAIT_QUEUE_HEAD(wq_manager_wait); /* wait for manager to go away */
 
 static LIST_HEAD(workqueues);		/* PR: list of all workqueues */
 static bool workqueue_freezing;		/* PL: have wqs started freezing? */
@@ -2129,10 +2128,21 @@ __acquires(&pool->lock)
 static bool manage_workers(struct worker *worker)
 {
 	struct worker_pool *pool = worker->pool;
+	struct work_struct *work = list_first_entry(&pool->worklist,
+					struct work_struct, entry);
+	struct pool_workqueue *pwq = get_work_pwq(work);
 
 	if (pool->flags & POOL_MANAGER_ACTIVE)
 		return false;
 
+	/*
+	 * If @pwq is for an unbound wq, its base ref and the @pool's
+	 * ref may be put at any time due to an attribute change.
+	 * Pin @pwq to also pin the @pool until the manager is done
+	 * with it.
+	 */
+	get_pwq(pwq);
+
 	pool->flags |= POOL_MANAGER_ACTIVE;
 	pool->manager = worker;
 
@@ -2140,7 +2150,7 @@ static bool manage_workers(struct worker *worker)
 
 	pool->manager = NULL;
 	pool->flags &= ~POOL_MANAGER_ACTIVE;
-	wake_up(&wq_manager_wait);
+	put_pwq(pwq);
 	return true;
 }
 
@@ -3535,16 +3545,7 @@ static void put_unbound_pool(struct worker_pool *pool)
 		idr_remove(&worker_pool_idr, pool->id);
 	hash_del(&pool->hash_node);
 
-	/*
-	 * Become the manager and destroy all workers.  This prevents
-	 * @pool's workers from blocking on attach_mutex.  We're the last
-	 * manager and @pool gets freed with the flag set.
-	 */
 	spin_lock_irq(&pool->lock);
-	wait_event_lock_irq(wq_manager_wait,
-			    !(pool->flags & POOL_MANAGER_ACTIVE), pool->lock);
-	pool->flags |= POOL_MANAGER_ACTIVE;
-
 	while ((worker = first_idle_worker(pool)))
 		destroy_worker(worker);
 	WARN_ON(pool->nr_workers || pool->nr_idle);
-- 
2.20.1


  reply	other threads:[~2020-05-28  3:07 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-27 19:46 [PATCH v2 0/2] workqueue: Make the workqueue code PREEMPT_RT safe Sebastian Andrzej Siewior
2020-05-27 19:46 ` [PATCH v2 1/2] workqueue: Use rcuwait for wq_manager_wait Sebastian Andrzej Siewior
2020-05-28  3:06   ` Lai Jiangshan [this message]
2020-05-28  3:06     ` [PATCH 2/2] workqueue: remove useless POOL_MANAGER_ACTIVE Lai Jiangshan
2020-05-28  8:08     ` [PATCH 1/2] workqueue: pin the pool while it is managing Sebastian Andrzej Siewior
2020-05-28  8:35       ` Lai Jiangshan
2020-05-28 14:35     ` Tejun Heo
2020-05-29  5:33       ` Lai Jiangshan
2020-05-27 19:46 ` [PATCH v2 2/2] workqueue: Convert the pool::lock and wq_mayday_lock to raw_spinlock_t Sebastian Andrzej Siewior
2020-05-28  0:33   ` Lai Jiangshan
2020-05-29 14:04   ` Tejun Heo
2020-05-27 22:57 ` [PATCH v2 0/2] workqueue: Make the workqueue code PREEMPT_RT safe Linus Torvalds

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200528030657.1690-1-laijs@linux.alibaba.com \
    --to=laijs@linux.alibaba.com \
    --cc=bigeasy@linutronix.de \
    --cc=jiangshanlai@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=tj@kernel.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).