mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Lai Jiangshan <laijs@cn.fujitsu.com>
To: <linux-kernel@vger.kernel.org>
Cc: Tejun Heo <tj@kernel.org>, Lai Jiangshan <laijs@cn.fujitsu.com>
Subject: [PATCH 10/10 V2] workqueue: use generic attach/detach routine for rescuers
Date: Mon, 12 May 2014 14:56:22 +0800	[thread overview]
Message-ID: <1399877792-13046-11-git-send-email-laijs@cn.fujitsu.com> (raw)
In-Reply-To: <1399877792-13046-1-git-send-email-laijs@cn.fujitsu.com>

There are several problems with the code that rescuers bind itself to the pool'
cpumask
  1) It uses a way different from the normal workers to bind to the cpumask
     So we can't maintain the normal/rescuer workers under the same framework.
  2) The the code of cpu-binding for rescuer is complicated
  3) If one or more cpuhotplugs happen while the rescuer processes the
     scheduled works, the rescuer may not be correctly bound to the cpumask of
     the pool. This is allowed behavior, but is not good. It will be better
     if the cpumask of the rescuer is always kept coordination with the pool
     across any cpuhotplugs.

Using generic attach/detach routine will solve the above problems,
and result much more simple code.

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

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 0ea0152..099f02c 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -1588,70 +1588,6 @@ static void worker_leave_idle(struct worker *worker)
 	list_del_init(&worker->entry);
 }
 
-/**
- * worker_maybe_bind_and_lock - try to bind %current to worker_pool and lock it
- * @pool: target worker_pool
- *
- * Bind %current to the cpu of @pool if it is associated and lock @pool.
- *
- * Works which are scheduled while the cpu is online must at least be
- * scheduled to a worker which is bound to the cpu so that if they are
- * flushed from cpu callbacks while cpu is going down, they are
- * guaranteed to execute on the cpu.
- *
- * This function is to be used by unbound workers and rescuers to bind
- * themselves to the target cpu and may race with cpu going down or
- * coming online.  kthread_bind() can't be used because it may put the
- * worker to already dead cpu and set_cpus_allowed_ptr() can't be used
- * verbatim as it's best effort and blocking and pool may be
- * [dis]associated in the meantime.
- *
- * This function tries set_cpus_allowed() and locks pool and verifies the
- * binding against %POOL_DISASSOCIATED which is set during
- * %CPU_DOWN_PREPARE and cleared during %CPU_ONLINE, so if the worker
- * enters idle state or fetches works without dropping lock, it can
- * guarantee the scheduling requirement described in the first paragraph.
- *
- * CONTEXT:
- * Might sleep.  Called without any lock but returns with pool->lock
- * held.
- *
- * Return:
- * %true if the associated pool is online (@worker is successfully
- * bound), %false if offline.
- */
-static bool worker_maybe_bind_and_lock(struct worker_pool *pool)
-__acquires(&pool->lock)
-{
-	while (true) {
-		/*
-		 * The following call may fail, succeed or succeed
-		 * without actually migrating the task to the cpu if
-		 * it races with cpu hotunplug operation.  Verify
-		 * against POOL_DISASSOCIATED.
-		 */
-		if (!(pool->flags & POOL_DISASSOCIATED))
-			set_cpus_allowed_ptr(current, pool->attrs->cpumask);
-
-		spin_lock_irq(&pool->lock);
-		if (pool->flags & POOL_DISASSOCIATED)
-			return false;
-		if (task_cpu(current) == pool->cpu &&
-		    cpumask_equal(&current->cpus_allowed, pool->attrs->cpumask))
-			return true;
-		spin_unlock_irq(&pool->lock);
-
-		/*
-		 * We've raced with CPU hot[un]plug.  Give it a breather
-		 * and retry migration.  cond_resched() is required here;
-		 * otherwise, we might deadlock against cpu_stop trying to
-		 * bring down the CPU on non-preemptive kernel.
-		 */
-		cpu_relax();
-		cond_resched();
-	}
-}
-
 static struct worker *alloc_worker(void)
 {
 	struct worker *worker;
@@ -2343,8 +2279,9 @@ repeat:
 
 		spin_unlock_irq(&wq_mayday_lock);
 
-		/* migrate to the target cpu if possible */
-		worker_maybe_bind_and_lock(pool);
+		worker_attach_to_pool(rescuer, pool);
+
+		spin_lock_irq(&pool->lock);
 		rescuer->pool = pool;
 
 		/*
@@ -2357,6 +2294,11 @@ repeat:
 				move_linked_works(work, scheduled, &n);
 
 		process_scheduled_works(rescuer);
+		spin_unlock_irq(&pool->lock);
+
+		worker_detach_from_pool(rescuer, pool);
+
+		spin_lock_irq(&pool->lock);
 
 		/*
 		 * Put the reference grabbed by send_mayday().  @pool won't
-- 
1.7.4.4


  parent reply	other threads:[~2014-05-12  6:53 UTC|newest]

Thread overview: 78+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-27  4:08 [PATCH 00/10] workqueue: async worker destruction and pool-binding synchronization Lai Jiangshan
2014-04-27  4:08 ` [PATCH 01/10] workqueue: use manager lock only to protect worker_idr Lai Jiangshan
2014-05-05 13:01   ` Tejun Heo
2014-05-06 15:43     ` Lai Jiangshan
2014-04-27  4:08 ` [PATCH 02/10] workqueue: destroy_worker() should destroy idle workers only Lai Jiangshan
2014-05-05 13:13   ` Tejun Heo
2014-05-06 15:58     ` Lai Jiangshan
2014-04-27  4:08 ` [PATCH 03/10] workqueue: async worker destruction Lai Jiangshan
2014-05-05 14:31   ` Tejun Heo
2014-05-05 15:02   ` Tejun Heo
2014-05-06 16:27     ` Lai Jiangshan
2014-05-06 16:31       ` Tejun Heo
2014-05-07  7:30   ` Lai Jiangshan
2014-05-07 13:15     ` Tejun Heo
2014-04-27  4:08 ` [PATCH 04/10] workqueue: destroy worker directly in the idle timeout handler Lai Jiangshan
2014-05-05 14:36   ` Tejun Heo
2014-05-07  7:10     ` Lai Jiangshan
2014-05-07 13:12       ` Tejun Heo
2014-05-07 13:38         ` Lai Jiangshan
2014-05-07 13:41           ` Tejun Heo
2014-05-07 15:30             ` Lai Jiangshan
2014-05-07 15:37               ` Tejun Heo
2014-04-27  4:09 ` [PATCH 05/10] workqueue: separate iteration role from worker_idr Lai Jiangshan
2014-05-05 14:57   ` Tejun Heo
2014-04-27  4:09 ` [PATCH 06/10] workqueue: convert worker_idr to worker_ida Lai Jiangshan
2014-05-05 14:59   ` Tejun Heo
2014-05-06 16:33     ` Lai Jiangshan
2014-05-06 16:35       ` Tejun Heo
2014-05-06 16:38         ` Tejun Heo
2014-04-27  4:09 ` [PATCH 07/10] workqueue: narrow the protection range of manager_mutex Lai Jiangshan
2014-04-27  4:09 ` [PATCH 08/10] workqueue: rename manager_mutex to bind_mutex Lai Jiangshan
2014-04-27  4:09 ` [PATCH 09/10] workqueue: separate pool-binding code out from create_worker() Lai Jiangshan
2014-04-27  4:09 ` [PATCH 10/10] workqueue: use generic pool-bind/unbind routine for rescuers Lai Jiangshan
2014-05-05 14:54   ` Tejun Heo
2014-05-05 15:05 ` [PATCH 00/10] workqueue: async worker destruction and pool-binding synchronization Tejun Heo
2014-05-12  6:56   ` [PATCH 00/10 V2] workqueue: async worker destruction and worker attaching/detaching Lai Jiangshan
2014-05-12  6:56     ` [PATCH 01/10 V2] workqueue: use manager lock only to protect worker_idr Lai Jiangshan
2014-05-12  6:56     ` [PATCH 02/10 V2] workqueue: destroy_worker() should destroy idle workers only Lai Jiangshan
2014-05-12 21:08       ` Tejun Heo
2014-05-13  6:08         ` Lai Jiangshan
2014-05-12  6:56     ` [PATCH 03/10 V2] workqueue: async worker destruction Lai Jiangshan
2014-05-12 21:20       ` Tejun Heo
2014-05-13  6:32         ` Lai Jiangshan
2014-05-13 14:14           ` Tejun Heo
2014-05-20  9:54             ` Lai Jiangshan
2014-05-12  6:56     ` [PATCH 04/10 V2] workqueue: destroy worker directly in the idle timeout handler Lai Jiangshan
2014-05-12 21:26       ` Tejun Heo
2014-05-12  6:56     ` [PATCH 05/10 V2] workqueue: separate iteration role from worker_idr Lai Jiangshan
2014-05-12 21:35       ` Tejun Heo
2014-05-12 21:37       ` Tejun Heo
2014-05-12  6:56     ` [PATCH 06/10 V2] workqueue: convert worker_idr to worker_ida Lai Jiangshan
2014-05-12 21:40       ` Tejun Heo
2014-05-13  6:43         ` Lai Jiangshan
2014-05-13 14:17           ` Tejun Heo
2014-05-12  6:56     ` [PATCH 07/10 V2] workqueue: narrow the protection range of manager_mutex Lai Jiangshan
2014-05-12  6:56     ` [PATCH 08/10 V2] workqueue: rename manager_mutex to attach_mutex Lai Jiangshan
2014-05-12 22:01       ` Tejun Heo
2014-05-13  6:45         ` Lai Jiangshan
2014-05-13 14:19           ` Tejun Heo
2014-05-12  6:56     ` [PATCH 09/10 V2] workqueue: separate pool-attaching code out from create_worker() Lai Jiangshan
2014-05-12  6:56     ` Lai Jiangshan [this message]
2014-05-12 22:05       ` [PATCH 10/10 V2] workqueue: use generic attach/detach routine for rescuers Tejun Heo
2014-05-13  6:55         ` Lai Jiangshan
2014-05-20  9:46     ` [PATCH V3 00/10] workqueue: async worker destruction and worker attaching/detaching Lai Jiangshan
2014-05-20  9:46       ` [PATCH V3 01/10] workqueue: use manager lock only to protect worker_idr Lai Jiangshan
2014-05-20  9:46       ` [PATCH V3 02/10] workqueue: destroy_worker() should destroy idle workers only Lai Jiangshan
2014-05-20  9:46       ` [PATCH V3 03/10] workqueue: async worker destruction Lai Jiangshan
2014-05-20 14:22         ` Tejun Heo
2014-05-20 14:23         ` Tejun Heo
2014-05-20 14:24         ` Tejun Heo
2014-05-20  9:46       ` [PATCH V3 04/10] workqueue: destroy worker directly in the idle timeout handler Lai Jiangshan
2014-05-20  9:46       ` [PATCH V3 05/10] workqueue: separate iteration role from worker_idr Lai Jiangshan
2014-05-20  9:46       ` [PATCH V3 06/10] workqueue: convert worker_idr to worker_ida Lai Jiangshan
2014-05-20  9:46       ` [PATCH V3 07/10] workqueue: narrow the protection range of manager_mutex Lai Jiangshan
2014-05-20  9:46       ` [PATCH V3 08/10] workqueue: rename manager_mutex to attach_mutex Lai Jiangshan
2014-05-20  9:46       ` [PATCH V3 09/10] workqueue: separate pool-attaching code out from create_worker() Lai Jiangshan
2014-05-20  9:46       ` [PATCH V3 10/10] workqueue: use generic attach/detach routine for rescuers Lai Jiangshan
2014-05-20 15:00       ` [PATCH V3 00/10] workqueue: async worker destruction and worker attaching/detaching Tejun Heo

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=1399877792-13046-11-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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®