* [PATCH 1/6] workqueue: rename wq_mutex to wq_pool_mutex
2013-03-26 0:07 [PATCHSET wq/for-3.10] workqueue: replace pwq_lock with wq->mutex Tejun Heo
@ 2013-03-26 0:07 ` Tejun Heo
2013-03-26 0:07 ` [PATCH 2/6] workqueue: rename wq->flush_mutex to wq->mutex Tejun Heo
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Tejun Heo @ 2013-03-26 0:07 UTC (permalink / raw)
To: laijs; +Cc: linux-kernel, js1304, Tejun Heo
From: Lai Jiangshan <laijs@cn.fujitsu.com>
wq->flush_mutex will be renamed to wq->mutex and cover all fields
specific to each workqueue and eventually replace pwq_lock, which will
make locking simpler and easier to understand.
Rename wq_mutex to wq_pool_mutex to avoid confusion with wq->mutex.
After the scheduled changes, wq_pool_mutex won't be protecting
anything specific to each workqueue instance anyway.
This patch is pure rename.
tj: s/wqs_mutex/wq_pool_mutex/. Rewrote description.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Lai Jiangshan <laijs@cn.fujitsu.com>
---
kernel/workqueue.c | 109 +++++++++++++++++++++++++++--------------------------
1 file changed, 55 insertions(+), 54 deletions(-)
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 47f2587..064157e 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -123,9 +123,9 @@ enum {
* MG: pool->manager_mutex and pool->lock protected. Writes require both
* locks. Reads can happen under either lock.
*
- * WQ: wq_mutex protected.
+ * PL: wq_pool_mutex protected.
*
- * WR: wq_mutex protected for writes. Sched-RCU protected for reads.
+ * PR: wq_pool_mutex protected for writes. Sched-RCU protected for reads.
*
* PW: pwq_lock protected.
*
@@ -163,8 +163,8 @@ struct worker_pool {
struct idr worker_idr; /* MG: worker IDs and iteration */
struct workqueue_attrs *attrs; /* I: worker attributes */
- struct hlist_node hash_node; /* WQ: unbound_pool_hash node */
- int refcnt; /* WQ: refcnt for unbound pools */
+ struct hlist_node hash_node; /* PL: unbound_pool_hash node */
+ int refcnt; /* PL: refcnt for unbound pools */
/*
* The current concurrency level. As it's likely to be accessed
@@ -226,10 +226,10 @@ struct wq_device;
* the appropriate worker_pool through its pool_workqueues.
*/
struct workqueue_struct {
- unsigned int flags; /* WQ: WQ_* flags */
+ unsigned int flags; /* PL: WQ_* flags */
struct pool_workqueue __percpu *cpu_pwqs; /* I: per-cpu pwq's */
struct list_head pwqs; /* FR: all pwqs of this wq */
- struct list_head list; /* WQ: list of all workqueues */
+ struct list_head list; /* PL: list of all workqueues */
struct mutex flush_mutex; /* protects wq flushing */
int work_color; /* F: current work color */
@@ -242,7 +242,7 @@ struct workqueue_struct {
struct list_head maydays; /* MD: pwqs requesting rescue */
struct worker *rescuer; /* I: rescue worker */
- int nr_drainers; /* WQ: drain in progress */
+ int nr_drainers; /* PL: drain in progress */
int saved_max_active; /* PW: saved pwq max_active */
#ifdef CONFIG_SYSFS
@@ -256,20 +256,20 @@ struct workqueue_struct {
static struct kmem_cache *pwq_cache;
-static DEFINE_MUTEX(wq_mutex); /* protects workqueues and pools */
+static DEFINE_MUTEX(wq_pool_mutex); /* protects pools and workqueues list */
static DEFINE_SPINLOCK(pwq_lock); /* protects pool_workqueues */
static DEFINE_SPINLOCK(wq_mayday_lock); /* protects wq->maydays list */
-static LIST_HEAD(workqueues); /* WQ: list of all workqueues */
-static bool workqueue_freezing; /* WQ: have wqs started freezing? */
+static LIST_HEAD(workqueues); /* PL: list of all workqueues */
+static bool workqueue_freezing; /* PL: have wqs started freezing? */
/* the per-cpu worker pools */
static DEFINE_PER_CPU_SHARED_ALIGNED(struct worker_pool [NR_STD_WORKER_POOLS],
cpu_worker_pools);
-static DEFINE_IDR(worker_pool_idr); /* WR: idr of all pools */
+static DEFINE_IDR(worker_pool_idr); /* PR: idr of all pools */
-/* WQ: hash of all unbound pools keyed by pool->attrs */
+/* PL: hash of all unbound pools keyed by pool->attrs */
static DEFINE_HASHTABLE(unbound_pool_hash, UNBOUND_POOL_HASH_ORDER);
/* I: attributes used when instantiating standard unbound pools on demand */
@@ -293,10 +293,10 @@ static void copy_workqueue_attrs(struct workqueue_attrs *to,
#define CREATE_TRACE_POINTS
#include <trace/events/workqueue.h>
-#define assert_rcu_or_wq_mutex() \
+#define assert_rcu_or_pool_mutex() \
rcu_lockdep_assert(rcu_read_lock_sched_held() || \
- lockdep_is_held(&wq_mutex), \
- "sched RCU or wq_mutex should be held")
+ lockdep_is_held(&wq_pool_mutex), \
+ "sched RCU or wq_pool_mutex should be held")
#define assert_rcu_or_pwq_lock() \
rcu_lockdep_assert(rcu_read_lock_sched_held() || \
@@ -323,16 +323,16 @@ static void copy_workqueue_attrs(struct workqueue_attrs *to,
* @pool: iteration cursor
* @pi: integer used for iteration
*
- * This must be called either with wq_mutex held or sched RCU read locked.
- * If the pool needs to be used beyond the locking in effect, the caller is
- * responsible for guaranteeing that the pool stays online.
+ * This must be called either with wq_pool_mutex held or sched RCU read
+ * locked. If the pool needs to be used beyond the locking in effect, the
+ * caller is responsible for guaranteeing that the pool stays online.
*
* The if/else clause exists only for the lockdep assertion and can be
* ignored.
*/
#define for_each_pool(pool, pi) \
idr_for_each_entry(&worker_pool_idr, pool, pi) \
- if (({ assert_rcu_or_wq_mutex(); false; })) { } \
+ if (({ assert_rcu_or_pool_mutex(); false; })) { } \
else
/**
@@ -489,7 +489,7 @@ static int worker_pool_assign_id(struct worker_pool *pool)
{
int ret;
- lockdep_assert_held(&wq_mutex);
+ lockdep_assert_held(&wq_pool_mutex);
do {
if (!idr_pre_get(&worker_pool_idr, GFP_KERNEL))
@@ -607,9 +607,9 @@ static struct pool_workqueue *get_work_pwq(struct work_struct *work)
*
* Return the worker_pool @work was last associated with. %NULL if none.
*
- * Pools are created and destroyed under wq_mutex, and allows read access
- * under sched-RCU read lock. As such, this function should be called
- * under wq_mutex or with preemption disabled.
+ * Pools are created and destroyed under wq_pool_mutex, and allows read
+ * access under sched-RCU read lock. As such, this function should be
+ * called under wq_pool_mutex or with preemption disabled.
*
* All fields of the returned pool are accessible as long as the above
* mentioned locking is in effect. If the returned pool needs to be used
@@ -621,7 +621,7 @@ static struct worker_pool *get_work_pool(struct work_struct *work)
unsigned long data = atomic_long_read(&work->data);
int pool_id;
- assert_rcu_or_wq_mutex();
+ assert_rcu_or_pool_mutex();
if (data & WORK_STRUCT_PWQ)
return ((struct pool_workqueue *)
@@ -2684,10 +2684,10 @@ void drain_workqueue(struct workqueue_struct *wq)
* hotter than drain_workqueue() and already looks at @wq->flags.
* Use __WQ_DRAINING so that queue doesn't have to check nr_drainers.
*/
- mutex_lock(&wq_mutex);
+ mutex_lock(&wq_pool_mutex);
if (!wq->nr_drainers++)
wq->flags |= __WQ_DRAINING;
- mutex_unlock(&wq_mutex);
+ mutex_unlock(&wq_pool_mutex);
reflush:
flush_workqueue(wq);
@@ -2714,10 +2714,10 @@ reflush:
local_irq_enable();
- mutex_lock(&wq_mutex);
+ mutex_lock(&wq_pool_mutex);
if (!--wq->nr_drainers)
wq->flags &= ~__WQ_DRAINING;
- mutex_unlock(&wq_mutex);
+ mutex_unlock(&wq_pool_mutex);
}
EXPORT_SYMBOL_GPL(drain_workqueue);
@@ -3430,16 +3430,16 @@ static void put_unbound_pool(struct worker_pool *pool)
{
struct worker *worker;
- mutex_lock(&wq_mutex);
+ mutex_lock(&wq_pool_mutex);
if (--pool->refcnt) {
- mutex_unlock(&wq_mutex);
+ mutex_unlock(&wq_pool_mutex);
return;
}
/* sanity checks */
if (WARN_ON(!(pool->flags & POOL_DISASSOCIATED)) ||
WARN_ON(!list_empty(&pool->worklist))) {
- mutex_unlock(&wq_mutex);
+ mutex_unlock(&wq_pool_mutex);
return;
}
@@ -3448,7 +3448,7 @@ static void put_unbound_pool(struct worker_pool *pool)
idr_remove(&worker_pool_idr, pool->id);
hash_del(&pool->hash_node);
- mutex_unlock(&wq_mutex);
+ mutex_unlock(&wq_pool_mutex);
/*
* Become the manager and destroy all workers. Grabbing
@@ -3489,7 +3489,7 @@ static struct worker_pool *get_unbound_pool(const struct workqueue_attrs *attrs)
u32 hash = wqattrs_hash(attrs);
struct worker_pool *pool;
- mutex_lock(&wq_mutex);
+ mutex_lock(&wq_pool_mutex);
/* do we already have a matching pool? */
hash_for_each_possible(unbound_pool_hash, pool, hash_node, hash) {
@@ -3520,10 +3520,10 @@ static struct worker_pool *get_unbound_pool(const struct workqueue_attrs *attrs)
/* install */
hash_add(unbound_pool_hash, &pool->hash_node, hash);
out_unlock:
- mutex_unlock(&wq_mutex);
+ mutex_unlock(&wq_pool_mutex);
return pool;
fail:
- mutex_unlock(&wq_mutex);
+ mutex_unlock(&wq_pool_mutex);
if (pool)
put_unbound_pool(pool);
return NULL;
@@ -3803,10 +3803,11 @@ struct workqueue_struct *__alloc_workqueue_key(const char *fmt,
goto err_destroy;
/*
- * wq_mutex protects global freeze state and workqueues list. Grab
- * it, adjust max_active and add the new @wq to workqueues list.
+ * wq_pool_mutex protects global freeze state and workqueues list.
+ * Grab it, adjust max_active and add the new @wq to workqueues
+ * list.
*/
- mutex_lock(&wq_mutex);
+ mutex_lock(&wq_pool_mutex);
spin_lock_irq(&pwq_lock);
for_each_pwq(pwq, wq)
@@ -3815,7 +3816,7 @@ struct workqueue_struct *__alloc_workqueue_key(const char *fmt,
list_add(&wq->list, &workqueues);
- mutex_unlock(&wq_mutex);
+ mutex_unlock(&wq_pool_mutex);
return wq;
@@ -3866,9 +3867,9 @@ void destroy_workqueue(struct workqueue_struct *wq)
* wq list is used to freeze wq, remove from list after
* flushing is complete in case freeze races us.
*/
- mutex_lock(&wq_mutex);
+ mutex_lock(&wq_pool_mutex);
list_del_init(&wq->list);
- mutex_unlock(&wq_mutex);
+ mutex_unlock(&wq_pool_mutex);
workqueue_sysfs_unregister(wq);
@@ -4198,7 +4199,7 @@ static int __cpuinit workqueue_cpu_up_callback(struct notifier_block *nfb,
case CPU_DOWN_FAILED:
case CPU_ONLINE:
- mutex_lock(&wq_mutex);
+ mutex_lock(&wq_pool_mutex);
for_each_pool(pool, pi) {
mutex_lock(&pool->manager_mutex);
@@ -4216,7 +4217,7 @@ static int __cpuinit workqueue_cpu_up_callback(struct notifier_block *nfb,
mutex_unlock(&pool->manager_mutex);
}
- mutex_unlock(&wq_mutex);
+ mutex_unlock(&wq_pool_mutex);
break;
}
return NOTIFY_OK;
@@ -4292,7 +4293,7 @@ EXPORT_SYMBOL_GPL(work_on_cpu);
* pool->worklist.
*
* CONTEXT:
- * Grabs and releases wq_mutex, pwq_lock and pool->lock's.
+ * Grabs and releases wq_pool_mutex, pwq_lock and pool->lock's.
*/
void freeze_workqueues_begin(void)
{
@@ -4301,7 +4302,7 @@ void freeze_workqueues_begin(void)
struct pool_workqueue *pwq;
int pi;
- mutex_lock(&wq_mutex);
+ mutex_lock(&wq_pool_mutex);
WARN_ON_ONCE(workqueue_freezing);
workqueue_freezing = true;
@@ -4322,7 +4323,7 @@ void freeze_workqueues_begin(void)
}
spin_unlock_irq(&pwq_lock);
- mutex_unlock(&wq_mutex);
+ mutex_unlock(&wq_pool_mutex);
}
/**
@@ -4332,7 +4333,7 @@ void freeze_workqueues_begin(void)
* between freeze_workqueues_begin() and thaw_workqueues().
*
* CONTEXT:
- * Grabs and releases wq_mutex.
+ * Grabs and releases wq_pool_mutex.
*
* RETURNS:
* %true if some freezable workqueues are still busy. %false if freezing
@@ -4344,7 +4345,7 @@ bool freeze_workqueues_busy(void)
struct workqueue_struct *wq;
struct pool_workqueue *pwq;
- mutex_lock(&wq_mutex);
+ mutex_lock(&wq_pool_mutex);
WARN_ON_ONCE(!workqueue_freezing);
@@ -4367,7 +4368,7 @@ bool freeze_workqueues_busy(void)
rcu_read_unlock_sched();
}
out_unlock:
- mutex_unlock(&wq_mutex);
+ mutex_unlock(&wq_pool_mutex);
return busy;
}
@@ -4378,7 +4379,7 @@ out_unlock:
* frozen works are transferred to their respective pool worklists.
*
* CONTEXT:
- * Grabs and releases wq_mutex, pwq_lock and pool->lock's.
+ * Grabs and releases wq_pool_mutex, pwq_lock and pool->lock's.
*/
void thaw_workqueues(void)
{
@@ -4387,7 +4388,7 @@ void thaw_workqueues(void)
struct worker_pool *pool;
int pi;
- mutex_lock(&wq_mutex);
+ mutex_lock(&wq_pool_mutex);
if (!workqueue_freezing)
goto out_unlock;
@@ -4410,7 +4411,7 @@ void thaw_workqueues(void)
workqueue_freezing = false;
out_unlock:
- mutex_unlock(&wq_mutex);
+ mutex_unlock(&wq_pool_mutex);
}
#endif /* CONFIG_FREEZER */
@@ -4442,9 +4443,9 @@ static int __init init_workqueues(void)
pool->attrs->nice = std_nice[i++];
/* alloc pool ID */
- mutex_lock(&wq_mutex);
+ mutex_lock(&wq_pool_mutex);
BUG_ON(worker_pool_assign_id(pool));
- mutex_unlock(&wq_mutex);
+ mutex_unlock(&wq_pool_mutex);
}
}
--
1.8.1.4
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 2/6] workqueue: rename wq->flush_mutex to wq->mutex
2013-03-26 0:07 [PATCHSET wq/for-3.10] workqueue: replace pwq_lock with wq->mutex Tejun Heo
2013-03-26 0:07 ` [PATCH 1/6] workqueue: rename wq_mutex to wq_pool_mutex Tejun Heo
@ 2013-03-26 0:07 ` Tejun Heo
2013-03-26 0:07 ` [PATCH 3/6] workqueue: protect wq->nr_drainers and ->flags with wq->mutex Tejun Heo
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Tejun Heo @ 2013-03-26 0:07 UTC (permalink / raw)
To: laijs; +Cc: linux-kernel, js1304, Tejun Heo
From: Lai Jiangshan <laijs@cn.fujitsu.com>
Currently pwq->flush_mutex protects many fields of a workqueue
including, especially, the pwqs list. We're going to expand this
mutex to protect most of a workqueue and eventually replace pwq_lock,
which will make locking simpler and easier to understand.
Drop the "flush_" prefix in preparation.
This patch is pure rename.
tj: Rebased on top of the current dev branch. Updated description.
Use WQ: and WR: instead of Q: and QR: for synchronization labels.
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
---
kernel/workqueue.c | 52 ++++++++++++++++++++++++++--------------------------
1 file changed, 26 insertions(+), 26 deletions(-)
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 064157e..d448eda 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -118,8 +118,6 @@ enum {
* cpu or grabbing pool->lock is enough for read access. If
* POOL_DISASSOCIATED is set, it's identical to L.
*
- * F: wq->flush_mutex protected.
- *
* MG: pool->manager_mutex and pool->lock protected. Writes require both
* locks. Reads can happen under either lock.
*
@@ -129,8 +127,10 @@ enum {
*
* PW: pwq_lock protected.
*
- * FR: wq->flush_mutex and pwq_lock protected for writes. Sched-RCU
- * protected for reads.
+ * WQ: wq->mutex protected.
+ *
+ * WR: wq->mutex and pwq_lock protected for writes. Sched-RCU protected
+ * for reads.
*
* MD: wq_mayday_lock protected.
*/
@@ -197,7 +197,7 @@ struct pool_workqueue {
int nr_active; /* L: nr of active works */
int max_active; /* L: max active works */
struct list_head delayed_works; /* L: delayed works */
- struct list_head pwqs_node; /* FR: node on wq->pwqs */
+ struct list_head pwqs_node; /* WR: node on wq->pwqs */
struct list_head mayday_node; /* MD: node on wq->maydays */
/*
@@ -214,8 +214,8 @@ struct pool_workqueue {
* Structure used to wait for workqueue flush.
*/
struct wq_flusher {
- struct list_head list; /* F: list of flushers */
- int flush_color; /* F: flush color waiting for */
+ struct list_head list; /* WQ: list of flushers */
+ int flush_color; /* WQ: flush color waiting for */
struct completion done; /* flush completion */
};
@@ -228,16 +228,16 @@ struct wq_device;
struct workqueue_struct {
unsigned int flags; /* PL: WQ_* flags */
struct pool_workqueue __percpu *cpu_pwqs; /* I: per-cpu pwq's */
- struct list_head pwqs; /* FR: all pwqs of this wq */
+ struct list_head pwqs; /* WR: all pwqs of this wq */
struct list_head list; /* PL: list of all workqueues */
- struct mutex flush_mutex; /* protects wq flushing */
- int work_color; /* F: current work color */
- int flush_color; /* F: current flush color */
+ struct mutex mutex; /* protects this wq */
+ int work_color; /* WQ: current work color */
+ int flush_color; /* WQ: current flush color */
atomic_t nr_pwqs_to_flush; /* flush in progress */
- struct wq_flusher *first_flusher; /* F: first flusher */
- struct list_head flusher_queue; /* F: flush waiters */
- struct list_head flusher_overflow; /* F: flush overflow list */
+ struct wq_flusher *first_flusher; /* WQ: first flusher */
+ struct list_head flusher_queue; /* WQ: flush waiters */
+ struct list_head flusher_overflow; /* WQ: flush overflow list */
struct list_head maydays; /* MD: pwqs requesting rescue */
struct worker *rescuer; /* I: rescue worker */
@@ -2460,7 +2460,7 @@ static void insert_wq_barrier(struct pool_workqueue *pwq,
* advanced to @work_color.
*
* CONTEXT:
- * mutex_lock(wq->flush_mutex).
+ * mutex_lock(wq->mutex).
*
* RETURNS:
* %true if @flush_color >= 0 and there's something to flush. %false
@@ -2529,7 +2529,7 @@ void flush_workqueue(struct workqueue_struct *wq)
lock_map_acquire(&wq->lockdep_map);
lock_map_release(&wq->lockdep_map);
- mutex_lock(&wq->flush_mutex);
+ mutex_lock(&wq->mutex);
/*
* Start-to-wait phase
@@ -2574,7 +2574,7 @@ void flush_workqueue(struct workqueue_struct *wq)
list_add_tail(&this_flusher.list, &wq->flusher_overflow);
}
- mutex_unlock(&wq->flush_mutex);
+ mutex_unlock(&wq->mutex);
wait_for_completion(&this_flusher.done);
@@ -2587,7 +2587,7 @@ void flush_workqueue(struct workqueue_struct *wq)
if (wq->first_flusher != &this_flusher)
return;
- mutex_lock(&wq->flush_mutex);
+ mutex_lock(&wq->mutex);
/* we might have raced, check again with mutex held */
if (wq->first_flusher != &this_flusher)
@@ -2659,7 +2659,7 @@ void flush_workqueue(struct workqueue_struct *wq)
}
out_unlock:
- mutex_unlock(&wq->flush_mutex);
+ mutex_unlock(&wq->mutex);
}
EXPORT_SYMBOL_GPL(flush_workqueue);
@@ -3550,15 +3550,15 @@ static void pwq_unbound_release_workfn(struct work_struct *work)
return;
/*
- * Unlink @pwq. Synchronization against flush_mutex isn't strictly
+ * Unlink @pwq. Synchronization against wq->mutex isn't strictly
* necessary on release but do it anyway. It's easier to verify
* and consistent with the linking path.
*/
- mutex_lock(&wq->flush_mutex);
+ mutex_lock(&wq->mutex);
spin_lock_irq(&pwq_lock);
list_del_rcu(&pwq->pwqs_node);
spin_unlock_irq(&pwq_lock);
- mutex_unlock(&wq->flush_mutex);
+ mutex_unlock(&wq->mutex);
put_unbound_pool(pool);
call_rcu_sched(&pwq->rcu, rcu_free_pwq);
@@ -3627,12 +3627,12 @@ static void init_and_link_pwq(struct pool_workqueue *pwq,
INIT_LIST_HEAD(&pwq->mayday_node);
INIT_WORK(&pwq->unbound_release_work, pwq_unbound_release_workfn);
- mutex_lock(&wq->flush_mutex);
+ mutex_lock(&wq->mutex);
spin_lock_irq(&pwq_lock);
/*
* Set the matching work_color. This is synchronized with
- * flush_mutex to avoid confusing flush_workqueue().
+ * wq->mutex to avoid confusing flush_workqueue().
*/
if (p_last_pwq)
*p_last_pwq = first_pwq(wq);
@@ -3645,7 +3645,7 @@ static void init_and_link_pwq(struct pool_workqueue *pwq,
list_add_rcu(&pwq->pwqs_node, &wq->pwqs);
spin_unlock_irq(&pwq_lock);
- mutex_unlock(&wq->flush_mutex);
+ mutex_unlock(&wq->mutex);
}
/**
@@ -3762,7 +3762,7 @@ struct workqueue_struct *__alloc_workqueue_key(const char *fmt,
/* init wq */
wq->flags = flags;
wq->saved_max_active = max_active;
- mutex_init(&wq->flush_mutex);
+ mutex_init(&wq->mutex);
atomic_set(&wq->nr_pwqs_to_flush, 0);
INIT_LIST_HEAD(&wq->pwqs);
INIT_LIST_HEAD(&wq->flusher_queue);
--
1.8.1.4
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 3/6] workqueue: protect wq->nr_drainers and ->flags with wq->mutex
2013-03-26 0:07 [PATCHSET wq/for-3.10] workqueue: replace pwq_lock with wq->mutex Tejun Heo
2013-03-26 0:07 ` [PATCH 1/6] workqueue: rename wq_mutex to wq_pool_mutex Tejun Heo
2013-03-26 0:07 ` [PATCH 2/6] workqueue: rename wq->flush_mutex to wq->mutex Tejun Heo
@ 2013-03-26 0:07 ` Tejun Heo
2013-03-26 0:07 ` [PATCH 4/6] workqueue: protect wq->pwqs and iteration " Tejun Heo
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Tejun Heo @ 2013-03-26 0:07 UTC (permalink / raw)
To: laijs; +Cc: linux-kernel, js1304, Tejun Heo
From: Lai Jiangshan <laijs@cn.fujitsu.com>
We're expanding wq->mutex to cover all fields specific to each
workqueue with the end goal of replacing pwq_lock which will make
locking simpler and easier to understand.
wq->nr_drainers and ->flags are specific to each workqueue. Protect
->nr_drainers and ->flags with wq->mutex instead of pool_mutex.
tj: Rebased on top of the current dev branch. Updated description.
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
---
kernel/workqueue.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index d448eda..3ac2c4d 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -226,7 +226,7 @@ struct wq_device;
* the appropriate worker_pool through its pool_workqueues.
*/
struct workqueue_struct {
- unsigned int flags; /* PL: WQ_* flags */
+ unsigned int flags; /* WQ: WQ_* flags */
struct pool_workqueue __percpu *cpu_pwqs; /* I: per-cpu pwq's */
struct list_head pwqs; /* WR: all pwqs of this wq */
struct list_head list; /* PL: list of all workqueues */
@@ -242,7 +242,7 @@ struct workqueue_struct {
struct list_head maydays; /* MD: pwqs requesting rescue */
struct worker *rescuer; /* I: rescue worker */
- int nr_drainers; /* PL: drain in progress */
+ int nr_drainers; /* WQ: drain in progress */
int saved_max_active; /* PW: saved pwq max_active */
#ifdef CONFIG_SYSFS
@@ -2684,10 +2684,10 @@ void drain_workqueue(struct workqueue_struct *wq)
* hotter than drain_workqueue() and already looks at @wq->flags.
* Use __WQ_DRAINING so that queue doesn't have to check nr_drainers.
*/
- mutex_lock(&wq_pool_mutex);
+ mutex_lock(&wq->mutex);
if (!wq->nr_drainers++)
wq->flags |= __WQ_DRAINING;
- mutex_unlock(&wq_pool_mutex);
+ mutex_unlock(&wq->mutex);
reflush:
flush_workqueue(wq);
@@ -2714,10 +2714,10 @@ reflush:
local_irq_enable();
- mutex_lock(&wq_pool_mutex);
+ mutex_lock(&wq->mutex);
if (!--wq->nr_drainers)
wq->flags &= ~__WQ_DRAINING;
- mutex_unlock(&wq_pool_mutex);
+ mutex_unlock(&wq->mutex);
}
EXPORT_SYMBOL_GPL(drain_workqueue);
--
1.8.1.4
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 4/6] workqueue: protect wq->pwqs and iteration with wq->mutex
2013-03-26 0:07 [PATCHSET wq/for-3.10] workqueue: replace pwq_lock with wq->mutex Tejun Heo
` (2 preceding siblings ...)
2013-03-26 0:07 ` [PATCH 3/6] workqueue: protect wq->nr_drainers and ->flags with wq->mutex Tejun Heo
@ 2013-03-26 0:07 ` Tejun Heo
2013-03-26 0:07 ` [PATCH 5/6] workqueue: protect wq->saved_max_active " Tejun Heo
2013-03-26 0:07 ` [PATCH 6/6] workqueue: remove pwq_lock which is no longer used Tejun Heo
5 siblings, 0 replies; 7+ messages in thread
From: Tejun Heo @ 2013-03-26 0:07 UTC (permalink / raw)
To: laijs; +Cc: linux-kernel, js1304, Tejun Heo
From: Lai Jiangshan <laijs@cn.fujitsu.com>
We're expanding wq->mutex to cover all fields specific to each
workqueue with the end goal of replacing pwq_lock which will make
locking simpler and easier to understand.
init_and_link_pwq() and pwq_unbound_release_workfn() already grab
wq->mutex when adding or removing a pwq from wq->pwqs list. This
patch makes it official that the list is wq->mutex protected for
writes and updates readers accoridingly. Explicit IRQ toggles for
sched-RCU read-locking in flush_workqueue_prep_pwqs() and
drain_workqueues() are removed as the surrounding wq->mutex can
provide sufficient synchronization.
Also, assert_rcu_or_pwq_lock() is renamed to assert_rcu_or_wq_mutex()
and checks for wq->mutex too.
pwq_lock locking and assertion are not removed by this patch and a
couple of for_each_pwq() iterations are still protected by it.
They'll be removed by future patches.
tj: Rebased on top of the current dev branch. Updated description.
Folded in assert_rcu_or_wq_mutex() renaming from a later patch
along with associated comment updates.
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
---
kernel/workqueue.c | 42 ++++++++++++++++++------------------------
1 file changed, 18 insertions(+), 24 deletions(-)
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 3ac2c4d..9c32fd1 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -204,7 +204,7 @@ struct pool_workqueue {
* Release of unbound pwq is punted to system_wq. See put_pwq()
* and pwq_unbound_release_workfn() for details. pool_workqueue
* itself is also sched-RCU protected so that the first pwq can be
- * determined without grabbing pwq_lock.
+ * determined without grabbing wq->mutex.
*/
struct work_struct unbound_release_work;
struct rcu_head rcu;
@@ -298,10 +298,11 @@ static void copy_workqueue_attrs(struct workqueue_attrs *to,
lockdep_is_held(&wq_pool_mutex), \
"sched RCU or wq_pool_mutex should be held")
-#define assert_rcu_or_pwq_lock() \
+#define assert_rcu_or_wq_mutex(wq) \
rcu_lockdep_assert(rcu_read_lock_sched_held() || \
+ lockdep_is_held(&wq->mutex) || \
lockdep_is_held(&pwq_lock), \
- "sched RCU or pwq_lock should be held")
+ "sched RCU or wq->mutex should be held")
#ifdef CONFIG_LOCKDEP
#define assert_manager_or_pool_lock(pool) \
@@ -356,7 +357,7 @@ static void copy_workqueue_attrs(struct workqueue_attrs *to,
* @pwq: iteration cursor
* @wq: the target workqueue
*
- * This must be called either with pwq_lock held or sched RCU read locked.
+ * This must be called either with wq->mutex held or sched RCU read locked.
* If the pwq needs to be used beyond the locking in effect, the caller is
* responsible for guaranteeing that the pwq stays online.
*
@@ -365,7 +366,7 @@ static void copy_workqueue_attrs(struct workqueue_attrs *to,
*/
#define for_each_pwq(pwq, wq) \
list_for_each_entry_rcu((pwq), &(wq)->pwqs, pwqs_node) \
- if (({ assert_rcu_or_pwq_lock(); false; })) { } \
+ if (({ assert_rcu_or_wq_mutex(wq); false; })) { } \
else
#ifdef CONFIG_DEBUG_OBJECTS_WORK
@@ -504,13 +505,13 @@ static int worker_pool_assign_id(struct worker_pool *pool)
* first_pwq - return the first pool_workqueue of the specified workqueue
* @wq: the target workqueue
*
- * This must be called either with pwq_lock held or sched RCU read locked.
+ * This must be called either with wq->mutex held or sched RCU read locked.
* If the pwq needs to be used beyond the locking in effect, the caller is
* responsible for guaranteeing that the pwq stays online.
*/
static struct pool_workqueue *first_pwq(struct workqueue_struct *wq)
{
- assert_rcu_or_pwq_lock();
+ assert_rcu_or_wq_mutex(wq);
return list_first_or_null_rcu(&wq->pwqs, struct pool_workqueue,
pwqs_node);
}
@@ -2477,12 +2478,10 @@ static bool flush_workqueue_prep_pwqs(struct workqueue_struct *wq,
atomic_set(&wq->nr_pwqs_to_flush, 1);
}
- local_irq_disable();
-
for_each_pwq(pwq, wq) {
struct worker_pool *pool = pwq->pool;
- spin_lock(&pool->lock);
+ spin_lock_irq(&pool->lock);
if (flush_color >= 0) {
WARN_ON_ONCE(pwq->flush_color != -1);
@@ -2499,11 +2498,9 @@ static bool flush_workqueue_prep_pwqs(struct workqueue_struct *wq,
pwq->work_color = work_color;
}
- spin_unlock(&pool->lock);
+ spin_unlock_irq(&pool->lock);
}
- local_irq_enable();
-
if (flush_color >= 0 && atomic_dec_and_test(&wq->nr_pwqs_to_flush))
complete(&wq->first_flusher->done);
@@ -2691,14 +2688,14 @@ void drain_workqueue(struct workqueue_struct *wq)
reflush:
flush_workqueue(wq);
- local_irq_disable();
+ mutex_lock(&wq->mutex);
for_each_pwq(pwq, wq) {
bool drained;
- spin_lock(&pwq->pool->lock);
+ spin_lock_irq(&pwq->pool->lock);
drained = !pwq->nr_active && list_empty(&pwq->delayed_works);
- spin_unlock(&pwq->pool->lock);
+ spin_unlock_irq(&pwq->pool->lock);
if (drained)
continue;
@@ -2708,13 +2705,10 @@ reflush:
pr_warn("workqueue %s: drain_workqueue() isn't complete after %u tries\n",
wq->name, flush_cnt);
- local_irq_enable();
+ mutex_unlock(&wq->mutex);
goto reflush;
}
- local_irq_enable();
-
- mutex_lock(&wq->mutex);
if (!--wq->nr_drainers)
wq->flags &= ~__WQ_DRAINING;
mutex_unlock(&wq->mutex);
@@ -3843,13 +3837,13 @@ void destroy_workqueue(struct workqueue_struct *wq)
drain_workqueue(wq);
/* sanity checks */
- spin_lock_irq(&pwq_lock);
+ mutex_lock(&wq->mutex);
for_each_pwq(pwq, wq) {
int i;
for (i = 0; i < WORK_NR_COLORS; i++) {
if (WARN_ON(pwq->nr_in_flight[i])) {
- spin_unlock_irq(&pwq_lock);
+ mutex_unlock(&wq->mutex);
return;
}
}
@@ -3857,11 +3851,11 @@ void destroy_workqueue(struct workqueue_struct *wq)
if (WARN_ON(pwq->refcnt > 1) ||
WARN_ON(pwq->nr_active) ||
WARN_ON(!list_empty(&pwq->delayed_works))) {
- spin_unlock_irq(&pwq_lock);
+ mutex_unlock(&wq->mutex);
return;
}
}
- spin_unlock_irq(&pwq_lock);
+ mutex_unlock(&wq->mutex);
/*
* wq list is used to freeze wq, remove from list after
--
1.8.1.4
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 5/6] workqueue: protect wq->saved_max_active with wq->mutex
2013-03-26 0:07 [PATCHSET wq/for-3.10] workqueue: replace pwq_lock with wq->mutex Tejun Heo
` (3 preceding siblings ...)
2013-03-26 0:07 ` [PATCH 4/6] workqueue: protect wq->pwqs and iteration " Tejun Heo
@ 2013-03-26 0:07 ` Tejun Heo
2013-03-26 0:07 ` [PATCH 6/6] workqueue: remove pwq_lock which is no longer used Tejun Heo
5 siblings, 0 replies; 7+ messages in thread
From: Tejun Heo @ 2013-03-26 0:07 UTC (permalink / raw)
To: laijs; +Cc: linux-kernel, js1304, Tejun Heo
From: Lai Jiangshan <laijs@cn.fujitsu.com>
We're expanding wq->mutex to cover all fields specific to each
workqueue with the end goal of replacing pwq_lock which will make
locking simpler and easier to understand.
This patch makes wq->saved_max_active protected by wq->mutex instead
of pwq_lock. As pwq_lock locking around pwq_adjust_mac_active() is no
longer necessary, this patch also replaces pwq_lock lockings of
for_each_pwq() around pwq_adjust_max_active() to wq->mutex.
tj: Rebased on top of the current dev branch. Updated description.
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
---
kernel/workqueue.c | 33 ++++++++++++++++-----------------
1 file changed, 16 insertions(+), 17 deletions(-)
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 9c32fd1..af6087a 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -243,7 +243,7 @@ struct workqueue_struct {
struct worker *rescuer; /* I: rescue worker */
int nr_drainers; /* WQ: drain in progress */
- int saved_max_active; /* PW: saved pwq max_active */
+ int saved_max_active; /* WQ: saved pwq max_active */
#ifdef CONFIG_SYSFS
struct wq_device *wq_dev; /* I: for sysfs interface */
@@ -3579,13 +3579,13 @@ static void pwq_adjust_max_active(struct pool_workqueue *pwq)
bool freezable = wq->flags & WQ_FREEZABLE;
/* for @wq->saved_max_active */
- lockdep_assert_held(&pwq_lock);
+ lockdep_assert_held(&wq->mutex);
/* fast exit for non-freezable wqs */
if (!freezable && pwq->max_active == wq->saved_max_active)
return;
- spin_lock(&pwq->pool->lock);
+ spin_lock_irq(&pwq->pool->lock);
if (!freezable || !(pwq->pool->flags & POOL_FREEZING)) {
pwq->max_active = wq->saved_max_active;
@@ -3603,7 +3603,7 @@ static void pwq_adjust_max_active(struct pool_workqueue *pwq)
pwq->max_active = 0;
}
- spin_unlock(&pwq->pool->lock);
+ spin_unlock_irq(&pwq->pool->lock);
}
static void init_and_link_pwq(struct pool_workqueue *pwq,
@@ -3622,7 +3622,6 @@ static void init_and_link_pwq(struct pool_workqueue *pwq,
INIT_WORK(&pwq->unbound_release_work, pwq_unbound_release_workfn);
mutex_lock(&wq->mutex);
- spin_lock_irq(&pwq_lock);
/*
* Set the matching work_color. This is synchronized with
@@ -3636,9 +3635,10 @@ static void init_and_link_pwq(struct pool_workqueue *pwq,
pwq_adjust_max_active(pwq);
/* link in @pwq */
+ spin_lock_irq(&pwq_lock);
list_add_rcu(&pwq->pwqs_node, &wq->pwqs);
-
spin_unlock_irq(&pwq_lock);
+
mutex_unlock(&wq->mutex);
}
@@ -3803,10 +3803,10 @@ struct workqueue_struct *__alloc_workqueue_key(const char *fmt,
*/
mutex_lock(&wq_pool_mutex);
- spin_lock_irq(&pwq_lock);
+ mutex_lock(&wq->mutex);
for_each_pwq(pwq, wq)
pwq_adjust_max_active(pwq);
- spin_unlock_irq(&pwq_lock);
+ mutex_unlock(&wq->mutex);
list_add(&wq->list, &workqueues);
@@ -3917,14 +3917,14 @@ void workqueue_set_max_active(struct workqueue_struct *wq, int max_active)
max_active = wq_clamp_max_active(max_active, wq->flags, wq->name);
- spin_lock_irq(&pwq_lock);
+ mutex_lock(&wq->mutex);
wq->saved_max_active = max_active;
for_each_pwq(pwq, wq)
pwq_adjust_max_active(pwq);
- spin_unlock_irq(&pwq_lock);
+ mutex_unlock(&wq->mutex);
}
EXPORT_SYMBOL_GPL(workqueue_set_max_active);
@@ -4287,7 +4287,7 @@ EXPORT_SYMBOL_GPL(work_on_cpu);
* pool->worklist.
*
* CONTEXT:
- * Grabs and releases wq_pool_mutex, pwq_lock and pool->lock's.
+ * Grabs and releases wq_pool_mutex, wq->mutex and pool->lock's.
*/
void freeze_workqueues_begin(void)
{
@@ -4309,13 +4309,12 @@ void freeze_workqueues_begin(void)
spin_unlock_irq(&pool->lock);
}
- /* suppress further executions by setting max_active to zero */
- spin_lock_irq(&pwq_lock);
list_for_each_entry(wq, &workqueues, list) {
+ mutex_lock(&wq->mutex);
for_each_pwq(pwq, wq)
pwq_adjust_max_active(pwq);
+ mutex_unlock(&wq->mutex);
}
- spin_unlock_irq(&pwq_lock);
mutex_unlock(&wq_pool_mutex);
}
@@ -4373,7 +4372,7 @@ out_unlock:
* frozen works are transferred to their respective pool worklists.
*
* CONTEXT:
- * Grabs and releases wq_pool_mutex, pwq_lock and pool->lock's.
+ * Grabs and releases wq_pool_mutex, wq->mutex and pool->lock's.
*/
void thaw_workqueues(void)
{
@@ -4396,12 +4395,12 @@ void thaw_workqueues(void)
}
/* restore max_active and repopulate worklist */
- spin_lock_irq(&pwq_lock);
list_for_each_entry(wq, &workqueues, list) {
+ mutex_lock(&wq->mutex);
for_each_pwq(pwq, wq)
pwq_adjust_max_active(pwq);
+ mutex_unlock(&wq->mutex);
}
- spin_unlock_irq(&pwq_lock);
workqueue_freezing = false;
out_unlock:
--
1.8.1.4
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 6/6] workqueue: remove pwq_lock which is no longer used
2013-03-26 0:07 [PATCHSET wq/for-3.10] workqueue: replace pwq_lock with wq->mutex Tejun Heo
` (4 preceding siblings ...)
2013-03-26 0:07 ` [PATCH 5/6] workqueue: protect wq->saved_max_active " Tejun Heo
@ 2013-03-26 0:07 ` Tejun Heo
5 siblings, 0 replies; 7+ messages in thread
From: Tejun Heo @ 2013-03-26 0:07 UTC (permalink / raw)
To: laijs; +Cc: linux-kernel, js1304, Tejun Heo
From: Lai Jiangshan <laijs@cn.fujitsu.com>
To simplify locking, the previous patches expanded wq->mutex to
protect all fields of each workqueue instance including the pwqs list
leaving pwq_lock without any user. Remove the unused pwq_lock.
tj: Rebased on top of the current dev branch. Updated description.
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
---
kernel/workqueue.c | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index af6087a..04a8b98 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -125,12 +125,9 @@ enum {
*
* PR: wq_pool_mutex protected for writes. Sched-RCU protected for reads.
*
- * PW: pwq_lock protected.
- *
* WQ: wq->mutex protected.
*
- * WR: wq->mutex and pwq_lock protected for writes. Sched-RCU protected
- * for reads.
+ * WR: wq->mutex protected for writes. Sched-RCU protected for reads.
*
* MD: wq_mayday_lock protected.
*/
@@ -257,7 +254,6 @@ struct workqueue_struct {
static struct kmem_cache *pwq_cache;
static DEFINE_MUTEX(wq_pool_mutex); /* protects pools and workqueues list */
-static DEFINE_SPINLOCK(pwq_lock); /* protects pool_workqueues */
static DEFINE_SPINLOCK(wq_mayday_lock); /* protects wq->maydays list */
static LIST_HEAD(workqueues); /* PL: list of all workqueues */
@@ -300,8 +296,7 @@ static void copy_workqueue_attrs(struct workqueue_attrs *to,
#define assert_rcu_or_wq_mutex(wq) \
rcu_lockdep_assert(rcu_read_lock_sched_held() || \
- lockdep_is_held(&wq->mutex) || \
- lockdep_is_held(&pwq_lock), \
+ lockdep_is_held(&wq->mutex), \
"sched RCU or wq->mutex should be held")
#ifdef CONFIG_LOCKDEP
@@ -3549,9 +3544,7 @@ static void pwq_unbound_release_workfn(struct work_struct *work)
* and consistent with the linking path.
*/
mutex_lock(&wq->mutex);
- spin_lock_irq(&pwq_lock);
list_del_rcu(&pwq->pwqs_node);
- spin_unlock_irq(&pwq_lock);
mutex_unlock(&wq->mutex);
put_unbound_pool(pool);
@@ -3635,9 +3628,7 @@ static void init_and_link_pwq(struct pool_workqueue *pwq,
pwq_adjust_max_active(pwq);
/* link in @pwq */
- spin_lock_irq(&pwq_lock);
list_add_rcu(&pwq->pwqs_node, &wq->pwqs);
- spin_unlock_irq(&pwq_lock);
mutex_unlock(&wq->mutex);
}
--
1.8.1.4
^ permalink raw reply [flat|nested] 7+ messages in thread