From b89deed32ccc96098bd6bc953c64bba6b847774f Mon Sep 17 00:00:00 2001 From: Oleg Nesterov Date: Wed, 9 May 2007 02:33:52 -0700 Subject: implement flush_work() A basic problem with flush_scheduled_work() is that it blocks behind _all_ presently-queued works, rather than just the work whcih the caller wants to flush. If the caller holds some lock, and if one of the queued work happens to want that lock as well then accidental deadlocks can occur. One example of this is the phy layer: it wants to flush work while holding rtnl_lock(). But if a linkwatch event happens to be queued, the phy code will deadlock because the linkwatch callback function takes rtnl_lock. So we implement a new function which will flush a *single* work - just the one which the caller wants to free up. Thus we avoid the accidental deadlocks which can arise from unrelated subsystems' callbacks taking shared locks. flush_work() non-blockingly dequeues the work_struct which we want to kill, then it waits for its handler to complete on all CPUs. Add ->current_work to the "struct cpu_workqueue_struct", it points to currently running "struct work_struct". When flush_work(work) detects ->current_work == work, it inserts a barrier at the _head_ of ->worklist (and thus right _after_ that work) and waits for completition. This means that the next work fired on that CPU will be this barrier, or another barrier queued by concurrent flush_work(), so the caller of flush_work() will be woken before any "regular" work has a chance to run. When wait_on_work() unlocks workqueue_mutex (or whatever we choose to protect against CPU hotplug), CPU may go away. But in that case take_over_work() will move a barrier we queued to another CPU, it will be fired sometime, and wait_on_work() will be woken. Actually, we are doing cleanup_workqueue_thread()->kthread_stop() before take_over_work(), so cwq->thread should complete its ->worklist (and thus the barrier), because currently we don't check kthread_should_stop() in run_workqueue(). But even if we did, everything should be ok. [akpm@osdl.org: cleanup] [akpm@osdl.org: add flush_work_keventd() wrapper] Signed-off-by: Oleg Nesterov Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- include/linux/workqueue.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'include/linux/workqueue.h') diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h index f16ba1e0687d..26a70992dec8 100644 --- a/include/linux/workqueue.h +++ b/include/linux/workqueue.h @@ -178,6 +178,8 @@ extern int FASTCALL(queue_delayed_work(struct workqueue_struct *wq, struct delay extern int queue_delayed_work_on(int cpu, struct workqueue_struct *wq, struct delayed_work *work, unsigned long delay); extern void FASTCALL(flush_workqueue(struct workqueue_struct *wq)); +extern void flush_work(struct workqueue_struct *wq, struct work_struct *work); +extern void flush_work_keventd(struct work_struct *work); extern int FASTCALL(schedule_work(struct work_struct *work)); extern int FASTCALL(run_scheduled_work(struct work_struct *work)); @@ -199,7 +201,7 @@ int execute_in_process_context(work_func_t fn, struct execute_work *); * Kill off a pending schedule_delayed_work(). Note that the work callback * function may still be running on return from cancel_delayed_work(), unless * it returns 1 and the work doesn't re-arm itself. Run flush_workqueue() or - * cancel_work_sync() to wait on it. + * flush_work() or cancel_work_sync() to wait on it. */ static inline int cancel_delayed_work(struct delayed_work *work) { -- cgit v1.2.2 From 7097a87afe937a5879528d52880c2d95f089e96c Mon Sep 17 00:00:00 2001 From: Oleg Nesterov Date: Wed, 9 May 2007 02:34:10 -0700 Subject: workqueue: kill run_scheduled_work() Because it has no callers. Actually, I think the whole idea of run_scheduled_work() was not right, not good to mix "unqueue this work and execute its ->func()" in one function. Signed-off-by: Oleg Nesterov Cc: Ingo Molnar Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- include/linux/workqueue.h | 1 - 1 file changed, 1 deletion(-) (limited to 'include/linux/workqueue.h') diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h index 26a70992dec8..2a58f16e1961 100644 --- a/include/linux/workqueue.h +++ b/include/linux/workqueue.h @@ -182,7 +182,6 @@ extern void flush_work(struct workqueue_struct *wq, struct work_struct *work); extern void flush_work_keventd(struct work_struct *work); extern int FASTCALL(schedule_work(struct work_struct *work)); -extern int FASTCALL(run_scheduled_work(struct work_struct *work)); extern int FASTCALL(schedule_delayed_work(struct delayed_work *work, unsigned long delay)); extern int schedule_delayed_work_on(int cpu, struct delayed_work *work, unsigned long delay); -- cgit v1.2.2 From 1634c48f8b85dcb05101f1eb2eab9af40b5976da Mon Sep 17 00:00:00 2001 From: Oleg Nesterov Date: Wed, 9 May 2007 02:34:18 -0700 Subject: make cancel_rearming_delayed_work() work on any workqueue, not just keventd_wq cancel_rearming_delayed_workqueue(wq, dwork) doesn't need the first parameter. We don't hang on un-queued dwork any longer, and work->data doesn't change its type. This means we can always figure out "wq" from dwork when it is needed. Remove this parameter, and rename the function to cancel_rearming_delayed_work(). Re-create an inline "obsolete" cancel_rearming_delayed_workqueue(wq) which just calls cancel_rearming_delayed_work(). Signed-off-by: Oleg Nesterov Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- include/linux/workqueue.h | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) (limited to 'include/linux/workqueue.h') diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h index 2a58f16e1961..27110c04f21e 100644 --- a/include/linux/workqueue.h +++ b/include/linux/workqueue.h @@ -191,9 +191,6 @@ extern int current_is_keventd(void); extern int keventd_up(void); extern void init_workqueues(void); -void cancel_rearming_delayed_work(struct delayed_work *work); -void cancel_rearming_delayed_workqueue(struct workqueue_struct *, - struct delayed_work *); int execute_in_process_context(work_func_t fn, struct execute_work *); /* @@ -212,4 +209,14 @@ static inline int cancel_delayed_work(struct delayed_work *work) return ret; } +extern void cancel_rearming_delayed_work(struct delayed_work *work); + +/* Obsolete. use cancel_rearming_delayed_work() */ +static inline +void cancel_rearming_delayed_workqueue(struct workqueue_struct *wq, + struct delayed_work *work) +{ + cancel_rearming_delayed_work(work); +} + #endif -- cgit v1.2.2 From 23b2e5991afde5af91a1a661d7f47ee56120759e Mon Sep 17 00:00:00 2001 From: Oleg Nesterov Date: Wed, 9 May 2007 02:34:19 -0700 Subject: workqueue: kill NOAUTOREL works We don't have any users, and it is not so trivial to use NOAUTOREL works correctly. It is better to simplify API. Delete NOAUTOREL support and rename work_release to work_clear_pending to avoid a confusion. Signed-off-by: Oleg Nesterov Acked-by: David Howells Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- include/linux/workqueue.h | 64 +++++++---------------------------------------- 1 file changed, 9 insertions(+), 55 deletions(-) (limited to 'include/linux/workqueue.h') diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h index 27110c04f21e..e1581dce5890 100644 --- a/include/linux/workqueue.h +++ b/include/linux/workqueue.h @@ -24,15 +24,13 @@ typedef void (*work_func_t)(struct work_struct *work); struct work_struct { atomic_long_t data; #define WORK_STRUCT_PENDING 0 /* T if work item pending execution */ -#define WORK_STRUCT_NOAUTOREL 1 /* F if work item automatically released on exec */ #define WORK_STRUCT_FLAG_MASK (3UL) #define WORK_STRUCT_WQ_DATA_MASK (~WORK_STRUCT_FLAG_MASK) struct list_head entry; work_func_t func; }; -#define WORK_DATA_INIT(autorelease) \ - ATOMIC_LONG_INIT((autorelease) << WORK_STRUCT_NOAUTOREL) +#define WORK_DATA_INIT() ATOMIC_LONG_INIT(0) struct delayed_work { struct work_struct work; @@ -44,14 +42,8 @@ struct execute_work { }; #define __WORK_INITIALIZER(n, f) { \ - .data = WORK_DATA_INIT(0), \ - .entry = { &(n).entry, &(n).entry }, \ - .func = (f), \ - } - -#define __WORK_INITIALIZER_NAR(n, f) { \ - .data = WORK_DATA_INIT(1), \ - .entry = { &(n).entry, &(n).entry }, \ + .data = WORK_DATA_INIT(), \ + .entry = { &(n).entry, &(n).entry }, \ .func = (f), \ } @@ -60,23 +52,12 @@ struct execute_work { .timer = TIMER_INITIALIZER(NULL, 0, 0), \ } -#define __DELAYED_WORK_INITIALIZER_NAR(n, f) { \ - .work = __WORK_INITIALIZER_NAR((n).work, (f)), \ - .timer = TIMER_INITIALIZER(NULL, 0, 0), \ - } - #define DECLARE_WORK(n, f) \ struct work_struct n = __WORK_INITIALIZER(n, f) -#define DECLARE_WORK_NAR(n, f) \ - struct work_struct n = __WORK_INITIALIZER_NAR(n, f) - #define DECLARE_DELAYED_WORK(n, f) \ struct delayed_work n = __DELAYED_WORK_INITIALIZER(n, f) -#define DECLARE_DELAYED_WORK_NAR(n, f) \ - struct dwork_struct n = __DELAYED_WORK_INITIALIZER_NAR(n, f) - /* * initialize a work item's function pointer */ @@ -95,16 +76,9 @@ struct execute_work { * assignment of the work data initializer allows the compiler * to generate better code. */ -#define INIT_WORK(_work, _func) \ - do { \ - (_work)->data = (atomic_long_t) WORK_DATA_INIT(0); \ - INIT_LIST_HEAD(&(_work)->entry); \ - PREPARE_WORK((_work), (_func)); \ - } while (0) - -#define INIT_WORK_NAR(_work, _func) \ +#define INIT_WORK(_work, _func) \ do { \ - (_work)->data = (atomic_long_t) WORK_DATA_INIT(1); \ + (_work)->data = (atomic_long_t) WORK_DATA_INIT(); \ INIT_LIST_HEAD(&(_work)->entry); \ PREPARE_WORK((_work), (_func)); \ } while (0) @@ -115,12 +89,6 @@ struct execute_work { init_timer(&(_work)->timer); \ } while (0) -#define INIT_DELAYED_WORK_NAR(_work, _func) \ - do { \ - INIT_WORK_NAR(&(_work)->work, (_func)); \ - init_timer(&(_work)->timer); \ - } while (0) - #define INIT_DELAYED_WORK_DEFERRABLE(_work, _func) \ do { \ INIT_WORK(&(_work)->work, (_func)); \ @@ -143,24 +111,10 @@ struct execute_work { work_pending(&(w)->work) /** - * work_release - Release a work item under execution - * @work: The work item to release - * - * This is used to release a work item that has been initialised with automatic - * release mode disabled (WORK_STRUCT_NOAUTOREL is set). This gives the work - * function the opportunity to grab auxiliary data from the container of the - * work_struct before clearing the pending bit as the work_struct may be - * subject to deallocation the moment the pending bit is cleared. - * - * In such a case, this should be called in the work function after it has - * fetched any data it may require from the containter of the work_struct. - * After this function has been called, the work_struct may be scheduled for - * further execution or it may be deallocated unless other precautions are - * taken. - * - * This should also be used to release a delayed work item. + * work_clear_pending - for internal use only, mark a work item as not pending + * @work: The work item in question */ -#define work_release(work) \ +#define work_clear_pending(work) \ clear_bit(WORK_STRUCT_PENDING, work_data_bits(work)) @@ -205,7 +159,7 @@ static inline int cancel_delayed_work(struct delayed_work *work) ret = del_timer(&work->timer); if (ret) - work_release(&work->work); + work_clear_pending(&work->work); return ret; } -- cgit v1.2.2 From 28e53bddf814485699a4142bc056fd37d4e11dd4 Mon Sep 17 00:00:00 2001 From: Oleg Nesterov Date: Wed, 9 May 2007 02:34:22 -0700 Subject: unify flush_work/flush_work_keventd and rename it to cancel_work_sync flush_work(wq, work) doesn't need the first parameter, we can use cwq->wq (this was possible from the very beginnig, I missed this). So we can unify flush_work_keventd and flush_work. Also, rename flush_work() to cancel_work_sync() and fix all callers. Perhaps this is not the best name, but "flush_work" is really bad. (akpm: this is why the earlier patches bypassed maintainers) Signed-off-by: Oleg Nesterov Cc: Jeff Garzik Cc: "David S. Miller" Cc: Jens Axboe Cc: Tejun Heo Cc: Auke Kok , Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- include/linux/workqueue.h | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) (limited to 'include/linux/workqueue.h') diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h index e1581dce5890..d555f31c0746 100644 --- a/include/linux/workqueue.h +++ b/include/linux/workqueue.h @@ -128,30 +128,33 @@ extern struct workqueue_struct *__create_workqueue(const char *name, extern void destroy_workqueue(struct workqueue_struct *wq); extern int FASTCALL(queue_work(struct workqueue_struct *wq, struct work_struct *work)); -extern int FASTCALL(queue_delayed_work(struct workqueue_struct *wq, struct delayed_work *work, unsigned long delay)); +extern int FASTCALL(queue_delayed_work(struct workqueue_struct *wq, + struct delayed_work *work, unsigned long delay)); extern int queue_delayed_work_on(int cpu, struct workqueue_struct *wq, - struct delayed_work *work, unsigned long delay); + struct delayed_work *work, unsigned long delay); + extern void FASTCALL(flush_workqueue(struct workqueue_struct *wq)); -extern void flush_work(struct workqueue_struct *wq, struct work_struct *work); -extern void flush_work_keventd(struct work_struct *work); +extern void flush_scheduled_work(void); extern int FASTCALL(schedule_work(struct work_struct *work)); -extern int FASTCALL(schedule_delayed_work(struct delayed_work *work, unsigned long delay)); - -extern int schedule_delayed_work_on(int cpu, struct delayed_work *work, unsigned long delay); +extern int FASTCALL(schedule_delayed_work(struct delayed_work *work, + unsigned long delay)); +extern int schedule_delayed_work_on(int cpu, struct delayed_work *work, + unsigned long delay); extern int schedule_on_each_cpu(work_func_t func); -extern void flush_scheduled_work(void); extern int current_is_keventd(void); extern int keventd_up(void); extern void init_workqueues(void); int execute_in_process_context(work_func_t fn, struct execute_work *); +extern void cancel_work_sync(struct work_struct *work); + /* * Kill off a pending schedule_delayed_work(). Note that the work callback * function may still be running on return from cancel_delayed_work(), unless * it returns 1 and the work doesn't re-arm itself. Run flush_workqueue() or - * flush_work() or cancel_work_sync() to wait on it. + * cancel_work_sync() to wait on it. */ static inline int cancel_delayed_work(struct delayed_work *work) { -- cgit v1.2.2