[PATCHSET] workqueue: prepare for concurrency managed workqueue, take#2
Tejun Heo
This patchset is the second take of prep-for-concurrent-managed-wq
patchset. Changes from the last take[L] are
* sched_notifier implementation redone such that it's more similar to
preempt_notifier and activate/deactivate ops are replaced with
wakeup/sleep.
I thought about doing state based callback[s] - three callbacks for
the three states - run, ready, sleep but it makes things more
complicated for the caller and there are only four transition edges
among the states anyway.
Another alternative to the current wakeup/sleep/in/out would be
wakeup/sleep/run/preempted where the only difference would be
preempted is not called after sleep is called while out is called
even after sleep is called. Either is not much better than the
other and the current use case fits in/out better, so I left it as
was.
* cpu_populated_map is killed but single thread implementation is now
left largely unchanged. I'll follow up with further patches on
this.
* force_cpus_allowed_ptr() renamed to force_cpus_allowed().
* Locking bug in introduce-worker fixed.
This patchset contains the following patches.
0001-sched-kvm-fix-race-condition-involving-sched_in_pree.patch
0002-workqueue-Add-debugobjects-support.patch
0003-sched-rename-preempt_notifier-to-sched_notifier-and-.patch
0004-sched-update-sched_notifier-and-add-wakeup-sleep-not.patch
0005-sched-implement-sched_notifier_wake_up_process.patch
0006-scheduler-implement-force_cpus_allowed.patch
0007-acpi-use-queue_work_on-instead-of-binding-workqueue-.patch
0008-stop_machine-reimplement-without-using-workqueue.patch
0009-workqueue-misc-cosmetic-updates.patch
0010-workqueue-merge-feature-parametesr-into-flags.patch
0011-workqueue-update-cwq-alignement-and-make-one-more-fl.patch
0012-workqueue-define-both-bit-position-and-mask-for-work.patch
0013-workqueue-separate-out-process_one_work.patch
0014-workqueue-temporarily-disable-workqueue-tracing.patch
0015-workqueue-kill-cpu_populated_map.patch
0016-workqueue-reimplement-workqueue-flushing-using-color.patch
0017-workqueue-introduce-worker.patch
0018-workqueue-reimplement-work-flushing-using-linked-wor.patch
0019-workqueue-reimplement-workqueue-freeze-using-cwq-fro.patch
If there's no objection (especially the scheduler notifier part), I
want to push the patches into linux-next and wait a while before
pushing concurrency managed workqueue. This prep patchset already
contains a lot of changes so I think it would be helpful to verify
things are okay at this point.
This patchset is available in the following git tree.
git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git review-wq-prep
Please note that the above tree is temporary. It needs to be rebased
once the fix patches appear upstream.
This patchset contains the following changes.
arch/ia64/kvm/Kconfig | 1
arch/powerpc/kvm/Kconfig | 1
arch/s390/kvm/Kconfig | 1
arch/x86/kernel/smpboot.c | 4
arch/x86/kvm/Kconfig | 1
drivers/acpi/osl.c | 41 -
include/linux/kvm_host.h | 4
include/linux/preempt.h | 43 -
include/linux/sched.h | 58 +
include/linux/stop_machine.h | 6
include/linux/workqueue.h | 110 ++-
init/Kconfig | 4
init/main.c | 2
kernel/power/process.c | 22
kernel/sched.c | 194 +++---
kernel/stop_machine.c | 151 +++-
kernel/trace/Kconfig | 4
kernel/workqueue.c | 1362 +++++++++++++++++++++++++++++++++----------
lib/Kconfig.debug | 8
virt/kvm/kvm_main.c | 28
20 files changed, 1507 insertions(+), 538 deletions(-)
Thanks.
--
tejun
[L] http://thread.gmane.org/gmane.linux.kernel/914545
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Tejun Heo
is temporary measure till concurrency managed workqueue is complete.
Signed-off-by: Tejun Heo <t...@kernel.org>
---
kernel/trace/Kconfig | 4 +++-
kernel/workqueue.c | 14 +++-----------
2 files changed, 6 insertions(+), 12 deletions(-)
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index b416512..0e14ecf 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -393,7 +393,9 @@ config KMEMTRACE
If unsure, say N.
config WORKQUEUE_TRACER
- bool "Trace workqueues"
+# Temporarily disabled during workqueue reimplementation
+# bool "Trace workqueues"
+ def_bool n
select GENERIC_TRACER
help
The workqueue tracer provides some statistical informations
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index d8ecc40..5392939 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -33,8 +33,6 @@
#include <linux/kallsyms.h>
#include <linux/debug_locks.h>
#include <linux/lockdep.h>
-#define CREATE_TRACE_POINTS
-#include <trace/events/workqueue.h>
/*
* Structure fields follow one of the following exclusion rules.
@@ -238,10 +236,10 @@ static inline void set_wq_data(struct work_struct *work,
WORK_STRUCT_PENDING | extra_flags);
}
-static inline
-struct cpu_workqueue_struct *get_wq_data(struct work_struct *work)
+static inline struct cpu_workqueue_struct *get_wq_data(struct work_struct *work)
{
- return (void *) (atomic_long_read(&work->data) & WORK_STRUCT_WQ_DATA_MASK);
+ return (void *)(atomic_long_read(&work->data) &
+ WORK_STRUCT_WQ_DATA_MASK);
}
/**
@@ -260,8 +258,6 @@ static void insert_work(struct cpu_workqueue_struct *cwq,
struct work_struct *work, struct list_head *head,
unsigned int extra_flags)
{
- trace_workqueue_insertion(cwq->thread, work);
-
/* we own @work, set data and link */
set_wq_data(work, cwq, extra_flags);
@@ -426,7 +422,6 @@ static void process_one_work(struct cpu_workqueue_struct *cwq,
struct lockdep_map lockdep_map = work->lockdep_map;
#endif
/* claim and process */
- trace_workqueue_execution(cwq->thread, work);
debug_work_deactivate(work);
cwq->current_work = work;
list_del_init(&work->entry);
@@ -987,8 +982,6 @@ static int create_workqueue_thread(struct cpu_workqueue_struct *cwq, int cpu)
return PTR_ERR(p);
cwq->thread = p;
- trace_workqueue_creation(cwq->thread, cpu);
-
return 0;
}
@@ -1093,7 +1086,6 @@ static void cleanup_workqueue_thread(struct cpu_workqueue_struct *cwq)
* checks list_empty(), and a "normal" queue_work() can't use
* a dead CPU.
*/
- trace_workqueue_destruction(cwq->thread);
kthread_stop(cwq->thread);
cwq->thread = NULL;
}
--
1.6.4.2
Tejun Heo
work_struct->data are considered to be available and used as flags.
Make the alignement requirement official by defining
WORK_STRUCT_FLAG_BITS and aligning cwqs to two's power of it.
This is in preparation of concurrency managed workqueue and cwqs being
aligned to cacheline wouldn't matter as much. While at it, this patch
reserves one more bit for work flags and make sure the resulting
alignment is at least equal to or larger than that of long long.
Signed-off-by: Tejun Heo <t...@kernel.org>
---
include/linux/workqueue.h | 19 +++++++++++++++----
kernel/workqueue.c | 15 +++++++++++++--
2 files changed, 28 insertions(+), 6 deletions(-)
diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index 495572a..5ff8c44 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -22,12 +22,23 @@ typedef void (*work_func_t)(struct work_struct *work);
*/
#define work_data_bits(work) ((unsigned long *)(&(work)->data))
+enum {
+ WORK_STRUCT_PENDING = 0, /* work item is pending execution */
+ WORK_STRUCT_STATIC = 1, /* static initializer (debugobjects) */
+
+ /*
+ * Reserve 3bits off of cwq pointer. This is enough and
+ * provides acceptable alignment on both 32 and 64bit
+ * machines.
+ */
+ WORK_STRUCT_FLAG_BITS = 3,
+
+ WORK_STRUCT_FLAG_MASK = (1UL << WORK_STRUCT_FLAG_BITS) - 1,
+ WORK_STRUCT_WQ_DATA_MASK = ~WORK_STRUCT_FLAG_MASK,
+};
+
struct work_struct {
atomic_long_t data;
-#define WORK_STRUCT_PENDING 0 /* T if work item pending execution */
-#define WORK_STRUCT_STATIC 1 /* static initializer (debugobjects) */
-#define WORK_STRUCT_FLAG_MASK (3UL)
-#define WORK_STRUCT_WQ_DATA_MASK (~WORK_STRUCT_FLAG_MASK)
struct list_head entry;
work_func_t func;
#ifdef CONFIG_LOCKDEP
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 579041f..f30977f 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -48,7 +48,9 @@
/*
* The per-CPU workqueue (if single thread, we always use the first
- * possible cpu).
+ * possible cpu). The lower WORK_STRUCT_FLAG_BITS of
+ * work_struct->data are used for flags and thus cwqs need to be
+ * aligned at two's power of the number of flag bits.
*/
struct cpu_workqueue_struct {
@@ -60,7 +62,7 @@ struct cpu_workqueue_struct {
struct workqueue_struct *wq; /* I: the owning workqueue */
struct task_struct *thread;
-} ____cacheline_aligned;
+} __attribute__((aligned(1 << WORK_STRUCT_FLAG_BITS)));
/*
* The externally visible workqueue abstraction is an array of
@@ -1198,6 +1200,15 @@ EXPORT_SYMBOL_GPL(work_on_cpu);
void __init init_workqueues(void)
{
+ /*
+ * cwqs are forced aligned according to WORK_STRUCT_FLAG_BITS.
+ * Make sure that the alignment isn't lower than that of
+ * unsigned long long in case this code survives for longer
+ * than twenty years. :-P
+ */
+ BUILD_BUG_ON(__alignof__(struct cpu_workqueue_struct) <
+ __alignof__(unsigned long long));
+
alloc_cpumask_var(&cpu_populated_map, GFP_KERNEL);
cpumask_copy(cpu_populated_map, cpu_online_mask);
--
1.6.4.2
Tejun Heo
struct cpu_workqueue_struct and implement helper functions to deal
with the new struct worker. The only change which is visible outside
is that now workqueue worker are all named "kworker/CPUID:WORKERID"
where WORKERID is allocated from per-cpu ida.
This is in preparation of concurrency managed workqueue where shared
multiple workers would be available per cpu.
Signed-off-by: Tejun Heo <t...@kernel.org>
---
kernel/workqueue.c | 220 +++++++++++++++++++++++++++++++++++++---------------
1 files changed, 157 insertions(+), 63 deletions(-)
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index dcb1c4b..4988e4d 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -33,6 +33,7 @@
#include <linux/kallsyms.h>
#include <linux/debug_locks.h>
#include <linux/lockdep.h>
+#include <linux/idr.h>
/*
* Structure fields follow one of the following exclusion rules.
@@ -46,6 +47,15 @@
* W: workqueue_lock protected.
*/
+struct cpu_workqueue_struct;
+
+struct worker {
+ struct work_struct *current_work; /* L: work being processed */
+ struct task_struct *task; /* I: worker task */
+ struct cpu_workqueue_struct *cwq; /* I: the associated cwq */
+ int id; /* I: worker id */
+};
+
/*
* The per-CPU workqueue (if single thread, we always use the first
* possible cpu). The lower WORK_STRUCT_FLAG_BITS of
@@ -58,14 +68,14 @@ struct cpu_workqueue_struct {
struct list_head worklist;
wait_queue_head_t more_work;
- struct work_struct *current_work;
+ unsigned int cpu;
+ struct worker *worker;
struct workqueue_struct *wq; /* I: the owning workqueue */
int work_color; /* L: current color */
int flush_color; /* L: flushing color */
int nr_in_flight[WORK_COLORS];
/* L: nr of in_flight works */
- struct task_struct *thread;
} __attribute__((aligned(1 << WORK_STRUCT_FLAG_BITS)));
/*
@@ -213,6 +223,9 @@ static inline void debug_work_deactivate(struct work_struct *work) { }
/* Serializes the accesses to the list of workqueues. */
static DEFINE_SPINLOCK(workqueue_lock);
static LIST_HEAD(workqueues);
+static DEFINE_PER_CPU(struct ida, worker_ida);
+
+static int worker_thread(void *__worker);
static int singlethread_cpu __read_mostly;
@@ -427,6 +440,105 @@ int queue_delayed_work_on(int cpu, struct workqueue_struct *wq,
}
EXPORT_SYMBOL_GPL(queue_delayed_work_on);
+static struct worker *alloc_worker(void)
+{
+ struct worker *worker;
+
+ worker = kzalloc(sizeof(*worker), GFP_KERNEL);
+ return worker;
+}
+
+/**
+ * create_worker - create a new workqueue worker
+ * @cwq: cwq the new worker will belong to
+ * @bind: whether to set affinity to @cpu or not
+ *
+ * Create a new worker which is bound to @cwq. The returned worker
+ * can be started by calling start_worker() or destroyed using
+ * destroy_worker().
+ *
+ * CONTEXT:
+ * Might sleep. Does GFP_KERNEL allocations.
+ *
+ * RETURNS:
+ * Pointer to the newly created worker.
+ */
+static struct worker *create_worker(struct cpu_workqueue_struct *cwq, bool bind)
+{
+ int id = -1;
+ struct worker *worker = NULL;
+
+ spin_lock(&workqueue_lock);
+ while (ida_get_new(&per_cpu(worker_ida, cwq->cpu), &id)) {
+ spin_unlock(&workqueue_lock);
+ if (!ida_pre_get(&per_cpu(worker_ida, cwq->cpu), GFP_KERNEL))
+ goto fail;
+ spin_lock(&workqueue_lock);
+ }
+ spin_unlock(&workqueue_lock);
+
+ worker = alloc_worker();
+ if (!worker)
+ goto fail;
+
+ worker->cwq = cwq;
+ worker->id = id;
+
+ worker->task = kthread_create(worker_thread, worker, "kworker/%u:%d",
+ cwq->cpu, id);
+ if (IS_ERR(worker->task))
+ goto fail;
+
+ if (bind)
+ kthread_bind(worker->task, cwq->cpu);
+
+ return worker;
+fail:
+ if (id >= 0) {
+ spin_lock(&workqueue_lock);
+ ida_remove(&per_cpu(worker_ida, cwq->cpu), id);
+ spin_unlock(&workqueue_lock);
+ }
+ kfree(worker);
+ return NULL;
+}
+
+/**
+ * start_worker - start a newly created worker
+ * @worker: worker to start
+ *
+ * Start @worker.
+ *
+ * CONTEXT:
+ * spin_lock_irq(cwq->lock).
+ */
+static void start_worker(struct worker *worker)
+{
+ wake_up_process(worker->task);
+}
+
+/**
+ * destroy_worker - destroy a workqueue worker
+ * @worker: worker to be destroyed
+ *
+ * Destroy @worker.
+ */
+static void destroy_worker(struct worker *worker)
+{
+ int cpu = worker->cwq->cpu;
+ int id = worker->id;
+
+ /* sanity check frenzy */
+ BUG_ON(worker->current_work);
+
+ kthread_stop(worker->task);
+ kfree(worker);
+
+ spin_lock(&workqueue_lock);
+ ida_remove(&per_cpu(worker_ida, cpu), id);
+ spin_unlock(&workqueue_lock);
+}
+
/**
* cwq_dec_nr_in_flight - decrement cwq's nr_in_flight
* @cwq: cwq of interest
@@ -464,7 +576,7 @@ static void cwq_dec_nr_in_flight(struct cpu_workqueue_struct *cwq,
/**
* process_one_work - process single work
- * @cwq: cwq to process work for
+ * @worker: self
* @work: work to process
*
* Process @work. This function contains all the logics necessary to
@@ -476,9 +588,9 @@ static void cwq_dec_nr_in_flight(struct cpu_workqueue_struct *cwq,
* CONTEXT:
* spin_lock_irq(cwq->lock) which is released and regrabbed.
*/
-static void process_one_work(struct cpu_workqueue_struct *cwq,
- struct work_struct *work)
+static void process_one_work(struct worker *worker, struct work_struct *work)
{
+ struct cpu_workqueue_struct *cwq = worker->cwq;
work_func_t f = work->func;
int work_color;
#ifdef CONFIG_LOCKDEP
@@ -493,7 +605,7 @@ static void process_one_work(struct cpu_workqueue_struct *cwq,
#endif
/* claim and process */
debug_work_deactivate(work);
- cwq->current_work = work;
+ worker->current_work = work;
work_color = work_flags_to_color(*work_data_bits(work));
list_del_init(&work->entry);
@@ -520,30 +632,33 @@ static void process_one_work(struct cpu_workqueue_struct *cwq,
spin_lock_irq(&cwq->lock);
/* we're done with it, release */
- cwq->current_work = NULL;
+ worker->current_work = NULL;
cwq_dec_nr_in_flight(cwq, work_color);
}
-static void run_workqueue(struct cpu_workqueue_struct *cwq)
+static void run_workqueue(struct worker *worker)
{
+ struct cpu_workqueue_struct *cwq = worker->cwq;
+
spin_lock_irq(&cwq->lock);
while (!list_empty(&cwq->worklist)) {
struct work_struct *work = list_entry(cwq->worklist.next,
struct work_struct, entry);
- process_one_work(cwq, work);
+ process_one_work(worker, work);
}
spin_unlock_irq(&cwq->lock);
}
/**
* worker_thread - the worker thread function
- * @__cwq: cwq to serve
+ * @__worker: self
*
* The cwq worker thread function.
*/
-static int worker_thread(void *__cwq)
+static int worker_thread(void *__worker)
{
- struct cpu_workqueue_struct *cwq = __cwq;
+ struct worker *worker = __worker;
+ struct cpu_workqueue_struct *cwq = worker->cwq;
DEFINE_WAIT(wait);
if (cwq->wq->flags & WQ_FREEZEABLE)
@@ -562,7 +677,7 @@ static int worker_thread(void *__cwq)
if (kthread_should_stop())
break;
- run_workqueue(cwq);
+ run_workqueue(worker);
}
return 0;
@@ -860,7 +975,7 @@ int flush_work(struct work_struct *work)
goto already_gone;
prev = &work->entry;
} else {
- if (cwq->current_work != work)
+ if (!cwq->worker || cwq->worker->current_work != work)
goto already_gone;
prev = &cwq->worklist;
}
@@ -925,7 +1040,7 @@ static void wait_on_cpu_work(struct cpu_workqueue_struct *cwq,
int running = 0;
spin_lock_irq(&cwq->lock);
- if (unlikely(cwq->current_work == work)) {
+ if (unlikely(cwq->worker && cwq->worker->current_work == work)) {
insert_wq_barrier(cwq, &barr, cwq->worklist.next);
running = 1;
}
@@ -1188,46 +1303,13 @@ int current_is_keventd(void)
BUG_ON(!keventd_wq);
cwq = get_cwq(cpu, keventd_wq);
- if (current == cwq->thread)
+ if (current == cwq->worker->task)
ret = 1;
return ret;
}
-static int create_workqueue_thread(struct cpu_workqueue_struct *cwq, int cpu)
-{
- struct workqueue_struct *wq = cwq->wq;
- const char *fmt = is_wq_single_threaded(wq) ? "%s" : "%s/%d";
- struct task_struct *p;
-
- p = kthread_create(worker_thread, cwq, fmt, wq->name, cpu);
- /*
- * Nobody can add the work_struct to this cwq,
- * if (caller is __create_workqueue)
- * nobody should see this wq
- * else // caller is CPU_UP_PREPARE
- * cpu is not on cpu_online_map
- * so we can abort safely.
- */
- if (IS_ERR(p))
- return PTR_ERR(p);
- cwq->thread = p;
-
- return 0;
-}
-
-static void start_workqueue_thread(struct cpu_workqueue_struct *cwq, int cpu)
-{
- struct task_struct *p = cwq->thread;
-
- if (p != NULL) {
- if (cpu >= 0)
- kthread_bind(p, cpu);
- wake_up_process(p);
- }
-}
-
struct workqueue_struct *__create_workqueue_key(const char *name,
unsigned int flags,
struct lock_class_key *key,
@@ -1235,7 +1317,8 @@ struct workqueue_struct *__create_workqueue_key(const char *name,
{
bool singlethread = flags & WQ_SINGLE_THREAD;
struct workqueue_struct *wq;
- int err = 0, cpu;
+ bool failed = false;
+ unsigned int cpu;
wq = kzalloc(sizeof(*wq), GFP_KERNEL);
if (!wq)
@@ -1273,22 +1356,26 @@ struct workqueue_struct *__create_workqueue_key(const char *name,
for_each_possible_cpu(cpu) {
struct cpu_workqueue_struct *cwq = get_cwq(cpu, wq);
+ cwq->cpu = cpu;
cwq->wq = wq;
cwq->flush_color = -1;
spin_lock_init(&cwq->lock);
INIT_LIST_HEAD(&cwq->worklist);
init_waitqueue_head(&cwq->more_work);
- if (err || !cpu_online(cpu))
+ if (failed || !cpu_online(cpu))
continue;
if (singlethread && cpu != singlethread_cpu)
continue;
- err = create_workqueue_thread(cwq, cpu);
- start_workqueue_thread(cwq, singlethread ? -1 : cpu);
+ cwq->worker = create_worker(cwq, !singlethread);
+ if (cwq->worker)
+ start_worker(cwq->worker);
+ else
+ failed = true;
}
cpu_maps_update_done();
- if (err) {
+ if (failed) {
destroy_workqueue(wq);
wq = NULL;
}
@@ -1324,9 +1411,9 @@ void destroy_workqueue(struct workqueue_struct *wq)
int i;
/* cpu_add_remove_lock protects cwq->thread */
- if (cwq->thread) {
- kthread_stop(cwq->thread);
- cwq->thread = NULL;
+ if (cwq->worker) {
+ destroy_worker(cwq->worker);
+ cwq->worker = NULL;
}
for (i = 0; i < WORK_COLORS; i++)
@@ -1360,7 +1447,8 @@ undo:
switch (action) {
case CPU_UP_PREPARE:
- if (!create_workqueue_thread(cwq, cpu))
+ cwq->worker = create_worker(cwq, false);
+ if (cwq->worker)
break;
printk(KERN_ERR "workqueue [%s] for %i failed\n",
wq->name, cpu);
@@ -1369,17 +1457,18 @@ undo:
goto undo;
case CPU_ONLINE:
- start_workqueue_thread(cwq, cpu);
+ kthread_bind(cwq->worker->task, cpu);
+ start_worker(cwq->worker);
break;
case CPU_UP_CANCELED:
- start_workqueue_thread(cwq, -1);
+ start_worker(cwq->worker);
case CPU_POST_DEAD:
flush_workqueue(wq);
/* cpu_add_remove_lock protects cwq->thread */
- if (cwq->thread) {
- kthread_stop(cwq->thread);
- cwq->thread = NULL;
+ if (cwq->worker) {
+ destroy_worker(cwq->worker);
+ cwq->worker = NULL;
}
break;
}
@@ -1437,6 +1526,8 @@ EXPORT_SYMBOL_GPL(work_on_cpu);
void __init init_workqueues(void)
{
+ unsigned int cpu;
+
/*
* cwqs are forced aligned according to WORK_STRUCT_FLAG_BITS.
* Make sure that the alignment isn't lower than that of
@@ -1446,6 +1537,9 @@ void __init init_workqueues(void)
BUILD_BUG_ON(__alignof__(struct cpu_workqueue_struct) <
__alignof__(unsigned long long));
+ for_each_possible_cpu(cpu)
+ ida_init(&per_cpu(worker_ida, cpu));
+
singlethread_cpu = cpumask_first(cpu_possible_mask);
hotcpu_notifier(workqueue_cpu_callback, 0);
keventd_wq = create_workqueue("events");
--
1.6.4.2
Tejun Heo
@freezeable paramters and store them separately in workqueue_struct.
Merge them into a single flags parameter and field and use
WQ_FREEZEABLE and WQ_SINGLE_THREAD.
Signed-off-by: Tejun Heo <t...@kernel.org>
---
include/linux/workqueue.h | 25 +++++++++++++++----------
kernel/workqueue.c | 17 +++++++----------
2 files changed, 22 insertions(+), 20 deletions(-)
diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index ac06c55..495572a 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -184,13 +184,17 @@ static inline bool work_static(struct work_struct *work) { return false; }
#define work_clear_pending(work) \
clear_bit(WORK_STRUCT_PENDING, work_data_bits(work))
+enum {
+ WQ_FREEZEABLE = 1 << 0, /* freeze during suspend */
+ WQ_SINGLE_THREAD = 1 << 1, /* no per-cpu worker */
+};
extern struct workqueue_struct *
-__create_workqueue_key(const char *name, int singlethread, int freezeable,
+__create_workqueue_key(const char *name, unsigned int flags,
struct lock_class_key *key, const char *lock_name);
#ifdef CONFIG_LOCKDEP
-#define __create_workqueue(name, singlethread, freezeable) \
+#define __create_workqueue(name, flags) \
({ \
static struct lock_class_key __key; \
const char *__lock_name; \
@@ -200,19 +204,20 @@ __create_workqueue_key(const char *name, int singlethread, int freezeable,
else \
__lock_name = #name; \
\
- __create_workqueue_key((name), (singlethread), \
- (freezeable), &__key, \
+ __create_workqueue_key((name), (flags), &__key, \
__lock_name); \
})
#else
-#define __create_workqueue(name, singlethread, freezeable) \
- __create_workqueue_key((name), (singlethread), (freezeable), \
- NULL, NULL)
+#define __create_workqueue(name, flags) \
+ __create_workqueue_key((name), (flags), NULL, NULL)
#endif
-#define create_workqueue(name) __create_workqueue((name), 0, 0)
-#define create_freezeable_workqueue(name) __create_workqueue((name), 1, 1)
-#define create_singlethread_workqueue(name) __create_workqueue((name), 1, 0)
+#define create_workqueue(name) \
+ __create_workqueue((name), 0)
+#define create_freezeable_workqueue(name) \
+ __create_workqueue((name), WQ_FREEZEABLE | WQ_SINGLE_THREAD)
+#define create_singlethread_workqueue(name) \
+ __create_workqueue((name), WQ_SINGLE_THREAD)
extern void destroy_workqueue(struct workqueue_struct *wq);
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index e16c457..579041f 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -67,11 +67,10 @@ struct cpu_workqueue_struct {
* per-CPU workqueues:
*/
struct workqueue_struct {
+ unsigned int flags; /* I: WQ_* flags */
struct cpu_workqueue_struct *cpu_wq; /* I: cwq's */
struct list_head list; /* W: list of all workqueues */
const char *name; /* I: workqueue name */
- int singlethread;
- int freezeable; /* Freeze threads during suspend */
#ifdef CONFIG_LOCKDEP
struct lockdep_map lockdep_map;
#endif
@@ -203,9 +202,9 @@ static const struct cpumask *cpu_singlethread_map __read_mostly;
static cpumask_var_t cpu_populated_map __read_mostly;
/* If it's single threaded, it isn't in the list of workqueues. */
-static inline int is_wq_single_threaded(struct workqueue_struct *wq)
+static inline bool is_wq_single_threaded(struct workqueue_struct *wq)
{
- return wq->singlethread;
+ return wq->flags & WQ_SINGLE_THREAD;
}
static const struct cpumask *wq_cpu_map(struct workqueue_struct *wq)
@@ -456,7 +455,7 @@ static int worker_thread(void *__cwq)
struct cpu_workqueue_struct *cwq = __cwq;
DEFINE_WAIT(wait);
- if (cwq->wq->freezeable)
+ if (cwq->wq->flags & WQ_FREEZEABLE)
set_freezable();
for (;;) {
@@ -981,8 +980,7 @@ static void start_workqueue_thread(struct cpu_workqueue_struct *cwq, int cpu)
}
struct workqueue_struct *__create_workqueue_key(const char *name,
- int singlethread,
- int freezeable,
+ unsigned int flags,
struct lock_class_key *key,
const char *lock_name)
{
@@ -998,13 +996,12 @@ struct workqueue_struct *__create_workqueue_key(const char *name,
if (!wq->cpu_wq)
goto err;
+ wq->flags = flags;
wq->name = name;
lockdep_init_map(&wq->lockdep_map, lock_name, key, 0);
- wq->singlethread = singlethread;
- wq->freezeable = freezeable;
INIT_LIST_HEAD(&wq->list);
- if (singlethread) {
+ if (flags & WQ_SINGLE_THREAD) {
cwq = init_cpu_workqueue(wq, singlethread_cpu);
err = create_workqueue_thread(cwq, singlethread_cpu);
start_workqueue_thread(cwq, -1);
--
1.6.4.2
Andy Walls
> Separate out worker thread related information to struct worker from
> struct cpu_workqueue_struct and implement helper functions to deal
> with the new struct worker. The only change which is visible outside
> is that now workqueue worker are all named "kworker/CPUID:WORKERID"
> where WORKERID is allocated from per-cpu ida.
>
> This is in preparation of concurrency managed workqueue where shared
> multiple workers would be available per cpu.
>
> Signed-off-by: Tejun Heo <t...@kernel.org>
> ---
> kernel/workqueue.c | 220 +++++++++++++++++++++++++++++++++++++---------------
> 1 files changed, 157 insertions(+), 63 deletions(-)
> +static struct worker *create_worker(struct cpu_workqueue_struct *cwq, bool bind)
> +{
> + int id = -1;
> + struct worker *worker = NULL;
> +
> + spin_lock(&workqueue_lock);
> + while (ida_get_new(&per_cpu(worker_ida, cwq->cpu), &id)) {
> + spin_unlock(&workqueue_lock);
> + if (!ida_pre_get(&per_cpu(worker_ida, cwq->cpu), GFP_KERNEL))
> + goto fail;
> + spin_lock(&workqueue_lock);
> + }
> + spin_unlock(&workqueue_lock);
> +
> + worker = alloc_worker();
> + if (!worker)
> + goto fail;
> +
> + worker->cwq = cwq;
> + worker->id = id;
> +
> + worker->task = kthread_create(worker_thread, worker, "kworker/%u:%d",
> + cwq->cpu, id);
> -static int create_workqueue_thread(struct cpu_workqueue_struct *cwq, int cpu)
> -{
> - struct workqueue_struct *wq = cwq->wq;
> - const char *fmt = is_wq_single_threaded(wq) ? "%s" : "%s/%d";
> - struct task_struct *p;
> -
> - p = kthread_create(worker_thread, cwq, fmt, wq->name, cpu);
Hi Tejun,
I think this change means a user can no longer see worker names that can
be easily mapped to kernel drivers in utiltites like top:
2499 root 20 0 196m 33m 9012 S 2.7 3.6 14:20.55 Xorg
1313 root 15 -5 0 0 0 S 1.3 0.0 0:37.76 cx18-0-in <---
2418 root 15 -5 0 0 0 D 1.3 0.0 1:14.59 kdvb-ad-0-fe-0 <---
2360 root 20 0 537m 16m 7012 S 0.7 1.8 3:20.32 mythbackend
877 root 15 -5 0 0 0 S 0.3 0.0 0:01.01 usb-storage
1311 root 15 -5 0 0 0 S 0.3 0.0 0:22.65 cx18-0-out/0 <---
3034 andy 20 0 277m 7832 6340 S 0.3 0.8 0:26.36 multiload-apple
4779 andy 20 0 14880 1172 872 R 0.3 0.1 0:00.13 top
1 root 20 0 4088 868 616 S 0.0 0.1 0:00.72 init
2 root 15 -5 0 0 0 S 0.0 0.0 0:00.00 kthreadd
If so, I think that it may hamper the ordinary user in submitting good
bug reports or trying to do some troubleshooting on their own. Also,
when forced to dump state with the magic-alt-syrq, I think this change
makes debugging the dump slightly more difficult.
Do you have a pressing need to use the naming convention you have chosen
over the current convention? I do realize that the "/cpu_numnber" part
of the current naming convention needs to be augmented. I just am
apprehensive about the descriptive names all being replaced with
"kworker".
Regards,
Andy
Tejun Heo
11/21/2009 08:44 AM, Andy Walls wrote:
> Do you have a pressing need to use the naming convention you have chosen
> over the current convention? I do realize that the "/cpu_numnber" part
> of the current naming convention needs to be augmented. I just am
> apprehensive about the descriptive names all being replaced with
> "kworker".
There will be no fixed relationship between worker thread and
workqueue, so it simply wouldn't be possible to give a specific name
to worker threads - much like we only have keventd for all the works
queued to the default workqueue for the current implementation. As
long as stack trace can be extracted, I don't think it will hamper
with debugging too much.
Thanks.
--
tejun
Tejun Heo
11/20/2009 01:46 PM, Tejun Heo wrote:
> 0001-sched-kvm-fix-race-condition-involving-sched_in_pree.patch
> 0002-workqueue-Add-debugobjects-support.patch
> 0003-sched-rename-preempt_notifier-to-sched_notifier-and-.patch
> 0004-sched-update-sched_notifier-and-add-wakeup-sleep-not.patch
> 0005-sched-implement-sched_notifier_wake_up_process.patch
> 0006-scheduler-implement-force_cpus_allowed.patch
> 0007-acpi-use-queue_work_on-instead-of-binding-workqueue-.patch
> 0008-stop_machine-reimplement-without-using-workqueue.patch
> 0009-workqueue-misc-cosmetic-updates.patch
> 0010-workqueue-merge-feature-parametesr-into-flags.patch
> 0011-workqueue-update-cwq-alignement-and-make-one-more-fl.patch
> 0012-workqueue-define-both-bit-position-and-mask-for-work.patch
> 0013-workqueue-separate-out-process_one_work.patch
> 0014-workqueue-temporarily-disable-workqueue-tracing.patch
I'm pushing out 0001-0014 to linux-next via the following tree.
git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git for-next
I intend to keep the tree stable from this point on unless drastic
change is necessary. I'll posting further patches against the above
tree.
Thank you.
--
tejun
Peter Zijlstra
> Hello,
>
> 11/20/2009 01:46 PM, Tejun Heo wrote:
> > 0001-sched-kvm-fix-race-condition-involving-sched_in_pree.patch
> > 0002-workqueue-Add-debugobjects-support.patch
> > 0003-sched-rename-preempt_notifier-to-sched_notifier-and-.patch
> > 0004-sched-update-sched_notifier-and-add-wakeup-sleep-not.patch
> > 0005-sched-implement-sched_notifier_wake_up_process.patch
> > 0006-scheduler-implement-force_cpus_allowed.patch
> > 0007-acpi-use-queue_work_on-instead-of-binding-workqueue-.patch
> > 0008-stop_machine-reimplement-without-using-workqueue.patch
> > 0009-workqueue-misc-cosmetic-updates.patch
> > 0010-workqueue-merge-feature-parametesr-into-flags.patch
> > 0011-workqueue-update-cwq-alignement-and-make-one-more-fl.patch
> > 0012-workqueue-define-both-bit-position-and-mask-for-work.patch
> > 0013-workqueue-separate-out-process_one_work.patch
> > 0014-workqueue-temporarily-disable-workqueue-tracing.patch
>
> I'm pushing out 0001-0014 to linux-next via the following tree.
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git for-next
>
> I intend to keep the tree stable from this point on unless drastic
> change is necessary. I'll posting further patches against the above
> tree.
05 is a full nak and needs reimplementing.
06 at the very least needs a changelog.
Tejun Heo
11/21/2009 09:07 PM, Peter Zijlstra wrote:
>> I'm pushing out 0001-0014 to linux-next via the following tree.
The sched fix patch hasn't showed up in upstream so I didn't do it
yet.
>> I intend to keep the tree stable from this point on unless drastic
>> change is necessary. I'll posting further patches against the above
>> tree.
>
> 05 is a full nak and needs reimplementing.
> 06 at the very least needs a changelog.
Alright, one more round to go then.
Thanks for reviewing.
--
tejun