Revert my ata_identify()/ata_reinit() related changes: r189166, r189091

and partially r188903. Revert breaks new drives detection on reinit to the
state as it was before me, but fixes series of new bugs reported by some
people.

Unconditional queueing of ata_completed() calls can lead to deadlock if
due to timeout ata_reinit() was called at the same thread by previous
ata_completed(). Calling of ata_identify() on ata_reinit() in current
implementation opens numerous races and deadlocks.

Problems I was touching here are still exist and should be addresed, but
probably in different way.

9 files changed, 69 insertions, 120 deletions

diff --git a/sys/dev/ata/ata-all.c b/sys/dev/ata/ata-all.c
index 25eefde..4530767 100644
--- a/sys/dev/ata/ata-all.c
+++ b/sys/dev/ata/ata-all.c
@@ -61,6 +61,7 @@ static struct cdevsw ata_cdevsw = {
 
 /* prototypes */
 static void ata_boot_attach(void);
+static device_t ata_add_child(device_t, struct ata_device *, int);
 static void ata_conn_event(void *, int);
 static void bswap(int8_t *, int);
 static void btrim(int8_t *, int);
@@ -179,7 +180,7 @@ ata_detach(device_t dev)
     if (!device_get_children(dev, &children, &nchildren)) {
 	for (i = 0; i < nchildren; i++)
 	    if (children[i])
-		ata_delete_child(dev, children[i]);
+		device_delete_child(dev, children[i]);
 	free(children, M_TEMP);
     } 
     taskqueue_drain(taskqueue_thread, &ch->conntask);
@@ -264,7 +265,7 @@ ata_reinit(device_t dev)
 			    ata_finish(request);
 		    request = NULL;
 		}
-		ata_delete_child(dev, children[i]);
+		device_delete_child(dev, children[i]);
 	    }
 	}
 	free(children, M_TEMP);
@@ -290,7 +291,7 @@ ata_reinit(device_t dev)
     ATA_LOCKING(dev, ATA_LF_UNLOCK);
 
     /* Add new children. */
-    ata_identify(dev);
+/*    ata_identify(dev); */
 
     if (bootverbose)
 	device_printf(dev, "reinit done ..\n");
@@ -589,44 +590,19 @@ ata_boot_attach(void)
 /*
  * misc support functions
  */
-device_t
-ata_add_child(device_t parent, int unit, int atapi)
+static device_t
+ata_add_child(device_t parent, struct ata_device *atadev, int unit)
 {
-    struct ata_device *atadev;
     device_t child;
-    int dev_unit = -1;
 
-#ifdef ATA_STATIC_ID
-    if (!atapi)
-	dev_unit = (device_get_unit(parent) << 1) + unit;
-#endif
-    if ((child = device_add_child(parent, NULL, dev_unit))) {
-	if (!(atadev = malloc(sizeof(struct ata_device),
-			  M_ATA, M_NOWAIT | M_ZERO))) {
-	    device_printf(parent, "out of memory\n");
-	    device_delete_child(parent, child);
-	    return (NULL);
-	}
+    if ((child = device_add_child(parent, NULL, unit))) {
 	device_set_softc(child, atadev);
 	device_quiet(child);
 	atadev->dev = child;
-	atadev->unit = unit;
-	atadev->type = atapi ? ATA_T_ATAPI : ATA_T_ATA;
 	atadev->max_iosize = DEV_BSIZE;
 	atadev->mode = ATA_PIO_MAX;
     }
-    return (child);
-}
-
-int
-ata_delete_child(device_t parent, device_t child)
-{
-    struct ata_device *atadev = device_get_softc(child);
-    int res;
-
-    res = device_delete_child(parent, child);
-    free(atadev, M_ATA);
-    return (res);
+    return child;
 }
 
 int
@@ -637,11 +613,11 @@ ata_getparam(struct ata_device *atadev, int init)
     u_int8_t command = 0;
     int error = ENOMEM, retries = 2;
 
-    if (atadev->type == ATA_T_ATA)
+    if (ch->devices & (ATA_ATA_MASTER << atadev->unit))
 	command = ATA_ATA_IDENTIFY;
-    else if (atadev->type == ATA_T_ATAPI)
+    if (ch->devices & (ATA_ATAPI_MASTER << atadev->unit))
 	command = ATA_ATAPI_IDENTIFY;
-    else
+    if (!command)
 	return ENXIO;
 
     while (retries-- > 0 && error) {
@@ -651,7 +627,7 @@ ata_getparam(struct ata_device *atadev, int init)
 	request->timeout = 1;
 	request->retries = 0;
 	request->u.ata.command = command;
-	request->flags = (ATA_R_READ|ATA_R_AT_HEAD|ATA_R_THREAD);
+	request->flags = (ATA_R_READ|ATA_R_AT_HEAD|ATA_R_DIRECT);
 	if (!bootverbose)
 	    request->flags |= ATA_R_QUIET;
 	request->data = (void *)&atadev->param;
@@ -687,18 +663,17 @@ ata_getparam(struct ata_device *atadev, int init)
 	btrim(atacap->serial, sizeof(atacap->serial));
 	bpack(atacap->serial, atacap->serial, sizeof(atacap->serial));
 
-	if (init) {
-	    char buffer[64];
-
-	    if (bootverbose) {
-		printf("ata%d-%s: pio=%s wdma=%s udma=%s cable=%s wire\n",
+	if (bootverbose)
+	    printf("ata%d-%s: pio=%s wdma=%s udma=%s cable=%s wire\n",
 		   device_get_unit(ch->dev),
 		   ata_unit2str(atadev),
 		   ata_mode2str(ata_pmode(atacap)),
 		   ata_mode2str(ata_wmode(atacap)),
 		   ata_mode2str(ata_umode(atacap)),
 		   (atacap->hwres & ATA_CABLE_ID) ? "80":"40");
-	    }
+
+	if (init) {
+	    char buffer[64];
 
 	    sprintf(buffer, "%.40s/%.8s", atacap->model, atacap->revision);
 	    device_set_desc_copy(atadev->dev, buffer);
@@ -731,6 +706,7 @@ ata_identify(device_t dev)
     struct ata_channel *ch = device_get_softc(dev);
     struct ata_device *atadev;
     device_t *children;
+    device_t child;
     int nchildren, i, n = ch->devices;
 
     if (bootverbose)
@@ -745,18 +721,37 @@ ata_identify(device_t dev)
 	}
 	free(children, M_TEMP);
     }
+    /* Create new devices. */
     if (bootverbose)
 	device_printf(dev, "New devices: %08x\n", n);
     if (n == 0) {
 	mtx_unlock(&Giant);
 	return (0);
     }
-    /* Create new devices. */
     for (i = 0; i < ATA_PM; ++i) {
-	if (n & (((ATA_ATA_MASTER | ATA_ATAPI_MASTER) << i)))
-	    ata_add_child(dev, i, n & (ATA_ATAPI_MASTER << i));
-    }
+	if (n & (((ATA_ATA_MASTER | ATA_ATAPI_MASTER) << i))) {
+	    int unit = -1;
 
+	    if (!(atadev = malloc(sizeof(struct ata_device),
+				  M_ATA, M_NOWAIT | M_ZERO))) {
+		device_printf(dev, "out of memory\n");
+		return ENOMEM;
+	    }
+	    atadev->unit = i;
+#ifdef ATA_STATIC_ID
+	    if (n & (ATA_ATA_MASTER << i))
+		unit = (device_get_unit(dev) << 1) + i;
+#endif
+	    if ((child = ata_add_child(dev, atadev, unit))) {
+		if (ata_getparam(atadev, 1)) {
+		    device_delete_child(dev, child);
+		    free(atadev, M_ATA);
+		}
+	    }
+	    else
+		free(atadev, M_ATA);
+	}
+    }
     bus_generic_probe(dev);
     bus_generic_attach(dev);
     mtx_unlock(&Giant);
diff --git a/sys/dev/ata/ata-all.h b/sys/dev/ata/ata-all.h
index 8c7f0d1..a721d80 100644
--- a/sys/dev/ata/ata-all.h
+++ b/sys/dev/ata/ata-all.h
@@ -367,6 +367,7 @@ struct ata_request {
 #define         ATA_R_AT_HEAD           0x00000200
 #define         ATA_R_REQUEUE           0x00000400
 #define         ATA_R_THREAD            0x00000800
+#define         ATA_R_DIRECT            0x00001000
 
 #define         ATA_R_DEBUG             0x10000000
 #define         ATA_R_DANGER1           0x20000000
@@ -410,10 +411,6 @@ struct ata_device {
 #define         ATA_MASTER              0x00
 #define         ATA_SLAVE               0x01
 #define         ATA_PM                  0x0f
-    int                         type;           /* device type */
-#define         ATA_T_ATA               0x00
-#define         ATA_T_ATAPI             0x01
-#define         ATA_T_ATAPI_CAM         0x02
 
     struct ata_params           param;          /* ata param structure */
     int                         mode;           /* current transfermode */
@@ -426,8 +423,6 @@ struct ata_device {
 #define         ATA_D_MEDIA_CHANGED     0x0002
 #define         ATA_D_ENC_PRESENT       0x0004
 #define         ATA_D_48BIT_ACTIVE      0x0008
-#define         ATA_D_PROBED            0x0010
-#define         ATA_D_VALID             0x0020
 };
 
 /* structure for holding DMA Physical Region Descriptors (PRD) entries */
@@ -565,8 +560,6 @@ void ata_interrupt(void *data);
 int ata_device_ioctl(device_t dev, u_long cmd, caddr_t data);
 int ata_getparam(struct ata_device *atadev, int init);
 int ata_identify(device_t dev);
-device_t ata_add_child(device_t, int, int);
-int ata_delete_child(device_t , device_t);
 void ata_default_registers(device_t dev);
 void ata_modify_if_48bit(struct ata_request *request);
 void ata_udelay(int interval);
diff --git a/sys/dev/ata/ata-disk.c b/sys/dev/ata/ata-disk.c
index a1280ac..e11f3f9 100644
--- a/sys/dev/ata/ata-disk.c
+++ b/sys/dev/ata/ata-disk.c
@@ -79,18 +79,6 @@ ad_probe(device_t dev)
 {
     struct ata_device *atadev = device_get_softc(dev);
 
-    if (atadev->type != ATA_T_ATA)
-	return (ENXIO);
-
-    if (!(atadev->flags & ATA_D_PROBED)) {
-	atadev->flags |= ATA_D_PROBED;
-	if (ata_getparam(atadev, 1) == 0)
-	    atadev->flags |= ATA_D_VALID;
-    }
-
-    if (!(atadev->flags & ATA_D_VALID))
-	return (ENXIO);
-
     if (!(atadev->param.config & ATA_PROTO_ATAPI) ||
 	(atadev->param.config == ATA_CFA_MAGIC1) ||
 	(atadev->param.config == ATA_CFA_MAGIC2) ||
diff --git a/sys/dev/ata/ata-queue.c b/sys/dev/ata/ata-queue.c
index b438467..707dfe9 100644
--- a/sys/dev/ata/ata-queue.c
+++ b/sys/dev/ata/ata-queue.c
@@ -237,8 +237,14 @@ ata_start(device_t dev)
 void
 ata_finish(struct ata_request *request)
 {
+    struct ata_channel *ch = device_get_softc(request->parent);
 
-    if (dumping) {
+    /*
+     * if in ATA_STALL_QUEUE state or request has ATA_R_DIRECT flags set
+     * we need to call ata_complete() directly here (no taskqueue involvement)
+     */
+    if (dumping ||
+	(ch->state & ATA_STALL_QUEUE) || (request->flags & ATA_R_DIRECT)) {
 	ATA_DEBUG_RQ(request, "finish directly");
 	ata_completed(request, 0);
     }
diff --git a/sys/dev/ata/ata-raid.c b/sys/dev/ata/ata-raid.c
index 2dea733..fc6a23c 100644
--- a/sys/dev/ata/ata-raid.c
+++ b/sys/dev/ata/ata-raid.c
@@ -275,7 +275,7 @@ ata_raid_flush(struct bio *bp)
 	request->u.ata.feature = 0;
 	request->timeout = 1;
 	request->retries = 0;
-	request->flags |= ATA_R_ORDERED | ATA_R_THREAD;
+	request->flags |= ATA_R_ORDERED | ATA_R_DIRECT;
 	ata_queue_request(request);
     }
     return 0;
@@ -1570,7 +1570,7 @@ ata_raid_wipe_metadata(struct ar_softc *rdp)
 	    if (!(meta = malloc(size, M_AR, M_NOWAIT | M_ZERO)))
 		return ENOMEM;
 	    if (ata_raid_rw(rdp->disks[disk].dev, lba, meta, size,
-			    ATA_R_WRITE | ATA_R_THREAD)) {
+			    ATA_R_WRITE | ATA_R_DIRECT)) {
 		device_printf(rdp->disks[disk].dev, "wipe metadata failed\n");
 		error = EIO;
 	    }
@@ -2264,7 +2264,7 @@ ata_raid_hptv2_write_meta(struct ar_softc *rdp)
 	    if (ata_raid_rw(rdp->disks[disk].dev,
 			    HPTV2_LBA(rdp->disks[disk].dev), meta,
 			    sizeof(struct promise_raid_conf),
-			    ATA_R_WRITE | ATA_R_THREAD)) {
+			    ATA_R_WRITE | ATA_R_DIRECT)) {
 		device_printf(rdp->disks[disk].dev, "write metadata failed\n");
 		error = EIO;
 	    }
@@ -2710,7 +2710,7 @@ ata_raid_intel_write_meta(struct ar_softc *rdp)
 	if (rdp->disks[disk].dev) {
 	    if (ata_raid_rw(rdp->disks[disk].dev,
 			    INTEL_LBA(rdp->disks[disk].dev),
-			    meta, 1024, ATA_R_WRITE | ATA_R_THREAD)) {
+			    meta, 1024, ATA_R_WRITE | ATA_R_DIRECT)) {
 		device_printf(rdp->disks[disk].dev, "write metadata failed\n");
 		error = EIO;
 	    }
@@ -3055,7 +3055,7 @@ ata_raid_jmicron_write_meta(struct ar_softc *rdp)
 	    if (ata_raid_rw(rdp->disks[disk].dev,
 			    JMICRON_LBA(rdp->disks[disk].dev),
 			    meta, sizeof(struct jmicron_raid_conf),
-			    ATA_R_WRITE | ATA_R_THREAD)) {
+			    ATA_R_WRITE | ATA_R_DIRECT)) {
 		device_printf(rdp->disks[disk].dev, "write metadata failed\n");
 		error = EIO;
 	    }
@@ -3778,7 +3778,7 @@ ata_raid_promise_write_meta(struct ar_softc *rdp)
 	    if (ata_raid_rw(rdp->disks[disk].dev,
 			    PROMISE_LBA(rdp->disks[disk].dev),
 			    meta, sizeof(struct promise_raid_conf),
-			    ATA_R_WRITE | ATA_R_THREAD)) {
+			    ATA_R_WRITE | ATA_R_DIRECT)) {
 		device_printf(rdp->disks[disk].dev, "write metadata failed\n");
 		error = EIO;
 	    }
@@ -4126,7 +4126,7 @@ ata_raid_sis_write_meta(struct ar_softc *rdp)
 	    if (ata_raid_rw(rdp->disks[disk].dev,
 			    SIS_LBA(rdp->disks[disk].dev),
 			    meta, sizeof(struct sis_raid_conf),
-			    ATA_R_WRITE | ATA_R_THREAD)) {
+			    ATA_R_WRITE | ATA_R_DIRECT)) {
 		device_printf(rdp->disks[disk].dev, "write metadata failed\n");
 		error = EIO;
 	    }
@@ -4351,7 +4351,7 @@ ata_raid_via_write_meta(struct ar_softc *rdp)
 	    if (ata_raid_rw(rdp->disks[disk].dev,
 			    VIA_LBA(rdp->disks[disk].dev),
 			    meta, sizeof(struct via_raid_conf),
-			    ATA_R_WRITE | ATA_R_THREAD)) {
+			    ATA_R_WRITE | ATA_R_DIRECT)) {
 		device_printf(rdp->disks[disk].dev, "write metadata failed\n");
 		error = EIO;
 	    }
diff --git a/sys/dev/ata/atapi-cam.c b/sys/dev/ata/atapi-cam.c
index 5fb729d..62e5251 100644
--- a/sys/dev/ata/atapi-cam.c
+++ b/sys/dev/ata/atapi-cam.c
@@ -147,7 +147,7 @@ static void
 atapi_cam_identify(driver_t *driver, device_t parent)
 {
 	struct atapi_xpt_softc *scp =
-	    malloc (sizeof (struct atapi_xpt_softc), M_ATA, M_NOWAIT|M_ZERO);
+	    malloc (sizeof (struct atapi_xpt_softc), M_ATACAM, M_NOWAIT|M_ZERO);
 	device_t child;
 
 	if (scp == NULL) {
@@ -159,11 +159,10 @@ atapi_cam_identify(driver_t *driver, device_t parent)
 	child = device_add_child(parent, "atapicam", -1);
 	if (child == NULL) {
 		printf ("atapi_cam_identify: out of memory, can't add child");
-		free (scp, M_ATA);
+		free (scp, M_ATACAM);
 		return;
 	}
 	scp->atapi_cam_dev.unit = -1;
-	scp->atapi_cam_dev.type = ATA_T_ATAPI_CAM;
 	scp->atapi_cam_dev.dev  = child;
 	device_quiet(child);
 	device_set_softc(child, scp);
@@ -175,10 +174,12 @@ atapi_cam_probe(device_t dev)
 	struct ata_device *atadev = device_get_softc (dev);
 
 	KASSERT(atadev != NULL, ("expect valid struct ata_device"));
-	if (atadev->type != ATA_T_ATAPI_CAM)
-		return (ENXIO);
-	device_set_desc(dev, "ATAPI CAM Attachment");
-	return (0);
+	if (atadev->unit < 0) {
+		device_set_desc(dev, "ATAPI CAM Attachment");
+		return (0);
+	} else {
+		return ENXIO;
+	}
 }
 
 static int
@@ -924,8 +925,11 @@ atapi_cam_event_handler(module_t mod, int what, void *arg) {
 	    if (devlist != NULL) {
 		while (devlist != NULL && devcount > 0) {
 		    device_t child = devlist[--devcount];
+		    struct atapi_xpt_softc *scp = device_get_softc(child);
 
-		    ata_delete_child(device_get_parent(child),child);
+		    device_delete_child(device_get_parent(child),child);
+		    if (scp != NULL)
+			free(scp, M_ATACAM);
 		}
 		free(devlist, M_TEMP);
 	    }
diff --git a/sys/dev/ata/atapi-cd.c b/sys/dev/ata/atapi-cd.c
index ba0c7c8..7fd80c4 100644
--- a/sys/dev/ata/atapi-cd.c
+++ b/sys/dev/ata/atapi-cd.c
@@ -108,18 +108,6 @@ acd_probe(device_t dev)
 {
     struct ata_device *atadev = device_get_softc(dev);
 
-    if (atadev->type != ATA_T_ATAPI)
-	return (ENXIO);
-
-    if (!(atadev->flags & ATA_D_PROBED)) {
-	atadev->flags |= ATA_D_PROBED;
-	if (ata_getparam(atadev, 1) == 0)
-	    atadev->flags |= ATA_D_VALID;
-    }
-
-    if (!(atadev->flags & ATA_D_VALID))
-	return (ENXIO);
-
     if ((atadev->param.config & ATA_PROTO_ATAPI) &&
 	(atadev->param.config & ATA_ATAPI_TYPE_MASK) == ATA_ATAPI_TYPE_CDROM)
 	return 0;
diff --git a/sys/dev/ata/atapi-fd.c b/sys/dev/ata/atapi-fd.c
index f8926bd..b8434b3 100644
--- a/sys/dev/ata/atapi-fd.c
+++ b/sys/dev/ata/atapi-fd.c
@@ -66,19 +66,6 @@ static int
 afd_probe(device_t dev)
 {
     struct ata_device *atadev = device_get_softc(dev);
-
-    if (atadev->type != ATA_T_ATAPI)
-	return (ENXIO);
-
-    if (!(atadev->flags & ATA_D_PROBED)) {
-	atadev->flags |= ATA_D_PROBED;
-	if (ata_getparam(atadev, 1) == 0)
-	    atadev->flags |= ATA_D_VALID;
-    }
-
-    if (!(atadev->flags & ATA_D_VALID))
-	return (ENXIO);
-
     if ((atadev->param.config & ATA_PROTO_ATAPI) &&
 	(atadev->param.config & ATA_ATAPI_TYPE_MASK) == ATA_ATAPI_TYPE_DIRECT)
 	return 0;  
diff --git a/sys/dev/ata/atapi-tape.c b/sys/dev/ata/atapi-tape.c
index 1dfdd5c..aa44afe 100644
--- a/sys/dev/ata/atapi-tape.c
+++ b/sys/dev/ata/atapi-tape.c
@@ -90,18 +90,6 @@ ast_probe(device_t dev)
 {
     struct ata_device *atadev = device_get_softc(dev);
 
-    if (atadev->type != ATA_T_ATAPI)
-	return (ENXIO);
-
-    if (!(atadev->flags & ATA_D_PROBED)) {
-	atadev->flags |= ATA_D_PROBED;
-	if (ata_getparam(atadev, 1) == 0)
-	    atadev->flags |= ATA_D_VALID;
-    }
-
-    if (!(atadev->flags & ATA_D_VALID))
-	return (ENXIO);
-
     if ((atadev->param.config & ATA_PROTO_ATAPI) &&
 	(atadev->param.config & ATA_ATAPI_TYPE_MASK) == ATA_ATAPI_TYPE_TAPE)
 	return 0;