Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/jdolecek-ncq]: src/sys/dev attend error paths, more strict asserts and c...
details: https://anonhg.NetBSD.org/src/rev/15feec74ffb7
branches: jdolecek-ncq
changeset: 822951:15feec74ffb7
user: jdolecek <jdolecek%NetBSD.org@localhost>
date: Tue Jun 27 18:36:03 2017 +0000
description:
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
diffstat:
sys/dev/ata/ata.c | 112 +++++++++++++++++++++++++++++---------------
sys/dev/ata/ata_wdc.c | 20 ++++---
sys/dev/ata/atavar.h | 11 ++--
sys/dev/ic/ahcisata_core.c | 98 +++++++++++++++++++++++++--------------
sys/dev/ic/mvsata.c | 71 +++++++++++++++++-----------
sys/dev/ic/siisata.c | 99 ++++++++++++++++++++++++---------------
sys/dev/ic/wdc.c | 30 ++++++++----
sys/dev/scsipi/atapi_wdc.c | 21 +++++---
8 files changed, 290 insertions(+), 172 deletions(-)
diffs (truncated from 1173 to 300 lines):
diff -r 9b71e70267cd -r 15feec74ffb7 sys/dev/ata/ata.c
--- a/sys/dev/ata/ata.c Tue Jun 27 18:16:50 2017 +0000
+++ b/sys/dev/ata/ata.c Tue Jun 27 18:36:03 2017 +0000
@@ -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_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 @@
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 @@
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 @@
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 @@
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 @@
}
/*
- * 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 @@
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 @@
/* Now try to kick off xfers on the current drive */
ATA_DRIVE_START(chp, drive);
+ splx(s);
#undef ATA_DRIVE_START
}
diff -r 9b71e70267cd -r 15feec74ffb7 sys/dev/ata/ata_wdc.c
--- a/sys/dev/ata/ata_wdc.c Tue Jun 27 18:16:50 2017 +0000
+++ b/sys/dev/ata/ata_wdc.c Tue Jun 27 18:36:03 2017 +0000
@@ -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 @@
{
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 @@
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 @@
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 @@
/* 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);
diff -r 9b71e70267cd -r 15feec74ffb7 sys/dev/ata/atavar.h
--- a/sys/dev/ata/atavar.h Tue Jun 27 18:16:50 2017 +0000
+++ b/sys/dev/ata/atavar.h Tue Jun 27 18:36:03 2017 +0000
@@ -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 @@
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 @@
#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 */
/*
Home |
Main Index |
Thread Index |
Old Index