Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/jdolecek-ncq]: src/sys/dev remove ata queue downsizing - every device, a...



details:   https://anonhg.NetBSD.org/src/rev/b744e795b607
branches:  jdolecek-ncq
changeset: 352680:b744e795b607
user:      jdolecek <jdolecek%NetBSD.org@localhost>
date:      Tue Jun 20 20:58:22 2017 +0000

description:
remove ata queue downsizing - every device, attached to the same channel,
uses slots according to it's own limits

wdc code changed to expect maximum one active xfer, and not check number
of openings in the channel; this is to facilitate using wdc functions
for e.g. handling of atapi commands  for drivers which support both ATAPI
and NCQ

diffstat:

 sys/dev/ata/TODO.ncq       |   8 ----
 sys/dev/ata/ata.c          |  88 ++++++++++++++++++++++++---------------------
 sys/dev/ata/ata_wdc.c      |   6 +-
 sys/dev/ata/atavar.h       |  25 ++++++++----
 sys/dev/ata/satapmp_subr.c |   8 ++--
 sys/dev/ata/wd.c           |  20 +++++----
 sys/dev/ic/ahcisata_core.c |   6 +-
 sys/dev/ic/mvsata.c        |  10 ++--
 sys/dev/ic/wdc.c           |  26 +++++-------
 sys/dev/scsipi/atapi_wdc.c |  10 ++--
 sys/dev/usb/umass_isdata.c |   8 ++--
 11 files changed, 109 insertions(+), 106 deletions(-)

diffs (truncated from 694 to 300 lines):

diff -r 3d1e8f49c4aa -r b744e795b607 sys/dev/ata/TODO.ncq
--- a/sys/dev/ata/TODO.ncq      Mon Jun 19 21:00:00 2017 +0000
+++ b/sys/dev/ata/TODO.ncq      Tue Jun 20 20:58:22 2017 +0000
@@ -22,14 +22,6 @@
 maybe do device error handling in not-interrupt-context (maybe this should be
 done on a mpata branch?)
 
-atabus(4) queue depth can only shrink, causing NCQ to not be available if NCQ
-drive rescaned after detach of non-NCQ drive
-- careful with PMP, must be minimum of openings supported by drives
-  attached to the same channel
-- the downsize can leak transfers with PMP if other device happens
-  to have active or pending transfers (e.g. when non-NCQ device is attached
-  while there is already NCQ device present)
-
 add mechanics to re-check queue when xfer is finished - needed on PMP
 and for IDE disks, when one drive uses up all available xfers nothing
 ever restarts queue for the other drives
diff -r 3d1e8f49c4aa -r b744e795b607 sys/dev/ata/ata.c
--- a/sys/dev/ata/ata.c Mon Jun 19 21:00:00 2017 +0000
+++ b/sys/dev/ata/ata.c Tue Jun 20 20:58:22 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ata.c,v 1.132.8.11 2017/06/19 21:00:00 jdolecek Exp $  */
+/*     $NetBSD: ata.c,v 1.132.8.12 2017/06/20 20:58:22 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.11 2017/06/19 21:00:00 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ata.c,v 1.132.8.12 2017/06/20 20:58:22 jdolecek Exp $");
 
 #include "opt_ata.h"
 
@@ -131,6 +131,7 @@
 
 static void ata_xfer_init(struct ata_xfer *, bool);
 static void ata_xfer_destroy(struct ata_xfer *);
+static void ata_channel_idle(struct ata_channel *);
 
 /*
  * atabus_init:
@@ -214,13 +215,14 @@
 
 /*
  * This interface is supposed only to be used when there is exactly
- * one outstanding command, and there is no information about the slot,
+ * one outstanding command, when there is no information about the slot,
  * which triggered the command. ata_queue_hwslot_to_xfer() interface
- * is preferred in all standard cases.
+ * is preferred in all NCQ cases.
  */
 struct ata_xfer *
-ata_queue_active_xfer_peek(struct ata_queue *chq)
+ata_queue_get_active_xfer(struct ata_queue *chq)
 {
+       KASSERT(chq->queue_active <= 1);
        return TAILQ_FIRST(&chq->active_xfers);
 }
 
@@ -246,13 +248,8 @@
        if (openings == 0)
                openings = 1;
 
-       /*
-        * While hw supports up to 32 tags, in practice we must never
-        * allow 32 active commands, since that would signal same as
-        * channel error. So just limit this to 31.
-        */
-       if (openings > 31)
-               openings = 31;
+       if (openings > ATA_MAX_OPENINGS)
+               openings = ATA_MAX_OPENINGS;
 
        struct ata_queue *chq = malloc(offsetof(struct ata_queue, queue_xfers[openings]),
            M_DEVBUF, M_WAITOK | M_ZERO);
@@ -268,17 +265,6 @@
        return chq;
 }
 
-static void
-ata_queue_downsize(struct ata_queue *chq, uint8_t openings)
-{
-       KASSERT(chq->queue_active == 0);
-       KASSERT(TAILQ_FIRST(&chq->queue_xfer) == NULL);
-       KASSERT(openings < chq->queue_openings);
-
-       chq->queue_openings = openings;
-       ata_queue_reset(chq);
-}
-
 void
 ata_queue_free(struct ata_queue *chq)
 {
@@ -588,13 +574,13 @@
                         * ata_reset_channel() will freeze 2 times, so
                         * unfreeze one time. Not a problem as we're at splbio
                         */
-                       chq->queue_freeze--;
+                       ata_channel_thaw(chp);
                        ata_reset_channel(chp, AT_WAIT | chp->ch_reset_flags);
                } else if (chq->queue_active > 0 && chq->queue_freeze == 1) {
                        /*
                         * Caller has bumped queue_freeze, decrease it.
                         */
-                       chq->queue_freeze--;
+                       ata_channel_thaw(chp);
                        u_int active __diagused = 0;
                        TAILQ_FOREACH(xfer, &chq->active_xfers, c_activechain) {
                                (*xfer->c_start)(xfer->c_chp, xfer);
@@ -1020,14 +1006,14 @@
  * freeze the queue and wait for the controller to be idle. Caller has to
  * unfreeze/restart the queue
  */
-void
-ata_queue_idle(struct ata_queue *queue)
+static void
+ata_channel_idle(struct ata_channel *chp)
 {
        int s = splbio();
-       queue->queue_freeze++;
-       while (queue->queue_active > 0) {
-               queue->queue_flags |= QF_IDLE_WAIT;
-               tsleep(&queue->queue_flags, PRIBIO, "qidl", 0);
+       ata_channel_freeze(chp);
+       while (chp->ch_queue->queue_active > 0) {
+               chp->ch_queue->queue_flags |= QF_IDLE_WAIT;
+               tsleep(&chp->ch_queue->queue_flags, PRIBIO, "qidl", 0);
        }
        splx(s);
 }
@@ -1160,18 +1146,28 @@
        xfer->c_start(chp, xfer);
 }
 
+/*
+ * Does it's own locking, does not require splbio().
+ * wait - whether to block waiting for free xfer
+ * openings - limit of openings supported by device, <= 0 means tag not
+ *     relevant, and any available xfer can be returned
+ */
 struct ata_xfer *
-ata_get_xfer(struct ata_channel *chp, bool wait)
+ata_get_xfer_ext(struct ata_channel *chp, bool wait, int8_t openings)
 {
        struct ata_queue *chq = chp->ch_queue;
        struct ata_xfer *xfer = NULL;
-       uint32_t avail, slot;
+       uint32_t avail, slot, mask;
        int error;
 
        mutex_enter(&chp->ch_lock);
 
 retry:
-       avail = ffs32(chq->queue_xfers_avail);
+       mask = (openings > 0)
+           ? (__BIT(MIN(ATA_MAX_OPENINGS, openings)) - 1)
+           : chq->queue_xfers_avail;
+
+       avail = ffs32(chq->queue_xfers_avail & mask);
        if (avail == 0) {
                if (wait) {
                        chq->queue_flags |= QF_NEED_XFER;
@@ -1356,6 +1352,18 @@
        splx(s);
 }
 
+void
+ata_channel_freeze(struct ata_channel *chp)
+{
+       chp->ch_queue->queue_freeze++;          /* XXX MPSAFE */
+}
+
+void
+ata_channel_thaw(struct ata_channel *chp)
+{
+       chp->ch_queue->queue_freeze--;          /* XXX MPSAFE */
+}
+
 /*
  * ata_reset_channel:
  *
@@ -1382,7 +1390,7 @@
        splx(spl1);
 #endif /* ATA_DEBUG */
 
-       chp->ch_queue->queue_freeze++;
+       ata_channel_freeze(chp);
 
        /*
         * If we can poll or wait it's OK, otherwise wake up the
@@ -1393,7 +1401,7 @@
        if ((flags & (AT_POLL | AT_WAIT)) == 0) {
                if (chp->ch_flags & ATACH_TH_RESET) {
                        /* No need to schedule a reset more than one time. */
-                       chp->ch_queue->queue_freeze--;
+                       ata_channel_thaw(chp);
                        return;
                }
                chp->ch_flags |= ATACH_TH_RESET;
@@ -1410,7 +1418,7 @@
 
        chp->ch_flags &= ~ATACH_TH_RESET;
        if ((flags & AT_RST_EMERG) == 0)  {
-               chp->ch_queue->queue_freeze--;
+               ata_channel_thaw(chp);
                atastart(chp);
        } else {
                /* make sure that we can use polled commands */
@@ -1831,8 +1839,6 @@
                        aprint_verbose(" w/PRIO");
                }
        }
-       if (drvp->drv_openings < chp->ch_queue->queue_openings)
-               ata_queue_downsize(chp->ch_queue, drvp->drv_openings);
        splx(s);
 
        if (printed)
@@ -1976,7 +1982,7 @@
        struct atabus_softc *sc = device_private(dv);
        struct ata_channel *chp = sc->sc_chan;
 
-       ata_queue_idle(chp->ch_queue);
+       ata_channel_idle(chp);
 
        return true;
 }
@@ -1999,7 +2005,7 @@
        }
        KASSERT(chp->ch_queue->queue_freeze > 0);
        /* unfreeze the queue and reset drives */
-       chp->ch_queue->queue_freeze--;
+       ata_channel_thaw(chp);
 
        /* reset channel only if there are drives attached */
        if (chp->ch_ndrives > 0)
diff -r 3d1e8f49c4aa -r b744e795b607 sys/dev/ata/ata_wdc.c
--- a/sys/dev/ata/ata_wdc.c     Mon Jun 19 21:00:00 2017 +0000
+++ b/sys/dev/ata/ata_wdc.c     Tue Jun 20 20:58:22 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ata_wdc.c,v 1.105.6.4 2017/06/16 20:40:49 jdolecek Exp $       */
+/*     $NetBSD: ata_wdc.c,v 1.105.6.5 2017/06/20 20:58:22 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.4 2017/06/16 20:40:49 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ata_wdc.c,v 1.105.6.5 2017/06/20 20:58:22 jdolecek Exp $");
 
 #include "opt_ata.h"
 #include "opt_wdc.h"
@@ -199,7 +199,7 @@
                /* If it's not a polled command, we need the kernel thread */
                if ((xfer->c_flags & C_POLL) == 0 &&
                    (chp->ch_flags & ATACH_TH_RUN) == 0) {
-                       chp->ch_queue->queue_freeze++;
+                       ata_channel_freeze(chp);
                        wakeup(&chp->ch_thread);
                        return;
                }
diff -r 3d1e8f49c4aa -r b744e795b607 sys/dev/ata/atavar.h
--- a/sys/dev/ata/atavar.h      Mon Jun 19 21:00:00 2017 +0000
+++ b/sys/dev/ata/atavar.h      Tue Jun 20 20:58:22 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: atavar.h,v 1.92.8.10 2017/06/19 21:00:00 jdolecek Exp $        */
+/*     $NetBSD: atavar.h,v 1.92.8.11 2017/06/20 20:58:22 jdolecek Exp $        */
 
 /*
  * Copyright (c) 1998, 2001 Manuel Bouyer.
@@ -183,16 +183,23 @@
 #define KILL_GONE 1 /* device is gone */
 #define KILL_RESET 2 /* xfer was reset */
 
+/*
+ * While hw supports up to 32 tags, in practice we must never
+ * allow 32 active commands, since that would signal same as
+ * channel error. So just limit this to 31.
+ */
+#define ATA_MAX_OPENINGS       31
+
 /* Per-channel queue of ata_xfers */
 struct ata_queue {
-       TAILQ_HEAD(, ata_xfer) queue_xfer;      /* queue of pending commands */
-       int queue_freeze;               /* freeze count for the queue */
        int8_t queue_flags;             /* flags for this queue */
 #define QF_IDLE_WAIT   0x01            /* someone wants the controller idle */
 #define QF_NEED_XFER   0x02            /* someone wants xfer */
        int8_t queue_active;            /* number of active transfers */
-       int8_t queue_openings;          /* max number of active transfers */
 #ifdef ATABUS_PRIVATE
+       int8_t queue_openings;                  /* max number of active xfers */
+       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 */
        TAILQ_HEAD(, ata_xfer) active_xfers;    /* active commands */



Home | Main Index | Thread Index | Old Index