From 4594bf159f1962cec3b727954b7c598b07e2e737 Mon Sep 17 00:00:00 2001 From: David Howells Date: Thu, 7 Dec 2006 11:33:26 +0000 Subject: [PATCH] WorkStruct: Use direct assignment rather than cmpxchg() Use direct assignment rather than cmpxchg() as the latter is unavailable and unimplementable on some platforms and is actually unnecessary. The use of cmpxchg() was to guard against two possibilities, neither of which can actually occur: (1) The pending flag may have been unset or may be cleared. However, given where it's called, the pending flag is _always_ set. I don't think it can be unset whilst we're in set_wq_data(). Once the work is enqueued to be actually run, the only way off the queue is for it to be actually run. If it's a delayed work item, then the bit can't be cleared by the timer because we haven't started the timer yet. Also, the pending bit can't be cleared by cancelling the delayed work _until_ the work item has had its timer started. (2) The workqueue pointer might change. This can only happen in two cases: (a) The work item has just been queued to actually run, and so we're protected by the appropriate workqueue spinlock. (b) A delayed work item is being queued, and so the timer hasn't been started yet, and so no one else knows about the work item or can access it (the pending bit protects us). Besides, set_wq_data() _sets_ the workqueue pointer unconditionally, so it can be assigned instead. So, replacing the set_wq_data() with a straight assignment would be okay in most cases. The problem is where we end up tangling with test_and_set_bit() emulated using spinlocks, and even then it's not a problem _provided_ test_and_set_bit() doesn't attempt to modify the word if the bit was set. If that's a problem, then a bitops-proofed assignment will be required - equivalent to atomic_set() vs other atomic_xxx() ops. Signed-off-by: David Howells Signed-off-by: Linus Torvalds --- kernel/workqueue.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) (limited to 'kernel/workqueue.c') diff --git a/kernel/workqueue.c b/kernel/workqueue.c index 6b186750e9be..db49886bfae1 100644 --- a/kernel/workqueue.c +++ b/kernel/workqueue.c @@ -85,22 +85,19 @@ static inline int is_single_threaded(struct workqueue_struct *wq) return list_empty(&wq->list); } +/* + * Set the workqueue on which a work item is to be run + * - Must *only* be called if the pending flag is set + */ static inline void set_wq_data(struct work_struct *work, void *wq) { - unsigned long new, old, res; + unsigned long new; + + BUG_ON(!work_pending(work)); - /* assume the pending flag is already set and that the task has already - * been queued on this workqueue */ new = (unsigned long) wq | (1UL << WORK_STRUCT_PENDING); - res = work->management; - if (res != new) { - do { - old = res; - new = (unsigned long) wq; - new |= (old & WORK_STRUCT_FLAG_MASK); - res = cmpxchg(&work->management, old, new); - } while (res != old); - } + new |= work->management & WORK_STRUCT_FLAG_MASK; + work->management = new; } static inline void *get_wq_data(struct work_struct *work) -- cgit v1.2.3 From a08727bae727fc2ca3a6ee9506d77786b71070b3 Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Sat, 16 Dec 2006 09:53:50 -0800 Subject: Make workqueue bit operations work on "atomic_long_t" On architectures where the atomicity of the bit operations is handled by external means (ie a separate spinlock to protect concurrent accesses), just doing a direct assignment on the workqueue data field (as done by commit 4594bf159f1962cec3b727954b7c598b07e2e737) can cause the assignment to be lost due to lack of serialization with the bitops on the same word. So we need to serialize the assignment with the locks on those architectures (notably older ARM chips, PA-RISC and sparc32). So rather than using an "unsigned long", let's use "atomic_long_t", which already has a safe assignment operation (atomic_long_set()) on such architectures. This requires that the atomic operations use the same atomicity locks as the bit operations do, but that is largely the case anyway. Sparc32 will probably need fixing. Architectures (including modern ARM with LL/SC) that implement sane atomic operations for SMP won't see any of this matter. Cc: Russell King Cc: David Howells Cc: David Miller Cc: Matthew Wilcox Cc: Linux Arch Maintainers Cc: Andrew Morton Signed-off-by: Linus Torvalds --- include/linux/workqueue.h | 32 ++++++++++++++++++++++---------- kernel/workqueue.c | 16 ++++++++-------- 2 files changed, 30 insertions(+), 18 deletions(-) (limited to 'kernel/workqueue.c') diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h index 5b13dcf02714..2a7b38d87018 100644 --- a/include/linux/workqueue.h +++ b/include/linux/workqueue.h @@ -8,16 +8,21 @@ #include #include #include +#include struct workqueue_struct; struct work_struct; typedef void (*work_func_t)(struct work_struct *work); +/* + * The first word is the work queue pointer and the flags rolled into + * one + */ +#define work_data_bits(work) ((unsigned long *)(&(work)->data)) + struct work_struct { - /* the first word is the work queue pointer and the flags rolled into - * one */ - unsigned long management; + 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) @@ -26,6 +31,9 @@ struct work_struct { work_func_t func; }; +#define WORK_DATA_INIT(autorelease) \ + ATOMIC_LONG_INIT((autorelease) << WORK_STRUCT_NOAUTOREL) + struct delayed_work { struct work_struct work; struct timer_list timer; @@ -36,13 +44,13 @@ struct execute_work { }; #define __WORK_INITIALIZER(n, f) { \ - .management = 0, \ + .data = WORK_DATA_INIT(0), \ .entry = { &(n).entry, &(n).entry }, \ .func = (f), \ } #define __WORK_INITIALIZER_NAR(n, f) { \ - .management = (1 << WORK_STRUCT_NOAUTOREL), \ + .data = WORK_DATA_INIT(1), \ .entry = { &(n).entry, &(n).entry }, \ .func = (f), \ } @@ -82,17 +90,21 @@ struct execute_work { /* * initialize all of a work item in one go + * + * NOTE! No point in using "atomic_long_set()": useing a direct + * assignment of the work data initializer allows the compiler + * to generate better code. */ #define INIT_WORK(_work, _func) \ do { \ - (_work)->management = 0; \ + (_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) \ do { \ - (_work)->management = (1 << WORK_STRUCT_NOAUTOREL); \ + (_work)->data = (atomic_long_t) WORK_DATA_INIT(1); \ INIT_LIST_HEAD(&(_work)->entry); \ PREPARE_WORK((_work), (_func)); \ } while (0) @@ -114,7 +126,7 @@ struct execute_work { * @work: The work item in question */ #define work_pending(work) \ - test_bit(WORK_STRUCT_PENDING, &(work)->management) + test_bit(WORK_STRUCT_PENDING, work_data_bits(work)) /** * delayed_work_pending - Find out whether a delayable work item is currently @@ -143,7 +155,7 @@ struct execute_work { * This should also be used to release a delayed work item. */ #define work_release(work) \ - clear_bit(WORK_STRUCT_PENDING, &(work)->management) + clear_bit(WORK_STRUCT_PENDING, work_data_bits(work)) extern struct workqueue_struct *__create_workqueue(const char *name, @@ -188,7 +200,7 @@ static inline int cancel_delayed_work(struct delayed_work *work) ret = del_timer_sync(&work->timer); if (ret) - clear_bit(WORK_STRUCT_PENDING, &work->work.management); + work_release(&work->work); return ret; } diff --git a/kernel/workqueue.c b/kernel/workqueue.c index db49886bfae1..742cbbe49bdc 100644 --- a/kernel/workqueue.c +++ b/kernel/workqueue.c @@ -96,13 +96,13 @@ static inline void set_wq_data(struct work_struct *work, void *wq) BUG_ON(!work_pending(work)); new = (unsigned long) wq | (1UL << WORK_STRUCT_PENDING); - new |= work->management & WORK_STRUCT_FLAG_MASK; - work->management = new; + new |= WORK_STRUCT_FLAG_MASK & *work_data_bits(work); + atomic_long_set(&work->data, new); } static inline void *get_wq_data(struct work_struct *work) { - return (void *) (work->management & WORK_STRUCT_WQ_DATA_MASK); + return (void *) (atomic_long_read(&work->data) & WORK_STRUCT_WQ_DATA_MASK); } static int __run_work(struct cpu_workqueue_struct *cwq, struct work_struct *work) @@ -133,7 +133,7 @@ static int __run_work(struct cpu_workqueue_struct *cwq, struct work_struct *work list_del_init(&work->entry); spin_unlock_irqrestore(&cwq->lock, flags); - if (!test_bit(WORK_STRUCT_NOAUTOREL, &work->management)) + if (!test_bit(WORK_STRUCT_NOAUTOREL, work_data_bits(work))) work_release(work); f(work); @@ -206,7 +206,7 @@ int fastcall queue_work(struct workqueue_struct *wq, struct work_struct *work) { int ret = 0, cpu = get_cpu(); - if (!test_and_set_bit(WORK_STRUCT_PENDING, &work->management)) { + if (!test_and_set_bit(WORK_STRUCT_PENDING, work_data_bits(work))) { if (unlikely(is_single_threaded(wq))) cpu = singlethread_cpu; BUG_ON(!list_empty(&work->entry)); @@ -248,7 +248,7 @@ int fastcall queue_delayed_work(struct workqueue_struct *wq, if (delay == 0) return queue_work(wq, work); - if (!test_and_set_bit(WORK_STRUCT_PENDING, &work->management)) { + if (!test_and_set_bit(WORK_STRUCT_PENDING, work_data_bits(work))) { BUG_ON(timer_pending(timer)); BUG_ON(!list_empty(&work->entry)); @@ -280,7 +280,7 @@ int queue_delayed_work_on(int cpu, struct workqueue_struct *wq, struct timer_list *timer = &dwork->timer; struct work_struct *work = &dwork->work; - if (!test_and_set_bit(WORK_STRUCT_PENDING, &work->management)) { + if (!test_and_set_bit(WORK_STRUCT_PENDING, work_data_bits(work))) { BUG_ON(timer_pending(timer)); BUG_ON(!list_empty(&work->entry)); @@ -321,7 +321,7 @@ static void run_workqueue(struct cpu_workqueue_struct *cwq) spin_unlock_irqrestore(&cwq->lock, flags); BUG_ON(get_wq_data(work) != cwq); - if (!test_bit(WORK_STRUCT_NOAUTOREL, &work->management)) + if (!test_bit(WORK_STRUCT_NOAUTOREL, work_data_bits(work))) work_release(work); f(work); -- cgit v1.2.3 From 9bfb18392ef586467277fa25d8f3a7a93611f6df Mon Sep 17 00:00:00 2001 From: Ingo Molnar Date: Mon, 18 Dec 2006 20:05:09 +0100 Subject: [PATCH] workqueue: fix schedule_on_each_cpu() fix the schedule_on_each_cpu() implementation: __queue_work() is now stricter, hence set the work-pending bit before passing in the new work. (found in the -rt tree, using Peter Zijlstra's files-lock scalability patchset) Signed-off-by: Ingo Molnar Signed-off-by: Linus Torvalds --- kernel/workqueue.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'kernel/workqueue.c') diff --git a/kernel/workqueue.c b/kernel/workqueue.c index 742cbbe49bdc..180a8ce11535 100644 --- a/kernel/workqueue.c +++ b/kernel/workqueue.c @@ -637,9 +637,11 @@ int schedule_on_each_cpu(work_func_t func) mutex_lock(&workqueue_mutex); for_each_online_cpu(cpu) { - INIT_WORK(per_cpu_ptr(works, cpu), func); - __queue_work(per_cpu_ptr(keventd_wq->cpu_wq, cpu), - per_cpu_ptr(works, cpu)); + struct work_struct *work = per_cpu_ptr(works, cpu); + + INIT_WORK(work, func); + set_bit(WORK_STRUCT_PENDING, work_data_bits(work)); + __queue_work(per_cpu_ptr(keventd_wq->cpu_wq, cpu), work); } mutex_unlock(&workqueue_mutex); flush_workqueue(keventd_wq); -- cgit v1.2.3 From af9997e426f9ddfe7a84cb4cd3c7ff938fabd41a Mon Sep 17 00:00:00 2001 From: Randy Dunlap Date: Fri, 22 Dec 2006 01:06:52 -0800 Subject: [PATCH] fix kernel-doc warnings in 2.6.20-rc1 Fix kernel-doc warnings in 2.6.20-rc1. Signed-off-by: Randy Dunlap Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- block/ll_rw_blk.c | 1 + drivers/base/firmware_class.c | 1 + drivers/message/i2o/exec-osm.c | 2 +- kernel/relay.c | 2 +- kernel/workqueue.c | 4 ++-- mm/slab.c | 1 + 6 files changed, 7 insertions(+), 4 deletions(-) (limited to 'kernel/workqueue.c') diff --git a/block/ll_rw_blk.c b/block/ll_rw_blk.c index e07c079e07e6..fb6789725e1b 100644 --- a/block/ll_rw_blk.c +++ b/block/ll_rw_blk.c @@ -2453,6 +2453,7 @@ EXPORT_SYMBOL(blk_rq_map_user); * @rq: request to map data to * @iov: pointer to the iovec * @iov_count: number of elements in the iovec + * @len: I/O byte count * * Description: * Data will be mapped directly for zero copy io, if possible. Otherwise diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c index 4bad2870c485..64558f45e6bc 100644 --- a/drivers/base/firmware_class.c +++ b/drivers/base/firmware_class.c @@ -127,6 +127,7 @@ static ssize_t firmware_loading_show(struct device *dev, /** * firmware_loading_store - set value in the 'loading' control file * @dev: device pointer + * @attr: device attribute pointer * @buf: buffer to scan for loading control value * @count: number of bytes in @buf * diff --git a/drivers/message/i2o/exec-osm.c b/drivers/message/i2o/exec-osm.c index a539d3b61e76..5278aad92bc4 100644 --- a/drivers/message/i2o/exec-osm.c +++ b/drivers/message/i2o/exec-osm.c @@ -367,7 +367,7 @@ static int i2o_exec_remove(struct device *dev) /** * i2o_exec_lct_modified - Called on LCT NOTIFY reply - * @work: work struct for a specific controller + * @_work: work struct for a specific controller * * This function handles asynchronus LCT NOTIFY replies. It parses the * new LCT and if the buffer for the LCT was to small sends a LCT NOTIFY diff --git a/kernel/relay.c b/kernel/relay.c index a4701e7ba7d0..3e076f2acd31 100644 --- a/kernel/relay.c +++ b/kernel/relay.c @@ -302,7 +302,7 @@ static struct rchan_callbacks default_channel_callbacks = { /** * wakeup_readers - wake up readers waiting on a channel - * @private: the channel buffer + * @work: work struct that contains the the channel buffer * * This is the work function used to defer reader waking. The * reason waking is deferred is that calling directly from write diff --git a/kernel/workqueue.c b/kernel/workqueue.c index 180a8ce11535..a3da07c5af28 100644 --- a/kernel/workqueue.c +++ b/kernel/workqueue.c @@ -233,7 +233,7 @@ static void delayed_work_timer_fn(unsigned long __data) /** * queue_delayed_work - queue work on a workqueue after delay * @wq: workqueue to use - * @work: delayable work to queue + * @dwork: delayable work to queue * @delay: number of jiffies to wait before queueing * * Returns 0 if @work was already on a queue, non-zero otherwise. @@ -268,7 +268,7 @@ EXPORT_SYMBOL_GPL(queue_delayed_work); * queue_delayed_work_on - queue work on specific CPU after delay * @cpu: CPU number to execute work on * @wq: workqueue to use - * @work: work to queue + * @dwork: work to queue * @delay: number of jiffies to wait before queueing * * Returns 0 if @work was already on a queue, non-zero otherwise. diff --git a/mm/slab.c b/mm/slab.c index 176037bcc66a..0d4e57431de4 100644 --- a/mm/slab.c +++ b/mm/slab.c @@ -3587,6 +3587,7 @@ out: * @cachep: The cache to allocate from. * @flags: See kmalloc(). * @nodeid: node number of the target node. + * @caller: return address of caller, used for debug information * * Identical to kmem_cache_alloc but it will allocate memory on the given * node, which can improve the performance for cpu bound structures. -- cgit v1.2.3