Re: +workqueue-proper-error-unwinding-in-cpu-hotplug-error-path.patchadded to -mm tree

From: Akinobu Mita
Date: Tue Jul 22 2008 - 05:55:56 EST


On Tue, Jul 22, 2008 at 01:16:36PM +0400, Oleg Nesterov wrote:
> Thanks Akinobu!

You are welcome!

> Can't we simplify the fix? I don't like the fact that the CPU_UP_CANCELED
> logic is duplicated.
>
> What do you think about the patch below?

Yes, it is no duplication. But the goto usage in this patch looks bit
tricky to me. Maybe it is better to factor out the list_for_each() block.

>
> Oleg.
>
> --- 26-rc2/kernel/workqueue.c~WQ_CPU_UP_PREPARE 2008-07-12 19:40:57.000000000 +0400
> +++ 26-rc2/kernel/workqueue.c 2008-07-22 13:15:16.000000000 +0400
> @@ -911,6 +911,7 @@ static int __devinit workqueue_cpu_callb
> unsigned int cpu = (unsigned long)hcpu;
> struct cpu_workqueue_struct *cwq;
> struct workqueue_struct *wq;
> + int ret = NOTIFY_OK;
>
> action &= ~CPU_TASKS_FROZEN;
>
> @@ -919,6 +920,7 @@ static int __devinit workqueue_cpu_callb
> cpu_set(cpu, cpu_populated_map);
> }
>
> +cancel:
> list_for_each_entry(wq, &workqueues, list) {
> cwq = per_cpu_ptr(wq->cpu_wq, cpu);
>
> @@ -928,7 +930,9 @@ static int __devinit workqueue_cpu_callb
> break;
> printk(KERN_ERR "workqueue [%s] for %i failed\n",
> wq->name, cpu);
> - return NOTIFY_BAD;
> + action = CPU_UP_CANCELED;
> + ret = NOTIFY_BAD;
> + goto cancel;
>
> case CPU_ONLINE:
> start_workqueue_thread(cwq, cpu);
> @@ -948,7 +952,7 @@ static int __devinit workqueue_cpu_callb
> cpu_clear(cpu, cpu_populated_map);
> }
>
> - return NOTIFY_OK;
> + return ret;
> }
>
> void __init init_workqueues(void)
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/