Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/jdolecek-ncqfixes]: src/sys/dev change channel reset and drive reset for...
details: https://anonhg.NetBSD.org/src/rev/b78922e72e93
branches: jdolecek-ncqfixes
changeset: 850587:b78922e72e93
user: jdolecek <jdolecek%NetBSD.org@localhost>
date: Wed Oct 03 19:20:48 2018 +0000
description:
change channel reset and drive reset for all ATA controllers to always
run via thread, and with channel lock held the whole time; the queue is
frozen while reset is pending
for this repurpose ata_reset_channel() into new ata_thread_run()
also adjust some device printfs to not leak xfer pointer, and avoid
aprint_* for non-autoconf messages
diffstat:
sys/dev/ata/TODO.ncq | 2 -
sys/dev/ata/ata.c | 146 ++++++++++++++++++++++++++------------------
sys/dev/ata/atavar.h | 9 ++-
sys/dev/ata/wd.c | 21 ++++--
sys/dev/ic/ahcisata_core.c | 11 +--
sys/dev/ic/mvsata.c | 18 +---
sys/dev/ic/siisata.c | 16 ++--
sys/dev/ic/wdc.c | 22 +++---
8 files changed, 134 insertions(+), 111 deletions(-)
diffs (truncated from 660 to 300 lines):
diff -r 6c8a9efc2a9f -r b78922e72e93 sys/dev/ata/TODO.ncq
--- a/sys/dev/ata/TODO.ncq Mon Sep 24 21:19:50 2018 +0000
+++ b/sys/dev/ata/TODO.ncq Wed Oct 03 19:20:48 2018 +0000
@@ -2,8 +2,6 @@
- fix ahci(4) error handling under paralles - invalid bio via WD_CHAOS_MONKEY
ends up being handled as NOERROR, triggering KASSERT() in wd(4)
- remove controller-specific slot bitmaps (ic/siisata.c, ic/ahcisata_core.c)
-- revert rev 1.431 of wd.c (AT_POOL removal) - ensure still works
- for the reset-via-thread case
Bugs
----
diff -r 6c8a9efc2a9f -r b78922e72e93 sys/dev/ata/ata.c
--- a/sys/dev/ata/ata.c Mon Sep 24 21:19:50 2018 +0000
+++ b/sys/dev/ata/ata.c Wed Oct 03 19:20:48 2018 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: ata.c,v 1.141.6.10 2018/09/24 19:48:02 jdolecek Exp $ */
+/* $NetBSD: ata.c,v 1.141.6.11 2018/10/03 19:20:48 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.141.6.10 2018/09/24 19:48:02 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ata.c,v 1.141.6.11 2018/10/03 19:20:48 jdolecek Exp $");
#include "opt_ata.h"
@@ -440,7 +440,7 @@
struct ata_channel *chp = sc->sc_chan;
struct ata_queue *chq = chp->ch_queue;
struct ata_xfer *xfer;
- int i, rv, s;
+ int i, rv;
ata_channel_lock(chp);
chp->ch_flags |= ATACH_TH_RUN;
@@ -462,7 +462,8 @@
ata_channel_lock(chp);
for (;;) {
- if ((chp->ch_flags & (ATACH_TH_RESET | ATACH_SHUTDOWN)) == 0 &&
+ if ((chp->ch_flags & (ATACH_TH_RESET | ATACH_TH_DRIVE_RESET
+ | ATACH_SHUTDOWN)) == 0 &&
(chq->queue_active == 0 || chq->queue_freeze == 0)) {
chp->ch_flags &= ~ATACH_TH_RUN;
cv_wait(&chp->ch_thr_idle, &chp->ch_lock);
@@ -478,12 +479,24 @@
ata_channel_lock(chp);
}
if (chp->ch_flags & ATACH_TH_RESET) {
- /* ata_reset_channel() will unfreeze the channel */
- ata_channel_unlock(chp);
- s = splbio();
- ata_reset_channel(chp, AT_WAIT | chp->ch_reset_flags);
- splx(s);
- ata_channel_lock(chp);
+ /* this will unfreeze the channel */
+ ata_thread_run(chp, AT_WAIT | chp->ch_reset_flags,
+ ATACH_TH_RESET, ATACH_NODRIVE);
+ } else if (chp->ch_flags & ATACH_TH_DRIVE_RESET) {
+ for (i = 0; i < chp->ch_ndrives; i++) {
+ struct ata_drive_datas *drvp;
+ int drv_reset_flags;
+
+ drvp = &chp->ch_drive[i];
+ drv_reset_flags = drvp->drive_reset_flags;
+
+ if (drvp->drive_flags & ATACH_TH_DRIVE_RESET) {
+ ata_thread_run(chp,
+ AT_WAIT | drv_reset_flags,
+ ATACH_TH_DRIVE_RESET, i);
+ }
+ }
+ chp->ch_flags &= ~ATACH_TH_DRIVE_RESET;
} else if (chq->queue_active > 0 && chq->queue_freeze == 1) {
/*
* Caller has bumped queue_freeze, decrease it. This
@@ -511,6 +524,11 @@
}
} else if (chq->queue_freeze > 1)
panic("%s: queue_freeze", __func__);
+
+ /* Try to run down the queue once after each event is handled */
+ ata_channel_unlock(chp);
+ atastart(chp);
+ ata_channel_lock(chp);
}
chp->ch_thread = NULL;
cv_signal(&chp->ch_thr_idle);
@@ -1471,7 +1489,8 @@
ata_channel_unlock(chp);
device_printf(drvp->drv_softc,
- "xfer %p freed while invoking timeout\n", xfer);
+ "xfer %"PRIxPTR" freed while invoking timeout\n",
+ (intptr_t)xfer & PAGE_MASK);
ata_free_xfer(chp, xfer);
return true;
@@ -1481,7 +1500,8 @@
ata_channel_unlock(chp);
device_printf(drvp->drv_softc,
- "xfer %p deactivated while invoking timeout\n", xfer);
+ "xfer %"PRIxPTR" deactivated while invoking timeout\n",
+ (intptr_t)xfer & PAGE_MASK);
return true;
}
@@ -1605,44 +1625,28 @@
}
/*
- * ata_reset_channel:
+ * ata_thread_run:
*
- * Reset and ATA channel.
- *
- * MUST BE CALLED AT splbio()!
+ * Reset and ATA channel. Channel lock must be held.
*/
void
-ata_reset_channel(struct ata_channel *chp, int flags)
+ata_thread_run(struct ata_channel *chp, int flags, int type, int drive)
{
struct atac_softc *atac = chp->ch_atac;
- int drive;
bool threset = false;
-
-#ifdef ATA_DEBUG
- int spl1, spl2;
+ struct ata_drive_datas *drvp;
- spl1 = splbio();
- spl2 = splbio();
- if (spl2 != spl1) {
- printf("ata_reset_channel: not at splbio()\n");
- panic("ata_reset_channel");
- }
- splx(spl2);
- splx(spl1);
-#endif /* ATA_DEBUG */
-
- ata_channel_lock(chp);
+ ata_channel_lock_owned(chp);
/*
* If we can poll or wait it's OK, otherwise wake up the
* kernel thread to do it for us.
*/
- ATADEBUG_PRINT(("ata_reset_channel flags 0x%x ch_flags 0x%x\n",
- flags, chp->ch_flags), DEBUG_FUNCS | DEBUG_XFERS);
+ ATADEBUG_PRINT(("%s flags 0x%x ch_flags 0x%x\n",
+ __func__, flags, chp->ch_flags), DEBUG_FUNCS | DEBUG_XFERS);
if ((flags & (AT_POLL | AT_WAIT)) == 0) {
- if (chp->ch_flags & ATACH_TH_RESET) {
+ if (chp->ch_flags & type) {
/* No need to schedule a reset more than one time. */
- ata_channel_unlock(chp);
return;
}
@@ -1651,10 +1655,24 @@
* to a thread.
*/
ata_channel_freeze_locked(chp);
- chp->ch_flags |= ATACH_TH_RESET;
- chp->ch_reset_flags = flags & AT_RST_EMERG;
+ chp->ch_flags |= type;
+
+
+ switch (type) {
+ case ATACH_TH_RESET:
+ chp->ch_reset_flags = flags & AT_RST_EMERG;
+ break;
+ case ATACH_TH_DRIVE_RESET:
+ drvp = &chp->ch_drive[drive];
+ drvp->drive_flags |= ATACH_TH_DRIVE_RESET;
+ drvp->drive_reset_flags = flags;
+ break;
+ default:
+ panic("%s: unknown type: %x", __func__, type);
+ /* NOTREACHED */
+ }
+
cv_signal(&chp->ch_thr_idle);
- ata_channel_unlock(chp);
return;
}
@@ -1666,19 +1684,31 @@
* the flag now so that the thread won't try to execute it if
* we happen to sleep, and thaw one more time after the reset.
*/
- if (chp->ch_flags & ATACH_TH_RESET) {
- chp->ch_flags &= ~ATACH_TH_RESET;
+ if (chp->ch_flags & type) {
+ chp->ch_flags &= ~type;
threset = true;
}
- ata_channel_unlock(chp);
+ switch (type) {
+ case ATACH_TH_RESET:
+ (*atac->atac_bustype_ata->ata_reset_channel)(chp, flags);
- (*atac->atac_bustype_ata->ata_reset_channel)(chp, flags);
+ KASSERT(chp->ch_ndrives == 0 || chp->ch_drive != NULL);
+ for (drive = 0; drive < chp->ch_ndrives; drive++)
+ chp->ch_drive[drive].state = 0;
+ break;
- ata_channel_lock(chp);
- KASSERT(chp->ch_ndrives == 0 || chp->ch_drive != NULL);
- for (drive = 0; drive < chp->ch_ndrives; drive++)
- chp->ch_drive[drive].state = 0;
+ case ATACH_TH_DRIVE_RESET:
+ KASSERT(drive <= chp->ch_ndrives);
+ drvp = &chp->ch_drive[drive];
+ (*atac->atac_bustype_ata->ata_reset_drive)(drvp, flags, NULL);
+ drvp->state = 0;
+ break;
+
+ default:
+ panic("%s: unknown type: %x", __func__, type);
+ /* NOTREACHED */
+ }
/*
* Thaw one extra time to clear the freeze done when the reset has
@@ -1693,13 +1723,9 @@
/* Signal the thread in case there is an xfer to run */
cv_signal(&chp->ch_thr_idle);
- ata_channel_unlock(chp);
-
if (flags & AT_RST_EMERG) {
/* make sure that we can use polled commands */
ata_queue_reset(chp->ch_queue);
- } else {
- atastart(chp);
}
}
@@ -1846,7 +1872,7 @@
(*atac->atac_set_modes)(chp);
ata_print_modes(chp);
/* reset the channel, which will schedule all drives for setup */
- ata_reset_channel(chp, flags);
+ ata_thread_run(chp, flags, ATACH_TH_RESET, ATACH_NODRIVE);
return 1;
}
#endif /* NATA_DMA */
@@ -2186,7 +2212,6 @@
struct ata_channel *chp = sc->sc_chan;
int min_drive, max_drive, drive;
int error;
- int s;
/*
* Enforce write permission for ioctls that change the
@@ -2203,9 +2228,10 @@
switch (cmd) {
case ATABUSIORESET:
- s = splbio();
- ata_reset_channel(sc->sc_chan, AT_WAIT | AT_POLL);
- splx(s);
+ ata_channel_lock(chp);
+ ata_thread_run(sc->sc_chan, AT_WAIT | AT_POLL,
+ ATACH_TH_RESET, ATACH_NODRIVE);
+ ata_channel_unlock(chp);
return 0;
case ATABUSIOSCAN:
{
@@ -2283,11 +2309,11 @@
/* unfreeze the queue and reset drives */
ata_channel_thaw_locked(chp);
- ata_channel_unlock(chp);
-
/* reset channel only if there are drives attached */
if (chp->ch_ndrives > 0)
- ata_reset_channel(chp, AT_WAIT);
+ ata_thread_run(chp, AT_WAIT, ATACH_TH_RESET, ATACH_NODRIVE);
+
+ ata_channel_unlock(chp);
out:
return true;
@@ -2336,6 +2362,7 @@
void
ata_delay(struct ata_channel *chp, int ms, const char *msg, int flags)
{
+ KASSERT(mutex_owned(&chp->ch_lock));
if ((flags & (AT_WAIT | AT_POLL)) == AT_POLL) {
Home |
Main Index |
Thread Index |
Old Index