Source-Changes-HG archive

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

[src/netbsd-8]: src/sys/dev/ic Pull up following revision(s) (requested by jd...



details:   https://anonhg.NetBSD.org/src/rev/ca2cbdbe1b87
branches:  netbsd-8
changeset: 434759:ca2cbdbe1b87
user:      martin <martin%NetBSD.org@localhost>
date:      Sun Mar 18 11:05:27 2018 +0000

description:
Pull up following revision(s) (requested by jdolecek in ticket #641):
        sys/dev/ic/nvme.c: revision 1.34
        sys/dev/ic/nvme.c: revision 1.35
        sys/dev/ic/nvme.c: revision 1.36
        sys/dev/ic/nvme.c: revision 1.37
        sys/dev/ic/ld_nvme.c: revision 1.19
        sys/dev/ic/nvmevar.h: revision 1.15

refactor the locking code around DIOCGCACHE handling to be reusable
for other infrequent commands,it uses single condvar for simplicity,
and uses it both when waiting for ccb or command completion - this
is fine, since usually there will be just one such command qeueued anyway
use this to finally properly implement DIOCCACHESYNC - return only after
the command is confirmed as completed by the controller.

switch handling of passthrough commands to use queue, instead of polling
should fix PR kern/53059 by Frank Kardel

fix passthrough command usage also in nvme_get_number_of_queues(), fixes
memory corruption and possible panic on boot

also remove now duplicate nvme_ccb_put() call from
nvme_get_number_of_queues()

diffstat:

 sys/dev/ic/ld_nvme.c |  124 +-----------------------
 sys/dev/ic/nvme.c    |  254 ++++++++++++++++++++++++++++++++++++++++----------
 sys/dev/ic/nvmevar.h |    9 +-
 3 files changed, 211 insertions(+), 176 deletions(-)

diffs (truncated from 750 to 300 lines):

diff -r e82382a236e8 -r ca2cbdbe1b87 sys/dev/ic/ld_nvme.c
--- a/sys/dev/ic/ld_nvme.c      Sun Mar 18 11:01:00 2018 +0000
+++ b/sys/dev/ic/ld_nvme.c      Sun Mar 18 11:05:27 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ld_nvme.c,v 1.16.2.1 2017/09/01 09:59:11 martin Exp $  */
+/*     $NetBSD: ld_nvme.c,v 1.16.2.2 2018/03/18 11:05:27 martin Exp $  */
 
 /*-
  * Copyright (C) 2016 NONAKA Kimihiro <nonaka%netbsd.org@localhost>
@@ -26,7 +26,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ld_nvme.c,v 1.16.2.1 2017/09/01 09:59:11 martin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ld_nvme.c,v 1.16.2.2 2018/03/18 11:05:27 martin Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -49,14 +49,6 @@
        struct nvme_softc       *sc_nvme;
 
        uint16_t                sc_nsid;
-
-       /* getcache handling */
-       kmutex_t                sc_getcache_lock;
-       kcondvar_t              sc_getcache_cv;
-       kcondvar_t              sc_getcache_ready_cv;
-       bool                    sc_getcache_waiting;
-       bool                    sc_getcache_ready;
-       int                     sc_getcache_result;
 };
 
 static int     ld_nvme_match(device_t, cfdata_t, void *);
@@ -73,8 +65,6 @@
 static int     ld_nvme_ioctl(struct ld_softc *, u_long, void *, int32_t, bool);
 
 static void    ld_nvme_biodone(void *, struct buf *, uint16_t, uint32_t);
-static void    ld_nvme_syncdone(void *, struct buf *, uint16_t, uint32_t);
-static void    ld_nvme_getcache_done(void *, struct buf *, uint16_t, uint32_t);
 
 static int
 ld_nvme_match(device_t parent, cfdata_t match, void *aux)
@@ -103,10 +93,6 @@
        sc->sc_nvme = nsc;
        sc->sc_nsid = naa->naa_nsid;
 
-       mutex_init(&sc->sc_getcache_lock, MUTEX_DEFAULT, IPL_SOFTBIO);
-       cv_init(&sc->sc_getcache_cv, "nvmegcq");
-       cv_init(&sc->sc_getcache_ready_cv, "nvmegcr");
-
        aprint_naive("\n");
        aprint_normal("\n");
 
@@ -203,116 +189,16 @@
 {
        struct ld_nvme_softc *sc = device_private(ld->sc_dv);
 
-       if (!nvme_has_volatile_write_cache(sc->sc_nvme)) {
-               /* cache not present, no value in trying to flush it */
-               return 0;
-       }
-
-       return nvme_ns_sync(sc->sc_nvme, sc->sc_nsid, sc,
-           poll ? NVME_NS_CTX_F_POLL : 0,
-           ld_nvme_syncdone);
-}
-
-static void
-ld_nvme_syncdone(void *xc, struct buf *bp, uint16_t cmd_status, uint32_t cdw0)
-{
-       /* nothing to do */
+       return nvme_ns_sync(sc->sc_nvme, sc->sc_nsid,
+           poll ? NVME_NS_CTX_F_POLL : 0);
 }
 
 static int
 ld_nvme_getcache(struct ld_softc *ld, int *addr)
 {
-       int error;
        struct ld_nvme_softc *sc = device_private(ld->sc_dv);
 
-       /*
-        * DPO not supported, Dataset Management (DSM) field doesn't specify
-        * the same semantics.
-        */ 
-       *addr = DKCACHE_FUA;
-
-       if (!nvme_has_volatile_write_cache(sc->sc_nvme)) {
-               /* cache simply not present */
-               return 0;
-       }
-
-       /*
-        * This is admin queue request. The queue is relatively limited in size,
-        * and this is not performance critical call, so have at most one pending
-        * cache request at a time to avoid spurious EWOULDBLOCK failures.
-        */ 
-       mutex_enter(&sc->sc_getcache_lock);
-       while (sc->sc_getcache_waiting) {
-               error = cv_wait_sig(&sc->sc_getcache_cv, &sc->sc_getcache_lock);
-               if (error)
-                       goto out;
-       }
-       sc->sc_getcache_waiting = true;
-       sc->sc_getcache_ready = false;
-       mutex_exit(&sc->sc_getcache_lock);
-
-       error = nvme_admin_getcache(sc->sc_nvme, sc, ld_nvme_getcache_done);
-       if (error) {
-               mutex_enter(&sc->sc_getcache_lock);
-               goto out;
-       }
-
-       mutex_enter(&sc->sc_getcache_lock);
-       while (!sc->sc_getcache_ready) {
-               error = cv_wait_sig(&sc->sc_getcache_ready_cv,
-                   &sc->sc_getcache_lock);
-               if (error)
-                       goto out;
-       }
-
-       KDASSERT(sc->sc_getcache_ready);
-
-       if (sc->sc_getcache_result >= 0)
-               *addr |= sc->sc_getcache_result;
-       else
-               error = EINVAL;
-
-    out:
-       sc->sc_getcache_waiting = false;
-
-       /* wake one of eventual waiters */
-       cv_signal(&sc->sc_getcache_cv);
-
-       mutex_exit(&sc->sc_getcache_lock);
-
-       return error;
-}
-
-static void
-ld_nvme_getcache_done(void *xc, struct buf *bp, uint16_t cmd_status, uint32_t cdw0)
-{
-       struct ld_nvme_softc *sc = xc;
-       uint16_t status = NVME_CQE_SC(cmd_status);
-       int result;
-
-       if (status == NVME_CQE_SC_SUCCESS) {
-               result = 0;
-
-               if (cdw0 & NVME_CQE_CDW0_VWC_WCE)
-                       result |= DKCACHE_WRITE;
-
-               /*
-                * If volatile write cache is present, the flag shall also be
-                * settable.
-                */
-               result |= DKCACHE_WCHANGE;
-       } else {
-               result = -1;
-       }
-
-       mutex_enter(&sc->sc_getcache_lock);
-       sc->sc_getcache_result = result;
-       sc->sc_getcache_ready = true;
-
-       /* wake up the waiter */
-       cv_signal(&sc->sc_getcache_ready_cv);
-
-       mutex_exit(&sc->sc_getcache_lock);
+       return nvme_admin_getcache(sc->sc_nvme, addr);
 }
 
 static int
diff -r e82382a236e8 -r ca2cbdbe1b87 sys/dev/ic/nvme.c
--- a/sys/dev/ic/nvme.c Sun Mar 18 11:01:00 2018 +0000
+++ b/sys/dev/ic/nvme.c Sun Mar 18 11:05:27 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: nvme.c,v 1.30.2.1 2018/03/17 08:11:18 martin Exp $     */
+/*     $NetBSD: nvme.c,v 1.30.2.2 2018/03/18 11:05:27 martin Exp $     */
 /*     $OpenBSD: nvme.c,v 1.49 2016/04/18 05:59:50 dlg Exp $ */
 
 /*
@@ -18,7 +18,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: nvme.c,v 1.30.2.1 2018/03/17 08:11:18 martin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: nvme.c,v 1.30.2.2 2018/03/18 11:05:27 martin Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -61,7 +61,7 @@
 static void    nvme_ccbs_free(struct nvme_queue *);
 
 static struct nvme_ccb *
-               nvme_ccb_get(struct nvme_queue *);
+               nvme_ccb_get(struct nvme_queue *, bool);
 static void    nvme_ccb_put(struct nvme_queue *, struct nvme_ccb *);
 
 static int     nvme_poll(struct nvme_softc *, struct nvme_queue *,
@@ -83,6 +83,8 @@
                    struct nvme_ccb *, void *));
 static int     nvme_q_complete(struct nvme_softc *, struct nvme_queue *q);
 static void    nvme_q_free(struct nvme_softc *, struct nvme_queue *);
+static void    nvme_q_wait_complete(struct nvme_softc *, struct nvme_queue *,
+                   bool (*)(void *), void *);
 
 static struct nvme_dmamem *
                nvme_dmamem_alloc(struct nvme_softc *, size_t);
@@ -564,7 +566,7 @@
 
        KASSERT(nsid > 0);
 
-       ccb = nvme_ccb_get(sc->sc_admin_q);
+       ccb = nvme_ccb_get(sc->sc_admin_q, false);
        KASSERT(ccb != NULL); /* it's a bug if we don't have spare ccb here */
 
        mem = nvme_dmamem_alloc(sc, sizeof(*identify));
@@ -617,7 +619,7 @@
        bus_dmamap_t dmap;
        int i, error;
 
-       ccb = nvme_ccb_get(q);
+       ccb = nvme_ccb_get(q, false);
        if (ccb == NULL)
                return EAGAIN;
 
@@ -736,31 +738,44 @@
  * If there is no volatile write cache, it makes no sense to issue
  * flush commands or query for the status.
  */
-bool
+static bool
 nvme_has_volatile_write_cache(struct nvme_softc *sc)
 {
        /* sc_identify is filled during attachment */
        return  ((sc->sc_identify.vwc & NVME_ID_CTRLR_VWC_PRESENT) != 0);
 }
 
+static bool
+nvme_ns_sync_finished(void *cookie)
+{
+       int *result = cookie;
+
+       return (*result != 0);
+}
+
 int
-nvme_ns_sync(struct nvme_softc *sc, uint16_t nsid, void *cookie,
-    int flags, nvme_nnc_done nnc_done)
+nvme_ns_sync(struct nvme_softc *sc, uint16_t nsid, int flags)
 {
        struct nvme_queue *q = nvme_get_q(sc);
        struct nvme_ccb *ccb;
+       int result = 0;
 
-       ccb = nvme_ccb_get(q);
+       if (!nvme_has_volatile_write_cache(sc)) {
+               /* cache not present, no value in trying to flush it */
+               return 0;
+       }
+
+       ccb = nvme_ccb_get(q, true);
        if (ccb == NULL)
                return EAGAIN;
 
        ccb->ccb_done = nvme_ns_sync_done;
-       ccb->ccb_cookie = cookie;
+       ccb->ccb_cookie = &result;
 
        /* namespace context */
        ccb->nnc_nsid = nsid;
        ccb->nnc_flags = flags;
-       ccb->nnc_done = nnc_done;
+       ccb->nnc_done = NULL;
 
        if (ISSET(flags, NVME_NS_CTX_F_POLL)) {
                if (nvme_poll(sc, q, ccb, nvme_ns_sync_fill, NVME_TIMO_SY) != 0)
@@ -769,7 +784,12 @@
        }
 
        nvme_q_submit(sc, q, ccb, nvme_ns_sync_fill);
-       return 0;
+
+       /* wait for completion */
+       nvme_q_wait_complete(sc, q, nvme_ns_sync_finished, &result);
+       KASSERT(result != 0);
+
+       return (result > 0) ? 0 : EIO;
 }
 
 static void
@@ -785,36 +805,64 @@
 nvme_ns_sync_done(struct nvme_queue *q, struct nvme_ccb *ccb,
     struct nvme_cqe *cqe)
 {
-       void *cookie = ccb->ccb_cookie;
-       nvme_nnc_done nnc_done = ccb->nnc_done;
+       int *result = ccb->ccb_cookie;
+       uint16_t status = NVME_CQE_SC(lemtoh16(&cqe->flags));



Home | Main Index | Thread Index | Old Index