Re: [PATCH] Fix deadlock in __create_workqueue

[Andrew Morton]

Srivatsa Vaddagiri <vatsa@xxxxxxxxxx> wrote:
>
> On Fri, Apr 30, 2004 at 07:19:01PM -0700, Andrew Morton wrote:
> > Yes, the logic in worker_thread() is a bit dorky, but I
> > don't believe that there is a race in there.
> 
> worker_thread examines kthread_should_stop() while its state
> is TASK_RUNNING, after which it sets its state to TASK_INTERRUPTIBLE.
> If kthread_stop were to come after kthread_should_stop and before
> worker_thread has set its state to TASK_INTERRUPTIBLE (which is possible
> because of a CPU going dead), wouldn't kthread_stop block forever? 
> Note that in case of CPU going dead, it is possible that a worker
> thread bound to the dead cpu continues executing on a different cpu 
> before it is killed in CPU_DEAD processing.

yup, sorry, I misread the code.  Like this?


--- 25/kernel/workqueue.c~worker_thread-race-fix	Mon May  3 13:02:25 2004
+++ 25-akpm/kernel/workqueue.c	Mon May  3 13:07:34 2004
@@ -201,19 +201,20 @@ static int worker_thread(void *__cwq)
 	siginitset(&sa.sa.sa_mask, sigmask(SIGCHLD));
 	do_sigaction(SIGCHLD, &sa, (struct k_sigaction *)0);
 
+	set_current_state(TASK_INTERRUPTIBLE);
 	while (!kthread_should_stop()) {
-		set_task_state(current, TASK_INTERRUPTIBLE);
-
 		add_wait_queue(&cwq->more_work, &wait);
 		if (list_empty(&cwq->worklist))
 			schedule();
 		else
-			set_task_state(current, TASK_RUNNING);
+			__set_current_state(TASK_RUNNING);
 		remove_wait_queue(&cwq->more_work, &wait);
 
 		if (!list_empty(&cwq->worklist))
 			run_workqueue(cwq);
+		set_current_state(TASK_INTERRUPTIBLE);
 	}
+	__set_current_state(TASK_RUNNING);
 	return 0;
 }
 

_

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