From eef6a7d5c2f38adadab8240fabf43730fe796482 Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Fri, 12 Feb 2010 17:39:21 +0900 Subject: workqueue: warn about flush_scheduled_work() This patch (as1319) adds kerneldoc and a pointed warning to flush_scheduled_work(). Signed-off-by: Alan Stern Signed-off-by: Tejun Heo --- kernel/workqueue.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) (limited to 'kernel/workqueue.c') diff --git a/kernel/workqueue.c b/kernel/workqueue.c index 5bfb213984b..0225fea8934 100644 --- a/kernel/workqueue.c +++ b/kernel/workqueue.c @@ -845,6 +845,30 @@ int schedule_on_each_cpu(work_func_t func) return 0; } +/** + * flush_scheduled_work - ensure that any scheduled work has run to completion. + * + * Forces execution of the kernel-global workqueue and blocks until its + * completion. + * + * Think twice before calling this function! It's very easy to get into + * trouble if you don't take great care. Either of the following situations + * will lead to deadlock: + * + * One of the work items currently on the workqueue needs to acquire + * a lock held by your code or its caller. + * + * Your code is running in the context of a work routine. + * + * They will be detected by lockdep when they occur, but the first might not + * occur very often. It depends on what work items are on the workqueue and + * what locks they need, which you have no control over. + * + * In most situations flushing the entire workqueue is overkill; you merely + * need to know that a particular work item isn't queued and isn't running. + * In such cases you should use cancel_delayed_work_sync() or + * cancel_work_sync() instead. + */ void flush_scheduled_work(void) { flush_workqueue(keventd_wq); -- cgit v1.2.3-70-g09d2 From 4d707b9f48e2c4aa94b96f1133813b73df71fb55 Mon Sep 17 00:00:00 2001 From: Oleg Nesterov Date: Fri, 23 Apr 2010 17:40:40 +0200 Subject: workqueue: change cancel_work_sync() to clear work->data In short: change cancel_work_sync(work) to mark this work as "never queued" upon return. When cancel_work_sync(work) succeeds, we know that this work can't be queued or running, and since we own WORK_STRUCT_PENDING nobody can change the bits in work->data under us. This means we can also clear the "cwq" part along with _PENDING bit lockless before return, unless the work is queued nobody can assume get_wq_data() is stable even under cwq->lock. This change can speedup the subsequent cancel/flush requests, and as Dmitry pointed out this simplifies the usage of work_struct's which can be queued on different workqueues. Consider this pseudo code from the input subsystem: struct workqueue_struct *WQ; struct work_struct *WORK; for (;;) { WQ = create_workqueue(); ... if (condition()) queue_work(WQ, WORK); ... cancel_work_sync(WORK); destroy_workqueue(WQ); } If condition() returns T and then F, cancel_work_sync() will crash the kernel because WORK->data still points to the already destroyed workqueue. With this patch the code like above becomes correct. Suggested-by: Dmitry Torokhov Signed-off-by: Oleg Nesterov Signed-off-by: Tejun Heo --- kernel/workqueue.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) (limited to 'kernel/workqueue.c') diff --git a/kernel/workqueue.c b/kernel/workqueue.c index 0225fea8934..77dabbf64b8 100644 --- a/kernel/workqueue.c +++ b/kernel/workqueue.c @@ -229,6 +229,16 @@ static inline void set_wq_data(struct work_struct *work, atomic_long_set(&work->data, new); } +/* + * Clear WORK_STRUCT_PENDING and the workqueue on which it was queued. + */ +static inline void clear_wq_data(struct work_struct *work) +{ + unsigned long flags = *work_data_bits(work) & + (1UL << WORK_STRUCT_STATIC); + atomic_long_set(&work->data, flags); +} + static inline struct cpu_workqueue_struct *get_wq_data(struct work_struct *work) { @@ -671,7 +681,7 @@ static int __cancel_work_timer(struct work_struct *work, wait_on_work(work); } while (unlikely(ret < 0)); - work_clear_pending(work); + clear_wq_data(work); return ret; } -- cgit v1.2.3-70-g09d2 From 80b5184cc537718122e036afe7e62d202b70d077 Mon Sep 17 00:00:00 2001 From: Akinobu Mita Date: Wed, 26 May 2010 14:43:32 -0700 Subject: kernel/: convert cpu notifier to return encapsulate errno value By the previous modification, the cpu notifier can return encapsulate errno value. This converts the cpu notifiers for kernel/*.c Signed-off-by: Akinobu Mita Cc: Ingo Molnar Cc: Peter Zijlstra Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- kernel/padata.c | 4 ++-- kernel/profile.c | 4 ++-- kernel/relay.c | 2 +- kernel/smp.c | 2 +- kernel/softirq.c | 2 +- kernel/timer.c | 7 +++++-- kernel/workqueue.c | 9 +++++---- 7 files changed, 17 insertions(+), 13 deletions(-) (limited to 'kernel/workqueue.c') diff --git a/kernel/padata.c b/kernel/padata.c index b1c9857f840..fdd8ae609ce 100644 --- a/kernel/padata.c +++ b/kernel/padata.c @@ -659,7 +659,7 @@ static int padata_cpu_callback(struct notifier_block *nfb, err = __padata_add_cpu(pinst, cpu); mutex_unlock(&pinst->lock); if (err) - return NOTIFY_BAD; + return notifier_from_errno(err); break; case CPU_DOWN_PREPARE: @@ -670,7 +670,7 @@ static int padata_cpu_callback(struct notifier_block *nfb, err = __padata_remove_cpu(pinst, cpu); mutex_unlock(&pinst->lock); if (err) - return NOTIFY_BAD; + return notifier_from_errno(err); break; case CPU_UP_CANCELED: diff --git a/kernel/profile.c b/kernel/profile.c index dfadc5b729f..1e6a0d94ea6 100644 --- a/kernel/profile.c +++ b/kernel/profile.c @@ -372,7 +372,7 @@ static int __cpuinit profile_cpu_callback(struct notifier_block *info, GFP_KERNEL | __GFP_ZERO, 0); if (!page) - return NOTIFY_BAD; + return notifier_from_errno(-ENOMEM); per_cpu(cpu_profile_hits, cpu)[1] = page_address(page); } if (!per_cpu(cpu_profile_hits, cpu)[0]) { @@ -388,7 +388,7 @@ out_free: page = virt_to_page(per_cpu(cpu_profile_hits, cpu)[1]); per_cpu(cpu_profile_hits, cpu)[1] = NULL; __free_page(page); - return NOTIFY_BAD; + return notifier_from_errno(-ENOMEM); case CPU_ONLINE: case CPU_ONLINE_FROZEN: if (prof_cpu_mask != NULL) diff --git a/kernel/relay.c b/kernel/relay.c index 4268287148c..c7cf397fb92 100644 --- a/kernel/relay.c +++ b/kernel/relay.c @@ -539,7 +539,7 @@ static int __cpuinit relay_hotcpu_callback(struct notifier_block *nb, "relay_hotcpu_callback: cpu %d buffer " "creation failed\n", hotcpu); mutex_unlock(&relay_channels_mutex); - return NOTIFY_BAD; + return notifier_from_errno(-ENOMEM); } } mutex_unlock(&relay_channels_mutex); diff --git a/kernel/smp.c b/kernel/smp.c index 3fc69733618..75c970c715d 100644 --- a/kernel/smp.c +++ b/kernel/smp.c @@ -52,7 +52,7 @@ hotplug_cfd(struct notifier_block *nfb, unsigned long action, void *hcpu) case CPU_UP_PREPARE_FROZEN: if (!zalloc_cpumask_var_node(&cfd->cpumask, GFP_KERNEL, cpu_to_node(cpu))) - return NOTIFY_BAD; + return notifier_from_errno(-ENOMEM); break; #ifdef CONFIG_HOTPLUG_CPU diff --git a/kernel/softirq.c b/kernel/softirq.c index 0db913a5c60..825e1126008 100644 --- a/kernel/softirq.c +++ b/kernel/softirq.c @@ -808,7 +808,7 @@ static int __cpuinit cpu_callback(struct notifier_block *nfb, p = kthread_create(run_ksoftirqd, hcpu, "ksoftirqd/%d", hotcpu); if (IS_ERR(p)) { printk("ksoftirqd for %i failed\n", hotcpu); - return NOTIFY_BAD; + return notifier_from_errno(PTR_ERR(p)); } kthread_bind(p, hotcpu); per_cpu(ksoftirqd, hotcpu) = p; diff --git a/kernel/timer.c b/kernel/timer.c index be394af5bc2..e3b8c697bde 100644 --- a/kernel/timer.c +++ b/kernel/timer.c @@ -1680,11 +1680,14 @@ static int __cpuinit timer_cpu_notify(struct notifier_block *self, unsigned long action, void *hcpu) { long cpu = (long)hcpu; + int err; + switch(action) { case CPU_UP_PREPARE: case CPU_UP_PREPARE_FROZEN: - if (init_timers_cpu(cpu) < 0) - return NOTIFY_BAD; + err = init_timers_cpu(cpu); + if (err < 0) + return notifier_from_errno(err); break; #ifdef CONFIG_HOTPLUG_CPU case CPU_DEAD: diff --git a/kernel/workqueue.c b/kernel/workqueue.c index 77dabbf64b8..327d2deb445 100644 --- a/kernel/workqueue.c +++ b/kernel/workqueue.c @@ -1110,7 +1110,7 @@ static int __devinit workqueue_cpu_callback(struct notifier_block *nfb, unsigned int cpu = (unsigned long)hcpu; struct cpu_workqueue_struct *cwq; struct workqueue_struct *wq; - int ret = NOTIFY_OK; + int err = 0; action &= ~CPU_TASKS_FROZEN; @@ -1124,12 +1124,13 @@ undo: switch (action) { case CPU_UP_PREPARE: - if (!create_workqueue_thread(cwq, cpu)) + err = create_workqueue_thread(cwq, cpu); + if (!err) break; printk(KERN_ERR "workqueue [%s] for %i failed\n", wq->name, cpu); action = CPU_UP_CANCELED; - ret = NOTIFY_BAD; + err = -ENOMEM; goto undo; case CPU_ONLINE: @@ -1150,7 +1151,7 @@ undo: cpumask_clear_cpu(cpu, cpu_populated_map); } - return ret; + return notifier_from_errno(err); } #ifdef CONFIG_SMP -- cgit v1.2.3-70-g09d2