login
Search
Hello, all. This is the sixth take of cmwq (concurrency managed workqueue) patchset. It's on top of v2.6.35-rc3 + sched/core branch. Git tree is available at git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git review-cmwq Linus, please read the merge plan section. Thanks. Table of contents ================= A. This take A-1. Merge plan A-2. Changes from the last take[L] A-3. TODOs A-4. Patches and diffstat B. General documentation of Concurrency Managed Workqueue (cmwq) B-1. Why? B-2. Overview B-3. Unified worklist B-4. Concurrency managed shared worker pool B-5. Performance test results A. This take ============ == A-1. Merge plan Until now, cmwq patches haven't been fixed into permanent commits mainly because sched patches which they are dependent upon made into sched/core tree only recently. After review, I'll put this take into permanent commits. Further developments or fixes will be done on top. I believe that expected users of cmwq are generally in favor of the flexibility added by cmwq. In the last take, the following issues were raised. * Andi Kleen wanted to use high priority dispatching for memory fault handlers. WQ_HIGHPRI is implemented to deal with this and padata integration. * Andrew Morton raised two issues - workqueue users which use RT priority setting (ivtv) and padata integration. kthread_worker which provides simple work based interface on top of kthread is added for cases where fixed association with a specific kthread is required for priority setting, cpuset and other task attributes adjustments. This will also be used by virtnet. WQ_CPU_INTENSIVE is added to address padata integration. When combined with WQ_HIGHPRI, all concurrency management logic is bypassed and cmwq works as a (conceptually) simple context provider and padata should operate without any noticeable difference. * Daniel Walker objected on the ground that cmwq would make it impossible to adjust priorities ...
libata has two concurrency related limitations.
a. ata_wq which is used for polling PIO has single thread per CPU. If
there are multiple devices doing polling PIO on the same CPU, they
can't be executed simultaneously.
b. ata_aux_wq which is used for SCSI probing has single thread. In
cases where SCSI probing is stalled for extended period of time
which is possible for ATAPI devices, this will stall all probing.
#a is solved by increasing maximum concurrency of ata_wq. Please note
that polling PIO might be used under allocation path and thus needs to
be served by a separate wq with a rescuer.
#b is solved by using the default wq instead and achieving exclusion
via per-port mutex.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Jeff Garzik <jgarzik@pobox.com>
---
drivers/ata/libata-core.c | 20 +++++---------------
drivers/ata/libata-eh.c | 4 ++--
drivers/ata/libata-scsi.c | 10 ++++++----
drivers/ata/libata-sff.c | 9 +--------
drivers/ata/libata.h | 1 -
include/linux/libata.h | 1 +
6 files changed, 15 insertions(+), 30 deletions(-)
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index ddf8e48..4f78741 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -98,8 +98,6 @@ static unsigned long ata_dev_blacklisted(const struct ata_device *dev);
unsigned int ata_print_id = 1;
-struct workqueue_struct *ata_aux_wq;
-
struct ata_force_param {
const char *name;
unsigned int cbl;
@@ -5611,6 +5609,7 @@ struct ata_port *ata_port_alloc(struct ata_host *host)
ap->msg_enable = ATA_MSG_DRV | ATA_MSG_ERR | ATA_MSG_WARN;
#endif
+ mutex_init(&ap->scsi_scan_mutex);
INIT_DELAYED_WORK(&ap->hotplug_task, ata_scsi_hotplug);
INIT_WORK(&ap->scsi_rescan_task, ata_scsi_dev_rescan);
INIT_LIST_HEAD(&ap->eh_done_q);
@@ -6549,29 +6548,20 @@ static int __init ata_init(void)
ata_parse_force_param();
- ata_aux_wq = create_singlethread_workqueue("ata_aux");
- if ...Acked-by: Jeff Garzik <jgarzik@redhat.com> --
To implement non-reentrant workqueue, the last gcwq a work was
executed on must be reliably obtainable as long as the work structure
is valid even if the previous workqueue has been destroyed.
To achieve this, work->data will be overloaded to carry the last cpu
number once execution starts so that the previous gcwq can be located
reliably. This means that cwq can't be obtained from work after
execution starts but only gcwq.
Implement set_work_{cwq|cpu}(), get_work_[g]cwq() and
clear_work_data() to set work data to the cpu number when starting
execution, access the overloaded work data and clear it after
cancellation.
queue_delayed_work_on() is updated to preserve the last cpu while
in-flight in timer and other callers which depended on getting cwq
from work after execution starts are converted to depend on gcwq
instead.
* Anton Blanchard fixed compile error on powerpc due to missing
linux/threads.h include.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Anton Blanchard <anton@samba.org>
---
include/linux/workqueue.h | 7 ++-
kernel/workqueue.c | 163 ++++++++++++++++++++++++++++----------------
2 files changed, 109 insertions(+), 61 deletions(-)
diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index 10611f7..0a78141 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -9,6 +9,7 @@
#include <linux/linkage.h>
#include <linux/bitops.h>
#include <linux/lockdep.h>
+#include <linux/threads.h>
#include <asm/atomic.h>
struct workqueue_struct;
@@ -59,6 +60,7 @@ enum {
WORK_STRUCT_FLAG_MASK = (1UL << WORK_STRUCT_FLAG_BITS) - 1,
WORK_STRUCT_WQ_DATA_MASK = ~WORK_STRUCT_FLAG_MASK,
+ WORK_STRUCT_NO_CPU = NR_CPUS << WORK_STRUCT_FLAG_BITS,
};
struct work_struct {
@@ -70,8 +72,9 @@ struct work_struct {
#endif
};
-#define WORK_DATA_INIT() ATOMIC_LONG_INIT(0)
-#define WORK_DATA_STATIC_INIT() ATOMIC_LONG_INIT(WORK_STRUCT_STATIC)
+#define ...Implement the following utility APIs.
workqueue_set_max_active() : adjust max_active of a wq
workqueue_congested() : test whether a wq is contested
work_cpu() : determine the last / current cpu of a work
work_busy() : query whether a work is busy
* Anton Blanchard fixed missing ret initialization in work_busy().
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Anton Blanchard <anton@samba.org>
---
include/linux/workqueue.h | 11 ++++-
kernel/workqueue.c | 108 ++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 117 insertions(+), 2 deletions(-)
diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index 48b7422..0a7f797 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -61,6 +61,10 @@ enum {
WORK_STRUCT_FLAG_MASK = (1UL << WORK_STRUCT_FLAG_BITS) - 1,
WORK_STRUCT_WQ_DATA_MASK = ~WORK_STRUCT_FLAG_MASK,
WORK_STRUCT_NO_CPU = NR_CPUS << WORK_STRUCT_FLAG_BITS,
+
+ /* bit mask for work_busy() return values */
+ WORK_BUSY_PENDING = 1 << 0,
+ WORK_BUSY_RUNNING = 1 << 1,
};
struct work_struct {
@@ -307,9 +311,14 @@ extern void init_workqueues(void);
int execute_in_process_context(work_func_t fn, struct execute_work *);
extern int flush_work(struct work_struct *work);
-
extern int cancel_work_sync(struct work_struct *work);
+extern void workqueue_set_max_active(struct workqueue_struct *wq,
+ int max_active);
+extern bool workqueue_congested(unsigned int cpu, struct workqueue_struct *wq);
+extern unsigned int work_cpu(struct work_struct *work);
+extern unsigned int work_busy(struct work_struct *work);
+
/*
* Kill off a pending schedule_delayed_work(). Note that the work callback
* function may still be running on return from cancel_delayed_work(), unless
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 08e4eb3..979f893 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -203,7 +203,7 @@ struct workqueue_struct {
cpumask_var_t mayday_mask; /* cpus ...Implement simple work processor for kthread. This is to ease using
kthread. Single thread workqueue used to be used for things like this
but workqueue won't guarantee fixed kthread association anymore to
enable worker sharing.
This can be used in cases where specific kthread association is
necessary, for example, when it should have RT priority or be assigned
to certain cgroup.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
include/linux/kthread.h | 64 ++++++++++++++++++++
kernel/kthread.c | 149 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 213 insertions(+), 0 deletions(-)
diff --git a/include/linux/kthread.h b/include/linux/kthread.h
index aabc8a1..f93cb69 100644
--- a/include/linux/kthread.h
+++ b/include/linux/kthread.h
@@ -34,4 +34,68 @@ int kthread_should_stop(void);
int kthreadd(void *unused);
extern struct task_struct *kthreadd_task;
+/*
+ * Simple work processor based on kthread.
+ *
+ * This provides easier way to make use of kthreads. A kthread_work
+ * can be queued and flushed using queue/flush_kthread_work()
+ * respectively. Queued kthread_works are processed by a kthread
+ * running kthread_worker_fn().
+ *
+ * A kthread_work can't be freed while it is executing.
+ */
+struct kthread_work;
+typedef void (*kthread_work_func_t)(struct kthread_work *work);
+
+struct kthread_worker {
+ spinlock_t lock;
+ struct list_head work_list;
+ struct task_struct *task;
+};
+
+struct kthread_work {
+ struct list_head node;
+ kthread_work_func_t func;
+ wait_queue_head_t done;
+ atomic_t flushing;
+ int queue_seq;
+ int done_seq;
+};
+
+#define KTHREAD_WORKER_INIT(worker) { \
+ .lock = SPIN_LOCK_UNLOCKED, \
+ .work_list = LIST_HEAD_INIT((worker).work_list), \
+ }
+
+#define KTHREAD_WORK_INIT(work, fn) { \
+ .node = LIST_HEAD_INIT((work).node), \
+ .func = (fn), \
+ .done = __WAIT_QUEUE_HEAD_INITIALIZER((work).done), \
+ .flushing = ...This patch implements high priority workqueue which can be specified
with WQ_HIGHPRI flag on creation. A high priority workqueue has the
following properties.
* A work queued to it is queued at the head of the worklist of the
respective gcwq after other highpri works, while normal works are
always appended at the end.
* As long as there are highpri works on gcwq->worklist,
[__]need_more_worker() remains %true and process_one_work() wakes up
another worker before it start executing a work.
The above two properties guarantee that works queued to high priority
workqueues are dispatched to workers and start execution as soon as
possible regardless of the state of other works.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
include/linux/workqueue.h | 1 +
kernel/workqueue.c | 70 +++++++++++++++++++++++++++++++++++++++++----
2 files changed, 65 insertions(+), 6 deletions(-)
diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index 0a7f797..006dcf7 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -231,6 +231,7 @@ enum {
WQ_SINGLE_CPU = 1 << 1, /* only single cpu at a time */
WQ_NON_REENTRANT = 1 << 2, /* guarantee non-reentrance */
WQ_RESCUER = 1 << 3, /* has an rescue worker */
+ WQ_HIGHPRI = 1 << 4, /* high priority */
WQ_MAX_ACTIVE = 512, /* I like 512, better ideas? */
WQ_DFL_ACTIVE = WQ_MAX_ACTIVE / 2,
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 979f893..e12f9aa 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -43,6 +43,7 @@ enum {
GCWQ_MANAGING_WORKERS = 1 << 1, /* managing workers */
GCWQ_DISASSOCIATED = 1 << 2, /* cpu can't serve workers */
GCWQ_FREEZING = 1 << 3, /* freeze in progress */
+ GCWQ_HIGHPRI_PENDING = 1 << 4, /* highpri works on queue */
/* worker flags */
WORKER_STARTED = 1 << 0, /* started */
@@ -452,15 +453,19 @@ static struct global_cwq ...Worker management is about to be overhauled. Simplify things by
removing cpu_populated_map, creating workers for all possible cpus and
making single threaded workqueues behave more like multi threaded
ones.
After this patch, all cwqs are always initialized, all workqueues are
linked on the workqueues list and workers for all possibles cpus
always exist. This also makes CPU hotplug support simpler - checking
->cpus_allowed before processing works in worker_thread() and flushing
cwqs on CPU_POST_DEAD are enough.
While at it, make get_cwq() always return the cwq for the specified
cpu, add target_cwq() for cases where single thread distinction is
necessary and drop all direct usage of per_cpu_ptr() on wq->cpu_wq.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
kernel/workqueue.c | 173 ++++++++++++++++++----------------------------------
1 files changed, 59 insertions(+), 114 deletions(-)
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index f7ab703..dc78956 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -55,6 +55,7 @@ struct cpu_workqueue_struct {
struct list_head worklist;
wait_queue_head_t more_work;
struct work_struct *current_work;
+ unsigned int cpu;
struct workqueue_struct *wq; /* I: the owning workqueue */
struct task_struct *thread;
@@ -189,34 +190,19 @@ static DEFINE_SPINLOCK(workqueue_lock);
static LIST_HEAD(workqueues);
static int singlethread_cpu __read_mostly;
-static const struct cpumask *cpu_singlethread_map __read_mostly;
-/*
- * _cpu_down() first removes CPU from cpu_online_map, then CPU_DEAD
- * flushes cwq->worklist. This means that flush_workqueue/wait_on_work
- * which comes in between can't use for_each_online_cpu(). We could
- * use cpu_possible_map, the cpumask below is more a documentation
- * than optimization.
- */
-static cpumask_var_t cpu_populated_map __read_mostly;
-
-/* If it's single threaded, it isn't in the list of workqueues. */
-static inline bool is_wq_single_threaded(struct ...Make the following updates in preparation of concurrency managed
workqueue. None of these changes causes any visible behavior
difference.
* Add comments and adjust indentations to data structures and several
functions.
* Rename wq_per_cpu() to get_cwq() and swap the position of two
parameters for consistency. Convert a direct per_cpu_ptr() access
to wq->cpu_wq to get_cwq().
* Add work_static() and Update set_wq_data() such that it sets the
flags part to WORK_STRUCT_PENDING | WORK_STRUCT_STATIC if static |
@extra_flags.
* Move santiy check on work->entry emptiness from queue_work_on() to
__queue_work() which all queueing paths share.
* Make __queue_work() take @cpu and @wq instead of @cwq.
* Restructure flush_work() and __create_workqueue_key() to make them
easier to modify.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
include/linux/workqueue.h | 5 ++
kernel/workqueue.c | 131 +++++++++++++++++++++++++++++----------------
2 files changed, 89 insertions(+), 47 deletions(-)
diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index 0697946..e724daf 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -96,9 +96,14 @@ struct execute_work {
#ifdef CONFIG_DEBUG_OBJECTS_WORK
extern void __init_work(struct work_struct *work, int onstack);
extern void destroy_work_on_stack(struct work_struct *work);
+static inline unsigned int work_static(struct work_struct *work)
+{
+ return *work_data_bits(work) & (1 << WORK_STRUCT_STATIC);
+}
#else
static inline void __init_work(struct work_struct *work, int onstack) { }
static inline void destroy_work_on_stack(struct work_struct *work) { }
+static inline unsigned int work_static(struct work_struct *work) { return 0; }
#endif
/*
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 1a47fbf..c56146a 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -37,6 +37,16 @@
#include <trace/events/workqueue.h>
/*
+ * Structure fields follow ...ACPI works need to be executed on cpu0 and acpi/osl.c achieves this by
creating singlethread workqueue and then binding it to cpu0 from a
work which is quite unorthodox. Make it create regular workqueues and
use queue_work_on() instead. This is in preparation of concurrency
managed workqueue and the extra workers won't be a problem after it's
implemented.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
drivers/acpi/osl.c | 40 +++++++++++-----------------------------
1 files changed, 11 insertions(+), 29 deletions(-)
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 78418ce..46cce39 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -191,36 +191,11 @@ acpi_status __init acpi_os_initialize(void)
return AE_OK;
}
-static void bind_to_cpu0(struct work_struct *work)
-{
- set_cpus_allowed_ptr(current, cpumask_of(0));
- kfree(work);
-}
-
-static void bind_workqueue(struct workqueue_struct *wq)
-{
- struct work_struct *work;
-
- work = kzalloc(sizeof(struct work_struct), GFP_KERNEL);
- INIT_WORK(work, bind_to_cpu0);
- queue_work(wq, work);
-}
-
acpi_status acpi_os_initialize1(void)
{
- /*
- * On some machines, a software-initiated SMI causes corruption unless
- * the SMI runs on CPU 0. An SMI can be initiated by any AML, but
- * typically it's done in GPE-related methods that are run via
- * workqueues, so we can avoid the known corruption cases by binding
- * the workqueues to CPU 0.
- */
- kacpid_wq = create_singlethread_workqueue("kacpid");
- bind_workqueue(kacpid_wq);
- kacpi_notify_wq = create_singlethread_workqueue("kacpi_notify");
- bind_workqueue(kacpi_notify_wq);
- kacpi_hotplug_wq = create_singlethread_workqueue("kacpi_hotplug");
- bind_workqueue(kacpi_hotplug_wq);
+ kacpid_wq = create_workqueue("kacpid");
+ kacpi_notify_wq = create_workqueue("kacpi_notify");
+ kacpi_hotplug_wq = create_workqueue("kacpi_hotplug");
BUG_ON(!kacpid_wq);
BUG_ON(!kacpi_notify_wq);
BUG_ON(!kacpi_hotplug_wq);
@@ -766,7 +741,14 @@ ...This patch implements cpu intensive workqueue which can be specified
with WQ_CPU_INTENSIVE flag on creation. Works queued to a cpu
intensive workqueue don't participate in concurrency management. IOW,
it doesn't contribute to gcwq->nr_running and thus doesn't delay
excution of other works.
Note that although cpu intensive works won't delay other works, they
can be delayed by other works. Combine with WQ_HIGHPRI to avoid being
delayed by other works too.
As the name suggests this is useful when using workqueue for cpu
intensive works. Workers executing cpu intensive works are not
considered for workqueue concurrency management and left for the
scheduler to manage.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
include/linux/workqueue.h | 1 +
kernel/workqueue.c | 16 +++++++++++++++-
2 files changed, 16 insertions(+), 1 deletions(-)
diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index 006dcf7..3f36d37 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -232,6 +232,7 @@ enum {
WQ_NON_REENTRANT = 1 << 2, /* guarantee non-reentrance */
WQ_RESCUER = 1 << 3, /* has an rescue worker */
WQ_HIGHPRI = 1 << 4, /* high priority */
+ WQ_CPU_INTENSIVE = 1 << 5, /* cpu instensive workqueue */
WQ_MAX_ACTIVE = 512, /* I like 512, better ideas? */
WQ_DFL_ACTIVE = WQ_MAX_ACTIVE / 2,
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index e12f9aa..25dce40 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -52,8 +52,10 @@ enum {
WORKER_PREP = 1 << 3, /* preparing to run works */
WORKER_ROGUE = 1 << 4, /* not bound to any cpu */
WORKER_REBIND = 1 << 5, /* mom is home, come back */
+ WORKER_CPU_INTENSIVE = 1 << 6, /* cpu intensive */
- WORKER_NOT_RUNNING = WORKER_PREP | WORKER_ROGUE | WORKER_REBIND,
+ WORKER_NOT_RUNNING = WORKER_PREP | WORKER_ROGUE | WORKER_REBIND |
+ WORKER_CPU_INTENSIVE,
/* gcwq->trustee_state */
...Use gcwq->worklist instead of cwq->worklist and break the strict
association between a cwq and its worker. All works queued on a cpu
are queued on gcwq->worklist and processed by any available worker on
the gcwq.
As there no longer is strict association between a cwq and its worker,
whether a work is executing can now only be determined by calling
[__]find_worker_executing_work().
After this change, the only association between a cwq and its worker
is that a cwq puts a worker into shared worker pool on creation and
kills it on destruction. As all workqueues are still limited to
max_active of one, this means that there are always at least as many
workers as active works and thus there's no danger for deadlock.
The break of strong association between cwqs and workers requires
somewhat clumsy changes to current_is_keventd() and
destroy_workqueue(). Dynamic worker pool management will remove both
clumsy changes. current_is_keventd() won't be necessary at all as the
only reason it exists is to avoid queueing a work from a work which
will be allowed just fine. The clumsy part of destroy_workqueue() is
added because a worker can only be destroyed while idle and there's no
guarantee a worker is idle when its wq is going down. With dynamic
pool management, workers are not associated with workqueues at all and
only idle ones will be submitted to destroy_workqueue() so the code
won't be necessary anymore.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
kernel/workqueue.c | 131 +++++++++++++++++++++++++++++++++++++++-------------
1 files changed, 99 insertions(+), 32 deletions(-)
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 7994edb..e0a7609 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -34,6 +34,7 @@
#include <linux/debug_locks.h>
#include <linux/lockdep.h>
#include <linux/idr.h>
+#include <linux/delay.h>
enum {
/* global_cwq flags */
@@ -72,7 +73,6 @@ enum {
*/
struct global_cwq;
-struct cpu_workqueue_struct;
struct ...Reimplement CPU hotplugging support using trustee thread. On CPU
down, a trustee thread is created and each step of CPU down is
executed by the trustee and workqueue_cpu_callback() simply drives and
waits for trustee state transitions.
CPU down operation no longer waits for works to be drained but trustee
sticks around till all pending works have been completed. If CPU is
brought back up while works are still draining,
workqueue_cpu_callback() tells trustee to step down and tell workers
to rebind to the cpu.
As it's difficult to tell whether cwqs are empty if it's freezing or
frozen, trustee doesn't consider draining to be complete while a gcwq
is freezing or frozen (tracked by new GCWQ_FREEZING flag). Also,
workers which get unbound from their cpu are marked with WORKER_ROGUE.
Trustee based implementation doesn't bring any new feature at this
point but it will be used to manage worker pool when dynamic shared
worker pool is implemented.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
include/linux/cpu.h | 2 +
kernel/workqueue.c | 293 ++++++++++++++++++++++++++++++++++++++++++++++++---
2 files changed, 279 insertions(+), 16 deletions(-)
diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index de6b172..4823af6 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -71,6 +71,8 @@ enum {
/* migration should happen before other stuff but after perf */
CPU_PRI_PERF = 20,
CPU_PRI_MIGRATION = 10,
+ /* prepare workqueues for other notifiers */
+ CPU_PRI_WORKQUEUE = 5,
};
#ifdef CONFIG_SMP
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 62d7cfd..5cd155d 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -36,14 +36,27 @@
#include <linux/idr.h>
enum {
+ /* global_cwq flags */
+ GCWQ_FREEZING = 1 << 3, /* freeze in progress */
+
/* worker flags */
WORKER_STARTED = 1 << 0, /* started */
WORKER_DIE = 1 << 1, /* die die die */
WORKER_IDLE = 1 << 2, /* is idle */
+ WORKER_ROGUE = 1 << 4, /* not bound ...work->data field is used for two purposes. It points to cwq it's
queued on and the lower bits are used for flags. Currently, two bits
are reserved which is always safe as 4 byte alignment is guaranteed on
every architecture. However, future changes will need more flag bits.
On SMP, the percpu allocator is capable of honoring larger alignment
(there are other users which depend on it) and larger alignment works
just fine. On UP, percpu allocator is a thin wrapper around
kzalloc/kfree() and don't honor alignment request.
This patch introduces WORK_STRUCT_FLAG_BITS and implements
alloc/free_cwqs() which guarantees (1 << WORK_STRUCT_FLAG_BITS)
alignment both on SMP and UP. On SMP, simply wrapping percpu
allocator is enouhg. On UP, extra space is allocated so that cwq can
be aligned and the original pointer can be stored after it which is
used in the free path.
While at it, as cwqs are now forced aligned, make sure the resulting
alignment is at least equal to or larger than that of long long.
Alignment problem on UP is reported by Michal Simek.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Christoph Lameter <cl@linux-foundation.org>
Cc: Ingo Molnar <mingo@elte.hu>
Reported-by: Michal Simek <michal.simek@petalogix.com>
---
include/linux/workqueue.h | 5 +++-
kernel/workqueue.c | 62 +++++++++++++++++++++++++++++++++++++++++---
2 files changed, 61 insertions(+), 6 deletions(-)
diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index d60c570..b90958a 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -26,6 +26,9 @@ enum {
WORK_STRUCT_PENDING_BIT = 0, /* work item is pending execution */
#ifdef CONFIG_DEBUG_OBJECTS_WORK
WORK_STRUCT_STATIC_BIT = 1, /* static initializer (debugobjects) */
+ WORK_STRUCT_FLAG_BITS = 2,
+#else
+ WORK_STRUCT_FLAG_BITS = 1,
#endif
WORK_STRUCT_PENDING = 1 << WORK_STRUCT_PENDING_BIT,
@@ -35,7 +38,7 @@ enum {
WORK_STRUCT_STATIC = 0,
#endif
- WORK_STRUCT_FLAG_MASK = ...But they are not allocated contiguously as we use the per cpu offsets. So why does the struct itself need to be aligned? Only the base pointer of its dynamic allocation needs to be aligned. Or am I missing something? This is crashing my build in x86-32, unless I force an alignment to 8, or I just remove this build check. --
Hello, work->data doesn't store the percpu pointer but the address of cwq of that specific cpu as returned by per_cpu_ptr(), so each element needs to be aligned. Besides, if the percpu ptr is aligned the elements are Heh, how did that happen? I'll investigate. Can you please attach your .config? Thanks. -- tejun --
But then, if each cpu pointers are aligned, the struct itself doesn't need to be aligned in its size right? It would need to if multiple elements were allocated per cpu but for this struct we only have one per cpu. So what seems to matter wrt alignment is only the base pointer of these structs, not the size. --
Hello, Yeap, sure, but how does it matter? Thanks. -- tejun --
It means the size of the struct itself doesn't need to aligned to anything. --
Hello, Frederic. Yeah, it could be that I'm kinda dense today but your mode of communication is a bit too implicit for me. So, can you please elaborate what problem you see in the patch and how you want it to be changed? Thanks. -- tejun --
So, imagine you allocate your struct with alloc_percpu(align). The per cpu pointer is 0x400 (purely imagination). Now you have two cpus and they have the following base offsets for per cpu allocations: CPU 0 = 0xf1000000 CPU 1 = 0xf2000000 So, the true pointers for your cpu workqueue structs will be: CPU 0 = 0xf1000400 CPU 1 = 0xf2000400 These addresses are aligned like you wanted to, and it seems it is what matters, to store these addresses in the work flags. So why does the size of the struct need to be aligned too? All you want is that the two above addresses are aligned. Now why the size of the struct itself needs this alignment too. That's the obscure point for me. If it's useless, this could avoid all this alignment maintainance, except during the allocation itself. --
Hello, What alignment maintenance? Are you talking about the UP code? If you're talking about the UP code, the ugliness there is because the current UP __alloc_percpu() can't honor the alignment parameter. Heh, it seems I'm still lost. Care to give one more shot at it? :-) Thanks. -- tejun --
Ah well, yesterday there was a BUILD_BUG_ON on init_workqueues that checked this structure size was well aligned. Now that I check again, it seems to have Heh no, it's about a leftover in your patchset that you have fixed My bad, I haven't looked your updated patch in detail... :) --
Why do we need alignment on UP? Cachelines typically dont bounce if a single processor accesses the data. --
Because work->data is multiplexed with pointer and flag bits, so the targets of the pointer (cwq's) need to be aligned so that the lower part of the pointer always stays zero. Thanks. -- tejun --
work->data field is used for two purposes. It points to cwq it's
queued on and the lower bits are used for flags. Currently, two bits
are reserved which is always safe as 4 byte alignment is guaranteed on
every architecture. However, future changes will need more flag bits.
On SMP, the percpu allocator is capable of honoring larger alignment
(there are other users which depend on it) and larger alignment works
just fine. On UP, percpu allocator is a thin wrapper around
kzalloc/kfree() and don't honor alignment request.
This patch introduces WORK_STRUCT_FLAG_BITS and implements
alloc/free_cwqs() which guarantees max(1 << WORK_STRUCT_FLAG_BITS,
__alignof__(unsigned long long) alignment both on SMP and UP. On SMP,
simply wrapping percpu allocator is enough. On UP, extra space is
allocated so that cwq can be aligned and the original pointer can be
stored after it which is used in the free path.
* Alignment problem on UP is reported by Michal Simek.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Christoph Lameter <cl@linux-foundation.org>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Reported-by: Michal Simek <michal.simek@petalogix.com>
---
Never mind of about config. This was originally after workqueue
flushing patch so there were enough flag bits to avoid triggering that
BUILD_BUG_ON(). I updated alloc_cwqs() to just take the larger
alignment. After this change, 0023 fails to apply but it's trivial
context conflict. I've updated the git tree with the refreshed
patches.
git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git review-cmwq
Thanks.
include/linux/workqueue.h | 5 +++
kernel/workqueue.c | 60 ++++++++++++++++++++++++++++++++++++++++++----
2 files changed, 59 insertions(+), 6 deletions(-)
Index: work/include/linux/workqueue.h
===================================================================
--- work.orig/include/linux/workqueue.h
+++ work/include/linux/workqueue.h
@@ -26,6 +26,9 @@ enum {
...Your tree now builds well on x86-32. Thanks! --
With stop_machine() converted to use cpu_stop, RT workqueue doesn't
have any user left. Kill RT workqueue support.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
include/linux/workqueue.h | 20 +++++++++-----------
kernel/workqueue.c | 6 ------
2 files changed, 9 insertions(+), 17 deletions(-)
diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index 9466e86..0697946 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -181,12 +181,11 @@ static inline void destroy_work_on_stack(struct work_struct *work) { }
extern struct workqueue_struct *
-__create_workqueue_key(const char *name, int singlethread,
- int freezeable, int rt, struct lock_class_key *key,
- const char *lock_name);
+__create_workqueue_key(const char *name, int singlethread, int freezeable,
+ struct lock_class_key *key, const char *lock_name);
#ifdef CONFIG_LOCKDEP
-#define __create_workqueue(name, singlethread, freezeable, rt) \
+#define __create_workqueue(name, singlethread, freezeable) \
({ \
static struct lock_class_key __key; \
const char *__lock_name; \
@@ -197,19 +196,18 @@ __create_workqueue_key(const char *name, int singlethread,
__lock_name = #name; \
\
__create_workqueue_key((name), (singlethread), \
- (freezeable), (rt), &__key, \
+ (freezeable), &__key, \
__lock_name); \
})
#else
-#define __create_workqueue(name, singlethread, freezeable, rt) \
- __create_workqueue_key((name), (singlethread), (freezeable), (rt), \
+#define __create_workqueue(name, singlethread, freezeable) \
+ __create_workqueue_key((name), (singlethread), (freezeable), \
NULL, NULL)
#endif
-#define create_workqueue(name) __create_workqueue((name), 0, 0, 0)
-#define create_rt_workqueue(name) __create_workqueue((name), 0, 0, 1)
-#define create_freezeable_workqueue(name) __create_workqueue((name), 1, 1, 0)
-#define create_singlethread_workqueue(name) ...Separate out process_one_work() out of run_workqueue(). This patch
doesn't cause any behavior change.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
kernel/workqueue.c | 100 +++++++++++++++++++++++++++++++--------------------
1 files changed, 61 insertions(+), 39 deletions(-)
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 5c49d76..8e3082b 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -402,51 +402,73 @@ int queue_delayed_work_on(int cpu, struct workqueue_struct *wq,
}
EXPORT_SYMBOL_GPL(queue_delayed_work_on);
+/**
+ * process_one_work - process single work
+ * @cwq: cwq to process work for
+ * @work: work to process
+ *
+ * Process @work. This function contains all the logics necessary to
+ * process a single work including synchronization against and
+ * interaction with other workers on the same cpu, queueing and
+ * flushing. As long as context requirement is met, any worker can
+ * call this function to process a work.
+ *
+ * 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)
+{
+ work_func_t f = work->func;
+#ifdef CONFIG_LOCKDEP
+ /*
+ * It is permissible to free the struct work_struct from
+ * inside the function that is called from it, this we need to
+ * take into account for lockdep too. To avoid bogus "held
+ * lock freed" warnings as well as problems when looking into
+ * work->lockdep_map, make a copy and use that here.
+ */
+ 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);
+
+ spin_unlock_irq(&cwq->lock);
+
+ BUG_ON(get_wq_data(work) != ...Implement worker_{set|clr}_flags() to manipulate worker flags. These
are currently simple wrappers but logics to track the current worker
state and the current level of concurrency will be added.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
kernel/workqueue.c | 48 ++++++++++++++++++++++++++++++++++++++++--------
1 files changed, 40 insertions(+), 8 deletions(-)
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index e0a7609..8b90d87 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -412,6 +412,38 @@ static void wake_up_worker(struct global_cwq *gcwq)
}
/**
+ * worker_set_flags - set worker flags
+ * @worker: worker to set flags for
+ * @flags: flags to set
+ * @wakeup: wakeup an idle worker if necessary
+ *
+ * Set @flags in @worker->flags.
+ *
+ * LOCKING:
+ * spin_lock_irq(gcwq->lock).
+ */
+static inline void worker_set_flags(struct worker *worker, unsigned int flags,
+ bool wakeup)
+{
+ worker->flags |= flags;
+}
+
+/**
+ * worker_clr_flags - clear worker flags
+ * @worker: worker to set flags for
+ * @flags: flags to clear
+ *
+ * Clear @flags in @worker->flags.
+ *
+ * LOCKING:
+ * spin_lock_irq(gcwq->lock).
+ */
+static inline void worker_clr_flags(struct worker *worker, unsigned int flags)
+{
+ worker->flags &= ~flags;
+}
+
+/**
* busy_worker_head - return the busy hash head for a work
* @gcwq: gcwq of interest
* @work: work to be hashed
@@ -776,7 +808,7 @@ static void worker_enter_idle(struct worker *worker)
BUG_ON(!list_empty(&worker->entry) &&
(worker->hentry.next || worker->hentry.pprev));
- worker->flags |= WORKER_IDLE;
+ worker_set_flags(worker, WORKER_IDLE, false);
gcwq->nr_idle++;
/* idle_list is LIFO */
@@ -800,7 +832,7 @@ static void worker_leave_idle(struct worker *worker)
struct global_cwq *gcwq = worker->gcwq;
BUG_ON(!(worker->flags & WORKER_IDLE));
- worker->flags &= ~WORKER_IDLE;
+ worker_clr_flags(worker, WORKER_IDLE);
gcwq->nr_idle--;
...It would be nice to get this in Documentation/workqueue-design.txt, Why this arbitrary limitation? Thanks. --
Hello, Yeah, I'm thinking about putting more technical description as the head comment in workqueue.c and putting overview and information for It's basically a safety mechanism to prevent a run away user from saturating the system with workers. 256 seemed high enough for most use cases yet low enough not to cause any major system failure. So, yeah, I pulled that number out of my ass. Thanks. -- tejun --
Hello, Commits for cmwq proper (patches 0001-0032) are now permanent. git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git cmwq-core These four patches fix bugs found since take#6 and available in the following branch. git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git for-next-candidate Thanks. -- tejun --
worker_set/clr_flags() assume that if none of NOT_RUNNING flags is set
the worker must be contributing to nr_running which is only true if
the worker is actually running.
As when called from self, it is guaranteed that the worker is running,
those functions can be safely used from the worker itself and they
aren't necessary from other places anyway. Make the following changes
to fix the bug.
* Make worker_set/clr_flags() whine if not called from self.
* Convert all places which called those functions from other tasks to
manipulate flags directly.
* Make trustee_thread() directly clear nr_running after setting
WORKER_ROGUE on all workers. This is the only place where
nr_running manipulation is necessary outside of workers themselves.
* While at it, add sanity check for nr_running in worker_enter_idle().
Signed-off-by: Tejun Heo <tj@kernel.org>
---
kernel/workqueue.c | 47 ++++++++++++++++++++++++++++-------------------
1 files changed, 28 insertions(+), 19 deletions(-)
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 6fa847c..5587338 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -601,7 +601,7 @@ struct task_struct *wq_worker_sleeping(struct task_struct *task,
/**
* worker_set_flags - set worker flags and adjust nr_running accordingly
- * @worker: worker to set flags for
+ * @worker: self
* @flags: flags to set
* @wakeup: wakeup an idle worker if necessary
*
@@ -609,14 +609,16 @@ struct task_struct *wq_worker_sleeping(struct task_struct *task,
* nr_running becomes zero and @wakeup is %true, an idle worker is
* woken up.
*
- * LOCKING:
- * spin_lock_irq(gcwq->lock).
+ * CONTEXT:
+ * spin_lock_irq(gcwq->lock)
*/
static inline void worker_set_flags(struct worker *worker, unsigned int flags,
bool wakeup)
{
struct global_cwq *gcwq = worker->gcwq;
+ WARN_ON_ONCE(worker->task != current);
+
/*
* If transitioning into NOT_RUNNING, adjust nr_running and
* wake up an idle worker as ...get_work_gcwq() was incorrectly triggering BUG_ON() if cpu number is equal to or higher than num_possible_cpus() instead of nr_cpu_ids. Fix it. Signed-off-by: Tejun Heo <tj@kernel.org> --- kernel/workqueue.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/kernel/workqueue.c b/kernel/workqueue.c index b59c946..0c485a5 100644 --- a/kernel/workqueue.c +++ b/kernel/workqueue.c @@ -445,7 +445,7 @@ static struct global_cwq *get_work_gcwq(struct work_struct *work) if (cpu == NR_CPUS) return NULL; - BUG_ON(cpu >= num_possible_cpus()); + BUG_ON(cpu >= nr_cpu_ids); return get_gcwq(cpu); } -- 1.6.4.2 --
When one flusher is cascading to the next flusher, it first sets wq->first_flusher to the next one and sets up the next flush cycle. If there's nothing to do for the next cycle, it clears wq->flush_flusher and proceeds to the one after that. If the woken up flusher checks wq->first_flusher before it gets cleared, it will incorrectly assume the role of the first flusher, which triggers BUG_ON() sanity check. Fix it by checking wq->first_flusher again after grabbing the mutex. Signed-off-by: Tejun Heo <tj@kernel.org> --- kernel/workqueue.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/kernel/workqueue.c b/kernel/workqueue.c index 5587338..b59c946 100644 --- a/kernel/workqueue.c +++ b/kernel/workqueue.c @@ -2138,6 +2138,10 @@ void flush_workqueue(struct workqueue_struct *wq) mutex_lock(&wq->flush_mutex); + /* we might have raced, check again with mutex held */ + if (wq->first_flusher != &this_flusher) + goto out_unlock; + wq->first_flusher = NULL; BUG_ON(!list_empty(&this_flusher.list)); -- 1.6.4.2 --
When there's no pending work to do, worker_thread() goes back to sleep after waking up without checking whether worker management is necessary. This means that idle worker exit requests can be ignored if the gcwq stays empty. Fix it by making worker_thread() always check whether worker management is necessary before going to sleep. Signed-off-by: Tejun Heo <tj@kernel.org> --- kernel/workqueue.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/kernel/workqueue.c b/kernel/workqueue.c index 0c485a5..2eb9fbd 100644 --- a/kernel/workqueue.c +++ b/kernel/workqueue.c @@ -1832,10 +1832,10 @@ recheck: } while (keep_working(gcwq)); worker_set_flags(worker, WORKER_PREP, false); - +sleep: if (unlikely(need_to_manage_workers(gcwq)) && manage_workers(worker)) goto recheck; -sleep: + /* * gcwq->lock is held and there's no work to process and no * need to manage, sleep. Workers are woken up only while -- 1.6.4.2 --
This should be in Documentation/workqueue.txt or something like that. David --
Yeap, once things settle a bit. I'll update it and put it under documentatino. Thanks. -- tejun --