Wed Oct 1 18:23:55 2008 UTC ()
Use a separate thread to probe/attach atabus's childrens. Fixes a deadlock
where the interrupt routine wants to wake up the atabus thread to perform a
reset, while the thread is blocked in wd's attach function.


(bouyer)
diff -r1.99 -r1.100 src/sys/dev/ata/ata.c

cvs diff -r1.99 -r1.100 src/sys/dev/ata/ata.c (expand / switch to context diff)
--- src/sys/dev/ata/ata.c 2008/06/12 21:46:35 1.99
+++ src/sys/dev/ata/ata.c 2008/10/01 18:23:55 1.100
@@ -1,4 +1,4 @@
-/*	$NetBSD: ata.c,v 1.99 2008/06/12 21:46:35 cegger Exp $	*/
+/*	$NetBSD: ata.c,v 1.100 2008/10/01 18:23:55 bouyer Exp $	*/
 
 /*
  * Copyright (c) 1998, 2001 Manuel Bouyer.  All rights reserved.
@@ -30,7 +30,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ata.c,v 1.99 2008/06/12 21:46:35 cegger Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ata.c,v 1.100 2008/10/01 18:23:55 bouyer Exp $");
 
 #include "opt_ata.h"
 
@@ -89,6 +89,9 @@
     TAILQ_HEAD_INITIALIZER(atabus_initq_head);
 struct simplelock atabus_interlock = SIMPLELOCK_INITIALIZER;
 
+/* kernel thread probing devices on a atabus. Only one probing at once */
+struct lwp *atabus_configlwp;
+
 /*****************************************************************************
  * ATA bus layer.
  *
@@ -110,6 +113,7 @@
 static void atabus_childdetached(device_t, device_t);
 static bool atabus_resume(device_t PMF_FN_PROTO);
 static bool atabus_suspend(device_t PMF_FN_PROTO);
+static void atabusconfig_thread(void *);
 
 /*
  * atabusprint:
@@ -174,9 +178,14 @@
 {
 	struct ata_channel *chp = atabus_sc->sc_chan;
 	struct atac_softc *atac = chp->ch_atac;
-	int i, s;
 	struct atabus_initq *atabus_initq = NULL;
+	int i, s, error;
 
+	/* we are in the atabus's thread context */
+	s = splbio();
+	chp->ch_flags |= ATACH_TH_RUN;
+	splx(s);
+
 	/* Probe for the drives. */
 	/* XXX for SATA devices we will power up all drives at once */
 	(*atac->atac_probe)(chp);
@@ -185,6 +194,22 @@
 	    chp->ch_drive[0].drive_flags, chp->ch_drive[1].drive_flags),
 	    DEBUG_PROBE);
 
+	/* next operations will occurs in a separate thread */
+	s = splbio();
+	chp->ch_flags &= ~ATACH_TH_RUN;
+	splx(s);
+
+	/* Make sure the devices probe in atabus order to avoid jitter. */
+	simple_lock(&atabus_interlock);
+	while(1) {
+		atabus_initq = TAILQ_FIRST(&atabus_initq_head);
+		if (atabus_initq->atabus_sc == atabus_sc)
+			break;
+		ltsleep(&atabus_initq_head, PRIBIO, "ata_initq", 0,
+		    &atabus_interlock);
+	}
+	simple_unlock(&atabus_interlock);
+
 	/* If no drives, abort here */
 	for (i = 0; i < chp->ch_ndrive; i++)
 		if ((chp->ch_drive[i].drive_flags & DRIVE) != 0)
@@ -196,17 +221,44 @@
 	if (chp->ch_flags & ATACH_SHUTDOWN)
 		goto out;
 
-	/* Make sure the devices probe in atabus order to avoid jitter. */
+
+	if ((error = kthread_create(PRI_NONE, 0, NULL, atabusconfig_thread,
+	    atabus_sc, &atabus_configlwp,
+	    "%scnf", device_xname(atac->atac_dev))) != 0)
+		aprint_error_dev(atac->atac_dev,
+		    "unable to create config thread: error %d\n", error);
+	return;
+
+ out:
 	simple_lock(&atabus_interlock);
-	while(1) {
-		atabus_initq = TAILQ_FIRST(&atabus_initq_head);
-		if (atabus_initq->atabus_sc == atabus_sc)
-			break;
-		ltsleep(&atabus_initq_head, PRIBIO, "ata_initq", 0,
-		    &atabus_interlock);
-	}
+	TAILQ_REMOVE(&atabus_initq_head, atabus_initq, atabus_initq);
 	simple_unlock(&atabus_interlock);
 
+	free(atabus_initq, M_DEVBUF);
+	wakeup(&atabus_initq_head);
+
+	ata_delref(chp);
+
+	config_pending_decr();
+}
+
+/*
+ * atabus_configthread: finish attach of atabus's childrens, in a separate
+ * kernel thread.
+ */
+static void
+atabusconfig_thread(void *arg)
+{
+	struct atabus_softc *atabus_sc = arg;
+	struct ata_channel *chp = atabus_sc->sc_chan;
+	struct atac_softc *atac = chp->ch_atac;
+	int i, s;
+	struct atabus_initq *atabus_initq = NULL;
+
+	simple_lock(&atabus_interlock);
+	atabus_initq = TAILQ_FIRST(&atabus_initq_head);
+	simple_unlock(&atabus_interlock);
+	KASSERT(atabus_initq->atabus_sc == atabus_sc);
 	/*
 	 * Attach an ATAPI bus, if needed.
 	 */
@@ -278,18 +330,6 @@
 	}
 	splx(s);
 
- out:
-	if (atabus_initq == NULL) {
-		simple_lock(&atabus_interlock);
-		while(1) {
-			atabus_initq = TAILQ_FIRST(&atabus_initq_head);
-			if (atabus_initq->atabus_sc == atabus_sc)
-				break;
-			ltsleep(&atabus_initq_head, PRIBIO, "ata_initq", 0,
-			    &atabus_interlock);
-		}
-		simple_unlock(&atabus_interlock);
-	}
 	simple_lock(&atabus_interlock);
 	TAILQ_REMOVE(&atabus_initq_head, atabus_initq, atabus_initq);
 	simple_unlock(&atabus_interlock);
@@ -300,6 +340,7 @@
 	ata_delref(chp);
 
 	config_pending_decr();
+	kthread_exit(0);
 }
 
 /*
@@ -981,6 +1022,8 @@
 	 * If we can poll or wait it's OK, otherwise wake up the
 	 * kernel thread to do it for us.
 	 */
+	ATADEBUG_PRINT(("ata_reset_channel flags 0x%x ch_flags 0x%x\n",
+	    flags, chp->ch_flags), DEBUG_FUNCS | DEBUG_XFERS);
 	if ((flags & (AT_POLL | AT_WAIT)) == 0) {
 		if (chp->ch_flags & ATACH_TH_RESET) {
 			/* No need to schedule a reset more than one time. */