Messages in this thread Patch in this message | | | Date | Thu, 14 Jun 2012 15:13:31 -0700 | From | Silas Boyd-Wickizer <> | Subject | Re: [RFC patch 1/5] kthread: Implement park/unpark facility |
| |
Hello,
Attached is a rough patch that uses park/unpark for workqueue idle threads. The patch is a hack, it probably has bugs, and it certainly doesn't simplify the workqueue code. Perhaps, however, the patch might be useful in guiding future changes to smp_boot_threads or workqueues.
A difficulty in applying park/unpark (or smp_boot_threads potentially), it that there is no single per-CPU thread for each CPU. That is, the thread that starts out as the initial worker/idle thread could be different from any thread that is idling when a CPU goes offline. An additional complication is that a per-cpu worker that is idle may have initially been unbound (e.g. worker creation in the trustee_thread can cause this).
Silas
Signed-off-by: Silas Boyd-Wickizer <sbw@mit.edu> --- kernel/workqueue.c | 114 +++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 87 insertions(+), 27 deletions(-)
diff --git a/kernel/workqueue.c b/kernel/workqueue.c index 9a3128d..fb5e18d 100644 --- a/kernel/workqueue.c +++ b/kernel/workqueue.c @@ -61,6 +61,7 @@ enum { WORKER_REBIND = 1 << 5, /* mom is home, come back */ WORKER_CPU_INTENSIVE = 1 << 6, /* cpu intensive */ WORKER_UNBOUND = 1 << 7, /* worker is unbound */ + WORKER_PARK = 1 << 8, WORKER_NOT_RUNNING = WORKER_PREP | WORKER_ROGUE | WORKER_REBIND | WORKER_CPU_INTENSIVE | WORKER_UNBOUND, @@ -1412,6 +1413,13 @@ fail: return NULL; } +static void __start_worker(struct worker *worker) +{ + worker->flags |= WORKER_STARTED; + worker->gcwq->nr_workers++; + worker_enter_idle(worker); +} + /** * start_worker - start a newly created worker * @worker: worker to start @@ -1423,12 +1431,29 @@ fail: */ static void start_worker(struct worker *worker) { - worker->flags |= WORKER_STARTED; - worker->gcwq->nr_workers++; - worker_enter_idle(worker); + __start_worker(worker); wake_up_process(worker->task); } +static void unpark_worker(struct worker *worker) +{ + worker->flags &= ~WORKER_PARK; + __start_worker(worker); + kthread_unpark(worker->task); +} + +static void __disconnect_worker(struct worker *worker) +{ + struct global_cwq *gcwq = worker->gcwq; + + if (worker->flags & WORKER_STARTED) + gcwq->nr_workers--; + if (worker->flags & WORKER_IDLE) + gcwq->nr_idle--; + + list_del_init(&worker->entry); +} + /** * destroy_worker - destroy a workqueue worker * @worker: worker to be destroyed @@ -1447,12 +1472,7 @@ static void destroy_worker(struct worker *worker) BUG_ON(worker->current_work); BUG_ON(!list_empty(&worker->scheduled)); - if (worker->flags & WORKER_STARTED) - gcwq->nr_workers--; - if (worker->flags & WORKER_IDLE) - gcwq->nr_idle--; - - list_del_init(&worker->entry); + __disconnect_worker(worker); worker->flags |= WORKER_DIE; spin_unlock_irq(&gcwq->lock); @@ -1464,6 +1484,23 @@ static void destroy_worker(struct worker *worker) ida_remove(&gcwq->worker_ida, id); } +static void park_idle_worker(struct worker *worker) +{ + struct global_cwq *gcwq = worker->gcwq; + + BUG_ON(worker->current_work); + BUG_ON(!list_empty(&worker->scheduled)); + BUG_ON(gcwq->first_idle); + + __disconnect_worker(worker); + worker->flags = WORKER_PARK | WORKER_PREP; + + spin_unlock_irq(&gcwq->lock); + kthread_park(worker->task); + spin_lock_irq(&gcwq->lock); + gcwq->first_idle = worker; +} + static void idle_worker_timeout(unsigned long __gcwq) { struct global_cwq *gcwq = (void *)__gcwq; @@ -1953,6 +1990,12 @@ woke_up: return 0; } + if (worker->flags & WORKER_PARK) { + spin_unlock_irq(&gcwq->lock); + kthread_parkme(); + WARN_ON(!worker_maybe_bind_and_lock(worker)); + } + worker_leave_idle(worker); recheck: /* no more worker necessary? */ @@ -3340,6 +3383,12 @@ static int __cpuinit trustee_thread(void *__gcwq) gcwq->flags |= GCWQ_MANAGING_WORKERS; + if (!gcwq->first_idle && !list_empty(&gcwq->idle_list)) { + worker = list_first_entry(&gcwq->idle_list, + struct worker, entry); + park_idle_worker(worker); + } + list_for_each_entry(worker, &gcwq->idle_list, entry) worker->flags |= WORKER_ROGUE; @@ -3516,15 +3565,7 @@ static int __devinit workqueue_cpu_callback(struct notifier_block *nfb, if (IS_ERR(new_trustee)) return notifier_from_errno(PTR_ERR(new_trustee)); kthread_bind(new_trustee, cpu); - /* fall through */ - case CPU_UP_PREPARE: - BUG_ON(gcwq->first_idle); - new_worker = create_worker(gcwq, false); - if (!new_worker) { - if (new_trustee) - kthread_stop(new_trustee); - return NOTIFY_BAD; - } + break; } /* some are called w/ irq disabled, don't disturb irq status */ @@ -3540,8 +3581,18 @@ static int __devinit workqueue_cpu_callback(struct notifier_block *nfb, wait_trustee_state(gcwq, TRUSTEE_IN_CHARGE); /* fall through */ case CPU_UP_PREPARE: - BUG_ON(gcwq->first_idle); - gcwq->first_idle = new_worker; + if (!gcwq->first_idle) { + spin_unlock_irq(&gcwq->lock); + new_worker = create_worker(gcwq, false); + if (!new_worker) { + if (new_trustee) + kthread_stop(new_trustee); + return NOTIFY_BAD; + } + spin_lock_irq(&gcwq->lock); + BUG_ON(gcwq->first_idle); + gcwq->first_idle = new_worker; + } break; case CPU_DYING: @@ -3556,10 +3607,14 @@ static int __devinit workqueue_cpu_callback(struct notifier_block *nfb, case CPU_POST_DEAD: gcwq->trustee_state = TRUSTEE_BUTCHER; - /* fall through */ + break; + case CPU_UP_CANCELED: - destroy_worker(gcwq->first_idle); - gcwq->first_idle = NULL; + /* If worker is parked, leave it parked for later */ + if (!(gcwq->first_idle->flags & WORKER_PARK)) { + destroy_worker(gcwq->first_idle); + gcwq->first_idle = NULL; + } break; case CPU_DOWN_FAILED: @@ -3570,17 +3625,22 @@ static int __devinit workqueue_cpu_callback(struct notifier_block *nfb, wake_up_process(gcwq->trustee); wait_trustee_state(gcwq, TRUSTEE_DONE); } + BUG_ON(!gcwq->first_idle); /* * Trustee is done and there might be no worker left. * Put the first_idle in and request a real manager to * take a look. */ - spin_unlock_irq(&gcwq->lock); - kthread_bind(gcwq->first_idle->task, cpu); - spin_lock_irq(&gcwq->lock); gcwq->flags |= GCWQ_MANAGE_WORKERS; - start_worker(gcwq->first_idle); + if (!(gcwq->first_idle->flags & WORKER_PARK)) { + spin_unlock_irq(&gcwq->lock); + kthread_bind(gcwq->first_idle->task, cpu); + spin_lock_irq(&gcwq->lock); + start_worker(gcwq->first_idle); + } else { + unpark_worker(gcwq->first_idle); + } gcwq->first_idle = NULL; break; }
| |