Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/jdolecek-ncq]: src/sys/dev refactor code so that xfer c_start() hook is ...
details: https://anonhg.NetBSD.org/src/rev/24d98d2ea6bc
branches: jdolecek-ncq
changeset: 823013:24d98d2ea6bc
user: jdolecek <jdolecek%NetBSD.org@localhost>
date: Sun Sep 10 19:31:15 2017 +0000
description:
refactor code so that xfer c_start() hook is called with channel mutex held,
and hence the controller submit code no longer relies on spl
tested all the affected drivers - wdc (via piixide), ahci, mvsata, siisata,
both disk and atapi I/O
diffstat:
sys/dev/ata/TODO.ncq | 6 +-
sys/dev/ata/ata.c | 246 ++++++++++++++++++++++++++++--------------
sys/dev/ata/ata_wdc.c | 88 ++++++++-------
sys/dev/ata/atavar.h | 15 ++-
sys/dev/ic/ahcisata_core.c | 139 ++++++++++++++++-------
sys/dev/ic/mvsata.c | 259 ++++++++++++++++++++++++++++++--------------
sys/dev/ic/siisata.c | 131 +++++++++++++++-------
sys/dev/ic/wdc.c | 77 ++++++++----
sys/dev/scsipi/atapi_wdc.c | 124 ++++++++++++++------
9 files changed, 721 insertions(+), 364 deletions(-)
diffs (truncated from 2919 to 300 lines):
diff -r cb3b5e58add5 -r 24d98d2ea6bc sys/dev/ata/TODO.ncq
--- a/sys/dev/ata/TODO.ncq Sun Sep 10 19:22:56 2017 +0000
+++ b/sys/dev/ata/TODO.ncq Sun Sep 10 19:31:15 2017 +0000
@@ -2,11 +2,13 @@
----
test wd* at umass?, confirm the ata_channel kludge works
-c_start() needs to be called on splbio to avoid spurious irq during reset,
-is not e.g. in ata thread and may not in atastart() neither
+revise calls to atastart() - now called alsoafter ATASTART_ABORT(), call
+only from intr routine
- wdc.c never calls atastart() (start always false)
- ata_wdc.c calls atastart() regardless if error
+reconsider freeze/thaw in error recovery - can it screw up with thread?
+
Other random notes (do outside the NCQ branch):
-----------------------------------------------------
do biodone() in wddone() starting the dump to not leak bufs when dumping from
diff -r cb3b5e58add5 -r 24d98d2ea6bc sys/dev/ata/ata.c
--- a/sys/dev/ata/ata.c Sun Sep 10 19:22:56 2017 +0000
+++ b/sys/dev/ata/ata.c Sun Sep 10 19:31:15 2017 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: ata.c,v 1.132.8.30 2017/09/10 19:22:56 jdolecek Exp $ */
+/* $NetBSD: ata.c,v 1.132.8.31 2017/09/10 19:31:15 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.30 2017/09/10 19:22:56 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ata.c,v 1.132.8.31 2017/09/10 19:31:15 jdolecek Exp $");
#include "opt_ata.h"
@@ -130,8 +130,12 @@
static void atabusconfig_thread(void *);
static void ata_channel_idle(struct ata_channel *);
+static void ata_channel_thaw_locked(struct ata_channel *);
static void ata_activate_xfer_locked(struct ata_channel *, struct ata_xfer *);
static void ata_channel_freeze_locked(struct ata_channel *);
+static struct ata_xfer *ata_queue_get_active_xfer_locked(struct ata_channel *);
+static void ata_thread_wake_locked(struct ata_channel *);
+
/*
* atabus_init:
*
@@ -200,7 +204,7 @@
struct ata_queue *chq = chp->ch_queue;
struct ata_xfer *xfer = NULL;
- mutex_enter(&chp->ch_lock);
+ ata_channel_lock(chp);
KASSERTMSG(hwslot < chq->queue_openings, "hwslot %d > openings %d",
hwslot, chq->queue_openings);
@@ -212,7 +216,7 @@
break;
}
- mutex_exit(&chp->ch_lock);
+ ata_channel_unlock(chp);
KASSERTMSG((xfer != NULL),
"%s: xfer with slot %d not found (active %x)", __func__,
@@ -221,6 +225,13 @@
return xfer;
}
+static struct ata_xfer *
+ata_queue_get_active_xfer_locked(struct ata_channel *chp)
+{
+ KASSERT(mutex_owned(&chp->ch_lock));
+ return TAILQ_FIRST(&chp->ch_queue->active_xfers);
+}
+
/*
* This interface is supposed only to be used when there is exactly
* one outstanding command, when there is no information about the slot,
@@ -232,12 +243,12 @@
{
struct ata_xfer *xfer = NULL;
- mutex_enter(&chp->ch_lock);
+ ata_channel_lock(chp);
KASSERT(chp->ch_queue->queue_active <= 1);
- xfer = TAILQ_FIRST(&chp->ch_queue->active_xfers);
+ xfer = ata_queue_get_active_xfer_locked(chp);
- mutex_exit(&chp->ch_lock);
+ ata_channel_unlock(chp);
return xfer;
}
@@ -247,7 +258,7 @@
{
struct ata_xfer *xfer = NULL;
- mutex_enter(&chp->ch_lock);
+ ata_channel_lock(chp);
TAILQ_FOREACH(xfer, &chp->ch_queue->active_xfers, c_activechain) {
if (xfer->c_drive == drive)
@@ -255,7 +266,7 @@
}
KASSERT(xfer != NULL);
- mutex_exit(&chp->ch_lock);
+ ata_channel_unlock(chp);
return xfer;
}
@@ -374,9 +385,9 @@
int i, error;
/* we are in the atabus's thread context */
- mutex_enter(&chp->ch_lock);
+ ata_channel_lock(chp);
chp->ch_flags |= ATACH_TH_RUN;
- mutex_exit(&chp->ch_lock);
+ ata_channel_unlock(chp);
/*
* Probe for the drives attached to controller, unless a PMP
@@ -393,9 +404,9 @@
}
/* next operations will occurs in a separate thread */
- mutex_enter(&chp->ch_lock);
+ ata_channel_lock(chp);
chp->ch_flags &= ~ATACH_TH_RUN;
- mutex_exit(&chp->ch_lock);
+ ata_channel_unlock(chp);
/* Make sure the devices probe in atabus order to avoid jitter. */
mutex_enter(&atabus_qlock);
@@ -407,7 +418,7 @@
}
mutex_exit(&atabus_qlock);
- mutex_enter(&chp->ch_lock);
+ ata_channel_lock(chp);
/* If no drives, abort here */
if (chp->ch_drive == NULL)
@@ -423,7 +434,7 @@
if (chp->ch_flags & ATACH_SHUTDOWN)
goto out;
- mutex_exit(&chp->ch_lock);
+ ata_channel_unlock(chp);
if ((error = kthread_create(PRI_NONE, 0, NULL, atabusconfig_thread,
atabus_sc, &atabus_cfg_lwp,
@@ -433,7 +444,7 @@
return;
out:
- mutex_exit(&chp->ch_lock);
+ ata_channel_unlock(chp);
mutex_enter(&atabus_qlock);
TAILQ_REMOVE(&atabus_initq_head, atabus_initq, atabus_initq);
@@ -586,9 +597,9 @@
struct ata_channel *chp = sc->sc_chan;
struct ata_queue *chq = chp->ch_queue;
struct ata_xfer *xfer;
- int i, s;
+ int i, rv, s;
- mutex_enter(&chp->ch_lock);
+ ata_channel_lock(chp);
chp->ch_flags |= ATACH_TH_RUN;
/*
@@ -602,11 +613,11 @@
chp->ch_drive[i].drive_flags = 0;
chp->ch_drive[i].drive_type = ATA_DRIVET_NONE;
}
- mutex_exit(&chp->ch_lock);
+ ata_channel_unlock(chp);
atabusconfig(sc);
- mutex_enter(&chp->ch_lock);
+ ata_channel_lock(chp);
for (;;) {
if ((chp->ch_flags & (ATACH_TH_RESET | ATACH_SHUTDOWN)) == 0 &&
(chq->queue_active == 0 || chq->queue_freeze == 0)) {
@@ -619,21 +630,21 @@
}
if (chp->ch_flags & ATACH_TH_RESCAN) {
chp->ch_flags &= ~ATACH_TH_RESCAN;
- mutex_exit(&chp->ch_lock);
+ ata_channel_unlock(chp);
atabusconfig(sc);
- mutex_enter(&chp->ch_lock);
+ ata_channel_lock(chp);
}
if (chp->ch_flags & ATACH_TH_RESET) {
/*
* ata_reset_channel() will freeze 2 times, so
* unfreeze one time. Not a problem as we're at splbio
*/
- mutex_exit(&chp->ch_lock);
- ata_channel_thaw(chp);
+ ata_channel_thaw_locked(chp);
+ ata_channel_unlock(chp);
s = splbio();
ata_reset_channel(chp, AT_WAIT | chp->ch_reset_flags);
splx(s);
- mutex_enter(&chp->ch_lock);
+ ata_channel_lock(chp);
} else if (chq->queue_active > 0 && chq->queue_freeze == 1) {
/*
* Caller has bumped queue_freeze, decrease it. This
@@ -641,31 +652,39 @@
*/
KASSERT((chp->ch_flags & ATACH_NCQ) == 0);
KASSERT(chq->queue_active == 1);
- mutex_exit(&chp->ch_lock);
- ata_channel_thaw(chp);
- xfer = ata_queue_get_active_xfer(chp);
+ ata_channel_thaw_locked(chp);
+ xfer = ata_queue_get_active_xfer_locked(chp);
+
KASSERT(xfer != NULL);
- s = splbio();
- (*xfer->c_start)(xfer->c_chp, xfer);
- splx(s);
- mutex_enter(&chp->ch_lock);
+ KASSERT((xfer->c_flags & C_POLL) == 0);
+
+ switch ((rv = ata_xfer_start(xfer))) {
+ case ATASTART_STARTED:
+ case ATASTART_POLL:
+ case ATASTART_ABORT:
+ break;
+ case ATASTART_TH:
+ default:
+ panic("%s: ata_xfer_start() unexpected rv %d",
+ __func__, rv);
+ /* NOTREACHED */
+ }
} else if (chq->queue_freeze > 1)
panic("%s: queue_freeze", __func__);
}
chp->ch_thread = NULL;
cv_signal(&chp->ch_thr_idle);
- mutex_exit(&chp->ch_lock);
+ ata_channel_unlock(chp);
kthread_exit(0);
}
-void
-ata_thread_wake(struct ata_channel *chp)
+static void
+ata_thread_wake_locked(struct ata_channel *chp)
{
- mutex_enter(&chp->ch_lock);
+ KASSERT(mutex_owned(&chp->ch_lock));
ata_channel_freeze_locked(chp);
cv_signal(&chp->ch_thr_idle);
- mutex_exit(&chp->ch_lock);
}
/*
@@ -743,13 +762,13 @@
int i, error = 0;
/* Shutdown the channel. */
- mutex_enter(&chp->ch_lock);
+ ata_channel_lock(chp);
chp->ch_flags |= ATACH_SHUTDOWN;
while (chp->ch_thread != NULL) {
cv_signal(&chp->ch_thr_idle);
cv_wait(&chp->ch_thr_idle, &chp->ch_lock);
}
- mutex_exit(&chp->ch_lock);
+ ata_channel_unlock(chp);
/*
* Detach atapibus and its children.
@@ -1199,7 +1218,7 @@
/* complete xfer setup */
xfer->c_chp = chp;
- mutex_enter(&chp->ch_lock);
+ ata_channel_lock(chp);
/*
* Standard commands are added to the end of command list, but
@@ -1235,7 +1254,7 @@
}
}
Home |
Main Index |
Thread Index |
Old Index