workqueue: implement mod_delayed_work[_on]()
Subsystems (15)
Tejun Heo
maintainerPATCH 1/1 workqueue: implement mod_delayed_work[_on]()
delayed_work has been annoyingly missing the mechanism to modify timer of a pending delayed_work - ie. mod_timer() counterpart. delayed_work users have been working around this using several methods - using an explicit timer + work item, messing directly with delayed_work->timer, and canceling before re-queueing, all of which are error-prone and/or ugly.
Gustavo Padovan posted a RFC implementation[1] of mod_delayed_work() a while back but it wasn't complete. To properly implement mod_delayed_work[_on](), it should be able to steal pending work items which may be on timer or worklist or anywhere inbetween. This is similar to what __cancel_work_timer() does but it turns out that there are a lot of holes around this area and try_to_grab_pending() needs considerable amount of work to be used for other purposes too.
This patchset improves canceling and try_to_grab_pending(), use it to implement mod_delayed_work[_on](), convert easy ones, and drop __cancel_delayed_work_sync() which doesn't have relevant users afterwards.
0001-workqueue-reorder-queueing-functions-so-that-_on-var.patch 0002-workqueue-make-queueing-functions-return-bool.patch 0003-workqueue-add-missing-smp_wmb-in-process_one_work.patch 0004-workqueue-disable-preemption-while-manipulating-PEND.patch 0005-workqueue-set-delayed_work-timer-function-on-initial.patch 0006-workqueue-unify-local-CPU-queueing-handling.patch 0007-workqueue-fix-zero-delay-handling-of-queue_delayed_w.patch 0008-workqueue-move-try_to_grab_pending-upwards.patch 0009-workqueue-introduce-WORK_OFFQ_FLAG_.patch 0010-workqueue-factor-out-__queue_delayed_work-from-queue.patch 0011-workqueue-reorganize-try_to_grab_pending-and-__cance.patch 0012-workqueue-mark-a-work-item-being-canceled-as-such.patch 0013-workqueue-implement-mod_delayed_work-_on.patch 0014-workqueue-use-mod_delayed_work-instead-of-cancel-que.patch 0015-workqueue-deprecate-__cancel_delayed_work.patch
0001-0003 are preparatory.
0004 removes the possibility of cancel_sync spinning for extended period of time while another task holding PENDING is preempted.
0005-0007 clean up local queueing handling.
0008-0011 prepare for try_to_grab_pending() improvements.
0012 makes try_to_grab_pending() distinguish transient failure which can be safely busy-retried and failure because the work item is being canceled, which may take arbitrary amount of time.
0013 uses the improve try_to_grab_pending() to implement mod_delayed_work[_on]().
0014 converts cancel + queue sequences to mod_delayed_work().
0015 drops __cancel_delayed_work() which doesn't have any relevant user left. Not all conversions are trivial. If you're involved with edac, thinkpad_acpi, charger-manager, thermal_sys or nic link_watch, please take a look at the conversion made in 0015.
This patchset is also available at the following git branch.
git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git review-wq-mod_delayed
If nobody objects, I'd like to route this series through wq/for-3.7. Changes to other subsystems are fairly localized and conflicts, if they occur, shouldn't be too painful to handle.
Although this ends up adding ~90 LOC, it contains a lot more documentation, converted only the apparent ones, and IMHO is worthwhile to have regardless as it removes an annoyance which is pretty easy to encounter while using delayed_work.
Thanks.
block/blk-core.c | 8 block/blk-throttle.c | 7 block/genhd.c | 6 drivers/block/floppy.c | 5 drivers/edac/edac_mc.c | 17 drivers/infiniband/core/addr.c | 4 drivers/infiniband/core/mad.c | 16 drivers/infiniband/hw/nes/nes_hw.c | 6 drivers/infiniband/hw/nes/nes_nic.c | 5 drivers/input/keyboard/qt2160.c | 3 drivers/input/mouse/synaptics_i2c.c | 7 drivers/net/wireless/ipw2x00/ipw2100.c | 8 drivers/net/wireless/zd1211rw/zd_usb.c | 3 drivers/platform/x86/thinkpad_acpi.c | 20 drivers/power/charger-manager.c | 9 drivers/power/ds2760_battery.c | 9 drivers/power/jz4740-battery.c | 6 drivers/thermal/thermal_sys.c | 15 drivers/video/omap2/dss/dsi.c | 2 fs/afs/callback.c | 4 fs/afs/server.c | 10 fs/afs/vlocation.c | 14 fs/nfs/nfs4renewd.c | 3 include/linux/workqueue.h | 49 +- kernel/workqueue.c | 690 ++++++++++++++++++++------------- net/core/dst.c | 4 net/core/link_watch.c | 21 - net/rfkill/input.c | 3 28 files changed, 521 insertions(+), 433 deletions(-)
--
tejun
Tejun Heo
maintainerPATCH 1/15 workqueue: reorder queueing functions so that _on() variants are on top
Currently, queue/schedule[_delayed]_work_on() are located below the counterpart without the _on postifx even though the latter is usually implemented using the former. Swap them.
This is cleanup and doesn't cause any functional difference.
--- include/linux/workqueue.h | 13 +++-- kernel/workqueue.c | 124 ++++++++++++++++++++++---------------------- 2 files changed, 69 insertions(+), 68 deletions(-)
include/linux/workqueue.h +7 -6
@@ -365,23 +365,24 @@ __alloc_workqueue_key(const char *fmt, unsigned int flags, int max_active,extern void destroy_workqueue (struct workqueue_struct *wq); -extern int queue_work (struct workqueue_struct *wq,struct work_struct *work);extern int queue_work_on (int cpu,struct workqueue_struct *wq,struct work_struct *work); -extern int queue_delayed_work (struct workqueue_struct *wq, -struct delayed_work *work,unsigned long delay); +extern int queue_work (struct workqueue_struct *wq,struct work_struct *work);extern int queue_delayed_work_on (int cpu,struct workqueue_struct *wq,struct delayed_work *work,unsigned long delay); +extern int queue_delayed_work (struct workqueue_struct *wq, +struct delayed_work *work,unsigned long delay);extern void flush_workqueue (struct workqueue_struct *wq);extern void drain_workqueue (struct workqueue_struct *wq);extern void flush_scheduled_work (void ); -extern int schedule_work (struct work_struct *work);extern int schedule_work_on (int cpu,struct work_struct *work); -extern int schedule_delayed_work (struct delayed_work *work,unsigned long delay); +extern int schedule_work (struct work_struct *work);extern int schedule_delayed_work_on (int cpu,struct delayed_work *work, -unsigned long delay); +unsigned long delay); +extern int schedule_delayed_work (struct delayed_work *work, +unsigned long delay);extern int schedule_on_each_cpu (work_func_t func);extern int keventd_up (void );
kernel/workqueue.c +62 -62
172 lines not on this page · raw message
Signature · 1 line
1.7.7.3
Tejun Heo
maintainerPATCH 2/15 workqueue: make queueing functions return bool
In reply to Tejun HeoAll queueing functions return 1 on success, 0 if the work item was already pending. Update them to return bool instead. This signifies better that they don't return 0 / -errno.
This is cleanup and doesn't cause any functional difference.
While at it, fix comment opening for schedule_work_on().
--- include/linux/workqueue.h | 20 +++++++++--------- kernel/workqueue.c | 47 ++++++++++++++++++++++----------------------- 2 files changed, 33 insertions(+), 34 deletions(-)
include/linux/workqueue.h +10 -10
@@ -365,24 +365,24 @@ __alloc_workqueue_key(const char *fmt, unsigned int flags, int max_active,extern void destroy_workqueue (struct workqueue_struct *wq); -extern int queue_work_on (int cpu,struct workqueue_struct *wq, +extern bool queue_work_on (int cpu,struct workqueue_struct *wq,struct work_struct *work); -extern int queue_work (struct workqueue_struct *wq,struct work_struct *work); -extern int queue_delayed_work_on (int cpu,struct workqueue_struct *wq, +extern bool queue_work (struct workqueue_struct *wq,struct work_struct *work); +extern bool queue_delayed_work_on (int cpu,struct workqueue_struct *wq,struct delayed_work *work,unsigned long delay); -extern int queue_delayed_work (struct workqueue_struct *wq, +extern bool queue_delayed_work (struct workqueue_struct *wq,struct delayed_work *work,unsigned long delay);extern void flush_workqueue (struct workqueue_struct *wq);extern void drain_workqueue (struct workqueue_struct *wq);extern void flush_scheduled_work (void ); -extern int schedule_work_on (int cpu,struct work_struct *work); -extern int schedule_work (struct work_struct *work); -extern int schedule_delayed_work_on (int cpu,struct delayed_work *work, -unsigned long delay); -extern int schedule_delayed_work (struct delayed_work *work, -unsigned long delay); +extern bool schedule_work_on (int cpu,struct work_struct *work); +extern bool schedule_work (struct work_struct *work); +extern bool schedule_delayed_work_on (int cpu,struct delayed_work *work, +unsigned long delay); +extern bool schedule_delayed_work (struct delayed_work *work, +unsigned long delay);extern int schedule_on_each_cpu (work_func_t func);extern int keventd_up (void );
kernel/workqueue.c +23 -24
130 lines not on this page · raw message
Signature · 1 line
1.7.7.3
Tejun Heo
maintainerPATCH 3/15 workqueue: add missing smp_wmb() in process_one_work()
In reply to Tejun HeoWORK_STRUCT_PENDING is used to claim ownership of a work item and process_one_work() releases it before starting execution. When someone else grabs PENDING, all pre-release updates to the work item should be visible and all updates made by the new owner should happen afterwards.
Grabbing PENDING uses test_and_set_bit() and thus has a full barrier; however, clearing doesn't have a matching wmb. Given the preceding spin_unlock and use of clear_bit, I don't believe this can be a problem on an actual machine and there hasn't been any related report but it still is theretically possible for clear_pending to permeate upwards and happen before work->entry update.
Add an explicit smp_wmb() before work_clear_pending().
--- kernel/workqueue.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
kernel/workqueue.c +2 -0
@@ -1997,7 +1997,9 @@ __acquires(&gcwq->lock)spin_unlock_irq (&gcwq->lock); +smp_wmb ();/* paired with test_and_set_bit(PENDING) */ work_clear_pending (work); +lock_map_acquire_read (&cwq->wq->lockdep_map);lock_map_acquire (&lockdep_map);trace_workqueue_execute_start (work);
Signature · 1 line
1.7.7.3
Tejun Heo
maintainerPATCH 4/15 workqueue: disable preemption while manipulating PENDING
In reply to Tejun HeoQueueing operations use WORK_STRUCT_PENDING_BIT to synchronize access to the target work item. They first try to claim the bit and proceed with queueing only after that succeeds and there's a window between PENDING being set and the actual queueing where the task can be preempted.
There's also a similar window in process_one_work() when clearing PENDING. A work item is dequeued, gcwq->lock is released and then PENDING is cleared and the worker might get preempted between releasing gcwq->lock and clearing PENDING.
cancel[_delayed]_work_sync() tries to claim or steal PENDING. The function assumes that a work item with PENDING is either queued, or in the process of being queued. In the latter case, it busy-loops until either the work item loses PENDING or is queued. If canceling coincides with the above described preemptions, the canceling task will busy-loop while the queueing or executing task is preempted.
This patch keeps preemption disabled across claiming PENDING and actual queueing and moves PENDING clearing in process_one_work() inside gcwq->lock so that busy looping from PENDING && !queued doesn't wait for preempted tasks. Note that, in process_one_work(), setting last CPU and clearing PENDING got merged into single operation.
--- kernel/workqueue.c | 55 +++++++++++++++++++++++++++++++++++++++++---------- 1 files changed, 44 insertions(+), 11 deletions(-)
kernel/workqueue.c +44 -11
@@ -537,9 +537,10 @@ static int work_next_color(int color)* work is on queue. Once execution starts, WORK_STRUCT_CWQ is * cleared and the work data contains the cpu number it was last on. * -* set_work_{cwq|cpu}() and clear_work_data() can be used to set the -* cwq, cpu or clear work->data. These functions should only be -* called while the work is owned - ie. while the PENDING bit is set. +* set_work_cwq(), set_work_cpu_and_clear_pending() and clear_work_data() +* can be used to set the cwq, cpu or clear work->data. These functions +* should only be called while the work is owned - ie. while the PENDING +* bit is set. * * get_work_[g]cwq() can be used to obtain the gcwq or cwq * corresponding to a work. gcwq is available once the work has been
@@ -561,9 +562,10 @@ static void set_work_cwq(struct work_struct *work, WORK_STRUCT_PENDING | WORK_STRUCT_CWQ | extra_flags); } -static void set_work_cpu (struct work_struct *work,unsigned int cpu) +static void set_work_cpu_and_clear_pending (struct work_struct *work, +unsigned int cpu) { -set_work_data (work, cpu <<WORK_STRUCT_FLAG_BITS ,WORK_STRUCT_PENDING ); +set_work_data (work, cpu <<WORK_STRUCT_FLAG_BITS ,0 ); }static void clear_work_data (struct work_struct *work)
@@ -983,6 +985,15 @@ static void __queue_work(unsigned int cpu, struct workqueue_struct *wq,unsigned int work_flags;unsigned long flags; +/* +* While a work item is PENDING && off queue, a task trying to +* steal the PENDING will busy-loop waiting for it to either get +* queued or lose PENDING, so a task shouldn't be preempted between +* grabbing PENDING and queueing @work. Make sure the caller has +* preemption disabled. +*/ +WARN_ON_ONCE (!preempt_count ()); +debug_work_activate (work);/* if dying, only works from the same workqueue are allowed */
@@ -1068,10 +1079,14 @@ bool queue_work_on(int cpu, struct workqueue_struct *wq, {bool ret = false; +preempt_disable (); +if (!test_and_set_bit (WORK_STRUCT_PENDING_BIT ,work_data_bits (work))) {__queue_work (cpu, wq, work); ret = true; } + +preempt_enable ();return ret; }EXPORT_SYMBOL_GPL (queue_work_on);
@@ -1121,6 +1136,12 @@ bool queue_delayed_work_on(int cpu, struct workqueue_struct *wq,struct work_struct *work = &dwork->work;bool ret = false; +/* +* We shouldn't get preempted between claiming PENDING and adding +* timer. Read the comment in __queue_work() for details. +*/ +preempt_disable (); +if (!test_and_set_bit (WORK_STRUCT_PENDING_BIT ,work_data_bits (work))) {unsigned int lcpu;
@@ -1156,6 +1177,8 @@ bool queue_delayed_work_on(int cpu, struct workqueue_struct *wq,add_timer (timer); ret = true; } + +preempt_enable ();return ret; }EXPORT_SYMBOL_GPL (queue_delayed_work_on);
@@ -1970,15 +1993,13 @@ __acquires(&gcwq->lock)return ; } -/* claim and process */ +/* claim and dequeue */ debug_work_deactivate (work);hlist_add_head (&worker->hentry, bwh); worker->current_work = work; worker->current_cwq = cwq; work_color =get_work_color (work); -/* record the current cpu number in the work data and dequeue */ -set_work_cpu (work, gcwq->cpu);list_del_init (&work->entry);/*
@@ -1995,10 +2016,18 @@ __acquires(&gcwq->lock)if ((worker->flags &WORKER_UNBOUND ) &&need_more_worker (pool))wake_up_worker (pool); -spin_unlock_irq (&gcwq->lock); +/* +* Record the last CPU and clear PENDING. The following wmb is +* paired with the implied mb in test_and_set_bit(PENDING) and +* ensures all updates to @work made here are visible to and +* precede any updates by the next PENDING owner. Also, clearing +* PENDING is inside @gcwq->lock so that we don't get preempted +* with PENDING set and @work off queue. +*/ +smp_wmb (); +set_work_cpu_and_clear_pending (work, gcwq->cpu); -smp_wmb ();/* paired with test_and_set_bit(PENDING) */ -work_clear_pending (work); +spin_unlock_irq (&gcwq->lock);lock_map_acquire_read (&cwq->wq->lockdep_map);lock_map_acquire (&lockdep_map);
@@ -2836,9 +2865,11 @@ EXPORT_SYMBOL_GPL(cancel_work_sync);*/ bool flush_delayed_work (struct delayed_work *dwork) { +preempt_disable ();if (del_timer_sync (&dwork->timer))__queue_work (raw_smp_processor_id (),get_work_cwq (&dwork->work)->wq, &dwork->work); +preempt_enable ();return flush_work (&dwork->work); }EXPORT_SYMBOL (flush_delayed_work);
@@ -2857,9 +2888,11 @@ EXPORT_SYMBOL(flush_delayed_work);*/ bool flush_delayed_work_sync (struct delayed_work *dwork) { +preempt_disable ();if (del_timer_sync (&dwork->timer))__queue_work (raw_smp_processor_id (),get_work_cwq (&dwork->work)->wq, &dwork->work); +preempt_enable ();return flush_work_sync (&dwork->work); }EXPORT_SYMBOL (flush_delayed_work_sync);
Signature · 1 line
1.7.7.3
Tejun Heo
maintainerPATCH v2 4/15 workqueue: disable preemption while manipulating PENDING
Quoted 1 line
From
bdbc04720254d1a84504074a6b25189961030803Mon Sep 17 00:00:00 2001
From: Tejun Heo <[email protected]>
Date: Mon, 30 Jul 2012 13:03:57 -0700
Queueing operations use WORK_STRUCT_PENDING_BIT to synchronize access to the target work item. They first try to claim the bit and proceed with queueing only after that succeeds and there's a window between PENDING being set and the actual queueing where the task can be preempted.
There's also a similar window in process_one_work() when clearing PENDING. A work item is dequeued, gcwq->lock is released and then PENDING is cleared and the worker might get preempted between releasing gcwq->lock and clearing PENDING.
cancel[_delayed]_work_sync() tries to claim or steal PENDING. The function assumes that a work item with PENDING is either queued, or in the process of being queued. In the latter case, it busy-loops until either the work item loses PENDING or is queued. If canceling coincides with the above described preemptions, the canceling task will busy-loop while the queueing or executing task is preempted.
This patch keeps preemption disabled across claiming PENDING and actual queueing and moves PENDING clearing in process_one_work() inside gcwq->lock so that busy looping from PENDING && !queued doesn't wait for preempted tasks. Note that, in process_one_work(), setting last CPU and clearing PENDING got merged into single operation.
v2: __queue_work() was testing preempt_count() to ensure that the
caller has disabled preemption. This triggers spuriously if !CONFIG_PREEMPT_COUNT. Use preemptible() instead. Reported by Fengguang Wu.
---
git branch updated accordingly.
kernel/workqueue.c | 55 +++++++++++++++++++++++++++++++++++++++++---------- 1 files changed, 44 insertions(+), 11 deletions(-)
kernel/workqueue.c +44 -11
@@ -537,9 +537,10 @@ static int work_next_color(int color)* work is on queue. Once execution starts, WORK_STRUCT_CWQ is * cleared and the work data contains the cpu number it was last on. * -* set_work_{cwq|cpu}() and clear_work_data() can be used to set the -* cwq, cpu or clear work->data. These functions should only be -* called while the work is owned - ie. while the PENDING bit is set. +* set_work_cwq(), set_work_cpu_and_clear_pending() and clear_work_data() +* can be used to set the cwq, cpu or clear work->data. These functions +* should only be called while the work is owned - ie. while the PENDING +* bit is set. * * get_work_[g]cwq() can be used to obtain the gcwq or cwq * corresponding to a work. gcwq is available once the work has been
@@ -561,9 +562,10 @@ static void set_work_cwq(struct work_struct *work, WORK_STRUCT_PENDING | WORK_STRUCT_CWQ | extra_flags); } -static void set_work_cpu (struct work_struct *work,unsigned int cpu) +static void set_work_cpu_and_clear_pending (struct work_struct *work, +unsigned int cpu) { -set_work_data (work, cpu <<WORK_STRUCT_FLAG_BITS ,WORK_STRUCT_PENDING ); +set_work_data (work, cpu <<WORK_STRUCT_FLAG_BITS ,0 ); }static void clear_work_data (struct work_struct *work)
@@ -983,6 +985,15 @@ static void __queue_work(unsigned int cpu, struct workqueue_struct *wq,unsigned int work_flags;unsigned long flags; +/* +* While a work item is PENDING && off queue, a task trying to +* steal the PENDING will busy-loop waiting for it to either get +* queued or lose PENDING, so a task shouldn't be preempted between +* grabbing PENDING and queueing @work. Make sure the caller has +* preemption disabled. +*/ +WARN_ON_ONCE (preemptible ()); +debug_work_activate (work);/* if dying, only works from the same workqueue are allowed */
@@ -1068,10 +1079,14 @@ bool queue_work_on(int cpu, struct workqueue_struct *wq, {bool ret = false; +preempt_disable (); +if (!test_and_set_bit (WORK_STRUCT_PENDING_BIT ,work_data_bits (work))) {__queue_work (cpu, wq, work); ret = true; } + +preempt_enable ();return ret; }EXPORT_SYMBOL_GPL (queue_work_on);
@@ -1121,6 +1136,12 @@ bool queue_delayed_work_on(int cpu, struct workqueue_struct *wq,struct work_struct *work = &dwork->work;bool ret = false; +/* +* We shouldn't get preempted between claiming PENDING and adding +* timer. Read the comment in __queue_work() for details. +*/ +preempt_disable (); +if (!test_and_set_bit (WORK_STRUCT_PENDING_BIT ,work_data_bits (work))) {unsigned int lcpu;
@@ -1156,6 +1177,8 @@ bool queue_delayed_work_on(int cpu, struct workqueue_struct *wq,add_timer (timer); ret = true; } + +preempt_enable ();return ret; }EXPORT_SYMBOL_GPL (queue_delayed_work_on);
@@ -1970,15 +1993,13 @@ __acquires(&gcwq->lock)return ; } -/* claim and process */ +/* claim and dequeue */ debug_work_deactivate (work);hlist_add_head (&worker->hentry, bwh); worker->current_work = work; worker->current_cwq = cwq; work_color =get_work_color (work); -/* record the current cpu number in the work data and dequeue */ -set_work_cpu (work, gcwq->cpu);list_del_init (&work->entry);/*
@@ -1995,10 +2016,18 @@ __acquires(&gcwq->lock)if ((worker->flags &WORKER_UNBOUND ) &&need_more_worker (pool))wake_up_worker (pool); -spin_unlock_irq (&gcwq->lock); +/* +* Record the last CPU and clear PENDING. The following wmb is +* paired with the implied mb in test_and_set_bit(PENDING) and +* ensures all updates to @work made here are visible to and +* precede any updates by the next PENDING owner. Also, clearing +* PENDING is inside @gcwq->lock so that we don't get preempted +* with PENDING set and @work off queue. +*/ +smp_wmb (); +set_work_cpu_and_clear_pending (work, gcwq->cpu); -smp_wmb ();/* paired with test_and_set_bit(PENDING) */ -work_clear_pending (work); +spin_unlock_irq (&gcwq->lock);lock_map_acquire_read (&cwq->wq->lockdep_map);lock_map_acquire (&lockdep_map);
@@ -2836,9 +2865,11 @@ EXPORT_SYMBOL_GPL(cancel_work_sync);*/ bool flush_delayed_work (struct delayed_work *dwork) { +preempt_disable ();if (del_timer_sync (&dwork->timer))__queue_work (raw_smp_processor_id (),get_work_cwq (&dwork->work)->wq, &dwork->work); +preempt_enable ();return flush_work (&dwork->work); }EXPORT_SYMBOL (flush_delayed_work);
@@ -2857,9 +2888,11 @@ EXPORT_SYMBOL(flush_delayed_work);*/ bool flush_delayed_work_sync (struct delayed_work *dwork) { +preempt_disable ();if (del_timer_sync (&dwork->timer))__queue_work (raw_smp_processor_id (),get_work_cwq (&dwork->work)->wq, &dwork->work); +preempt_enable ();return flush_work_sync (&dwork->work); }EXPORT_SYMBOL (flush_delayed_work_sync);
Signature · 1 line
1.7.7.3
Tejun Heo
maintainerPATCH 5/15 workqueue: set delayed_work->timer function on initialization
In reply to Tejun Heodelayed_work->timer.function is currently initialized during queue_delayed_work_on(). Export delayed_work_timer_fn() and set delayed_work timer function during delayed_work initialization together with other fields.
This ensures the timer function is always valid on an initialized delayed_work. This is to help mod_delayed_work() implementation.
To detect delayed_work users which diddle with the internal timer, trigger WARN if timer function doesn't match on queue.
--- include/linux/workqueue.h | 13 +++++++++++-- kernel/workqueue.c | 7 ++++--- 2 files changed, 15 insertions(+), 5 deletions(-)
include/linux/workqueue.h +11 -2
@@ -16,6 +16,7 @@ struct workqueue_struct;struct work_struct ;typedef void (*work_func_t )(struct work_struct *work); +void delayed_work_timer_fn (unsigned long __data);/* * The first word is the work queue pointer and the flags rolled into
@@ -124,12 +125,14 @@ struct execute_work {#define __DELAYED_WORK_INITIALIZER (n, f) { \ .work = __WORK_INITIALIZER((n).work, (f)), \ - .timer = TIMER_INITIALIZER(NULL, 0, 0), \ + .timer = TIMER_INITIALIZER(delayed_work_timer_fn, \ + 0, (unsigned long)&(n)), \ }#define __DEFERRED_WORK_INITIALIZER (n, f) { \ .work = __WORK_INITIALIZER((n).work, (f)), \ - .timer = TIMER_DEFERRED_INITIALIZER(NULL, 0, 0), \ + .timer = TIMER_DEFERRED_INITIALIZER(delayed_work_timer_fn, \ + 0, (unsigned long)&(n)), \ }#define DECLARE_WORK (n, f) \
@@ -207,18 +210,24 @@ static inline unsigned int work_static(struct work_struct *work) { return 0; }do { \INIT_WORK (&(_work)->work, (_func)); \init_timer (&(_work)->timer); \ + (_work)->timer.function = delayed_work_timer_fn;\ + (_work)->timer.data = (unsigned long )(_work); \ } while (0)#define INIT_DELAYED_WORK_ONSTACK (_work, _func) \ do { \ INIT_WORK_ONSTACK(&(_work)->work, (_func)); \ init_timer_on_stack(&(_work)->timer); \ + (_work)->timer.function = delayed_work_timer_fn;\ + (_work)->timer.data = (unsigned long)(_work); \ } while (0)#define INIT_DELAYED_WORK_DEFERRABLE (_work, _func) \ do { \ INIT_WORK(&(_work)->work, (_func)); \ init_timer_deferrable(&(_work)->timer); \ + (_work)->timer.function = delayed_work_timer_fn;\ + (_work)->timer.data = (unsigned long)(_work); \ } while (0)/**
kernel/workqueue.c +4 -3
@@ -1112,13 +1112,14 @@ bool queue_work(struct workqueue_struct *wq, struct work_struct *work) }EXPORT_SYMBOL_GPL (queue_work); -static void delayed_work_timer_fn (unsigned long __data) +void delayed_work_timer_fn (unsigned long __data) {struct delayed_work *dwork = (struct delayed_work *)__data;struct cpu_workqueue_struct *cwq =get_work_cwq (&dwork->work);__queue_work (smp_processor_id (), cwq->wq, &dwork->work); } +EXPORT_SYMBOL_GPL (delayed_work_timer_fn);/** * queue_delayed_work_on - queue work on specific CPU after delay
@@ -1145,6 +1146,8 @@ bool queue_delayed_work_on(int cpu, struct workqueue_struct *wq,if (!test_and_set_bit (WORK_STRUCT_PENDING_BIT ,work_data_bits (work))) {unsigned int lcpu; +WARN_ON_ONCE (timer->function != delayed_work_timer_fn || + timer->data != (unsigned long )dwork);BUG_ON (timer_pending (timer));BUG_ON (!list_empty (&work->entry));
@@ -1168,8 +1171,6 @@ bool queue_delayed_work_on(int cpu, struct workqueue_struct *wq,set_work_cwq (work,get_cwq (lcpu, wq),0 ); timer->expires = jiffies + delay; - timer->data = (unsigned long )dwork; - timer->function = delayed_work_timer_fn;if (unlikely (cpu >=0 ))add_timer_on (timer, cpu);
Signature · 1 line
1.7.7.3
Tejun Heo
maintainerPATCH 6/15 workqueue: unify local CPU queueing handling
In reply to Tejun HeoQueueing functions have been using different methods to determine the local CPU.
* queue_work() superflously uses get/put_cpu() to acquire and hold the local CPU across queue_work_on().
* delayed_work_timer_fn() uses smp_processor_id().
* queue_delayed_work() calls queue_delayed_work_on() with -1 @cpu which is interpreted as the local CPU.
* flush_delayed_work[_sync]() were using raw_smp_processor_id().
* __queue_work() interprets %WORK_CPU_UNBOUND as local CPU if the target workqueue is bound one but nobody uses this.
This patch converts all functions to uniformly use %WORK_CPU_UNBOUND to indicate local CPU and use the local binding feature of __queue_work(). unlikely() is dropped from %WORK_CPU_UNBOUND handling in __queue_work().
--- kernel/workqueue.c | 19 +++++++------------ 1 files changed, 7 insertions(+), 12 deletions(-)
kernel/workqueue.c +7 -12
@@ -1005,7 +1005,7 @@ static void __queue_work(unsigned int cpu, struct workqueue_struct *wq,if (!(wq->flags &WQ_UNBOUND )) {struct global_cwq *last_gcwq; -if (unlikely (cpu ==WORK_CPU_UNBOUND )) +if (cpu ==WORK_CPU_UNBOUND ) cpu =raw_smp_processor_id ();/*
@@ -1103,12 +1103,7 @@ EXPORT_SYMBOL_GPL(queue_work_on);*/ bool queue_work (struct workqueue_struct *wq,struct work_struct *work) { -bool ret; - - ret =queue_work_on (get_cpu (), wq, work); -put_cpu (); - -return ret; +return queue_work_on (WORK_CPU_UNBOUND , wq, work); }EXPORT_SYMBOL_GPL (queue_work);
@@ -1117,7 +1112,7 @@ void delayed_work_timer_fn(unsigned long __data)struct delayed_work *dwork = (struct delayed_work *)__data;struct cpu_workqueue_struct *cwq =get_work_cwq (&dwork->work); -__queue_work (smp_processor_id (), cwq->wq, &dwork->work); +__queue_work (WORK_CPU_UNBOUND , cwq->wq, &dwork->work); }EXPORT_SYMBOL_GPL (delayed_work_timer_fn);
@@ -1172,7 +1167,7 @@ bool queue_delayed_work_on(int cpu, struct workqueue_struct *wq, timer->expires = jiffies + delay; -if (unlikely (cpu >=0 )) +if (unlikely (cpu !=WORK_CPU_UNBOUND ))add_timer_on (timer, cpu);else add_timer (timer);
@@ -1198,7 +1193,7 @@ bool queue_delayed_work(struct workqueue_struct *wq,if (delay ==0 )return queue_work (wq, &dwork->work); -return queue_delayed_work_on (-1 , wq, dwork, delay); +return queue_delayed_work_on (WORK_CPU_UNBOUND , wq, dwork, delay); }EXPORT_SYMBOL_GPL (queue_delayed_work);
@@ -2868,7 +2863,7 @@ bool flush_delayed_work(struct delayed_work *dwork) {preempt_disable ();if (del_timer_sync (&dwork->timer)) -__queue_work (raw_smp_processor_id (), +__queue_work (WORK_CPU_UNBOUND ,get_work_cwq (&dwork->work)->wq, &dwork->work);preempt_enable ();return flush_work (&dwork->work);
@@ -2891,7 +2886,7 @@ bool flush_delayed_work_sync(struct delayed_work *dwork) {preempt_disable ();if (del_timer_sync (&dwork->timer)) -__queue_work (raw_smp_processor_id (), +__queue_work (WORK_CPU_UNBOUND ,get_work_cwq (&dwork->work)->wq, &dwork->work);preempt_enable ();return flush_work_sync (&dwork->work);
Signature · 1 line
1.7.7.3
Tejun Heo
maintainerPATCH 7/15 workqueue: fix zero @delay handling of queue_delayed_work_on()
In reply to Tejun HeoIf @delay is zero and the dealyed_work is idle, queue_delayed_work() queues it for immediate execution; however, queue_delayed_work_on() lacks this logic and always goes through timer regardless of @delay.
This patch moves 0 @delay handling logic from queue_delayed_work() to queue_delayed_work_on() so that both functions behave the same.
--- kernel/workqueue.c | 12 +++++++----- 1 files changed, 7 insertions(+), 5 deletions(-)
kernel/workqueue.c +7 -5
@@ -1123,7 +1123,9 @@ EXPORT_SYMBOL_GPL(delayed_work_timer_fn);* @dwork: work to queue * @delay: number of jiffies to wait before queueing * -* Returns %false if @work was already on a queue, %true otherwise. +* Returns %false if @work was already on a queue, %true otherwise. If +* @delay is zero and @dwork is idle, it will be scheduled for immediate +* execution. */ bool queue_delayed_work_on (int cpu,struct workqueue_struct *wq,struct delayed_work *dwork,unsigned long delay)
@@ -1132,6 +1134,9 @@ bool queue_delayed_work_on(int cpu, struct workqueue_struct *wq,struct work_struct *work = &dwork->work;bool ret = false; +if (!delay) +return queue_work_on (cpu, wq, &dwork->work); +/* * We shouldn't get preempted between claiming PENDING and adding * timer. Read the comment in __queue_work() for details.
@@ -1185,14 +1190,11 @@ EXPORT_SYMBOL_GPL(queue_delayed_work_on);* @dwork: delayable work to queue * @delay: number of jiffies to wait before queueing * -* Returns %false if @work was already on a queue, %true otherwise. +* Equivalent to queue_delayed_work_on() but tries to use the local CPU. */ bool queue_delayed_work (struct workqueue_struct *wq,struct delayed_work *dwork,unsigned long delay) { -if (delay ==0 ) -return queue_work (wq, &dwork->work); -return queue_delayed_work_on (WORK_CPU_UNBOUND , wq, dwork, delay); }EXPORT_SYMBOL_GPL (queue_delayed_work);
Signature · 1 line
1.7.7.3
Tejun Heo
maintainerPATCH 8/15 workqueue: move try_to_grab_pending() upwards
In reply to Tejun Heotry_to_grab_pending() will be used by to-be-implemented mod_delayed_work[_on](). Move try_to_grab_pending() and related functions above queueing functions.
This patch only moves functions around.
--- kernel/workqueue.c | 286 ++++++++++++++++++++++++++-------------------------- 1 files changed, 143 insertions(+), 143 deletions(-)
kernel/workqueue.c +143 -143
@@ -904,6 +904,149 @@ static struct worker *find_worker_executing_work(struct global_cwq *gcwq, }/** +* move_linked_works - move linked works to a list +* @work: start of series of works to be scheduled +* @head: target list to append @work to +* @nextp: out paramter for nested worklist walking +* +* Schedule linked works starting from @work to @head. Work series to +* be scheduled starts at @work and includes any consecutive work with +* WORK_STRUCT_LINKED set in its predecessor. +* +* If @nextp is not NULL, it's updated to point to the next work of +* the last scheduled work. This allows move_linked_works() to be +* nested inside outer list_for_each_entry_safe(). +* +* CONTEXT: +* spin_lock_irq(gcwq->lock). +*/ +static void move_linked_works (struct work_struct *work,struct list_head *head, +struct work_struct **nextp) +{ +struct work_struct *n; + +/* +* Linked worklist will always end before the end of the list, +* use NULL for list head. +*/ +list_for_each_entry_safe_from (work, n,NULL , entry) { +list_move_tail (&work->entry, head); +if (!(*work_data_bits (work) &WORK_STRUCT_LINKED )) +break ; + } + +/* +* If we're already inside safe list traversal and have moved +* multiple works to the scheduled queue, the next position +* needs to be updated. +*/ +if (nextp) + *nextp = n; +} + +static void cwq_activate_first_delayed (struct cpu_workqueue_struct *cwq) +{ +struct work_struct *work =list_first_entry (&cwq->delayed_works, + struct work_struct, entry); + +trace_workqueue_activate_work (work); +move_linked_works (work, &cwq->pool->worklist,NULL ); +__clear_bit (WORK_STRUCT_DELAYED_BIT ,work_data_bits (work)); + cwq->nr_active++; +} + +/** +* cwq_dec_nr_in_flight - decrement cwq's nr_in_flight +* @cwq: cwq of interest +* @color: color of work which left the queue +* @delayed: for a delayed work +* +* A work either has completed or is removed from pending queue, +* decrement nr_in_flight of its cwq and handle workqueue flushing. +* +* CONTEXT: +* spin_lock_irq(gcwq->lock). +*/ +static void cwq_dec_nr_in_flight (struct cpu_workqueue_struct *cwq,int color, +bool delayed) +{ +/* ignore uncolored works */ +if (color ==WORK_NO_COLOR ) +return ; + + cwq->nr_in_flight[color]--; + +if (!delayed) { + cwq->nr_active--; +if (!list_empty (&cwq->delayed_works)) { +/* one down, submit a delayed one */ +if (cwq->nr_active < cwq->max_active) +cwq_activate_first_delayed (cwq); + } + } + +/* is flush in progress and are we at the flushing tip? */ +if (likely (cwq->flush_color != color)) +return ; + +/* are there still in-flight works? */ +if (cwq->nr_in_flight[color]) +return ; + +/* this cwq is done, clear flush_color */ + cwq->flush_color =-1 ; + +/* +* If this was the last cwq, wake up the first flusher. It +* will handle the rest. +*/ +if (atomic_dec_and_test (&cwq->wq->nr_cwqs_to_flush)) +complete (&cwq->wq->first_flusher->done); +} + +/* +* Upon a successful return (>= 0), the caller "owns" WORK_STRUCT_PENDING bit, +* so this work can't be re-armed in any way. +*/ +static int try_to_grab_pending (struct work_struct *work) +{ +struct global_cwq *gcwq; +int ret =-1 ; + +if (!test_and_set_bit (WORK_STRUCT_PENDING_BIT ,work_data_bits (work))) +return 0 ; + +/* +* The queueing is in progress, or it is already queued. Try to +* steal it from ->worklist without clearing WORK_STRUCT_PENDING. +*/ + gcwq =get_work_gcwq (work); +if (!gcwq) +return ret; + +spin_lock_irq (&gcwq->lock); +if (!list_empty (&work->entry)) { +/* +* This work is queued, but perhaps we locked the wrong gcwq. +* In that case we must see the new value after rmb(), see +* insert_work()->wmb(). +*/ +smp_rmb (); +if (gcwq ==get_work_gcwq (work)) { +debug_work_deactivate (work); +list_del_init (&work->entry); +cwq_dec_nr_in_flight (get_work_cwq (work), +get_work_color (work), + *work_data_bits (work) &WORK_STRUCT_DELAYED ); + ret =1 ; + } + } +spin_unlock_irq (&gcwq->lock); + +return ret; +} + +/** * insert_work - insert a work into gcwq * @cwq: cwq @work belongs to * @work: work to insert
@@ -1832,107 +1975,6 @@ static bool manage_workers(struct worker *worker) }/** -* move_linked_works - move linked works to a list -* @work: start of series of works to be scheduled -* @head: target list to append @work to -* @nextp: out paramter for nested worklist walking -* -* Schedule linked works starting from @work to @head. Work series to -* be scheduled starts at @work and includes any consecutive work with -* WORK_STRUCT_LINKED set in its predecessor. -* -* If @nextp is not NULL, it's updated to point to the next work of -* the last scheduled work. This allows move_linked_works() to be -* nested inside outer list_for_each_entry_safe(). -* -* CONTEXT: -* spin_lock_irq(gcwq->lock). -*/ -static void move_linked_works (struct work_struct *work,struct list_head *head, -struct work_struct **nextp) -{ -struct work_struct *n; - -/* -* Linked worklist will always end before the end of the list, -* use NULL for list head. -*/ -list_for_each_entry_safe_from (work, n,NULL , entry) { -list_move_tail (&work->entry, head); -if (!(*work_data_bits (work) &WORK_STRUCT_LINKED )) -break ; - } - -/* -* If we're already inside safe list traversal and have moved -* multiple works to the scheduled queue, the next position -* needs to be updated. -*/ -if (nextp) - *nextp = n; -} - -static void cwq_activate_first_delayed (struct cpu_workqueue_struct *cwq) -{ -struct work_struct *work =list_first_entry (&cwq->delayed_works, - struct work_struct, entry); - -trace_workqueue_activate_work (work); -move_linked_works (work, &cwq->pool->worklist,NULL ); -__clear_bit (WORK_STRUCT_DELAYED_BIT ,work_data_bits (work)); - cwq->nr_active++; -} - -/** -* cwq_dec_nr_in_flight - decrement cwq's nr_in_flight -* @cwq: cwq of interest -* @color: color of work which left the queue -* @delayed: for a delayed work -* -* A work either has completed or is removed from pending queue, -* decrement nr_in_flight of its cwq and handle workqueue flushing. -* -* CONTEXT: -* spin_lock_irq(gcwq->lock). -*/ -static void cwq_dec_nr_in_flight (struct cpu_workqueue_struct *cwq,int color, -bool delayed) -{ -/* ignore uncolored works */ -if (color ==WORK_NO_COLOR ) -return ; - - cwq->nr_in_flight[color]--; - -if (!delayed) { - cwq->nr_active--; -if (!list_empty (&cwq->delayed_works)) { -/* one down, submit a delayed one */ -if (cwq->nr_active < cwq->max_active) -cwq_activate_first_delayed (cwq); - } - } - -/* is flush in progress and are we at the flushing tip? */ -if (likely (cwq->flush_color != color)) -return ; - -/* are there still in-flight works? */ -if (cwq->nr_in_flight[color]) -return ; - -/* this cwq is done, clear flush_color */ - cwq->flush_color =-1 ; - -/* -* If this was the last cwq, wake up the first flusher. It -* will handle the rest. -*/ -if (atomic_dec_and_test (&cwq->wq->nr_cwqs_to_flush)) -complete (&cwq->wq->first_flusher->done); -} - -/** * process_one_work - process single work * @worker: self * @work: work to process
@@ -2767,48 +2809,6 @@ bool flush_work_sync(struct work_struct *work) }EXPORT_SYMBOL_GPL (flush_work_sync); -/* -* Upon a successful return (>= 0), the caller "owns" WORK_STRUCT_PENDING bit, -* so this work can't be re-armed in any way. -*/ -static int try_to_grab_pending (struct work_struct *work) -{ -struct global_cwq *gcwq; -int ret =-1 ; - -if (!test_and_set_bit (WORK_STRUCT_PENDING_BIT ,work_data_bits (work))) -return 0 ; - -/* -* The queueing is in progress, or it is already queued. Try to -* steal it from ->worklist without clearing WORK_STRUCT_PENDING. -*/ - gcwq =get_work_gcwq (work); -if (!gcwq) -return ret; - -spin_lock_irq (&gcwq->lock); -if (!list_empty (&work->entry)) { -/* -* This work is queued, but perhaps we locked the wrong gcwq. -* In that case we must see the new value after rmb(), see -* insert_work()->wmb(). -*/ -smp_rmb (); -if (gcwq ==get_work_gcwq (work)) { -debug_work_deactivate (work); -list_del_init (&work->entry); -cwq_dec_nr_in_flight (get_work_cwq (work), -get_work_color (work), - *work_data_bits (work) &WORK_STRUCT_DELAYED ); - ret =1 ; - } - } -spin_unlock_irq (&gcwq->lock); - -return ret; -} -static bool __cancel_work_timer (struct work_struct *work,struct timer_list * timer) {
Signature · 1 line
1.7.7.3
Tejun Heo
maintainerPATCH 9/15 workqueue: introduce WORK_OFFQ_FLAG_*
In reply to Tejun HeoLow WORK_STRUCT_FLAG_BITS bits of work_struct->data contain WORK_STRUCT_FLAG_* and flush color. If the work item is queued, the rest point to the cpu_workqueue with WORK_STRUCT_CWQ set; otherwise, WORK_STRUCT_CWQ is clear and the bits contain the last CPU number - either a real CPU number or one of WORK_CPU_*.
Scheduled addition of mod_delayed_work[_on]() requires an additional flag, which is used only while a work item is off queue. There are more than enough bits to represent off-queue CPU number on both 32 and 64bits. This patch introduces WORK_OFFQ_FLAG_* which occupy the lower part of the @work->data high bits while off queue. This patch doesn't define any actual OFFQ flag yet.
Off-queue CPU number is now shifted by WORK_OFFQ_CPU_SHIFT, which adds the number of bits used by OFFQ flags to WORK_STRUCT_FLAG_SHIFT, to make room for OFFQ flags.
To avoid shift width warning with large WORK_OFFQ_FLAG_BITS, ulong cast is added to WORK_STRUCT_NO_CPU and, just in case, BUILD_BUG_ON() to check that there are enough bits to accomodate off-queue CPU number is added.
This patch doesn't make any functional difference.
--- include/linux/workqueue.h | 8 +++++++- kernel/workqueue.c | 14 +++++++++----- 2 files changed, 16 insertions(+), 6 deletions(-)
include/linux/workqueue.h +7 -1
@@ -68,9 +68,15 @@ enum {WORK_STRUCT_FLAG_BITS =WORK_STRUCT_COLOR_SHIFT +WORK_STRUCT_COLOR_BITS , +/* data contains off-queue information when !WORK_STRUCT_CWQ */ +WORK_OFFQ_FLAG_BASE =WORK_STRUCT_FLAG_BITS , +WORK_OFFQ_FLAG_BITS =0 , +WORK_OFFQ_CPU_SHIFT =WORK_OFFQ_FLAG_BASE +WORK_OFFQ_FLAG_BITS , + +/* convenience constants */ WORK_STRUCT_FLAG_MASK = (1UL <<WORK_STRUCT_FLAG_BITS ) -1 ,WORK_STRUCT_WQ_DATA_MASK = ~WORK_STRUCT_FLAG_MASK , -WORK_STRUCT_NO_CPU =WORK_CPU_NONE <<WORK_STRUCT_FLAG_BITS , +WORK_STRUCT_NO_CPU = (unsigned long )WORK_CPU_NONE <<WORK_OFFQ_CPU_SHIFT ,/* bit mask for work_busy() return values */ WORK_BUSY_PENDING = 1 << 0,
kernel/workqueue.c +9 -5
@@ -533,9 +533,9 @@ static int work_next_color(int color) }/* -* A work's data points to the cwq with WORK_STRUCT_CWQ set while the -* work is on queue. Once execution starts, WORK_STRUCT_CWQ is -* cleared and the work data contains the cpu number it was last on. +* While queued, %WORK_STRUCT_CWQ is set and non flag bits of a work's data +* contain the pointer to the queued cwq. Once execution starts, the flag +* is cleared and the high bits contain OFFQ flags and CPU number. * * set_work_cwq(), set_work_cpu_and_clear_pending() and clear_work_data() * can be used to set the cwq, cpu or clear work->data. These functions
@@ -565,7 +565,7 @@ static void set_work_cwq(struct work_struct *work,static void set_work_cpu_and_clear_pending (struct work_struct *work,unsigned int cpu) { -set_work_data (work, cpu <<WORK_STRUCT_FLAG_BITS ,0 ); +set_work_data (work, (unsigned long )cpu <<WORK_OFFQ_CPU_SHIFT ,0 ); }static void clear_work_data (struct work_struct *work)
@@ -592,7 +592,7 @@ static struct global_cwq *get_work_gcwq(struct work_struct *work)return ((struct cpu_workqueue_struct *) (data &WORK_STRUCT_WQ_DATA_MASK ))->pool->gcwq; - cpu = data >>WORK_STRUCT_FLAG_BITS ; + cpu = data >>WORK_OFFQ_CPU_SHIFT ;if (cpu ==WORK_CPU_NONE )return NULL ;
@@ -3724,6 +3724,10 @@ static int __init init_workqueues(void)unsigned int cpu;int i; +/* make sure we have enough bits for OFFQ CPU number */ +BUILD_BUG_ON ((1LU << (BITS_PER_LONG -WORK_OFFQ_CPU_SHIFT )) < +WORK_CPU_LAST ); +cpu_notifier (workqueue_cpu_up_callback,CPU_PRI_WORKQUEUE_UP );cpu_notifier (workqueue_cpu_down_callback,CPU_PRI_WORKQUEUE_DOWN );
Signature · 1 line
1.7.7.3
Tejun Heo
maintainerPATCH 10/15 workqueue: factor out __queue_delayed_work() from queue_delayed_work_on()
In reply to Tejun HeoThis is to prepare for mod_delayed_work[_on]() and doesn't cause any functional difference.
--- kernel/workqueue.c | 74 ++++++++++++++++++++++++++++----------------------- 1 files changed, 41 insertions(+), 33 deletions(-)
kernel/workqueue.c +41 -33
@@ -1259,6 +1259,46 @@ void delayed_work_timer_fn(unsigned long __data) }EXPORT_SYMBOL_GPL (delayed_work_timer_fn); +static void __queue_delayed_work (int cpu,struct workqueue_struct *wq, +struct delayed_work *dwork,unsigned long delay) +{ +struct timer_list *timer = &dwork->timer; +struct work_struct *work = &dwork->work; +unsigned int lcpu; + +WARN_ON_ONCE (timer->function != delayed_work_timer_fn || + timer->data != (unsigned long )dwork); +BUG_ON (timer_pending (timer)); +BUG_ON (!list_empty (&work->entry)); + +timer_stats_timer_set_start_info (&dwork->timer); + +/* +* This stores cwq for the moment, for the timer_fn. Note that the +* work's gcwq is preserved to allow reentrance detection for +* delayed works. +*/ +if (!(wq->flags &WQ_UNBOUND )) { +struct global_cwq *gcwq =get_work_gcwq (work); + +if (gcwq && gcwq->cpu !=WORK_CPU_UNBOUND ) + lcpu = gcwq->cpu; +else + lcpu =raw_smp_processor_id (); + }else { + lcpu =WORK_CPU_UNBOUND ; + } + +set_work_cwq (work,get_cwq (lcpu, wq),0 ); + + timer->expires = jiffies + delay; + +if (unlikely (cpu !=WORK_CPU_UNBOUND )) +add_timer_on (timer, cpu); +else +add_timer (timer); +} +/** * queue_delayed_work_on - queue work on specific CPU after delay * @cpu: CPU number to execute work on
@@ -1273,7 +1313,6 @@ EXPORT_SYMBOL_GPL(delayed_work_timer_fn);bool queue_delayed_work_on (int cpu,struct workqueue_struct *wq,struct delayed_work *dwork,unsigned long delay) { -struct timer_list *timer = &dwork->timer;struct work_struct *work = &dwork->work;bool ret = false;
@@ -1287,38 +1326,7 @@ bool queue_delayed_work_on(int cpu, struct workqueue_struct *wq,preempt_disable ();if (!test_and_set_bit (WORK_STRUCT_PENDING_BIT ,work_data_bits (work))) { -unsigned int lcpu; - -WARN_ON_ONCE (timer->function != delayed_work_timer_fn || - timer->data != (unsigned long )dwork); -BUG_ON (timer_pending (timer)); -BUG_ON (!list_empty (&work->entry)); - -timer_stats_timer_set_start_info (&dwork->timer); - -/* -* This stores cwq for the moment, for the timer_fn. -* Note that the work's gcwq is preserved to allow -* reentrance detection for delayed works. -*/ -if (!(wq->flags &WQ_UNBOUND )) { -struct global_cwq *gcwq =get_work_gcwq (work); - -if (gcwq && gcwq->cpu !=WORK_CPU_UNBOUND ) - lcpu = gcwq->cpu; -else - lcpu =raw_smp_processor_id (); - }else - lcpu =WORK_CPU_UNBOUND ; - -set_work_cwq (work,get_cwq (lcpu, wq),0 ); - - timer->expires = jiffies + delay; - -if (unlikely (cpu !=WORK_CPU_UNBOUND )) -add_timer_on (timer, cpu); -else -add_timer (timer); +__queue_delayed_work (cpu, wq, dwork, delay); ret = true; }
Signature · 1 line
1.7.7.3
Tejun Heo
maintainerPATCH 11/15 workqueue: reorganize try_to_grab_pending() and __cancel_timer_work()
In reply to Tejun Heo* Move timer handling from __cancel_work_timer() to try_to_grab_pending().
* Make try_to_grab_pending() use -EAGAIN instead of -1 for busy-looping and drop the ret local variable.
* Add proper function comment to try_to_grab_pending().
This makes the code a bit easier to understand and will ease further changes. This patch doesn't make any functional change.
--- kernel/workqueue.c | 36 +++++++++++++++++++++++++----------- 1 files changed, 25 insertions(+), 11 deletions(-)
kernel/workqueue.c +25 -11
@@ -1004,15 +1004,29 @@ static void cwq_dec_nr_in_flight(struct cpu_workqueue_struct *cwq, int color,complete (&cwq->wq->first_flusher->done); } -/* -* Upon a successful return (>= 0), the caller "owns" WORK_STRUCT_PENDING bit, -* so this work can't be re-armed in any way. +/** +* try_to_grab_pending - steal work item from worklist +* @work: work item to steal +* +* Try to grab PENDING bit of @work. This function can handle @work in any +* stable state - idle, on timer or on worklist. Return values are +* +* 1 if @work was pending and we successfully stole PENDING +* 0 if @work was idle and we claimed PENDING +* -EAGAIN if PENDING couldn't be grabbed at the moment, safe to busy-retry +* +* On >= 0 return, the caller owns @work's PENDING bit. */ -static int try_to_grab_pending (struct work_struct *work) +static int try_to_grab_pending (struct work_struct *work, +struct timer_list *timer) {struct global_cwq *gcwq; -int ret =-1 ; +/* try to steal the timer if it exists */ +if (timer &&likely (del_timer (timer))) +return 1 ; + +/* try to claim PENDING the normal way */ if (!test_and_set_bit (WORK_STRUCT_PENDING_BIT ,work_data_bits (work)))return 0 ;
@@ -1022,7 +1036,7 @@ static int try_to_grab_pending(struct work_struct *work)*/ gcwq =get_work_gcwq (work);if (!gcwq) -return ret; +return -EAGAIN ;spin_lock_irq (&gcwq->lock);if (!list_empty (&work->entry)) {
@@ -1038,12 +1052,14 @@ static int try_to_grab_pending(struct work_struct *work)cwq_dec_nr_in_flight (get_work_cwq (work),get_work_color (work), *work_data_bits (work) &WORK_STRUCT_DELAYED ); - ret =1 ; + +spin_unlock_irq (&gcwq->lock); +return 1 ; } }spin_unlock_irq (&gcwq->lock); -return ret; +return -EAGAIN ; }/**
@@ -2823,9 +2839,7 @@ static bool __cancel_work_timer(struct work_struct *work,int ret;do { - ret = (timer &&likely (del_timer (timer))); -if (!ret) - ret =try_to_grab_pending (work); + ret =try_to_grab_pending (work, timer);wait_on_work (work); }while (unlikely (ret <0 ));
Signature · 1 line
1.7.7.3
Tejun Heo
maintainerPATCH 12/15 workqueue: mark a work item being canceled as such
In reply to Tejun HeoThere can be two reasons try_to_grab_pending() can fail with -EAGAIN. One is when someone else is queueing or deqeueing the work item. With the previous patches, it is guaranteed that PENDING and queued state will soon agree making it safe to busy-retry in this case.
The other is if multiple __cancel_work_timer() invocations are racing one another. __cancel_work_timer() grabs PENDING and then waits for running instances of the target work item on all CPUs while holding PENDING and !queued. try_to_grab_pending() invoked from another task will keep returning -EAGAIN while the current owner is waiting.
Not distinguishing the two cases is okay because __cancel_work_timer() is the only user of try_to_grab_pending() and it invokes wait_on_work() whenever grabbing fails. For the first case, busy looping should be fine but wait_on_work() doesn't cause any critical problem. For the latter case, the new contender usually waits for the same condition as the current owner, so no unnecessarily extended busy-looping happens. Combined, these make __cancel_work_timer() technically correct even without preemption protection while grabbing PENDING or distinguishing the two different cases.
While the current code is technically correct, not distinguishing the two cases makes it difficult to use try_to_grab_pending() for other purposes than canceling because it's impossible to tell whether it's safe to busy-retry grabbing.
This patch adds a mechanism to mark a work item being canceled. try_to_grab_pending() now requires the caller to have preemption disabled and returns -EAGAIN to indicate that grabbing failed but PENDING and queued states are gonna agree soon and it's safe to busy-loop. It returns -ENOENT if the work item is being canceled and it may stay PENDING && !queued for arbitrary amount of time.
__cancel_work_timer() is modified to mark the work canceling with WORK_OFFQ_CANCELING after grabbing PENDING, thus making try_to_grab_pending() fail with -ENOENT instead of -EAGAIN. Also, it invokes wait_on_work() iff grabbing failed with -ENOENT. This isn't necessary for correctness but makes it consistent with other future users of try_to_grab_pending().
--- include/linux/workqueue.h | 5 +++- kernel/workqueue.c | 67 ++++++++++++++++++++++++++++++++++++++------- 2 files changed, 61 insertions(+), 11 deletions(-)
include/linux/workqueue.h +4 -1
@@ -70,7 +70,10 @@ enum {/* data contains off-queue information when !WORK_STRUCT_CWQ */ WORK_OFFQ_FLAG_BASE =WORK_STRUCT_FLAG_BITS , -WORK_OFFQ_FLAG_BITS =0 , + +WORK_OFFQ_CANCELING = (1 <<WORK_OFFQ_FLAG_BASE ), + +WORK_OFFQ_FLAG_BITS =1 , WORK_OFFQ_CPU_SHIFT = WORK_OFFQ_FLAG_BASE + WORK_OFFQ_FLAG_BITS,/* convenience constants */
kernel/workqueue.c +57 -10
109 lines not on this page · raw message
Signature · 1 line
1.7.7.3
Tejun Heo
maintainerPATCH v2 12/15 workqueue: mark a work item being canceled as such
Quoted 1 line
From
7199d2757a11ab19bf28713490b898a5a7b24d65Mon Sep 17 00:00:00 2001
From: Tejun Heo <[email protected]>
Date: Mon, 30 Jul 2012 13:03:59 -0700
There can be two reasons try_to_grab_pending() can fail with -EAGAIN. One is when someone else is queueing or deqeueing the work item. With the previous patches, it is guaranteed that PENDING and queued state will soon agree making it safe to busy-retry in this case.
The other is if multiple __cancel_work_timer() invocations are racing one another. __cancel_work_timer() grabs PENDING and then waits for running instances of the target work item on all CPUs while holding PENDING and !queued. try_to_grab_pending() invoked from another task will keep returning -EAGAIN while the current owner is waiting.
Not distinguishing the two cases is okay because __cancel_work_timer() is the only user of try_to_grab_pending() and it invokes wait_on_work() whenever grabbing fails. For the first case, busy looping should be fine but wait_on_work() doesn't cause any critical problem. For the latter case, the new contender usually waits for the same condition as the current owner, so no unnecessarily extended busy-looping happens. Combined, these make __cancel_work_timer() technically correct even without preemption protection while grabbing PENDING or distinguishing the two different cases.
While the current code is technically correct, not distinguishing the two cases makes it difficult to use try_to_grab_pending() for other purposes than canceling because it's impossible to tell whether it's safe to busy-retry grabbing.
This patch adds a mechanism to mark a work item being canceled. try_to_grab_pending() now requires the caller to have preemption disabled and returns -EAGAIN to indicate that grabbing failed but PENDING and queued states are gonna agree soon and it's safe to busy-loop. It returns -ENOENT if the work item is being canceled and it may stay PENDING && !queued for arbitrary amount of time.
__cancel_work_timer() is modified to mark the work canceling with WORK_OFFQ_CANCELING after grabbing PENDING, thus making try_to_grab_pending() fail with -ENOENT instead of -EAGAIN. Also, it invokes wait_on_work() iff grabbing failed with -ENOENT. This isn't necessary for correctness but makes it consistent with other future users of try_to_grab_pending().
v2: try_to_grab_pending() was testing preempt_count() to ensure that
the caller has disabled preemption. This triggers spuriously if !CONFIG_PREEMPT_COUNT. Use preemptible() instead. Reported by Fengguang Wu.
---
Spurious WARN bug fixed. git branch updated accordingly.
Thanks.
include/linux/workqueue.h | 5 +++- kernel/workqueue.c | 67 ++++++++++++++++++++++++++++++++++++++------- 2 files changed, 61 insertions(+), 11 deletions(-)
include/linux/workqueue.h +4 -1
@@ -70,7 +70,10 @@ enum {/* data contains off-queue information when !WORK_STRUCT_CWQ */ WORK_OFFQ_FLAG_BASE =WORK_STRUCT_FLAG_BITS , -WORK_OFFQ_FLAG_BITS =0 , + +WORK_OFFQ_CANCELING = (1 <<WORK_OFFQ_FLAG_BASE ), + +WORK_OFFQ_FLAG_BITS =1 , WORK_OFFQ_CPU_SHIFT = WORK_OFFQ_FLAG_BASE + WORK_OFFQ_FLAG_BITS,/* convenience constants */
kernel/workqueue.c +57 -10
109 lines not on this page · raw message
Signature · 1 line
1.7.7.3
Tejun Heo
maintainerPATCH 13/15 workqueue: implement mod_delayed_work[_on]()
In reply to Tejun HeoWorkqueue was lacking a mechanism to modify the timeout of an already pending delayed_work. delayed_work users have been working around this using several methods - using an explicit timer + work item, messing directly with delayed_work->timer, and canceling before re-queueing, all of which are error-prone and/or ugly.
This patch implements mod_delayed_work[_on]() which behaves similarly to mod_timer() - if the delayed_work is idle, it's queued with the given delay; otherwise, its timeout is modified to the new value. Zero @delay guarantees immediate execution.
--- include/linux/workqueue.h | 4 +++ kernel/workqueue.c | 51 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 0 deletions(-)
include/linux/workqueue.h +4 -0
@@ -390,6 +390,10 @@ extern bool queue_delayed_work_on(int cpu, struct workqueue_struct *wq, struct delayed_work *work, unsigned long delay);extern bool queue_delayed_work (struct workqueue_struct *wq,struct delayed_work *work,unsigned long delay); +extern bool mod_delayed_work_on (int cpu,struct workqueue_struct *wq, +struct delayed_work *dwork,unsigned long delay); +extern bool mod_delayed_work (struct workqueue_struct *wq, +struct delayed_work *dwork,unsigned long delay);extern void flush_workqueue (struct workqueue_struct *wq);extern void drain_workqueue (struct workqueue_struct *wq);
kernel/workqueue.c +51 -0
57 lines not on this page · raw message
Signature · 1 line
1.7.7.3
Tejun Heo
maintainerPATCH 14/15 workqueue: use mod_delayed_work() instead of cancel + queue
In reply to Tejun HeoConvert delayed_work users doing [__]cancel_delayed_work() + queue_delayed_work() to mod_delayed_work().
Most conversions are straight-forward. Ones worth mentioning are,
* drivers/edac/edac_mc.c: edac_mc_workq_setup() converted to always use mod_delayed_work() and cancel loop in edac_mc_reset_delay_period() is dropped.
* drivers/platform/x86/thinkpad_acpi.c: No need to remember whether watchdog is active or not. @fan_watchdog_active and related code dropped.
* drivers/power/charger-manager.c: Seemingly a lot of delayed_work_pending() abuse going on here. [delayed_]work_pending() are unsynchronized and racy when used like this. I converted one instance in fullbatt_handler(). Please conver the rest so that it invokes workqueue APIs for the intended target state rather than trying to game work item pending state transitions. e.g. if timer should be modified - call mod_delayed_work(), canceled - call cancel_delayed_work[_sync]().
* drivers/thermal/thermal_sys.c: thermal_zone_device_set_polling() simplified. Note that round_jiffies() calls in this function are meaningless. round_jiffies() work on absolute jiffies not delta delay used by delayed_work.
* net/core/link_watch.c: linkwatch_schedule_work() was doing a quite elaborate dancing around its delayed_work. Collapse it such that linkwatch_work is queued for immediate execution if LW_URGENT and existing timer is kept otherwise.
--- block/blk-core.c | 6 ++---- block/blk-throttle.c | 7 +------ block/genhd.c | 6 ++---- drivers/block/floppy.c | 3 +-- drivers/edac/edac_mc.c | 17 +---------------- drivers/infiniband/core/addr.c | 4 +--- drivers/infiniband/core/mad.c | 14 +++++--------- drivers/infiniband/hw/nes/nes_hw.c | 6 ++---- drivers/infiniband/hw/nes/nes_nic.c | 5 ++--- drivers/input/keyboard/qt2160.c | 3 +-- drivers/input/mouse/synaptics_i2c.c | 7 +------ drivers/net/wireless/ipw2x00/ipw2100.c | 8 +++----- drivers/net/wireless/zd1211rw/zd_usb.c | 3 +-- drivers/platform/x86/thinkpad_acpi.c | 20 +++++--------------- drivers/power/charger-manager.c | 9 +++------ drivers/power/ds2760_battery.c | 9 +++------ drivers/power/jz4740-battery.c | 6 ++---- drivers/thermal/thermal_sys.c | 15 ++++++--------- fs/afs/callback.c | 4 +--- fs/afs/server.c | 10 ++-------- fs/afs/vlocation.c | 14 +++----------- fs/nfs/nfs4renewd.c | 3 +-- net/core/dst.c | 4 ++-- net/core/link_watch.c | 21 ++++++--------------- net/rfkill/input.c | 3 +-- 25 files changed, 58 insertions(+), 149 deletions(-)
block/blk-core.c +2 -4
@@ -319,10 +319,8 @@ EXPORT_SYMBOL(__blk_run_queue);*/ void blk_run_queue_async (struct request_queue *q) { -if (likely (!blk_queue_stopped (q))) { -__cancel_delayed_work (&q->delay_work); -queue_delayed_work (kblockd_workqueue, &q->delay_work,0 ); - } +if (likely (!blk_queue_stopped (q))) +mod_delayed_work (kblockd_workqueue, &q->delay_work,0 ); }EXPORT_SYMBOL (blk_run_queue_async);
block/blk-throttle.c +1 -6
@@ -930,12 +930,7 @@ throtl_schedule_delayed_work(struct throtl_data *td, unsigned long delay)/* schedule work if limits changed even if no bio is queued */ if (total_nr_queued (td) || td->limits_changed) { -/* -* We might have a work scheduled to be executed in future. -* Cancel that and schedule a new one. -*/ -__cancel_delayed_work (dwork); -queue_delayed_work (kthrotld_workqueue, dwork, delay); +mod_delayed_work (kthrotld_workqueue, dwork, delay);throtl_log (td,"schedule work. delay=%lu jiffies=%lu" , delay, jiffies); }
block/genhd.c +2 -4
@@ -1524,10 +1524,8 @@ void disk_flush_events(struct gendisk *disk, unsigned int mask)spin_lock_irq (&ev->lock); ev->clearing |= mask; -if (!ev->block) { -cancel_delayed_work (&ev->dwork); -queue_delayed_work (system_nrt_freezable_wq, &ev->dwork,0 ); - } +if (!ev->block) +mod_delayed_work (system_nrt_freezable_wq, &ev->dwork,0 );spin_unlock_irq (&ev->lock); }
drivers/block/floppy.c +1 -2
@@ -671,7 +671,6 @@ static void __reschedule_timeout(int drive, const char *message)if (drive == current_reqD) drive = current_drive; -__cancel_delayed_work (&fd_timeout);if (drive <0 || drive >=N_DRIVE ) { delay =20UL *HZ ;
@@ -679,7 +678,7 @@ static void __reschedule_timeout(int drive, const char *message) }else delay =UDP ->timeout; -queue_delayed_work (floppy_wq, &fd_timeout, delay); +mod_delayed_work (floppy_wq, &fd_timeout, delay);if (UDP ->flags &FD_DEBUG )DPRINT ("reschedule timeout %s\n" , message); timeout_message = message;
drivers/edac/edac_mc.c +1 -16
@@ -492,7 +492,7 @@ static void edac_mc_workq_setup(struct mem_ctl_info *mci, unsigned msec)return ;INIT_DELAYED_WORK (&mci->work, edac_mc_workq_function); -queue_delayed_work (edac_workqueue, &mci->work,msecs_to_jiffies (msec)); +mod_delayed_work (edac_workqueue, &mci->work,msecs_to_jiffies (msec)); }/*
@@ -533,21 +533,6 @@ void edac_mc_reset_delay_period(int value)mutex_lock (&mem_ctls_mutex); -/* scan the list and turn off all workq timers, doing so under lock -*/ -list_for_each (item, &mc_devices) { - mci = list_entry(item, struct mem_ctl_info, link); - -if (mci->op_state ==OP_RUNNING_POLL ) -cancel_delayed_work (&mci->work); - } - -mutex_unlock (&mem_ctls_mutex); - - -/* re-walk the list, and reset the poll delay */ -mutex_lock (&mem_ctls_mutex); -list_for_each (item, &mc_devices) { mci = list_entry(item, struct mem_ctl_info, link);
drivers/infiniband/core/addr.c +1 -3
@@ -152,13 +152,11 @@ static void set_timeout(unsigned long time) {unsigned long delay; -cancel_delayed_work (&work); - delay = time - jiffies;if ((long )delay <=0 ) delay =1 ; -queue_delayed_work (addr_wq, &work, delay); +mod_delayed_work (addr_wq, &work, delay); }static void queue_req (struct addr_req *req)
drivers/infiniband/core/mad.c +5 -9
@@ -2013,13 +2013,11 @@ static void adjust_timeout(struct ib_mad_agent_private *mad_agent_priv)if (time_after (mad_agent_priv->timeout, mad_send_wr->timeout)) { mad_agent_priv->timeout = mad_send_wr->timeout; -__cancel_delayed_work (&mad_agent_priv->timed_work); delay = mad_send_wr->timeout - jiffies;if ((long )delay <=0 ) delay =1 ; -queue_delayed_work (mad_agent_priv->qp_info-> - port_priv->wq, - &mad_agent_priv->timed_work, delay); +mod_delayed_work (mad_agent_priv->qp_info->port_priv->wq, + &mad_agent_priv->timed_work, delay); } } }
@@ -2052,11 +2050,9 @@ static void wait_for_response(struct ib_mad_send_wr_private *mad_send_wr)list_add (&mad_send_wr->agent_list, list_item);/* Reschedule a work item if we have a shorter timeout */ -if (mad_agent_priv->wait_list.next == &mad_send_wr->agent_list) { -__cancel_delayed_work (&mad_agent_priv->timed_work); -queue_delayed_work (mad_agent_priv->qp_info->port_priv->wq, - &mad_agent_priv->timed_work, delay); - } +if (mad_agent_priv->wait_list.next == &mad_send_wr->agent_list) +mod_delayed_work (mad_agent_priv->qp_info->port_priv->wq, + &mad_agent_priv->timed_work, delay); } void ib_reset_mad_timeout(struct ib_mad_send_wr_private *mad_send_wr,
drivers/infiniband/hw/nes/nes_hw.c +2 -4
@@ -2679,11 +2679,9 @@ static void nes_process_mac_intr(struct nes_device *nesdev, u32 mac_number) } }if (nesadapter->phy_type[mac_index] ==NES_PHY_TYPE_SFP_D ) { -if (nesdev->link_recheck) -cancel_delayed_work (&nesdev->work); nesdev->link_recheck =1 ; -schedule_delayed_work (&nesdev->work, -NES_LINK_RECHECK_DELAY ); +mod_delayed_work (system_wq, &nesdev->work, +NES_LINK_RECHECK_DELAY ); } }
drivers/infiniband/hw/nes/nes_nic.c +2 -3
@@ -243,10 +243,9 @@ static int nes_netdev_open(struct net_device *netdev)spin_lock_irqsave (&nesdev->nesadapter->phy_lock, flags);if (nesdev->nesadapter->phy_type[nesdev->mac_index] ==NES_PHY_TYPE_SFP_D ) { -if (nesdev->link_recheck) -cancel_delayed_work (&nesdev->work); nesdev->link_recheck =1 ; -schedule_delayed_work (&nesdev->work,NES_LINK_RECHECK_DELAY ); +mod_delayed_work (system_wq, &nesdev->work, +NES_LINK_RECHECK_DELAY ); }spin_unlock_irqrestore (&nesdev->nesadapter->phy_lock, flags);
drivers/input/keyboard/qt2160.c +1 -2
@@ -156,8 +156,7 @@ static irqreturn_t qt2160_irq(int irq, void *_qt2160)spin_lock_irqsave (&qt2160->lock, flags); -__cancel_delayed_work (&qt2160->dwork); -schedule_delayed_work (&qt2160->dwork,0 ); +mod_delayed_work (system_wq, &qt2160->dwork,0 );spin_unlock_irqrestore (&qt2160->lock, flags);
drivers/input/mouse/synaptics_i2c.c +1 -6
@@ -376,12 +376,7 @@ static void synaptics_i2c_reschedule_work(struct synaptics_i2c *touch,spin_lock_irqsave (&touch->lock, flags); -/* -* If work is already scheduled then subsequent schedules will not -* change the scheduled time that's why we have to cancel it first. -*/ -__cancel_delayed_work (&touch->dwork); -schedule_delayed_work (&touch->dwork, delay); +mod_delayed_work (system_wq, &touch->dwork, delay);spin_unlock_irqrestore (&touch->lock, flags); }
drivers/net/wireless/ipw2x00/ipw2100.c +3 -5
@@ -2180,8 +2180,7 @@ static void isr_indicate_rf_kill(struct ipw2100_priv *priv, u32 status)/* Make sure the RF Kill check timer is running */ priv->stop_rf_kill =0 ; -cancel_delayed_work (&priv->rf_kill); -schedule_delayed_work (&priv->rf_kill,round_jiffies_relative (HZ )); +mod_delayed_work (system_wq, &priv->rf_kill,round_jiffies_relative (HZ )); }static void send_scan_event (void *data)
@@ -4321,9 +4320,8 @@ static int ipw_radio_kill_sw(struct ipw2100_priv *priv, int disable_radio) "disabled by HW switch\n");/* Make sure the RF_KILL check timer is running */ priv->stop_rf_kill =0 ; -cancel_delayed_work (&priv->rf_kill); -schedule_delayed_work (&priv->rf_kill, -round_jiffies_relative (HZ )); +mod_delayed_work (system_wq, &priv->rf_kill, +round_jiffies_relative (HZ )); }else schedule_reset (priv); }
drivers/net/wireless/zd1211rw/zd_usb.c +1 -2
@@ -1164,8 +1164,7 @@ void zd_usb_reset_rx_idle_timer(struct zd_usb *usb) {struct zd_usb_rx *rx = &usb->rx; -cancel_delayed_work (&rx->idle_work); -queue_delayed_work (zd_workqueue, &rx->idle_work,ZD_RX_IDLE_INTERVAL ); +mod_delayed_work (zd_workqueue, &rx->idle_work,ZD_RX_IDLE_INTERVAL ); }static inline void init_usb_interrupt (struct zd_usb *usb)
drivers/platform/x86/thinkpad_acpi.c +5 -15
@@ -7683,25 +7683,15 @@ static int fan_set_speed(int speed)static void fan_watchdog_reset (void ) { -static int fan_watchdog_active; -if (fan_control_access_mode ==TPACPI_FAN_WR_NONE )return ; -if (fan_watchdog_active) -cancel_delayed_work (&fan_watchdog_task); -if (fan_watchdog_maxinterval >0 && - tpacpi_lifecycle !=TPACPI_LIFE_EXITING ) { - fan_watchdog_active =1 ; -if (!queue_delayed_work (tpacpi_wq, &fan_watchdog_task, -msecs_to_jiffies (fan_watchdog_maxinterval - *1000 ))) { -pr_err ("failed to queue the fan watchdog, " -"watchdog will not trigger\n" ); - } - }else - fan_watchdog_active =0 ; + tpacpi_lifecycle !=TPACPI_LIFE_EXITING ) +mod_delayed_work (tpacpi_wq, &fan_watchdog_task, +msecs_to_jiffies (fan_watchdog_maxinterval *1000 )); +else +cancel_delayed_work (&fan_watchdog_task); }static void fan_watchdog_fire (struct work_struct *ignored)
drivers/power/charger-manager.c +3 -6
@@ -512,9 +512,8 @@ static void _setup_polling(struct work_struct *work)if (!delayed_work_pending (&cm_monitor_work) || (delayed_work_pending (&cm_monitor_work) &&time_after (next_polling, _next_polling))) { -cancel_delayed_work_sync (&cm_monitor_work); next_polling = jiffies + polling_jiffy; -queue_delayed_work (cm_wq, &cm_monitor_work, polling_jiffy); +mod_delayed_work (cm_wq, &cm_monitor_work, polling_jiffy); } out:
@@ -549,10 +548,8 @@ static void fullbatt_handler(struct charger_manager *cm)if (cm_suspended)device_set_wakeup_capable (cm->dev, true); -if (delayed_work_pending (&cm->fullbatt_vchk_work)) -cancel_delayed_work (&cm->fullbatt_vchk_work); -queue_delayed_work (cm_wq, &cm->fullbatt_vchk_work, -msecs_to_jiffies (desc->fullbatt_vchkdrop_ms)); +mod_delayed_work (cm_wq, &cm->fullbatt_vchk_work, +msecs_to_jiffies (desc->fullbatt_vchkdrop_ms)); cm->fullbatt_vchk_jiffies_at = jiffies +msecs_to_jiffies ( desc->fullbatt_vchkdrop_ms);
drivers/power/ds2760_battery.c +3 -6
@@ -355,8 +355,7 @@ static void ds2760_battery_external_power_changed(struct power_supply *psy)dev_dbg (di->dev,"%s\n" , __func__); -cancel_delayed_work (&di->monitor_work); -queue_delayed_work (di->monitor_wqueue, &di->monitor_work,HZ /10 ); +mod_delayed_work (di->monitor_wqueue, &di->monitor_work,HZ /10 ); }
@@ -401,8 +400,7 @@ static void ds2760_battery_set_charged(struct power_supply *psy)/* postpone the actual work by 20 secs. This is for debouncing GPIO * signals and to let the current value settle. See AN4188. */ -cancel_delayed_work (&di->set_charged_work); -queue_delayed_work (di->monitor_wqueue, &di->set_charged_work,HZ *20 ); +mod_delayed_work (di->monitor_wqueue, &di->set_charged_work,HZ *20 ); } static int ds2760_battery_get_property(struct power_supply *psy,
@@ -616,8 +614,7 @@ static int ds2760_battery_resume(struct platform_device *pdev) di->charge_status =POWER_SUPPLY_STATUS_UNKNOWN ;power_supply_changed (&di->bat); -cancel_delayed_work (&di->monitor_work); -queue_delayed_work (di->monitor_wqueue, &di->monitor_work,HZ ); +mod_delayed_work (di->monitor_wqueue, &di->monitor_work,HZ );return 0 ; }
drivers/power/jz4740-battery.c +2 -4
@@ -173,16 +173,14 @@ static void jz_battery_external_power_changed(struct power_supply *psy) {struct jz_battery *jz_battery =psy_to_jz_battery (psy); -cancel_delayed_work (&jz_battery->work); -schedule_delayed_work (&jz_battery->work,0 ); +mod_delayed_work (system_wq, &jz_battery->work,0 ); }static irqreturn_t jz_battery_charge_irq (int irq,void *data) {struct jz_battery *jz_battery = data; -cancel_delayed_work (&jz_battery->work); -schedule_delayed_work (&jz_battery->work,0 ); +mod_delayed_work (system_wq, &jz_battery->work,0 );return IRQ_HANDLED ; }
drivers/thermal/thermal_sys.c +6 -9
@@ -694,17 +694,14 @@ thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz)static void thermal_zone_device_set_polling (struct thermal_zone_device *tz,int delay) { -cancel_delayed_work (&(tz->poll_queue)); - -if (!delay) -return ; -if (delay >1000 ) -queue_delayed_work (system_freezable_wq, &(tz->poll_queue), -round_jiffies (msecs_to_jiffies (delay))); +mod_delayed_work (system_freezable_wq, &tz->poll_queue, +round_jiffies (msecs_to_jiffies (delay))); +else if (delay) +mod_delayed_work (system_freezable_wq, &tz->poll_queue, +msecs_to_jiffies (delay));else -queue_delayed_work (system_freezable_wq, &(tz->poll_queue), -msecs_to_jiffies (delay)); +cancel_delayed_work (&tz->poll_queue); } static void thermal_zone_device_passive(struct thermal_zone_device *tz,
fs/afs/callback.c +1 -3
@@ -351,9 +351,7 @@ void afs_dispatch_give_up_callbacks(struct work_struct *work)*/ void afs_flush_callback_breaks (struct afs_server *server) { -cancel_delayed_work (&server->cb_break_work); -queue_delayed_work (afs_callback_update_worker, - &server->cb_break_work,0 ); +mod_delayed_work (afs_callback_update_worker, &server->cb_break_work,0 ); }#if 0
fs/afs/server.c +2 -8
@@ -285,12 +285,7 @@ static void afs_reap_server(struct work_struct *work) expiry = server->time_of_death + afs_server_timeout;if (expiry > now) { delay = (expiry - now) *HZ ; -if (!queue_delayed_work (afs_wq, &afs_server_reaper, - delay)) { -cancel_delayed_work (&afs_server_reaper); -queue_delayed_work (afs_wq, &afs_server_reaper, - delay); - } +mod_delayed_work (afs_wq, &afs_server_reaper, delay);break ; }
@@ -323,6 +318,5 @@ static void afs_reap_server(struct work_struct *work) void __exit afs_purge_servers(void) { afs_server_timeout =0 ; -cancel_delayed_work (&afs_server_reaper); -queue_delayed_work (afs_wq, &afs_server_reaper,0 ); +mod_delayed_work (afs_wq, &afs_server_reaper,0 ); }
fs/afs/vlocation.c +3 -11
@@ -561,12 +561,7 @@ static void afs_vlocation_reaper(struct work_struct *work)if (expiry > now) { delay = (expiry - now) *HZ ;_debug ("delay %lu" , delay); -if (!queue_delayed_work (afs_wq, &afs_vlocation_reap, - delay)) { -cancel_delayed_work (&afs_vlocation_reap); -queue_delayed_work (afs_wq, &afs_vlocation_reap, - delay); - } +mod_delayed_work (afs_wq, &afs_vlocation_reap, delay);break ; }
@@ -614,13 +609,10 @@ void afs_vlocation_purge(void)spin_lock (&afs_vlocation_updates_lock);list_del_init (&afs_vlocation_updates);spin_unlock (&afs_vlocation_updates_lock); -cancel_delayed_work (&afs_vlocation_update); -queue_delayed_work (afs_vlocation_update_worker, - &afs_vlocation_update,0 ); +mod_delayed_work (afs_vlocation_update_worker, &afs_vlocation_update,0 );destroy_workqueue (afs_vlocation_update_worker); -cancel_delayed_work (&afs_vlocation_reap); -queue_delayed_work (afs_wq, &afs_vlocation_reap,0 ); +mod_delayed_work (afs_wq, &afs_vlocation_reap,0 ); }/*
fs/nfs/nfs4renewd.c +1 -2
@@ -117,8 +117,7 @@ nfs4_schedule_state_renewal(struct nfs_client *clp) timeout =5 *HZ ;dprintk ("%s: requeueing work. Lease period = %ld\n" , __func__, (timeout +HZ -1 ) /HZ ); -cancel_delayed_work (&clp->cl_renewd); -schedule_delayed_work (&clp->cl_renewd, timeout); +mod_delayed_work (system_wq, &clp->cl_renewd, timeout);set_bit (NFS_CS_RENEWD , &clp->cl_res_state);spin_unlock (&clp->cl_lock); }
net/core/dst.c +2 -2
@@ -214,8 +214,8 @@ void __dst_free(struct dst_entry *dst)if (dst_garbage.timer_inc >DST_GC_INC ) { dst_garbage.timer_inc =DST_GC_INC ; dst_garbage.timer_expires =DST_GC_MIN ; -cancel_delayed_work (&dst_gc_work); -schedule_delayed_work (&dst_gc_work, dst_garbage.timer_expires); +mod_delayed_work (system_wq, &dst_gc_work, + dst_garbage.timer_expires); }spin_unlock_bh (&dst_garbage.lock); }
net/core/link_watch.c +6 -15
@@ -120,22 +120,13 @@ static void linkwatch_schedule_work(int urgent) delay =0 ;/* -* This is true if we've scheduled it immeditately or if we don't -* need an immediate execution and it's already pending. +* If urgent, schedule immediate execution; otherwise, don't +* override the existing timer. */ -if (schedule_delayed_work (&linkwatch_work, delay) == !delay) -return ; - -/* Don't bother if there is nothing urgent. */ -if (!test_bit (LW_URGENT , &linkwatch_flags)) -return ; - -/* It's already running which is good enough. */ -if (!__cancel_delayed_work (&linkwatch_work)) -return ; - -/* Otherwise we reschedule it again for immediate execution. */ -schedule_delayed_work (&linkwatch_work,0 ); +if (test_bit (LW_URGENT , &linkwatch_flags)) +mod_delayed_work (system_wq, &linkwatch_work,0 ); +else +schedule_delayed_work (&linkwatch_work, delay); }
net/rfkill/input.c +1 -2
@@ -164,8 +164,7 @@ static void rfkill_schedule_global_op(enum rfkill_sched_op op) rfkill_op_pending = true;if (op ==RFKILL_GLOBAL_OP_EPO && !rfkill_is_epo_lock_active ()) {/* bypass the limiter for EPO */ -cancel_delayed_work (&rfkill_op_work); -schedule_delayed_work (&rfkill_op_work,0 ); +mod_delayed_work (system_wq, &rfkill_op_work,0 ); rfkill_last_scheduled = jiffies; }else rfkill_schedule_ratelimited ();
Signature · 1 line
1.7.7.3
Re: [PATCH 14/15] workqueue: use mod_delayed_work() instead of cancel + queue
Quoted 5 lines · Tejun Heo
Convert
delayed_workusers doing [__]cancel_delayed_work() +queue_delayed_work()tomod_delayed_work().Most conversions are straight-forward. Ones worth mentioning are,
Quoted 7 lines
*
drivers/edac/edac_mc.c:edac_mc_workq_setup()converted to always usemod_delayed_work()and cancel loop inedac_mc_reset_delay_period()is dropped.*
drivers/platform/x86/thinkpad_acpi.c: No need to remember whether watchdog is active or not. @fan_watchdog_active and related code dropped.
For the thinkpad_acpi.c part:
Signature · 4 lines
"One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh
Re: [PATCH 14/15] workqueue: use mod_delayed_work() instead of cancel + queue
In reply to Tejun HeoQuoted 32 lines · Tejun Heo
Convert
delayed_workusers doing [__]cancel_delayed_work() +queue_delayed_work()tomod_delayed_work().Most conversions are straight-forward. Ones worth mentioning are,
*
drivers/edac/edac_mc.c:edac_mc_workq_setup()converted to always usemod_delayed_work()and cancel loop inedac_mc_reset_delay_period()is dropped.*
drivers/platform/x86/thinkpad_acpi.c: No need to remember whether watchdog is active or not. @fan_watchdog_active and related code dropped.*
drivers/power/charger-manager.c: Seemingly a lot ofdelayed_work_pending()abuse going on here. [delayed_]work_pending() are unsynchronized and racy when used like this. I converted one instance infullbatt_handler(). Please conver the rest so that it invokes workqueue APIs for the intended target state rather than trying to game work item pending state transitions. e.g. if timer should be modified - callmod_delayed_work(), canceled - call cancel_delayed_work[_sync]().*
drivers/thermal/thermal_sys.c:thermal_zone_device_set_polling()simplified. Note thatround_jiffies()calls in this function are meaningless.round_jiffies()work on absolute jiffies not delta delay used bydelayed_work.*
net/core/link_watch.c:linkwatch_schedule_work()was doing a quite elaborate dancing around itsdelayed_work. Collapse it such thatlinkwatch_workis queued for immediate execution ifLW_URGENTand existing timer is kept otherwise.
Quoted 7 lines
Signed-off-by Tejun Heo <[email protected]>Cc Jens Axboe <[email protected]>Cc Jiri Kosina <[email protected]>Cc Doug Thompson <[email protected]>Cc David Airlie <[email protected]>Cc Roland Dreier <[email protected]>Cc Dmitry Torokhov <[email protected]>
For the input parts:
Thanks.
Signature · 1 line
Dmitry
Re: [PATCH 14/15] workqueue: use mod_delayed_work() instead of cancel + queue
In reply to Tejun HeoQuoted 27 lines · Tejun Heo
Convert
delayed_workusers doing [__]cancel_delayed_work() +queue_delayed_work()tomod_delayed_work().Most conversions are straight-forward. Ones worth mentioning are,
*
drivers/edac/edac_mc.c:edac_mc_workq_setup()converted to always usemod_delayed_work()and cancel loop inedac_mc_reset_delay_period()is dropped.*
drivers/platform/x86/thinkpad_acpi.c: No need to remember whether watchdog is active or not. @fan_watchdog_active and related code dropped.*
drivers/power/charger-manager.c: Seemingly a lot ofdelayed_work_pending()abuse going on here. [delayed_]work_pending() are unsynchronized and racy when used like this. I converted one instance infullbatt_handler(). Please conver the rest so that it invokes workqueue APIs for the intended target state rather than trying to game work item pending state transitions. e.g. if timer should be modified - callmod_delayed_work(), canceled - call cancel_delayed_work[_sync]().*
drivers/thermal/thermal_sys.c:thermal_zone_device_set_polling()simplified. Note thatround_jiffies()calls in this function are meaningless.round_jiffies()work on absolute jiffies not delta delay used bydelayed_work.
Quoted 6 lines
*
net/core/link_watch.c:linkwatch_schedule_work()was doing a quite elaborate dancing around itsdelayed_work. Collapse it such thatlinkwatch_workis queued for immediate execution ifLW_URGENTand existing timer is kept otherwise.Signed-off-by Tejun Heo <[email protected]>
drivers/power/ bits:
Thanks!
Signature · 2 lines
Anton Vorontsov Email: [email protected]
Tejun Heo
maintainerPATCH v2 14/15 workqueue: use mod_delayed_work() instead of cancel + queue
In reply to Tejun HeoQuoted 1 line
From
18fe17017c7de052f116ef771dc8c4088f0d1ebbMon Sep 17 00:00:00 2001
From: Tejun Heo <[email protected]>
Date: Tue, 31 Jul 2012 10:53:21 -0700
Convert delayed_work users doing cancel_delayed_work() followed by queue_delayed_work() to mod_delayed_work().
Most conversions are straight-forward. Ones worth mentioning are,
* drivers/edac/edac_mc.c: edac_mc_workq_setup() converted to always use mod_delayed_work() and cancel loop in edac_mc_reset_delay_period() is dropped.
* drivers/platform/x86/thinkpad_acpi.c: No need to remember whether watchdog is active or not. @fan_watchdog_active and related code dropped.
* drivers/power/charger-manager.c: Seemingly a lot of delayed_work_pending() abuse going on here. [delayed_]work_pending() are unsynchronized and racy when used like this. I converted one instance in fullbatt_handler(). Please conver the rest so that it invokes workqueue APIs for the intended target state rather than trying to game work item pending state transitions. e.g. if timer should be modified - call mod_delayed_work(), canceled - call cancel_delayed_work[_sync]().
* drivers/thermal/thermal_sys.c: thermal_zone_device_set_polling() simplified. Note that round_jiffies() calls in this function are meaningless. round_jiffies() work on absolute jiffies not delta delay used by delayed_work.
v2: Tomi pointed out that __cancel_delayed_work() users can't be
safely converted to mod_delayed_work(). They could be calling it from irq context and if that happens while delayed_work_timer_fn() is running, it could deadlock. __cancel_delayed_work() users are dropped.
--- block/genhd.c | 6 ++---- drivers/edac/edac_mc.c | 17 +---------------- drivers/infiniband/core/addr.c | 4 +--- drivers/infiniband/hw/nes/nes_hw.c | 6 ++---- drivers/infiniband/hw/nes/nes_nic.c | 5 ++--- drivers/net/wireless/ipw2x00/ipw2100.c | 8 +++----- drivers/net/wireless/zd1211rw/zd_usb.c | 3 +-- drivers/platform/x86/thinkpad_acpi.c | 20 +++++--------------- drivers/power/charger-manager.c | 9 +++------ drivers/power/ds2760_battery.c | 9 +++------ drivers/power/jz4740-battery.c | 6 ++---- drivers/thermal/thermal_sys.c | 15 ++++++--------- fs/afs/callback.c | 4 +--- fs/afs/server.c | 10 ++-------- fs/afs/vlocation.c | 14 +++----------- fs/nfs/nfs4renewd.c | 3 +-- net/core/dst.c | 4 ++-- net/rfkill/input.c | 3 +-- 18 files changed, 41 insertions(+), 105 deletions(-)
block/genhd.c +2 -4
@@ -1524,10 +1524,8 @@ void disk_flush_events(struct gendisk *disk, unsigned int mask)spin_lock_irq (&ev->lock); ev->clearing |= mask; -if (!ev->block) { -cancel_delayed_work (&ev->dwork); -queue_delayed_work (system_nrt_freezable_wq, &ev->dwork,0 ); - } +if (!ev->block) +mod_delayed_work (system_nrt_freezable_wq, &ev->dwork,0 );spin_unlock_irq (&ev->lock); }
drivers/edac/edac_mc.c +1 -16
@@ -492,7 +492,7 @@ static void edac_mc_workq_setup(struct mem_ctl_info *mci, unsigned msec)return ;INIT_DELAYED_WORK (&mci->work, edac_mc_workq_function); -queue_delayed_work (edac_workqueue, &mci->work,msecs_to_jiffies (msec)); +mod_delayed_work (edac_workqueue, &mci->work,msecs_to_jiffies (msec)); }/*
@@ -533,21 +533,6 @@ void edac_mc_reset_delay_period(int value)mutex_lock (&mem_ctls_mutex); -/* scan the list and turn off all workq timers, doing so under lock -*/ -list_for_each (item, &mc_devices) { - mci = list_entry(item, struct mem_ctl_info, link); - -if (mci->op_state ==OP_RUNNING_POLL ) -cancel_delayed_work (&mci->work); - } - -mutex_unlock (&mem_ctls_mutex); - - -/* re-walk the list, and reset the poll delay */ -mutex_lock (&mem_ctls_mutex); -list_for_each (item, &mc_devices) { mci = list_entry(item, struct mem_ctl_info, link);
drivers/infiniband/core/addr.c +1 -3
@@ -152,13 +152,11 @@ static void set_timeout(unsigned long time) {unsigned long delay; -cancel_delayed_work (&work); - delay = time - jiffies;if ((long )delay <=0 ) delay =1 ; -queue_delayed_work (addr_wq, &work, delay); +mod_delayed_work (addr_wq, &work, delay); }static void queue_req (struct addr_req *req)
drivers/infiniband/hw/nes/nes_hw.c +2 -4
@@ -2679,11 +2679,9 @@ static void nes_process_mac_intr(struct nes_device *nesdev, u32 mac_number) } }if (nesadapter->phy_type[mac_index] ==NES_PHY_TYPE_SFP_D ) { -if (nesdev->link_recheck) -cancel_delayed_work (&nesdev->work); nesdev->link_recheck =1 ; -schedule_delayed_work (&nesdev->work, -NES_LINK_RECHECK_DELAY ); +mod_delayed_work (system_wq, &nesdev->work, +NES_LINK_RECHECK_DELAY ); } }
drivers/infiniband/hw/nes/nes_nic.c +2 -3
@@ -243,10 +243,9 @@ static int nes_netdev_open(struct net_device *netdev)spin_lock_irqsave (&nesdev->nesadapter->phy_lock, flags);if (nesdev->nesadapter->phy_type[nesdev->mac_index] ==NES_PHY_TYPE_SFP_D ) { -if (nesdev->link_recheck) -cancel_delayed_work (&nesdev->work); nesdev->link_recheck =1 ; -schedule_delayed_work (&nesdev->work,NES_LINK_RECHECK_DELAY ); +mod_delayed_work (system_wq, &nesdev->work, +NES_LINK_RECHECK_DELAY ); }spin_unlock_irqrestore (&nesdev->nesadapter->phy_lock, flags);
drivers/net/wireless/ipw2x00/ipw2100.c +3 -5
@@ -2180,8 +2180,7 @@ static void isr_indicate_rf_kill(struct ipw2100_priv *priv, u32 status)/* Make sure the RF Kill check timer is running */ priv->stop_rf_kill =0 ; -cancel_delayed_work (&priv->rf_kill); -schedule_delayed_work (&priv->rf_kill,round_jiffies_relative (HZ )); +mod_delayed_work (system_wq, &priv->rf_kill,round_jiffies_relative (HZ )); }static void send_scan_event (void *data)
@@ -4321,9 +4320,8 @@ static int ipw_radio_kill_sw(struct ipw2100_priv *priv, int disable_radio) "disabled by HW switch\n");/* Make sure the RF_KILL check timer is running */ priv->stop_rf_kill =0 ; -cancel_delayed_work (&priv->rf_kill); -schedule_delayed_work (&priv->rf_kill, -round_jiffies_relative (HZ )); +mod_delayed_work (system_wq, &priv->rf_kill, +round_jiffies_relative (HZ )); }else schedule_reset (priv); }
drivers/net/wireless/zd1211rw/zd_usb.c +1 -2
@@ -1164,8 +1164,7 @@ void zd_usb_reset_rx_idle_timer(struct zd_usb *usb) {struct zd_usb_rx *rx = &usb->rx; -cancel_delayed_work (&rx->idle_work); -queue_delayed_work (zd_workqueue, &rx->idle_work,ZD_RX_IDLE_INTERVAL ); +mod_delayed_work (zd_workqueue, &rx->idle_work,ZD_RX_IDLE_INTERVAL ); }static inline void init_usb_interrupt (struct zd_usb *usb)
drivers/platform/x86/thinkpad_acpi.c +5 -15
@@ -7683,25 +7683,15 @@ static int fan_set_speed(int speed)static void fan_watchdog_reset (void ) { -static int fan_watchdog_active; -if (fan_control_access_mode ==TPACPI_FAN_WR_NONE )return ; -if (fan_watchdog_active) -cancel_delayed_work (&fan_watchdog_task); -if (fan_watchdog_maxinterval >0 && - tpacpi_lifecycle !=TPACPI_LIFE_EXITING ) { - fan_watchdog_active =1 ; -if (!queue_delayed_work (tpacpi_wq, &fan_watchdog_task, -msecs_to_jiffies (fan_watchdog_maxinterval - *1000 ))) { -pr_err ("failed to queue the fan watchdog, " -"watchdog will not trigger\n" ); - } - }else - fan_watchdog_active =0 ; + tpacpi_lifecycle !=TPACPI_LIFE_EXITING ) +mod_delayed_work (tpacpi_wq, &fan_watchdog_task, +msecs_to_jiffies (fan_watchdog_maxinterval *1000 )); +else +cancel_delayed_work (&fan_watchdog_task); }static void fan_watchdog_fire (struct work_struct *ignored)
drivers/power/charger-manager.c +3 -6
@@ -512,9 +512,8 @@ static void _setup_polling(struct work_struct *work)if (!delayed_work_pending (&cm_monitor_work) || (delayed_work_pending (&cm_monitor_work) &&time_after (next_polling, _next_polling))) { -cancel_delayed_work_sync (&cm_monitor_work); next_polling = jiffies + polling_jiffy; -queue_delayed_work (cm_wq, &cm_monitor_work, polling_jiffy); +mod_delayed_work (cm_wq, &cm_monitor_work, polling_jiffy); } out:
@@ -549,10 +548,8 @@ static void fullbatt_handler(struct charger_manager *cm)if (cm_suspended)device_set_wakeup_capable (cm->dev, true); -if (delayed_work_pending (&cm->fullbatt_vchk_work)) -cancel_delayed_work (&cm->fullbatt_vchk_work); -queue_delayed_work (cm_wq, &cm->fullbatt_vchk_work, -msecs_to_jiffies (desc->fullbatt_vchkdrop_ms)); +mod_delayed_work (cm_wq, &cm->fullbatt_vchk_work, +msecs_to_jiffies (desc->fullbatt_vchkdrop_ms)); cm->fullbatt_vchk_jiffies_at = jiffies +msecs_to_jiffies ( desc->fullbatt_vchkdrop_ms);
drivers/power/ds2760_battery.c +3 -6
@@ -355,8 +355,7 @@ static void ds2760_battery_external_power_changed(struct power_supply *psy)dev_dbg (di->dev,"%s\n" , __func__); -cancel_delayed_work (&di->monitor_work); -queue_delayed_work (di->monitor_wqueue, &di->monitor_work,HZ /10 ); +mod_delayed_work (di->monitor_wqueue, &di->monitor_work,HZ /10 ); }
@@ -401,8 +400,7 @@ static void ds2760_battery_set_charged(struct power_supply *psy)/* postpone the actual work by 20 secs. This is for debouncing GPIO * signals and to let the current value settle. See AN4188. */ -cancel_delayed_work (&di->set_charged_work); -queue_delayed_work (di->monitor_wqueue, &di->set_charged_work,HZ *20 ); +mod_delayed_work (di->monitor_wqueue, &di->set_charged_work,HZ *20 ); } static int ds2760_battery_get_property(struct power_supply *psy,
@@ -616,8 +614,7 @@ static int ds2760_battery_resume(struct platform_device *pdev) di->charge_status =POWER_SUPPLY_STATUS_UNKNOWN ;power_supply_changed (&di->bat); -cancel_delayed_work (&di->monitor_work); -queue_delayed_work (di->monitor_wqueue, &di->monitor_work,HZ ); +mod_delayed_work (di->monitor_wqueue, &di->monitor_work,HZ );return 0 ; }
drivers/power/jz4740-battery.c +2 -4
@@ -173,16 +173,14 @@ static void jz_battery_external_power_changed(struct power_supply *psy) {struct jz_battery *jz_battery =psy_to_jz_battery (psy); -cancel_delayed_work (&jz_battery->work); -schedule_delayed_work (&jz_battery->work,0 ); +mod_delayed_work (system_wq, &jz_battery->work,0 ); }static irqreturn_t jz_battery_charge_irq (int irq,void *data) {struct jz_battery *jz_battery = data; -cancel_delayed_work (&jz_battery->work); -schedule_delayed_work (&jz_battery->work,0 ); +mod_delayed_work (system_wq, &jz_battery->work,0 );return IRQ_HANDLED ; }
drivers/thermal/thermal_sys.c +6 -9
@@ -694,17 +694,14 @@ thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz)static void thermal_zone_device_set_polling (struct thermal_zone_device *tz,int delay) { -cancel_delayed_work (&(tz->poll_queue)); - -if (!delay) -return ; -if (delay >1000 ) -queue_delayed_work (system_freezable_wq, &(tz->poll_queue), -round_jiffies (msecs_to_jiffies (delay))); +mod_delayed_work (system_freezable_wq, &tz->poll_queue, +round_jiffies (msecs_to_jiffies (delay))); +else if (delay) +mod_delayed_work (system_freezable_wq, &tz->poll_queue, +msecs_to_jiffies (delay));else -queue_delayed_work (system_freezable_wq, &(tz->poll_queue), -msecs_to_jiffies (delay)); +cancel_delayed_work (&tz->poll_queue); } static void thermal_zone_device_passive(struct thermal_zone_device *tz,
fs/afs/callback.c +1 -3
@@ -351,9 +351,7 @@ void afs_dispatch_give_up_callbacks(struct work_struct *work)*/ void afs_flush_callback_breaks (struct afs_server *server) { -cancel_delayed_work (&server->cb_break_work); -queue_delayed_work (afs_callback_update_worker, - &server->cb_break_work,0 ); +mod_delayed_work (afs_callback_update_worker, &server->cb_break_work,0 ); }#if 0
fs/afs/server.c +2 -8
@@ -285,12 +285,7 @@ static void afs_reap_server(struct work_struct *work) expiry = server->time_of_death + afs_server_timeout;if (expiry > now) { delay = (expiry - now) *HZ ; -if (!queue_delayed_work (afs_wq, &afs_server_reaper, - delay)) { -cancel_delayed_work (&afs_server_reaper); -queue_delayed_work (afs_wq, &afs_server_reaper, - delay); - } +mod_delayed_work (afs_wq, &afs_server_reaper, delay);break ; }
@@ -323,6 +318,5 @@ static void afs_reap_server(struct work_struct *work) void __exit afs_purge_servers(void) { afs_server_timeout =0 ; -cancel_delayed_work (&afs_server_reaper); -queue_delayed_work (afs_wq, &afs_server_reaper,0 ); +mod_delayed_work (afs_wq, &afs_server_reaper,0 ); }
fs/afs/vlocation.c +3 -11
@@ -561,12 +561,7 @@ static void afs_vlocation_reaper(struct work_struct *work)if (expiry > now) { delay = (expiry - now) *HZ ;_debug ("delay %lu" , delay); -if (!queue_delayed_work (afs_wq, &afs_vlocation_reap, - delay)) { -cancel_delayed_work (&afs_vlocation_reap); -queue_delayed_work (afs_wq, &afs_vlocation_reap, - delay); - } +mod_delayed_work (afs_wq, &afs_vlocation_reap, delay);break ; }
@@ -614,13 +609,10 @@ void afs_vlocation_purge(void)spin_lock (&afs_vlocation_updates_lock);list_del_init (&afs_vlocation_updates);spin_unlock (&afs_vlocation_updates_lock); -cancel_delayed_work (&afs_vlocation_update); -queue_delayed_work (afs_vlocation_update_worker, - &afs_vlocation_update,0 ); +mod_delayed_work (afs_vlocation_update_worker, &afs_vlocation_update,0 );destroy_workqueue (afs_vlocation_update_worker); -cancel_delayed_work (&afs_vlocation_reap); -queue_delayed_work (afs_wq, &afs_vlocation_reap,0 ); +mod_delayed_work (afs_wq, &afs_vlocation_reap,0 ); }/*
fs/nfs/nfs4renewd.c +1 -2
@@ -117,8 +117,7 @@ nfs4_schedule_state_renewal(struct nfs_client *clp) timeout =5 *HZ ;dprintk ("%s: requeueing work. Lease period = %ld\n" , __func__, (timeout +HZ -1 ) /HZ ); -cancel_delayed_work (&clp->cl_renewd); -schedule_delayed_work (&clp->cl_renewd, timeout); +mod_delayed_work (system_wq, &clp->cl_renewd, timeout);set_bit (NFS_CS_RENEWD , &clp->cl_res_state);spin_unlock (&clp->cl_lock); }
net/core/dst.c +2 -2
@@ -214,8 +214,8 @@ void __dst_free(struct dst_entry *dst)if (dst_garbage.timer_inc >DST_GC_INC ) { dst_garbage.timer_inc =DST_GC_INC ; dst_garbage.timer_expires =DST_GC_MIN ; -cancel_delayed_work (&dst_gc_work); -schedule_delayed_work (&dst_gc_work, dst_garbage.timer_expires); +mod_delayed_work (system_wq, &dst_gc_work, + dst_garbage.timer_expires); }spin_unlock_bh (&dst_garbage.lock); }
net/rfkill/input.c +1 -2
@@ -164,8 +164,7 @@ static void rfkill_schedule_global_op(enum rfkill_sched_op op) rfkill_op_pending = true;if (op ==RFKILL_GLOBAL_OP_EPO && !rfkill_is_epo_lock_active ()) {/* bypass the limiter for EPO */ -cancel_delayed_work (&rfkill_op_work); -schedule_delayed_work (&rfkill_op_work,0 ); +mod_delayed_work (system_wq, &rfkill_op_work,0 ); rfkill_last_scheduled = jiffies; }else rfkill_schedule_ratelimited ();
Signature · 1 line
1.7.7.3
Tejun Heo
maintainerPATCH 15/15 workqueue: deprecate __cancel_delayed_work()
In reply to Tejun Heo__cancel_delayed_work() is different from cancel_delayed_work() in that it uses del_timer() instead of del_timer_sync(). This adds confusion to already complicated flush / cancel API and given that the only thing delayed_work->timer does is queueing the work, the difference between cancel_delayed_work() and __cancel_delayed_work() isn't anything material.
Furthermore, none of the remaining users are on hot path racing against high-frequency work item making the chance of actually waiting for delayed_work_timer_fn() very slim.
Use cancel_delayed_work() instead of __cancel_delayed_work() and mark the latter deprecated.
--- block/blk-core.c | 2 +- drivers/block/floppy.c | 2 +- drivers/infiniband/core/mad.c | 2 +- drivers/video/omap2/dss/dsi.c | 2 +- include/linux/workqueue.h | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-)
block/blk-core.c +1 -1
@@ -262,7 +262,7 @@ EXPORT_SYMBOL(blk_start_queue);**/ void blk_stop_queue (struct request_queue *q) { -__cancel_delayed_work (&q->delay_work); +cancel_delayed_work (&q->delay_work);queue_flag_set (QUEUE_FLAG_STOPPED , q); }EXPORT_SYMBOL (blk_stop_queue);
drivers/block/floppy.c +1 -1
@@ -889,7 +889,7 @@ static void unlock_fdc(void) raw_cmd =NULL ; command_status =FD_COMMAND_NONE ; -__cancel_delayed_work (&fd_timeout); +cancel_delayed_work (&fd_timeout); do_floppy =NULL ; cont =NULL ;clear_bit (0 , &fdc_busy);
drivers/infiniband/core/mad.c +1 -1
@@ -2004,7 +2004,7 @@ static void adjust_timeout(struct ib_mad_agent_private *mad_agent_priv)unsigned long delay;if (list_empty (&mad_agent_priv->wait_list)) { -__cancel_delayed_work (&mad_agent_priv->timed_work); +cancel_delayed_work (&mad_agent_priv->timed_work); }else { mad_send_wr = list_entry(mad_agent_priv->wait_list.next, struct ib_mad_send_wr_private,
drivers/video/omap2/dss/dsi.c +1 -1
@@ -4307,7 +4307,7 @@ static void dsi_framedone_irq_callback(void *data, u32 mask)* and is sending the data. */ -__cancel_delayed_work (&dsi->framedone_timeout_work); +cancel_delayed_work (&dsi->framedone_timeout_work);dsi_handle_framedone (dsidev,0 ); }
include/linux/workqueue.h +1 -1
@@ -445,7 +445,7 @@ static inline bool cancel_delayed_work(struct delayed_work *work)* if it returns 0 the timer function may be running and the queueing is in * progress. */ -static inline bool __cancel_delayed_work (struct delayed_work *work) +static inline bool __deprecated __cancel_delayed_work (struct delayed_work *work) {bool ret;
Signature · 1 line
1.7.7.3
Re: [PATCH 15/15] workqueue: deprecate __cancel_delayed_work()
Hi,
Quoted 7 lines · Tejun Heo
__cancel_delayed_work()is different fromcancel_delayed_work()in that it usesdel_timer()instead ofdel_timer_sync(). This adds confusion to already complicated flush / cancel API and given that the only thingdelayed_work->timerdoes is queueing the work, the difference betweencancel_delayed_work()and__cancel_delayed_work()isn't anything material.
Quoted 6 lines
Furthermore, none of the remaining users are on hot path racing against high-frequency work item making the chance of actually waiting for
delayed_work_timer_fn()very slim.Use
cancel_delayed_work()instead of__cancel_delayed_work()and mark the latter deprecated.
I used __cancel_delayed_work() in drivers/video/omap2/dss/dsi.c as the cancel is done in an interrupt handler. Is it safe to use cancel_delayed_work() in atomic context? I presume not, as it uses del_timer_sync().
Tomi
Tejun Heo
maintainerRe: [PATCH 15/15] workqueue: deprecate __cancel_delayed_work()
Quoted 7 lines · Tomi Valkeinen
Quoted 2 lines
Use
cancel_delayed_work()instead of__cancel_delayed_work()and mark the latter deprecated.I used
__cancel_delayed_work()indrivers/video/omap2/dss/dsi.cas the cancel is done in an interrupt handler. Is it safe to usecancel_delayed_work()in atomic context? I presume not, as it usesdel_timer_sync().
Ah, you're right. __cancel_delayed_work() invoked from irq context can't be converted to cancel_delayed_work() or mod_delayed_work(). I'll skip those from the previous patch and drop this patch. I really hope this could be solved somehow tho. :(
Thank you.
Signature · 1 line
tejun
Tejun Heo
maintainerRe: [PATCH 15/15] workqueue: deprecate __cancel_delayed_work()
In reply to Tejun HeoThis patch is dropped for now as cancel_delayed_work() cannot be called from irq context as pointed out by Tomi Valkeinen. git branch accordingly updated. I'm working on another patchset to remove the restriction.
Thanks.
Signature · 1 line
tejun