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>,
Ingo Molnar <mingo@redhat.com>,
Peter Zijlstra <peterz@infradead.org>
Subject: [PATCH 5/8] workqueue: remove disabled wq_worker_waking_up()
Date: Mon, 15 Apr 2013 00:41:53 +0800 [thread overview]
Message-ID: <1365957716-7631-6-git-send-email-laijs@cn.fujitsu.com> (raw)
In-Reply-To: <1365957716-7631-1-git-send-email-laijs@cn.fujitsu.com>
When a worker is sleeping, its flags has WORKER_QUIT_CM, which means
worker->flags & WORKER_NOT_RUNNING is always non-zero, and which means
wq_worker_waking_up() is disabled.
so we removed wq_worker_waking_up(). (the access to worker->flags
in wq_worker_waking_up() is not protected by "LI". after this, it is alwasy
protected by "LI")
The patch also do these changes after removal:
1) because wq_worker_waking_up() is removed, we don't need schedule()
before zapping nr_running in wq_unbind_fn(), and don't need to
release/regain pool->lock.
2) the sanity check in worker_enter_idle() is changed to also check for
unbound/disassociated pools. (because the above change and nr_running
is expected always reliable in worker_enter_idle() now.)
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
kernel/sched/core.c | 4 ---
kernel/workqueue.c | 58 +++++++-----------------------------------
kernel/workqueue_internal.h | 3 +-
3 files changed, 11 insertions(+), 54 deletions(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index ffc06ad..18f95884 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1276,10 +1276,6 @@ static void ttwu_activate(struct rq *rq, struct task_struct *p, int en_flags)
{
activate_task(rq, p, en_flags);
p->on_rq = 1;
-
- /* if a worker is waking up, notify workqueue */
- if (p->flags & PF_WQ_WORKER)
- wq_worker_waking_up(p, cpu_of(rq));
}
/*
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 668e9b7..9f1ebdf 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -790,27 +790,6 @@ static void wake_up_worker(struct worker_pool *pool)
}
/**
- * wq_worker_waking_up - a worker is waking up
- * @task: task waking up
- * @cpu: CPU @task is waking up to
- *
- * This function is called during try_to_wake_up() when a worker is
- * being awoken.
- *
- * CONTEXT:
- * spin_lock_irq(rq->lock)
- */
-void wq_worker_waking_up(struct task_struct *task, int cpu)
-{
- struct worker *worker = kthread_data(task);
-
- if (!(worker->flags & WORKER_NOT_RUNNING)) {
- WARN_ON_ONCE(worker->pool->cpu != cpu);
- atomic_inc(&worker->pool->nr_running);
- }
-}
-
-/**
* wq_worker_sleeping - a worker is going to sleep
* @task: task going to sleep
*
@@ -1564,14 +1543,8 @@ static void worker_enter_idle(struct worker *worker)
if (too_many_workers(pool) && !timer_pending(&pool->idle_timer))
mod_timer(&pool->idle_timer, jiffies + IDLE_WORKER_TIMEOUT);
- /*
- * Sanity check nr_running. Because wq_unbind_fn() releases
- * pool->lock between setting %WORKER_UNBOUND and zapping
- * nr_running, the warning may trigger spuriously. Check iff
- * unbind is not in progress.
- */
- WARN_ON_ONCE(!(pool->flags & POOL_DISASSOCIATED) &&
- pool->nr_workers == pool->nr_idle &&
+ /* Sanity check nr_running. */
+ WARN_ON_ONCE(pool->nr_workers == pool->nr_idle &&
atomic_read(&pool->nr_running));
}
@@ -4385,24 +4358,12 @@ static void wq_unbind_fn(struct work_struct *work)
pool->flags |= POOL_DISASSOCIATED;
- spin_unlock_irq(&pool->lock);
- mutex_unlock(&pool->manager_mutex);
-
/*
- * Call schedule() so that we cross rq->lock and thus can
- * guarantee sched callbacks see the %WORKER_UNBOUND flag.
- * This is necessary as scheduler callbacks may be invoked
- * from other cpus.
- */
- schedule();
-
- /*
- * Sched callbacks are disabled now. Zap nr_running.
- * After this, nr_running stays zero and need_more_worker()
- * and keep_working() are always true as long as the
- * worklist is not empty. This pool now behaves as an
- * unbound (in terms of concurrency management) pool which
- * are served by workers tied to the pool.
+ * Zap nr_running. After this, nr_running stays zero
+ * and need_more_worker() and keep_working() are always true
+ * as long as the worklist is not empty. This pool now
+ * behaves as an unbound (in terms of concurrency management)
+ * pool which are served by workers tied to the pool.
*/
atomic_set(&pool->nr_running, 0);
@@ -4411,9 +4372,9 @@ static void wq_unbind_fn(struct work_struct *work)
* worker blocking could lead to lengthy stalls. Kick off
* unbound chain execution of currently pending work items.
*/
- spin_lock_irq(&pool->lock);
wake_up_worker(pool);
spin_unlock_irq(&pool->lock);
+ mutex_unlock(&pool->manager_mutex);
}
}
@@ -4466,9 +4427,10 @@ static void rebind_workers(struct worker_pool *pool)
* concurrency management. Note that when or whether
* @worker clears REBOUND doesn't affect correctness.
*
+ * Current CPU may not the cpu of this rebinding pool,
* ACCESS_ONCE() is necessary because @worker->flags may be
* tested without holding any lock in
- * wq_worker_waking_up(). Without it, NOT_RUNNING test may
+ * wq_worker_sleeping(). Without it, NOT_RUNNING test may
* fail incorrectly leading to premature concurrency
* management operations.
*/
diff --git a/kernel/workqueue_internal.h b/kernel/workqueue_internal.h
index 1713ae7..e9fd05f 100644
--- a/kernel/workqueue_internal.h
+++ b/kernel/workqueue_internal.h
@@ -54,9 +54,8 @@ static inline struct worker *current_wq_worker(void)
/*
* Scheduler hooks for concurrency managed workqueue. Only to be used from
- * sched.c and workqueue.c.
+ * kernel/sched/core.c and kernel/workqueue.c.
*/
-void wq_worker_waking_up(struct task_struct *task, int cpu);
struct task_struct *wq_worker_sleeping(struct task_struct *task);
#endif /* _KERNEL_WORKQUEUE_INTERNAL_H */
--
1.7.7.6
next prev parent reply other threads:[~2013-04-14 16:43 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-04-14 16:41 [PATCH 0/8] workqueue: advance concurrency management Lai Jiangshan
2013-04-14 16:41 ` [PATCH 1/8] workqueue: remove @cpu from wq_worker_sleeping() Lai Jiangshan
2013-04-14 16:41 ` [PATCH 2/8] workqueue: use create_and_start_worker() in manage_workers() Lai Jiangshan
2013-04-14 16:41 ` [PATCH 3/8] workqueue: remove cpu_intensive from process_one_work() Lai Jiangshan
2013-04-14 16:41 ` [PATCH 4/8] workqueue: quit cm mode when sleeping Lai Jiangshan
2013-04-14 16:41 ` Lai Jiangshan [this message]
2013-04-14 16:41 ` [PATCH 6/8] workqueue: make nr_running non-atomic Lai Jiangshan
2013-04-14 16:41 ` [PATCH 7/8] workqueue: move worker->flags up Lai Jiangshan
2013-04-14 16:41 ` [PATCH 8/8] workqueue: rename ->nr_running to ->nr_cm_workers Lai Jiangshan
2013-04-18 22:10 ` [PATCH 0/8] workqueue: advance concurrency management Lai Jiangshan
2013-04-19 18:11 ` Tejun Heo
2013-04-20 16:02 ` 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=1365957716-7631-6-git-send-email-laijs@cn.fujitsu.com \
--to=laijs@cn.fujitsu.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=peterz@infradead.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®