2008-07-12 15:29:40

by 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 <[email protected]>

--- 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;

2008-07-12 15:32:12

by Oleg Nesterov

Subject: [PATCH] workqueues: queue_work() can use queue_work_on()

queue_work() can use queue_work_on() to avoid the code duplication.

Signed-off-by: Oleg Nesterov <[email protected]>

--- 26-rc2/kernel/workqueue.c~WQ_2_QWON_FACTOR 2008-07-12 19:11:39.000000000 +0400
+++ 26-rc2/kernel/workqueue.c 2008-07-12 19:19:23.000000000 +0400
@@ -159,14 +159,11 @@ static void __queue_work(struct cpu_work
*/
int queue_work(struct workqueue_struct *wq, struct work_struct *work)
{
- int ret = 0;
+ int ret;
+
+ ret = queue_work_on(get_cpu(), wq, work);
+ put_cpu();

- if (!test_and_set_bit(WORK_STRUCT_PENDING, work_data_bits(work))) {
- BUG_ON(!list_empty(&work->entry));
- __queue_work(wq_per_cpu(wq, get_cpu()), work);
- put_cpu();
- ret = 1;
- }
return ret;
}
EXPORT_SYMBOL_GPL(queue_work);

2008-07-12 15:41:51

by Oleg Nesterov

Subject: [PATCH] workqueues: schedule_on_each_cpu() can use schedule_work_on()

schedule_on_each_cpu() can use schedule_work_on() to avoid the code
duplication.

Signed-off-by: Oleg Nesterov <[email protected]>

--- 26-rc2/kernel/workqueue.c~WQ_3_SOEC_QWON 2008-07-12 19:19:23.000000000 +0400
+++ 26-rc2/kernel/workqueue.c 2008-07-12 19:40:57.000000000 +0400
@@ -689,8 +689,7 @@ int schedule_on_each_cpu(work_func_t fun
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);
+ schedule_work_on(cpu, work);
}
for_each_online_cpu(cpu)
flush_work(per_cpu_ptr(works, cpu));