All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lai Jiangshan <laijs@cn.fujitsu.com>
To: Tejun Heo <tj@kernel.org>, linux-kernel@vger.kernel.org
Cc: Lai Jiangshan <laijs@cn.fujitsu.com>
Subject: [PATCH 05/11 V5] workqueue: Add @bind arguement back without change any thing
Date: Wed, 5 Sep 2012 18:37:42 +0800	[thread overview]
Message-ID: <1346841475-4422-6-git-send-email-laijs@cn.fujitsu.com> (raw)
In-Reply-To: <1346841475-4422-1-git-send-email-laijs@cn.fujitsu.com>

Ensure the gcwq->flags is only accessed with gcwq->lock held.
And make the code more easier to understand.

In all current callsite of create_worker(), DISASSOCIATED can't
be flipped while create_worker().
So the whole behavior is unchanged with this patch.

Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
 kernel/workqueue.c |   33 ++++++++++++++++++---------------
 1 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 6bf4185..1946be4 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -56,10 +56,6 @@ enum {
 	 * While DISASSOCIATED, the cpu may be offline and all workers have
 	 * %WORKER_UNBOUND set and concurrency management disabled, and may
 	 * be executing on any CPU.  The gcwq behaves as an unbound one.
-	 *
-	 * Note that DISASSOCIATED can be flipped only while holding
-	 * managership of all pools on the gcwq to avoid changing binding
-	 * state while create_worker() is in progress.
 	 */
 	GCWQ_DISASSOCIATED	= 1 << 0,	/* cpu can't serve workers */
 	GCWQ_FREEZING		= 1 << 1,	/* freeze in progress */
@@ -1727,6 +1723,7 @@ static struct worker *alloc_worker(void)
 /**
  * create_worker - create a new workqueue worker
  * @pool: pool the new worker will belong to
+ * @bind: whether to set affinity to @cpu or not
  *
  * Create a new worker which is bound to @pool.  The returned worker
  * can be started by calling start_worker() or destroyed using
@@ -1738,7 +1735,7 @@ static struct worker *alloc_worker(void)
  * RETURNS:
  * Pointer to the newly created worker.
  */
-static struct worker *create_worker(struct worker_pool *pool)
+static struct worker *create_worker(struct worker_pool *pool, bool bind)
 {
 	struct global_cwq *gcwq = pool->gcwq;
 	const char *pri = worker_pool_pri(pool) ? "H" : "";
@@ -1775,15 +1772,10 @@ static struct worker *create_worker(struct worker_pool *pool)
 		set_user_nice(worker->task, HIGHPRI_NICE_LEVEL);
 
 	/*
-	 * Determine CPU binding of the new worker depending on
-	 * %GCWQ_DISASSOCIATED.  The caller is responsible for ensuring the
-	 * flag remains stable across this function.  See the comments
-	 * above the flag definition for details.
-	 *
 	 * As an unbound worker may later become a regular one if CPU comes
 	 * online, make sure every worker has %PF_THREAD_BOUND set.
 	 */
-	if (!(gcwq->flags & GCWQ_DISASSOCIATED)) {
+	if (bind) {
 		kthread_bind(worker->task, gcwq->cpu);
 	} else {
 		worker->task->flags |= PF_THREAD_BOUND;
@@ -1951,6 +1943,14 @@ __releases(&gcwq->lock)
 __acquires(&gcwq->lock)
 {
 	struct global_cwq *gcwq = pool->gcwq;
+	bool bind;
+
+	/*
+	 * Note we have held the manage_mutex, so DISASSOCIATED can't be
+	 * flipped and it is correct that we calculate @bind only once
+	 * and then release the gcwq->lock.
+	 */
+	bind = !(gcwq->flags & GCWQ_DISASSOCIATED);
 
 	if (!need_to_create_worker(pool))
 		return false;
@@ -1963,7 +1963,7 @@ restart:
 	while (true) {
 		struct worker *worker;
 
-		worker = create_worker(pool);
+		worker = create_worker(pool, bind);
 		if (worker) {
 			del_timer_sync(&pool->mayday_timer);
 			spin_lock_irq(&gcwq->lock);
@@ -3479,7 +3479,7 @@ static int __devinit workqueue_cpu_up_callback(struct notifier_block *nfb,
 			if (pool->nr_workers)
 				continue;
 
-			worker = create_worker(pool);
+			worker = create_worker(pool, false);
 			if (!worker)
 				return NOTIFY_BAD;
 
@@ -3757,14 +3757,17 @@ static int __init init_workqueues(void)
 	for_each_online_gcwq_cpu(cpu) {
 		struct global_cwq *gcwq = get_gcwq(cpu);
 		struct worker_pool *pool;
+		bool bind = false;
 
-		if (cpu != WORK_CPU_UNBOUND)
+		if (cpu != WORK_CPU_UNBOUND) {
 			gcwq->flags &= ~GCWQ_DISASSOCIATED;
+			bind = true;
+		}
 
 		for_each_worker_pool(pool, gcwq) {
 			struct worker *worker;
 
-			worker = create_worker(pool);
+			worker = create_worker(pool, bind);
 			BUG_ON(!worker);
 			spin_lock_irq(&gcwq->lock);
 			start_worker(worker);
-- 
1.7.4.4


  parent reply	other threads:[~2012-09-05 10:36 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-05 10:37 [PATCH 00/11 V5] workqueue: reimplement unbind/rebind Lai Jiangshan
2012-09-05 10:37 ` [PATCH 01/11 V5] workqueue: ensure the wq_worker_sleeping() see the right flags Lai Jiangshan
2012-09-05 10:37 ` [PATCH 02/11 V5] workqueue: async idle rebinding Lai Jiangshan
2012-09-05 18:06   ` Tejun Heo
2012-09-06  1:28     ` Lai Jiangshan
2012-09-05 10:37 ` [PATCH 03/11 V5] workqueue: new day don't need WORKER_REBIND for busy rebinding Lai Jiangshan
2012-09-05 18:31   ` Tejun Heo
2012-09-06  2:10     ` Lai Jiangshan
2012-09-05 10:37 ` [PATCH 04/11 V5] workqueue: remove WORKER_REBIND Lai Jiangshan
2012-09-05 10:37 ` Lai Jiangshan [this message]
2012-09-05 19:49   ` [PATCH 05/11 V5] workqueue: Add @bind arguement back without change any thing Tejun Heo
2012-09-06  1:04     ` Lai Jiangshan
2012-09-06 16:51       ` Tejun Heo
2012-09-07  2:11         ` Lai Jiangshan
2012-09-07 19:37           ` Tejun Heo
2012-09-05 10:37 ` [PATCH 06/11 V5] workqueue: unbind manager Lai Jiangshan
2012-09-05 10:37 ` [PATCH 07/11 V5] workqueue: rebind manager Lai Jiangshan
2012-09-05 10:37 ` [PATCH 08/11 V5] workqueue: unbind newly created worker Lai Jiangshan
2012-09-05 10:37 ` [PATCH 09/11 V5] workqueue: rebind " Lai Jiangshan
2012-09-05 16:19   ` Lai Jiangshan
2012-09-05 10:37 ` [PATCH 10/11 V5] workqueue: unbind/rebind without manager_mutex Lai Jiangshan
2012-09-05 20:04   ` Tejun Heo
2012-09-06 10:44     ` Lai Jiangshan
2012-09-06 17:00       ` Tejun Heo
2012-09-05 10:37 ` [PATCH 11/11 V5] workqueue: remove manager_mutex Lai Jiangshan

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=1346841475-4422-6-git-send-email-laijs@cn.fujitsu.com \
    --to=laijs@cn.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tj@kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.