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",