public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Srivatsa Vaddagiri <vatsa@in.ibm.com>
To: Rusty Russell <rusty@rustcorp.com.au>
Cc: Ingo Molnar <mingo@elte.hu>, Andrew Morton <akpm@osdl.org>,
	linux-kernel@vger.kernel.org
Subject: [PATCH] Fix deadlock in __create_workqueue
Date: Fri, 30 Apr 2004 17:07:51 +0530	[thread overview]
Message-ID: <20040430113751.GA18296@in.ibm.com> (raw)

Noticed a possible deadlock in __create_workqueue when CONFIG_HOTPLUG_CPU is
set.  This can happen when create_workqueue_thread fails to create a worker
thread. In that case, we call destroy_workqueue with cpu hotplug lock held.
destroy_workqueue however also attempts to take the same lock.

Patch below address this deadlock as well as a kthread_stop race. 
Its tested against 2.6.6-rc2-mm2 on a 4-way Intel SMP box.

---

 linux-2.6.6-rc2-mm2-root/kernel/workqueue.c |   58 +++++++++++++++++-----------
 1 files changed, 37 insertions(+), 21 deletions(-)

diff -puN kernel/workqueue.c~__create_workqueue_fix kernel/workqueue.c
--- linux-2.6.6-rc2-mm2/kernel/workqueue.c~__create_workqueue_fix	2004-04-29 18:53:03.722704312 +0530
+++ linux-2.6.6-rc2-mm2-root/kernel/workqueue.c	2004-04-29 18:53:09.623807208 +0530
@@ -201,8 +201,9 @@ static int worker_thread(void *__cwq)
 	siginitset(&sa.sa.sa_mask, sigmask(SIGCHLD));
 	do_sigaction(SIGCHLD, &sa, (struct k_sigaction *)0);
 
+	set_task_state(current, TASK_INTERRUPTIBLE);
+
 	while (!kthread_should_stop()) {
-		set_task_state(current, TASK_INTERRUPTIBLE);
 
 		add_wait_queue(&cwq->more_work, &wait);
 		if (list_empty(&cwq->worklist))
@@ -213,7 +214,11 @@ static int worker_thread(void *__cwq)
 
 		if (!list_empty(&cwq->worklist))
 			run_workqueue(cwq);
+
+		set_task_state(current, TASK_INTERRUPTIBLE);
 	}
+
+	__set_current_state(TASK_RUNNING);
 	return 0;
 }
 
@@ -297,6 +302,21 @@ static struct task_struct *create_workqu
 	return p;
 }
 
+static void cleanup_workqueue_thread(struct workqueue_struct *wq, int cpu)
+{
+	struct cpu_workqueue_struct *cwq;
+	unsigned long flags;
+	struct task_struct *p;
+
+	cwq = wq->cpu_wq + cpu;
+	spin_lock_irqsave(&cwq->lock, flags);
+	p = cwq->thread;
+	cwq->thread = NULL;
+	spin_unlock_irqrestore(&cwq->lock, flags);
+	if (p)
+		kthread_stop(p);
+}
+
 struct workqueue_struct *__create_workqueue(const char *name,
 					    int singlethread)
 {
@@ -322,16 +342,27 @@ struct workqueue_struct *__create_workqu
 		else
 			wake_up_process(p);
 	} else {
-		spin_lock(&workqueue_lock);
-		list_add(&wq->list, &workqueues);
-		spin_unlock_irq(&workqueue_lock);
 		for_each_online_cpu(cpu) {
 			p = create_workqueue_thread(wq, cpu);
 			if (p) {
 				kthread_bind(p, cpu);
 				wake_up_process(p);
-			} else
+			} else {
+				int i;
+
+				for (i=cpu-1; i>=0; --i)
+					if (cpu_online(i))
+						cleanup_workqueue_thread(wq, i);
+
 				destroy = 1;
+				break;
+			}
+		}
+
+		if (!destroy) {
+			spin_lock(&workqueue_lock);
+			list_add(&wq->list, &workqueues);
+			spin_unlock_irq(&workqueue_lock);
 		}
 	}
 
@@ -339,28 +370,13 @@ struct workqueue_struct *__create_workqu
 	 * Was there any error during startup? If yes then clean up:
 	 */
 	if (destroy) {
-		destroy_workqueue(wq);
+		kfree(wq);
 		wq = NULL;
 	}
 	unlock_cpu_hotplug();
 	return wq;
 }
 
-static void cleanup_workqueue_thread(struct workqueue_struct *wq, int cpu)
-{
-	struct cpu_workqueue_struct *cwq;
-	unsigned long flags;
-	struct task_struct *p;
-
-	cwq = wq->cpu_wq + cpu;
-	spin_lock_irqsave(&cwq->lock, flags);
-	p = cwq->thread;
-	cwq->thread = NULL;
-	spin_unlock_irqrestore(&cwq->lock, flags);
-	if (p)
-		kthread_stop(p);
-}
-
 void destroy_workqueue(struct workqueue_struct *wq)
 {
 	int cpu;

_


-- 


Thanks and Regards,
Srivatsa Vaddagiri,
Linux Technology Center,
IBM Software Labs,
Bangalore, INDIA - 560017

             reply	other threads:[~2004-04-30 11:36 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-04-30 11:37 Srivatsa Vaddagiri [this message]
2004-05-01  2:19 ` [PATCH] Fix deadlock in __create_workqueue Andrew Morton
2004-05-03 12:24   ` Srivatsa Vaddagiri
2004-05-03 20:08     ` Andrew Morton
2004-05-04  6:51       ` Srivatsa Vaddagiri
2004-05-01  2:27 ` Andrew Morton
2004-05-03 12:23   ` Srivatsa Vaddagiri
2004-05-03 19:25     ` Andrew Morton
2004-05-04  6:50       ` Srivatsa Vaddagiri
2004-05-04  7:35         ` Andrew Morton
2004-05-05  8:37     ` Rusty Russell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20040430113751.GA18296@in.ibm.com \
    --to=vatsa@in.ibm.com \
    --cc=akpm@osdl.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=rusty@rustcorp.com.au \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox