Messages in this thread Patch in this message | | | Date | Sat, 12 Jul 2008 19:32:34 +0400 | From | Oleg Nesterov <> | Subject | [PATCH] pm-introduce-new-interfaces-schedule_work_on-and-queue_work_on-cleanup |
| |
On 07/11, Andrew Morton wrote: > > +queue_work_on(int cpu, struct workqueue_struct *wq, struct work_struct *work) > +{ > + int ret = 0; > + > + if (!test_and_set_bit(WORK_STRUCT_PENDING, work_data_bits(work))) { > + BUG_ON(!list_empty(&work->entry)); > + preempt_disable(); > + __queue_work(wq_per_cpu(wq, cpu), work); > + preempt_enable();
The comment above __queue_work() is wrong, we don't need to disable preemtion.
What it actually means is: the caller of __queue_work() must ensure we can't race with CPU_DEAD, but preempt_disable() can't help for queue_work_on(). CPU can die even before preempt_disable().
Remove preempt_disable() and update the comment.
Signed-off-by: Oleg Nesterov <oleg@tv-sign.ru>
--- 26-rc2/kernel/workqueue.c~WQ_1_QWON_CLEANUP 2008-07-12 19:04:48.000000000 +0400 +++ 26-rc2/kernel/workqueue.c 2008-07-12 19:11:39.000000000 +0400 @@ -137,7 +137,6 @@ static void insert_work(struct cpu_workq wake_up(&cwq->more_work); } -/* Preempt must be disabled. */ static void __queue_work(struct cpu_workqueue_struct *cwq, struct work_struct *work) { @@ -180,7 +179,8 @@ EXPORT_SYMBOL_GPL(queue_work); * * Returns 0 if @work was already on a queue, non-zero otherwise. * - * We queue the work to a specific CPU + * We queue the work to a specific CPU, the caller must ensure it + * can't go away. */ int queue_work_on(int cpu, struct workqueue_struct *wq, struct work_struct *work) @@ -189,9 +189,7 @@ queue_work_on(int cpu, struct workqueue_ if (!test_and_set_bit(WORK_STRUCT_PENDING, work_data_bits(work))) { BUG_ON(!list_empty(&work->entry)); - preempt_disable(); __queue_work(wq_per_cpu(wq, cpu), work); - preempt_enable(); ret = 1; } return ret;
| |