Module Name:    src
Committed By:   jdolecek
Date:           Tue Jun 27 18:36:04 UTC 2017

Modified Files:
        src/sys/dev/ata [jdolecek-ncq]: ata.c ata_wdc.c atavar.h
        src/sys/dev/ic [jdolecek-ncq]: ahcisata_core.c mvsata.c siisata.c wdc.c
        src/sys/dev/scsipi [jdolecek-ncq]: atapi_wdc.c

Log Message:
attend error paths, more strict asserts and code consistency

- atastart() and ata_kill_pending() now KASSERT() that all xfers on queue
  have same channel
- inactive xfers are killed via new reason KILL_GONE_INACTIVE, controller
  code must not call any resource deactivation in that case
- c_intr() must call ata_waitdrain_xfer_check() as first thing, and must not
  further touch any xfer structures on exit path; any resource cleanup
  is supposed to be done in c_kill_xfer()
- c_kill_xfer() should never call atastart()
- ata_waitdrain_check() removed, replaced by ata_waitdrain_xfer_check()
- ATA_DRIVE_WAITDRAIN handling converted to use condvar
- removed unused ata_c callback


To generate a diff of this commit:
cvs rdiff -u -r1.132.8.17 -r1.132.8.18 src/sys/dev/ata/ata.c
cvs rdiff -u -r1.105.6.5 -r1.105.6.6 src/sys/dev/ata/ata_wdc.c
cvs rdiff -u -r1.92.8.15 -r1.92.8.16 src/sys/dev/ata/atavar.h
cvs rdiff -u -r1.57.6.16 -r1.57.6.17 src/sys/dev/ic/ahcisata_core.c
cvs rdiff -u -r1.35.6.16 -r1.35.6.17 src/sys/dev/ic/mvsata.c
cvs rdiff -u -r1.30.4.23 -r1.30.4.24 src/sys/dev/ic/siisata.c
cvs rdiff -u -r1.283.2.9 -r1.283.2.10 src/sys/dev/ic/wdc.c
cvs rdiff -u -r1.123.4.8 -r1.123.4.9 src/sys/dev/scsipi/atapi_wdc.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/ata/ata.c
diff -u src/sys/dev/ata/ata.c:1.132.8.17 src/sys/dev/ata/ata.c:1.132.8.18
--- src/sys/dev/ata/ata.c:1.132.8.17	Sat Jun 24 14:57:17 2017
+++ src/sys/dev/ata/ata.c	Tue Jun 27 18:36:03 2017
@@ -1,4 +1,4 @@
-/*	$NetBSD: ata.c,v 1.132.8.17 2017/06/24 14:57:17 jdolecek Exp $	*/
+/*	$NetBSD: ata.c,v 1.132.8.18 2017/06/27 18:36:03 jdolecek Exp $	*/
 
 /*
  * Copyright (c) 1998, 2001 Manuel Bouyer.  All rights reserved.
@@ -25,7 +25,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ata.c,v 1.132.8.17 2017/06/24 14:57:17 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ata.c,v 1.132.8.18 2017/06/27 18:36:03 jdolecek Exp $");
 
 #include "opt_ata.h"
 
@@ -275,6 +275,7 @@ ata_queue_alloc(uint8_t openings)
 	ata_queue_reset(chq);
 
 	cv_init(&chq->queue_busy, "ataqbusy");
+	cv_init(&chq->queue_drain, "atdrn");
 
 	for (uint8_t i = 0; i < openings; i++)
 		ata_xfer_init(&chq->queue_xfers[i], false);
@@ -288,6 +289,9 @@ ata_queue_free(struct ata_queue *chq)
 	for (uint8_t i = 0; i < chq->queue_openings; i++)
 		ata_xfer_destroy(&chq->queue_xfers[i]);
 
+	cv_destroy(&chq->queue_busy);
+	cv_destroy(&chq->queue_drain);
+
 	free(chq, M_DEVBUF);
 }
 
@@ -1140,6 +1144,9 @@ atastart(struct ata_channel *chp)
 	if ((xfer = TAILQ_FIRST(&chp->ch_queue->queue_xfer)) == NULL)
 		goto out;
 
+	/* all xfers on same queue must belong to the same channel */
+	KASSERT(xfer->c_chp == chp);
+
 	/*
 	 * Can only take NCQ command if there are no current active
 	 * commands, or if the active commands are NCQ. Need only check
@@ -1149,10 +1156,6 @@ atastart(struct ata_channel *chp)
 	if (axfer && (axfer->c_flags & C_NCQ) == 0)
 		goto out;
 
-	/* adjust chp, in case we have a shared queue */
-	chp = xfer->c_chp;
-	KASSERT(chp->ch_queue == chq);
-
 	struct ata_drive_datas * const drvp = &chp->ch_drive[xfer->c_drive];
 
 	/*
@@ -1322,29 +1325,37 @@ ata_deactivate_xfer(struct ata_channel *
 	mutex_exit(&chp->ch_lock);
 }
 
-
-bool
-ata_waitdrain_check(struct ata_channel *chp, int drive)
-{
-	if (chp->ch_drive[drive].drive_flags & ATA_DRIVE_WAITDRAIN) {
-		chp->ch_drive[drive].drive_flags &= ~ATA_DRIVE_WAITDRAIN;
-		wakeup(&chp->ch_queue->active_xfers);
-		return true;
-	}
-	return false;
-}
-
+/*
+ * Called in c_intr hook. Must be called before before any deactivations
+ * are done - if there is drain pending, it calls c_kill_xfer hook which
+ * deactivates the xfer.
+ * Calls c_kill_xfer with channel lock free.
+ * Returns true if caller should just exit without further processing.
+ * Caller must not further access any part of xfer or any related controller
+ * structures in that case, it should just return.
+ */
 bool
 ata_waitdrain_xfer_check(struct ata_channel *chp, struct ata_xfer *xfer)
 {
 	int drive = xfer->c_drive;
+	bool draining = false;
+
+	mutex_enter(&chp->ch_lock);
+
 	if (chp->ch_drive[drive].drive_flags & ATA_DRIVE_WAITDRAIN) {
+		mutex_exit(&chp->ch_lock);
+
 		(*xfer->c_kill_xfer)(chp, xfer, KILL_GONE);
+
+		mutex_enter(&chp->ch_lock);
 		chp->ch_drive[drive].drive_flags &= ~ATA_DRIVE_WAITDRAIN;
-		wakeup(&chp->ch_queue->active_xfers);
-		return true;
+		cv_signal(&chp->ch_queue->queue_drain);
+		draining = true;
 	}
-	return false;
+
+	mutex_exit(&chp->ch_lock);
+
+	return draining;
 }
 
 /*
@@ -1367,38 +1378,58 @@ ata_kill_active(struct ata_channel *chp,
 }
 
 /*
- * Kill off all pending xfers for a ata_channel.
- *
- * Must be called at splbio().
+ * Kill off all pending xfers for a drive.
  */
 void
 ata_kill_pending(struct ata_drive_datas *drvp)
 {
 	struct ata_channel * const chp = drvp->chnl_softc;
 	struct ata_queue * const chq = chp->ch_queue;
-	struct ata_xfer *xfer, *next_xfer;
-	int s = splbio();
+	struct ata_xfer *xfer, *xfernext;
+
+	mutex_enter(&chp->ch_lock);
+
+	/* Kill all pending transfers */
+	TAILQ_FOREACH_SAFE(xfer, &chq->queue_xfer, c_xferchain, xfernext) {
+		KASSERT(xfer->c_chp == chp);
 
-	for (xfer = TAILQ_FIRST(&chq->queue_xfer);
-	    xfer != NULL; xfer = next_xfer) {
-		next_xfer = TAILQ_NEXT(xfer, c_xferchain);
-		if (xfer->c_chp != chp || xfer->c_drive != drvp->drive)
+		if (xfer->c_drive != drvp->drive)
 			continue;
-		TAILQ_REMOVE(&chq->queue_xfer, xfer, c_xferchain);
-		(*xfer->c_kill_xfer)(chp, xfer, KILL_GONE);
+
+		TAILQ_REMOVE(&chp->ch_queue->queue_xfer, xfer, c_xferchain);
+
+		/*
+		 * Keep the lock, so that we get deadlock (and 'locking against
+		 * myself' with LOCKDEBUG), instead of silent
+		 * data corruption, if the hook tries to call back into
+		 * middle layer for inactive xfer.
+		 */
+		(*xfer->c_kill_xfer)(chp, xfer, KILL_GONE_INACTIVE);
 	}
 
+	/* Wait until all active transfers on the drive finish */
 	while (chq->queue_active > 0) {
-		if (chq->queue_openings == 1 && chp->ch_ndrives > 1) {
-			xfer = TAILQ_FIRST(&chq->active_xfers);
-			KASSERT(xfer != NULL);
-			if (xfer->c_chp != chp || xfer->c_drive != drvp->drive)
+		bool drv_active = false;
+
+		TAILQ_FOREACH(xfer, &chq->active_xfers, c_activechain) {
+			KASSERT(xfer->c_chp == chp);
+
+			if (xfer->c_drive == drvp->drive) {
+				drv_active = true;
 				break;
+			}
+		}
+		
+		if (!drv_active) {
+			/* all finished */
+			break;
 		}
+
 		drvp->drive_flags |= ATA_DRIVE_WAITDRAIN;
-		(void) tsleep(&chq->active_xfers, PRIBIO, "atdrn", 0);
+		cv_wait(&chq->queue_drain, &chp->ch_lock);
 	}
-	splx(s);
+
+	mutex_exit(&chp->ch_lock);
 }
 
 void
@@ -2152,9 +2183,11 @@ atacmd_toncq(struct ata_xfer *xfer, uint
 void
 ata_channel_start(struct ata_channel *chp, int drive)
 {
-	int i;
+	int i, s;
 	struct ata_drive_datas *drvp;
 
+	s = splbio();
+
 	KASSERT(chp->ch_ndrives > 0);
 
 #define ATA_DRIVE_START(chp, drive) \
@@ -2186,5 +2219,6 @@ ata_channel_start(struct ata_channel *ch
 	/* Now try to kick off xfers on the current drive */
 	ATA_DRIVE_START(chp, drive);
 
+	splx(s);
 #undef ATA_DRIVE_START
 }

Index: src/sys/dev/ata/ata_wdc.c
diff -u src/sys/dev/ata/ata_wdc.c:1.105.6.5 src/sys/dev/ata/ata_wdc.c:1.105.6.6
--- src/sys/dev/ata/ata_wdc.c:1.105.6.5	Tue Jun 20 20:58:22 2017
+++ src/sys/dev/ata/ata_wdc.c	Tue Jun 27 18:36:03 2017
@@ -1,4 +1,4 @@
-/*	$NetBSD: ata_wdc.c,v 1.105.6.5 2017/06/20 20:58:22 jdolecek Exp $	*/
+/*	$NetBSD: ata_wdc.c,v 1.105.6.6 2017/06/27 18:36:03 jdolecek Exp $	*/
 
 /*
  * Copyright (c) 1998, 2001, 2003 Manuel Bouyer.
@@ -54,7 +54,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ata_wdc.c,v 1.105.6.5 2017/06/20 20:58:22 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ata_wdc.c,v 1.105.6.6 2017/06/27 18:36:03 jdolecek Exp $");
 
 #include "opt_ata.h"
 #include "opt_wdc.h"
@@ -774,11 +774,13 @@ wdc_ata_bio_kill_xfer(struct ata_channel
 {
 	struct ata_bio *ata_bio = &xfer->c_bio;
 	int drive = xfer->c_drive;
-
-	ata_deactivate_xfer(chp, xfer);
+	bool deactivate = true;
 
 	ata_bio->flags |= ATA_ITSDONE;
 	switch (reason) {
+	case KILL_GONE_INACTIVE:
+		deactivate = false;
+		/* FALLTHROUGH */
 	case KILL_GONE:
 		ata_bio->error = ERR_NODEV;
 		break;
@@ -791,6 +793,10 @@ wdc_ata_bio_kill_xfer(struct ata_channel
 		panic("wdc_ata_bio_kill_xfer");
 	}
 	ata_bio->r_error = WDCE_ABRT;
+
+	if (deactivate)
+		ata_deactivate_xfer(chp, xfer);
+
 	ATADEBUG_PRINT(("wdc_ata_bio_kill_xfer: drv_done\n"), DEBUG_XFERS);
 	(*chp->ch_drive[drive].drv_done)(chp->ch_drive[drive].drv_softc, xfer);
 }
@@ -806,7 +812,8 @@ wdc_ata_bio_done(struct ata_channel *chp
 	    xfer->c_drive, (u_int)xfer->c_flags),
 	    DEBUG_XFERS);
 
-	callout_stop(&xfer->c_timo_callout);
+	if (ata_waitdrain_xfer_check(chp, xfer))
+		return;
 
 	/* feed back residual bcount to our caller */
 	ata_bio->bcount = xfer->c_bcount;
@@ -814,9 +821,6 @@ wdc_ata_bio_done(struct ata_channel *chp
 	/* mark controller inactive and free xfer */
 	ata_deactivate_xfer(chp, xfer);
 
-	if (ata_waitdrain_check(chp, drive)) {
-		ata_bio->error = ERR_NODEV;
-	}
 	ata_bio->flags |= ATA_ITSDONE;
 	ATADEBUG_PRINT(("wdc_ata_done: drv_done\n"), DEBUG_XFERS);
 	(*chp->ch_drive[drive].drv_done)(chp->ch_drive[drive].drv_softc, xfer);

Index: src/sys/dev/ata/atavar.h
diff -u src/sys/dev/ata/atavar.h:1.92.8.15 src/sys/dev/ata/atavar.h:1.92.8.16
--- src/sys/dev/ata/atavar.h:1.92.8.15	Fri Jun 23 20:40:51 2017
+++ src/sys/dev/ata/atavar.h	Tue Jun 27 18:36:03 2017
@@ -1,4 +1,4 @@
-/*	$NetBSD: atavar.h,v 1.92.8.15 2017/06/23 20:40:51 jdolecek Exp $	*/
+/*	$NetBSD: atavar.h,v 1.92.8.16 2017/06/27 18:36:03 jdolecek Exp $	*/
 
 /*
  * Copyright (c) 1998, 2001 Manuel Bouyer.
@@ -121,8 +121,6 @@ struct ata_command {
 	int timeout;		/* timeout (in ms) */
 	void *data;		/* Data buffer address */
 	int bcount;		/* number of bytes to transfer */
-	void (*callback)(void *); /* command to call once command completed */
-	void *callback_arg;	/* argument passed to *callback() */
 };
 
 /* Forward declaration for ata_xfer */
@@ -181,8 +179,9 @@ struct ata_xfer {
 #define	C_NCQ		0x0100		/* command is queued  */
 
 /* reasons for c_kill_xfer() */
-#define KILL_GONE 1 /* device is gone */
-#define KILL_RESET 2 /* xfer was reset */
+#define KILL_GONE 1		/* device is gone while xfer was active */
+#define KILL_RESET 2		/* xfer was reset */
+#define KILL_GONE_INACTIVE 3	/* device is gone while xfer was pending */
 
 /*
  * While hw supports up to 32 tags, in practice we must never
@@ -204,6 +203,7 @@ struct ata_queue {
 	TAILQ_HEAD(, ata_xfer) queue_xfer; 	/* queue of pending commands */
 	int queue_freeze; 			/* freeze count for the queue */
 	kcondvar_t queue_busy;			/* c: waiting of xfer */
+	kcondvar_t queue_drain;			/* c: waiting of queue drain */
 	TAILQ_HEAD(, ata_xfer) active_xfers; 	/* active commands */
 	uint32_t active_xfers_used;		/* mask of active commands */
 	uint32_t queue_xfers_avail;		/* available xfers mask */
@@ -524,7 +524,6 @@ struct ata_xfer *
 
 void	ata_delay(int, const char *, int);
 
-bool	ata_waitdrain_check(struct ata_channel *, int);
 bool	ata_waitdrain_xfer_check(struct ata_channel *, struct ata_xfer *);
 
 void	atacmd_toncq(struct ata_xfer *, uint8_t *, uint16_t *, uint16_t *,

Index: src/sys/dev/ic/ahcisata_core.c
diff -u src/sys/dev/ic/ahcisata_core.c:1.57.6.16 src/sys/dev/ic/ahcisata_core.c:1.57.6.17
--- src/sys/dev/ic/ahcisata_core.c:1.57.6.16	Wed Jun 21 19:38:42 2017
+++ src/sys/dev/ic/ahcisata_core.c	Tue Jun 27 18:36:03 2017
@@ -1,4 +1,4 @@
-/*	$NetBSD: ahcisata_core.c,v 1.57.6.16 2017/06/21 19:38:42 jdolecek Exp $	*/
+/*	$NetBSD: ahcisata_core.c,v 1.57.6.17 2017/06/27 18:36:03 jdolecek Exp $	*/
 
 /*
  * Copyright (c) 2006 Manuel Bouyer.
@@ -26,7 +26,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ahcisata_core.c,v 1.57.6.16 2017/06/21 19:38:42 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ahcisata_core.c,v 1.57.6.17 2017/06/27 18:36:03 jdolecek Exp $");
 
 #include <sys/types.h>
 #include <sys/malloc.h>
@@ -69,6 +69,7 @@ static void ahci_killpending(struct ata_
 static void ahci_cmd_start(struct ata_channel *, struct ata_xfer *);
 static int  ahci_cmd_complete(struct ata_channel *, struct ata_xfer *, int);
 static void ahci_cmd_done(struct ata_channel *, struct ata_xfer *);
+static void ahci_cmd_done_end(struct ata_channel *, struct ata_xfer *);
 static void ahci_cmd_kill_xfer(struct ata_channel *, struct ata_xfer *, int);
 static void ahci_bio_start(struct ata_channel *, struct ata_xfer *);
 static int  ahci_bio_complete(struct ata_channel *, struct ata_xfer *, int);
@@ -1045,13 +1046,17 @@ ahci_cmd_start(struct ata_channel *chp, 
 static void
 ahci_cmd_kill_xfer(struct ata_channel *chp, struct ata_xfer *xfer, int reason)
 {
+	struct ahci_channel *achp = (struct ahci_channel *)chp;
 	struct ata_command *ata_c = &xfer->c_ata_c;
+	bool deactivate = true;
+
 	AHCIDEBUG_PRINT(("ahci_cmd_kill_xfer channel %d\n", chp->ch_channel),
 	    DEBUG_FUNCS);
 
-	ata_deactivate_xfer(chp, xfer);
-
 	switch (reason) {
+	case KILL_GONE_INACTIVE:
+		deactivate = false;
+		/* FALLTHROUGH */
 	case KILL_GONE:
 		ata_c->flags |= AT_GONE;
 		break;
@@ -1062,7 +1067,13 @@ ahci_cmd_kill_xfer(struct ata_channel *c
 		printf("ahci_cmd_kill_xfer: unknown reason %d\n", reason);
 		panic("ahci_cmd_kill_xfer");
 	}
-	ahci_cmd_done(chp, xfer);
+
+	if (deactivate) {
+		achp->ahcic_cmds_active &= ~(1 << xfer->c_slot);
+		ata_deactivate_xfer(chp, xfer);
+	}
+
+	ahci_cmd_done_end(chp, xfer);
 }
 
 static int
@@ -1076,15 +1087,16 @@ ahci_cmd_complete(struct ata_channel *ch
 	    chp->ch_channel, AHCI_READ(sc, AHCI_P_CMD(chp->ch_channel)),
 	    AHCI_READ(sc, AHCI_P_CI(chp->ch_channel))),
 	    DEBUG_FUNCS);
-	chp->ch_flags &= ~ATACH_IRQ_WAIT;
-	if (xfer->c_flags & C_TIMEOU) {
-		ata_c->flags |= AT_TIMEOU;
-	}
 
+	if (ata_waitdrain_xfer_check(chp, xfer))
+		return 0;
+
+	achp->ahcic_cmds_active &= ~(1 << xfer->c_slot);
 	ata_deactivate_xfer(chp, xfer);
 
-	if (ata_waitdrain_xfer_check(chp, xfer)) {
-		return 0;
+	chp->ch_flags &= ~ATACH_IRQ_WAIT;
+	if (xfer->c_flags & C_TIMEOU) {
+		ata_c->flags |= AT_TIMEOU;
 	}
 
 	if (chp->ch_status & WDCS_BSY) {
@@ -1113,9 +1125,6 @@ ahci_cmd_done(struct ata_channel *chp, s
 	AHCIDEBUG_PRINT(("ahci_cmd_done channel %d (status %#x) flags %#x/%#x\n",
 	    chp->ch_channel, chp->ch_status, xfer->c_flags, ata_c->flags), DEBUG_FUNCS);
 
-	/* this comamnd is not active any more */
-	achp->ahcic_cmds_active &= ~(1 << xfer->c_slot);
-
 	if (ata_c->flags & (AT_READ|AT_WRITE) && ata_c->bcount > 0) {
 		bus_dmamap_t map = achp->ahcic_datad[xfer->c_slot];
 		bus_dmamap_sync(sc->sc_dmat, map, 0, map->dm_mapsize,
@@ -1136,15 +1145,22 @@ ahci_cmd_done(struct ata_channel *chp, s
 		}
 	}
 
-	ata_c->flags |= AT_DONE;
 	if (achp->ahcic_cmdh[xfer->c_slot].cmdh_prdbc)
 		ata_c->flags |= AT_XFDONE;
 
+	ahci_cmd_done_end(chp, xfer);
+	atastart(chp);
+}
+
+static void
+ahci_cmd_done_end(struct ata_channel *chp, struct ata_xfer *xfer)
+{
+	struct ata_command *ata_c = &xfer->c_ata_c;
+	
+	ata_c->flags |= AT_DONE;
+
 	if (ata_c->flags & AT_WAIT)
 		wakeup(ata_c);
-	else if (ata_c->callback)
-		ata_c->callback(ata_c->callback_arg);
-	atastart(chp);
 	return;
 }
 
@@ -1257,14 +1273,16 @@ ahci_bio_kill_xfer(struct ata_channel *c
 	int drive = xfer->c_drive;
 	struct ata_bio *ata_bio = &xfer->c_bio;
 	struct ahci_channel *achp = (struct ahci_channel *)chp;
+	bool deactivate = true;
+
 	AHCIDEBUG_PRINT(("ahci_bio_kill_xfer channel %d\n", chp->ch_channel),
 	    DEBUG_FUNCS);
 
-	achp->ahcic_cmds_active &= ~(1 << xfer->c_slot);
-	ata_deactivate_xfer(chp, xfer);
-
 	ata_bio->flags |= ATA_ITSDONE;
 	switch (reason) {
+	case KILL_GONE_INACTIVE:
+		deactivate = false;
+		/* FALLTHROUGH */
 	case KILL_GONE:
 		ata_bio->error = ERR_NODEV;
 		break;
@@ -1276,6 +1294,12 @@ ahci_bio_kill_xfer(struct ata_channel *c
 		panic("ahci_bio_kill_xfer");
 	}
 	ata_bio->r_error = WDCE_ABRT;
+
+	if (deactivate) {
+		achp->ahcic_cmds_active &= ~(1 << xfer->c_slot);
+		ata_deactivate_xfer(chp, xfer);
+	}
+
 	(*chp->ch_drive[drive].drv_done)(chp->ch_drive[drive].drv_softc, xfer);
 }
 
@@ -1290,7 +1314,11 @@ ahci_bio_complete(struct ata_channel *ch
 	AHCIDEBUG_PRINT(("ahci_bio_complete channel %d\n", chp->ch_channel),
 	    DEBUG_FUNCS);
 
+	if (ata_waitdrain_xfer_check(chp, xfer))
+		return 0;
+
 	achp->ahcic_cmds_active &= ~(1 << xfer->c_slot);
+	ata_deactivate_xfer(chp, xfer);
 
 	chp->ch_flags &= ~ATACH_IRQ_WAIT;
 	if (xfer->c_flags & C_TIMEOU) {
@@ -1299,17 +1327,12 @@ ahci_bio_complete(struct ata_channel *ch
 		ata_bio->error = NOERROR;
 	}
 
-	ata_deactivate_xfer(chp, xfer);
-
 	bus_dmamap_sync(sc->sc_dmat, achp->ahcic_datad[xfer->c_slot], 0,
 	    achp->ahcic_datad[xfer->c_slot]->dm_mapsize,
 	    (ata_bio->flags & ATA_READ) ? BUS_DMASYNC_POSTREAD :
 	    BUS_DMASYNC_POSTWRITE);
 	bus_dmamap_unload(sc->sc_dmat, achp->ahcic_datad[xfer->c_slot]);
 
-	if (ata_waitdrain_xfer_check(chp, xfer)) {
-		return 0;
-	}
 	ata_bio->flags |= ATA_ITSDONE;
 	if (chp->ch_status & WDCS_DWF) { 
 		ata_bio->error = ERR_DF;
@@ -1695,7 +1718,11 @@ ahci_atapi_complete(struct ata_channel *
 	AHCIDEBUG_PRINT(("ahci_atapi_complete channel %d\n", chp->ch_channel),
 	    DEBUG_FUNCS);
 
+	if (ata_waitdrain_xfer_check(chp, xfer))
+		return 0;
+
 	achp->ahcic_cmds_active &= ~(1 << xfer->c_slot);
+	ata_deactivate_xfer(chp, xfer);
 
 	chp->ch_flags &= ~ATACH_IRQ_WAIT;
 	if (xfer->c_flags & C_TIMEOU) {
@@ -1704,8 +1731,6 @@ ahci_atapi_complete(struct ata_channel *
 		sc_xfer->error = 0;
 	}
 
-	ata_deactivate_xfer(chp, xfer);
-
 	if (xfer->c_bcount > 0) {
 		bus_dmamap_sync(sc->sc_dmat, achp->ahcic_datad[xfer->c_slot], 0,
 		    achp->ahcic_datad[xfer->c_slot]->dm_mapsize,
@@ -1714,10 +1739,6 @@ ahci_atapi_complete(struct ata_channel *
 		bus_dmamap_unload(sc->sc_dmat, achp->ahcic_datad[xfer->c_slot]);
 	}
 
-	if (ata_waitdrain_xfer_check(chp, xfer)) {
-		sc_xfer->error = XS_DRIVER_STUFFUP;
-		return 0;
-	}
 	ata_free_xfer(chp, xfer);
 
 	AHCI_CMDH_SYNC(sc, achp, xfer->c_slot,
@@ -1748,12 +1769,13 @@ ahci_atapi_kill_xfer(struct ata_channel 
 {
 	struct scsipi_xfer *sc_xfer = xfer->c_scsipi;
 	struct ahci_channel *achp = (struct ahci_channel *)chp;
-
-	achp->ahcic_cmds_active &= ~(1 << xfer->c_slot);
-	ata_deactivate_xfer(chp, xfer);
+	bool deactivate = true;
 
 	/* remove this command from xfer queue */
 	switch (reason) {
+	case KILL_GONE_INACTIVE:
+		deactivate = false;
+		/* FALLTHROUGH */
 	case KILL_GONE:
 		sc_xfer->error = XS_DRIVER_STUFFUP;
 		break;
@@ -1764,6 +1786,12 @@ ahci_atapi_kill_xfer(struct ata_channel 
 		printf("ahci_ata_atapi_kill_xfer: unknown reason %d\n", reason);
 		panic("ahci_ata_atapi_kill_xfer");
 	}
+
+	if (deactivate) {
+		achp->ahcic_cmds_active &= ~(1 << xfer->c_slot);
+		ata_deactivate_xfer(chp, xfer);
+	}
+
 	ata_free_xfer(chp, xfer);
 	scsipi_done(sc_xfer);
 }

Index: src/sys/dev/ic/mvsata.c
diff -u src/sys/dev/ic/mvsata.c:1.35.6.16 src/sys/dev/ic/mvsata.c:1.35.6.17
--- src/sys/dev/ic/mvsata.c:1.35.6.16	Sat Jun 24 14:59:10 2017
+++ src/sys/dev/ic/mvsata.c	Tue Jun 27 18:36:03 2017
@@ -1,4 +1,4 @@
-/*	$NetBSD: mvsata.c,v 1.35.6.16 2017/06/24 14:59:10 jdolecek Exp $	*/
+/*	$NetBSD: mvsata.c,v 1.35.6.17 2017/06/27 18:36:03 jdolecek Exp $	*/
 /*
  * Copyright (c) 2008 KIYOHARA Takashi
  * All rights reserved.
@@ -26,7 +26,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: mvsata.c,v 1.35.6.16 2017/06/24 14:59:10 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: mvsata.c,v 1.35.6.17 2017/06/27 18:36:03 jdolecek Exp $");
 
 #include "opt_mvsata.h"
 
@@ -1421,6 +1421,7 @@ mvsata_bio_kill_xfer(struct ata_channel 
 	struct atac_softc *atac = chp->ch_atac;
 	struct ata_bio *ata_bio = &xfer->c_bio;
 	int drive = xfer->c_drive;
+	bool deactivate = true;
 
 	DPRINTFN(2, ("%s:%d: mvsata_bio_kill_xfer: drive=%d\n",
 	    device_xname(atac->atac_dev), chp->ch_channel, xfer->c_drive));
@@ -1431,11 +1432,11 @@ mvsata_bio_kill_xfer(struct ata_channel 
 		mvsata_edma_enable(mvport);
 	}
 
-	mvsata_quetag_put(mvport, xfer->c_slot);
-	ata_deactivate_xfer(chp, xfer);
-
 	ata_bio->flags |= ATA_ITSDONE;
 	switch (reason) {
+	case KILL_GONE_INACTIVE:
+		deactivate = false;
+		/* FALLTHROUGH */
 	case KILL_GONE:
 		ata_bio->error = ERR_NODEV;
 		break;
@@ -1448,6 +1449,12 @@ mvsata_bio_kill_xfer(struct ata_channel 
 		panic("mvsata_bio_kill_xfer");
 	}
 	ata_bio->r_error = WDCE_ABRT;
+
+	if (deactivate) {
+		mvsata_quetag_put(mvport, xfer->c_slot);
+		ata_deactivate_xfer(chp, xfer);
+	}
+
 	(*chp->ch_drive[drive].drv_done)(chp->ch_drive[drive].drv_softc, xfer);
 }
 
@@ -1462,14 +1469,15 @@ mvsata_bio_done(struct ata_channel *chp,
 	    device_xname(MVSATA_DEV2(mvport)), chp->ch_channel, xfer->c_drive,
 	    (u_int)xfer->c_flags));
 
-	callout_stop(&xfer->c_timo_callout);
-
 	/* EDMA restart, if enabled */
 	if (!(xfer->c_flags & C_DMA) && mvport->port_edmamode_curr != nodma) {
 		mvsata_edma_reset_qptr(mvport);
 		mvsata_edma_enable(mvport);
 	}
 
+	if (ata_waitdrain_xfer_check(chp, xfer))
+		return;
+
 	/* feed back residual bcount to our caller */
 	ata_bio->bcount = xfer->c_bcount;
 
@@ -1477,8 +1485,6 @@ mvsata_bio_done(struct ata_channel *chp,
 	mvsata_quetag_put(mvport, xfer->c_slot);
 	ata_deactivate_xfer(chp, xfer);
 
-	if (ata_waitdrain_check(chp, drive))
-		ata_bio->error = ERR_NODEV;
 	ata_bio->flags |= ATA_ITSDONE;
 	(*chp->ch_drive[drive].drv_done)(chp->ch_drive[drive].drv_softc, xfer);
 	atastart(chp);
@@ -1758,14 +1764,15 @@ mvsata_wdc_cmd_kill_xfer(struct ata_chan
 {
 	struct mvsata_port *mvport = (struct mvsata_port *)chp;
 	struct ata_command *ata_c = &xfer->c_ata_c;
+	bool deactivate = true;
 
 	DPRINTFN(1, ("%s:%d: mvsata_cmd_kill_xfer: drive=%d\n",
 	    device_xname(MVSATA_DEV2(mvport)), chp->ch_channel, xfer->c_drive));
 
-	mvsata_quetag_put(mvport, xfer->c_slot);
-	ata_deactivate_xfer(chp, xfer);
-
 	switch (reason) {
+	case KILL_GONE_INACTIVE:
+		deactivate = false;
+		/* FALLTHROUGH */
 	case KILL_GONE:
 		ata_c->flags |= AT_GONE;
 		break;
@@ -1777,6 +1784,12 @@ mvsata_wdc_cmd_kill_xfer(struct ata_chan
 		    "mvsata_cmd_kill_xfer: unknown reason %d\n", reason);
 		panic("mvsata_cmd_kill_xfer");
 	}
+
+	if (deactivate) {
+		mvsata_quetag_put(mvport, xfer->c_slot);
+		ata_deactivate_xfer(chp, xfer);
+	}
+
 	mvsata_wdc_cmd_done_end(chp, xfer);
 }
 
@@ -1791,6 +1804,9 @@ mvsata_wdc_cmd_done(struct ata_channel *
 	    device_xname(atac->atac_dev), chp->ch_channel, xfer->c_drive,
 	    ata_c->flags));
 
+	if (ata_waitdrain_xfer_check(chp, xfer))
+		return;
+
 	if (chp->ch_status & WDCS_DWF)
 		ata_c->flags |= AT_DF;
 	if (chp->ch_status & WDCS_ERR) {
@@ -1848,8 +1864,8 @@ mvsata_wdc_cmd_done(struct ata_channel *
 		delay(10);	/* some drives need a little delay here */
 	}
 
-	if (!ata_waitdrain_xfer_check(chp, xfer))
-		mvsata_wdc_cmd_done_end(chp, xfer);
+	mvsata_wdc_cmd_done_end(chp, xfer);
+	atastart(chp);
 }
 
 static void
@@ -1867,11 +1883,6 @@ mvsata_wdc_cmd_done_end(struct ata_chann
 	ata_c->flags |= AT_DONE;
 	if (ata_c->flags & AT_WAIT)
 		wakeup(ata_c);
-	else if (ata_c->callback)
-		ata_c->callback(ata_c->callback_arg);
-	atastart(chp);
-
-	return;
 }
 
 #if NATAPIBUS > 0
@@ -2277,25 +2288,30 @@ mvsata_atapi_kill_xfer(struct ata_channe
 {
 	struct mvsata_port *mvport = (struct mvsata_port *)chp;
 	struct scsipi_xfer *sc_xfer = xfer->c_scsipi;
-
-	mvsata_quetag_put(mvport, xfer->c_slot);
-	ata_deactivate_xfer(chp, xfer);
+	bool deactivate = true;
 
 	/* remove this command from xfer queue */
 	switch (reason) {
+	case KILL_GONE_INACTIVE:
+		deactivate = false;
+		/* FALLTHROUGH */
 	case KILL_GONE:
 		sc_xfer->error = XS_DRIVER_STUFFUP;
 		break;
-
 	case KILL_RESET:
 		sc_xfer->error = XS_RESET;
 		break;
-
 	default:
 		aprint_error_dev(MVSATA_DEV2(mvport),
 		    "mvsata_atapi_kill_xfer: unknown reason %d\n", reason);
 		panic("mvsata_atapi_kill_xfer");
 	}
+
+	if (deactivate) {
+		mvsata_quetag_put(mvport, xfer->c_slot);
+		ata_deactivate_xfer(chp, xfer);
+	}
+
 	ata_free_xfer(chp, xfer);
 	scsipi_done(sc_xfer);
 }
@@ -2408,19 +2424,18 @@ mvsata_atapi_done(struct ata_channel *ch
 {
 	struct mvsata_port *mvport = (struct mvsata_port *)chp;
 	struct scsipi_xfer *sc_xfer = xfer->c_scsipi;
-	int drive = xfer->c_drive;
 
 	DPRINTFN(1, ("%s:%d:%d: mvsata_atapi_done: flags 0x%x\n",
 	    device_xname(chp->ch_atac->atac_dev), chp->ch_channel,
 	    xfer->c_drive, (u_int)xfer->c_flags));
 
+	if (ata_waitdrain_xfer_check(chp, xfer))
+		return;
+
 	/* mark controller inactive and free the command */
 	mvsata_quetag_put(mvport, xfer->c_slot);
 	ata_deactivate_xfer(chp, xfer);
 
-	if (ata_waitdrain_check(chp, drive))
-		sc_xfer->error = XS_DRIVER_STUFFUP;
-
 	ata_free_xfer(chp, xfer);
 
 	DPRINTFN(1, ("%s:%d: mvsata_atapi_done: scsipi_done\n",

Index: src/sys/dev/ic/siisata.c
diff -u src/sys/dev/ic/siisata.c:1.30.4.23 src/sys/dev/ic/siisata.c:1.30.4.24
--- src/sys/dev/ic/siisata.c:1.30.4.23	Mon Jun 26 20:36:14 2017
+++ src/sys/dev/ic/siisata.c	Tue Jun 27 18:36:04 2017
@@ -1,4 +1,4 @@
-/* $NetBSD: siisata.c,v 1.30.4.23 2017/06/26 20:36:14 jdolecek Exp $ */
+/* $NetBSD: siisata.c,v 1.30.4.24 2017/06/27 18:36:04 jdolecek Exp $ */
 
 /* from ahcisata_core.c */
 
@@ -79,7 +79,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: siisata.c,v 1.30.4.23 2017/06/26 20:36:14 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: siisata.c,v 1.30.4.24 2017/06/27 18:36:04 jdolecek Exp $");
 
 #include <sys/types.h>
 #include <sys/param.h>
@@ -149,6 +149,7 @@ void siisata_killpending(struct ata_driv
 void siisata_cmd_start(struct ata_channel *, struct ata_xfer *);
 int siisata_cmd_complete(struct ata_channel *, struct ata_xfer *, int);
 void siisata_cmd_done(struct ata_channel *, struct ata_xfer *);
+static void siisata_cmd_done_end(struct ata_channel *, struct ata_xfer *);
 void siisata_cmd_kill_xfer(struct ata_channel *, struct ata_xfer *, int);
 
 void siisata_bio_start(struct ata_channel *, struct ata_xfer *);
@@ -970,11 +971,12 @@ siisata_cmd_kill_xfer(struct ata_channel
 {
 	struct ata_command *ata_c = &xfer->c_ata_c;
 	struct siisata_channel *schp = (struct siisata_channel *)chp;
+	bool deactivate = true;
 
-	siisata_deactivate_prb(schp, xfer->c_slot);
-	ata_deactivate_xfer(chp, xfer);
-	
 	switch (reason) {
+	case KILL_GONE_INACTIVE:
+		deactivate = false;
+		/* FALLTHROUGH */
 	case KILL_GONE:
 		ata_c->flags |= AT_GONE;
 		break;
@@ -985,7 +987,13 @@ siisata_cmd_kill_xfer(struct ata_channel
 		panic("%s: port %d: unknown reason %d",
 		   __func__, chp->ch_channel, reason);
 	}
-	siisata_cmd_done(chp, xfer);
+
+	if (deactivate) {
+		siisata_deactivate_prb(schp, xfer->c_slot);
+		ata_deactivate_xfer(chp, xfer);
+	}
+	
+	siisata_cmd_done_end(chp, xfer);
 }
 
 int
@@ -1003,12 +1011,15 @@ siisata_cmd_complete(struct ata_channel 
 	SIISATA_DEBUG_PRINT(("%s: %s\n", SIISATANAME(sc), __func__),
 	    DEBUG_FUNCS|DEBUG_XFERS);
 
+	if (ata_waitdrain_xfer_check(chp, xfer))
+		return 0;
+
 	siisata_deactivate_prb(schp, xfer->c_slot);
+	ata_deactivate_xfer(chp, xfer);
+
 	chp->ch_flags &= ~ATACH_IRQ_WAIT;
 	if (xfer->c_flags & C_TIMEOU)
 		ata_c->flags |= AT_TIMEOU;
-	else
-		callout_stop(&xfer->c_timo_callout);
 
 	if (chp->ch_status & WDCS_BSY) {
 		ata_c->flags |= AT_TIMEOU;
@@ -1017,11 +1028,7 @@ siisata_cmd_complete(struct ata_channel 
 		ata_c->flags |= AT_ERROR;
 	}
 
-	ata_deactivate_xfer(chp, xfer);
-
-	if (!ata_waitdrain_xfer_check(chp, xfer)) {
-		siisata_cmd_done(chp, xfer);
-	}
+	siisata_cmd_done(chp, xfer);
 
 	return 0;
 }
@@ -1063,15 +1070,22 @@ siisata_cmd_done(struct ata_channel *chp
 		}
 	}
 
-	ata_c->flags |= AT_DONE;
 	if (PRREAD(sc, PRSX(chp->ch_channel, xfer->c_slot, PRSO_RTC)))
 		ata_c->flags |= AT_XFDONE;
 
+	siisata_cmd_done_end(chp, xfer);
+	atastart(chp);
+}
+
+static void
+siisata_cmd_done_end(struct ata_channel *chp, struct ata_xfer *xfer)
+{
+	struct ata_command *ata_c = &xfer->c_ata_c;
+
+	ata_c->flags |= AT_DONE;
+
 	if (ata_c->flags & AT_WAIT)
 		wakeup(ata_c);
-	else if (ata_c->callback)
-		ata_c->callback(ata_c->callback_arg);
-	atastart(chp);
 	return;
 }
 
@@ -1170,16 +1184,17 @@ siisata_bio_kill_xfer(struct ata_channel
 	struct siisata_channel *schp = (struct siisata_channel *)chp;
 	struct ata_bio *ata_bio = &xfer->c_bio;
 	int drive = xfer->c_drive;
+	bool deactivate = true;
 
 	SIISATA_DEBUG_PRINT(("%s: %s: port %d slot %d\n",
 	    SIISATANAME((struct siisata_softc *)chp->ch_atac), __func__,
 	    chp->ch_channel, xfer->c_slot), DEBUG_FUNCS);
 
-	siisata_deactivate_prb(schp, xfer->c_slot);
-	ata_deactivate_xfer(chp, xfer);
-
 	ata_bio->flags |= ATA_ITSDONE;
 	switch (reason) {
+	case KILL_GONE_INACTIVE:
+		deactivate = false;
+		/* FALLTHROUGH */
 	case KILL_GONE:
 		ata_bio->error = ERR_NODEV;
 		break;
@@ -1191,6 +1206,12 @@ siisata_bio_kill_xfer(struct ata_channel
 		   __func__, chp->ch_channel, reason);
 	}
 	ata_bio->r_error = WDCE_ABRT;
+
+	if (deactivate) {
+		siisata_deactivate_prb(schp, xfer->c_slot);
+		ata_deactivate_xfer(chp, xfer);
+	}
+
 	(*chp->ch_drive[drive].drv_done)(chp->ch_drive[drive].drv_softc, xfer);
 }
 
@@ -1206,12 +1227,16 @@ siisata_bio_complete(struct ata_channel 
 	    SIISATANAME((struct siisata_softc *)chp->ch_atac), __func__,
 	    chp->ch_channel, xfer->c_slot, xfer->c_drive), DEBUG_FUNCS);
 
+	if (ata_waitdrain_xfer_check(chp, xfer))
+		return 0;
+
 	siisata_deactivate_prb(schp, xfer->c_slot);
+	ata_deactivate_xfer(chp, xfer);
+
 	chp->ch_flags &= ~ATACH_IRQ_WAIT;
 	if (xfer->c_flags & C_TIMEOU) {
 		ata_bio->error = TIMEOUT;
 	} else {
-		callout_stop(&xfer->c_timo_callout);
 		ata_bio->error = NOERROR;
 	}
 
@@ -1221,12 +1246,6 @@ siisata_bio_complete(struct ata_channel 
 	    BUS_DMASYNC_POSTWRITE);
 	bus_dmamap_unload(sc->sc_dmat, schp->sch_datad[xfer->c_slot]);
 
-	ata_deactivate_xfer(chp, xfer);
-
-	if (ata_waitdrain_xfer_check(chp, xfer)) {
-		return 0;
-	}
-
 	ata_bio->flags |= ATA_ITSDONE;
 	if (chp->ch_status & WDCS_DWF) {
 		ata_bio->error = ERR_DF;
@@ -1440,12 +1459,13 @@ siisata_atapi_kill_xfer(struct ata_chann
 {
 	struct scsipi_xfer *sc_xfer = xfer->c_scsipi;
 	struct siisata_channel *schp = (struct siisata_channel *)chp;
-
-	siisata_deactivate_prb(schp, xfer->c_slot);
-	ata_deactivate_xfer(chp, xfer);
+	bool deactivate = true;
 
 	/* remove this command from xfer queue */
 	switch (reason) {
+	case KILL_GONE_INACTIVE:
+		deactivate = false;
+		/* FALLTHROUGH */
 	case KILL_GONE:
 		sc_xfer->error = XS_DRIVER_STUFFUP;
 		break;
@@ -1456,6 +1476,12 @@ siisata_atapi_kill_xfer(struct ata_chann
 		panic("%s: port %d: unknown reason %d",
 		   __func__, chp->ch_channel, reason);
 	}
+
+	if (deactivate) {
+		siisata_deactivate_prb(schp, xfer->c_slot);
+		ata_deactivate_xfer(chp, xfer);
+	}
+
 	ata_free_xfer(chp, xfer);
 	scsipi_done(sc_xfer);
 }
@@ -1722,13 +1748,17 @@ siisata_atapi_complete(struct ata_channe
 	SIISATA_DEBUG_PRINT(("%s: %s()\n", SIISATANAME(sc), __func__),
 	    DEBUG_INTR);
 
+	if (ata_waitdrain_xfer_check(chp, xfer))
+		return 0;
+
 	/* this command is not active any more */
 	siisata_deactivate_prb(schp, xfer->c_slot);
+	ata_deactivate_xfer(chp, xfer);
+
 	chp->ch_flags &= ~ATACH_IRQ_WAIT;
 	if (xfer->c_flags & C_TIMEOU) {
 		sc_xfer->error = XS_TIMEOUT;
 	} else {
-		callout_stop(&xfer->c_timo_callout);
 		sc_xfer->error = XS_NOERROR;
 	}
 
@@ -1738,13 +1768,6 @@ siisata_atapi_complete(struct ata_channe
 	    BUS_DMASYNC_POSTREAD : BUS_DMASYNC_POSTWRITE);
 	bus_dmamap_unload(sc->sc_dmat, schp->sch_datad[xfer->c_slot]);
 
-	ata_deactivate_xfer(chp, xfer);
-
-	if (ata_waitdrain_xfer_check(chp, xfer)) {
-		sc_xfer->error = XS_DRIVER_STUFFUP;
-		return 0; /* XXX verify */
-	}
-
 	ata_free_xfer(chp, xfer);
 	sc_xfer->resid = sc_xfer->datalen;
 	sc_xfer->resid -= PRREAD(sc, PRSX(chp->ch_channel, xfer->c_slot,

Index: src/sys/dev/ic/wdc.c
diff -u src/sys/dev/ic/wdc.c:1.283.2.9 src/sys/dev/ic/wdc.c:1.283.2.10
--- src/sys/dev/ic/wdc.c:1.283.2.9	Wed Jun 21 19:38:43 2017
+++ src/sys/dev/ic/wdc.c	Tue Jun 27 18:36:04 2017
@@ -1,4 +1,4 @@
-/*	$NetBSD: wdc.c,v 1.283.2.9 2017/06/21 19:38:43 jdolecek Exp $ */
+/*	$NetBSD: wdc.c,v 1.283.2.10 2017/06/27 18:36:04 jdolecek Exp $ */
 
 /*
  * Copyright (c) 1998, 2001, 2003 Manuel Bouyer.  All rights reserved.
@@ -58,7 +58,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: wdc.c,v 1.283.2.9 2017/06/21 19:38:43 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: wdc.c,v 1.283.2.10 2017/06/27 18:36:04 jdolecek Exp $");
 
 #include "opt_ata.h"
 #include "opt_wdc.h"
@@ -1561,11 +1561,16 @@ __wdccommand_done(struct ata_channel *ch
 	struct wdc_softc *wdc = CHAN_TO_WDC(chp);
 	struct wdc_regs *wdr = &wdc->regs[chp->ch_channel];
 	struct ata_command *ata_c = &xfer->c_ata_c;
+	bool start = false;
 
 	ATADEBUG_PRINT(("__wdccommand_done %s:%d:%d flags 0x%x\n",
 	    device_xname(atac->atac_dev), chp->ch_channel, xfer->c_drive,
 	    ata_c->flags), DEBUG_FUNCS);
 
+	if (ata_waitdrain_xfer_check(chp, xfer)) {
+		start = false;
+		goto out;
+	}
 
 	if (chp->ch_status & WDCS_DWF)
 		ata_c->flags |= AT_DF;
@@ -1629,6 +1634,9 @@ __wdccommand_done(struct ata_channel *ch
 
 	ata_deactivate_xfer(chp, xfer);
 
+	__wdccommand_done_end(chp, xfer);
+
+out:
 	if (ata_c->flags & AT_POLL) {
 		/* enable interrupts */
 		if (! (wdc->cap & WDC_CAPABILITY_NO_AUXCTL)) 
@@ -1637,9 +1645,8 @@ __wdccommand_done(struct ata_channel *ch
 		delay(10); /* some drives need a little delay here */
 	}
 
-	if (!ata_waitdrain_xfer_check(chp, xfer)) {
-		__wdccommand_done_end(chp, xfer);
-	}
+	if (start)
+		atastart(chp);
 }
 
 static void
@@ -1650,9 +1657,6 @@ __wdccommand_done_end(struct ata_channel
 	ata_c->flags |= AT_DONE;
 	if (ata_c->flags & AT_WAIT)
 		wakeup(ata_c);
-	else if (ata_c->callback)
-		ata_c->callback(ata_c->callback_arg);
-	atastart(chp);
 	return;
 }
 
@@ -1661,10 +1665,12 @@ __wdccommand_kill_xfer(struct ata_channe
     int reason)
 {
 	struct ata_command *ata_c = &xfer->c_ata_c;
-
-	ata_deactivate_xfer(chp, xfer);
+	bool deactivate = true;
 
 	switch (reason) {
+	case KILL_GONE_INACTIVE:
+		deactivate = false;
+		/* FALLTHROUGH */
 	case KILL_GONE:
 		ata_c->flags |= AT_GONE;
 		break;
@@ -1676,6 +1682,10 @@ __wdccommand_kill_xfer(struct ata_channe
 		    reason);
 		panic("__wdccommand_kill_xfer");
 	}
+
+	if (deactivate)
+		ata_deactivate_xfer(chp, xfer);
+
 	__wdccommand_done_end(chp, xfer);
 }
 

Index: src/sys/dev/scsipi/atapi_wdc.c
diff -u src/sys/dev/scsipi/atapi_wdc.c:1.123.4.8 src/sys/dev/scsipi/atapi_wdc.c:1.123.4.9
--- src/sys/dev/scsipi/atapi_wdc.c:1.123.4.8	Wed Jun 21 22:34:46 2017
+++ src/sys/dev/scsipi/atapi_wdc.c	Tue Jun 27 18:36:03 2017
@@ -1,4 +1,4 @@
-/*	$NetBSD: atapi_wdc.c,v 1.123.4.8 2017/06/21 22:34:46 jdolecek Exp $	*/
+/*	$NetBSD: atapi_wdc.c,v 1.123.4.9 2017/06/27 18:36:03 jdolecek Exp $	*/
 
 /*
  * Copyright (c) 1998, 2001 Manuel Bouyer.
@@ -25,7 +25,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: atapi_wdc.c,v 1.123.4.8 2017/06/21 22:34:46 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: atapi_wdc.c,v 1.123.4.9 2017/06/27 18:36:03 jdolecek Exp $");
 
 #ifndef ATADEBUG
 #define ATADEBUG
@@ -170,11 +170,13 @@ static void
 wdc_atapi_kill_xfer(struct ata_channel *chp, struct ata_xfer *xfer, int reason)
 {
 	struct scsipi_xfer *sc_xfer = xfer->c_scsipi;
-
-	ata_deactivate_xfer(chp, xfer);
+	bool deactivate = true;
 
 	/* remove this command from xfer queue */
 	switch (reason) {
+	case KILL_GONE_INACTIVE:
+		deactivate = false;
+		/* FALLTHROUGH */
 	case KILL_GONE:
 		sc_xfer->error = XS_DRIVER_STUFFUP;
 		break;
@@ -186,6 +188,10 @@ wdc_atapi_kill_xfer(struct ata_channel *
 		    reason);
 		panic("wdc_ata_bio_kill_xfer");
 	}
+
+	if (deactivate)
+		ata_deactivate_xfer(chp, xfer);
+
 	ata_free_xfer(chp, xfer);
 	scsipi_done(sc_xfer);
 }
@@ -1096,18 +1102,17 @@ wdc_atapi_done(struct ata_channel *chp, 
 {
 	struct atac_softc *atac = chp->ch_atac;
 	struct scsipi_xfer *sc_xfer = xfer->c_scsipi;
-	int drive = xfer->c_drive;
 
 	ATADEBUG_PRINT(("wdc_atapi_done %s:%d:%d: flags 0x%x\n",
 	    device_xname(atac->atac_dev), chp->ch_channel, xfer->c_drive,
 	    (u_int)xfer->c_flags), DEBUG_XFERS);
 
+	if (ata_waitdrain_xfer_check(chp, xfer))
+		return;
+
 	ata_deactivate_xfer(chp, xfer);
 	ata_free_xfer(chp, xfer);
 
-	if (ata_waitdrain_check(chp, drive))
-		sc_xfer->error = XS_DRIVER_STUFFUP;
-
 	ATADEBUG_PRINT(("wdc_atapi_done: scsipi_done\n"), DEBUG_XFERS);
 	scsipi_done(sc_xfer);
 	ATADEBUG_PRINT(("atastart from wdc_atapi_done, flags 0x%x\n",

Reply via email to