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

[Akinobu Mita <akinobu.mita@gmail.com>]

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