[src/trunk]: src/sys/dev/ic stop using q_nccbs_avail for deciding whether the...

[jdolecek <jdolecek%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/e2199aab44cb
branches:  trunk
changeset: 321450:e2199aab44cb
user:      jdolecek <jdolecek%NetBSD.org@localhost>
date:      Fri Mar 16 18:49:18 2018 +0000

description:
stop using q_nccbs_avail for deciding whether there are available ccbs;
no need to maintain a counter _and_ q_ccb_list

this fixes deadlock when all ccbs happen to be taken before completion
interrupt - nvme_q_complete() increased q_nccbs_avail only after
processing all the completed commands, by then there was nothing
left to actually kick the disk queue again into action

this also fixes ccb leak on command errors e.g. with bus_dmamem_alloc()
or bus_dmamel_load() - q_nccbs_avail was never decreased on the error path

fixes PR kern/52769 by Martin Husemann, thanks to Paul Goyette
for testing

diffstat:

 sys/dev/ic/nvme.c    |  18 ++++--------------
 sys/dev/ic/nvmevar.h |   3 +--
 2 files changed, 5 insertions(+), 16 deletions(-)

diffs (70 lines):

diff -r 231b6ec74897 -r e2199aab44cb sys/dev/ic/nvme.c
--- a/sys/dev/ic/nvme.c Fri Mar 16 17:56:31 2018 +0000
+++ b/sys/dev/ic/nvme.c Fri Mar 16 18:49:18 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: nvme.c,v 1.32 2018/02/27 12:59:53 christos Exp $       */
+/*     $NetBSD: nvme.c,v 1.33 2018/03/16 18:49:18 jdolecek 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.32 2018/02/27 12:59:53 christos Exp $");
+__KERNEL_RCSID(0, "$NetBSD: nvme.c,v 1.33 2018/03/16 18:49:18 jdolecek Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -1149,12 +1149,6 @@
 
        mutex_exit(&q->q_cq_mtx);
 
-       if (rv) {
-               mutex_enter(&q->q_ccb_mtx);
-               q->q_nccbs_avail += rv;
-               mutex_exit(&q->q_ccb_mtx);
-       }
-
        return rv;
 }
 
@@ -1367,7 +1361,6 @@
        q->q_ccbs = kmem_alloc(sizeof(*ccb) * nccbs, KM_SLEEP);
 
        q->q_nccbs = nccbs;
-       q->q_nccbs_avail = nccbs;
        q->q_ccb_prpls = nvme_dmamem_alloc(sc,
            sizeof(*prpl) * sc->sc_max_sgl * nccbs);
 
@@ -1407,11 +1400,8 @@
        struct nvme_ccb *ccb = NULL;
 
        mutex_enter(&q->q_ccb_mtx);
-       if (q->q_nccbs_avail > 0) {
-               ccb = SIMPLEQ_FIRST(&q->q_ccb_list);
-               KASSERT(ccb != NULL);
-               q->q_nccbs_avail--;
-
+       ccb = SIMPLEQ_FIRST(&q->q_ccb_list);
+       if (ccb != NULL) {
                SIMPLEQ_REMOVE_HEAD(&q->q_ccb_list, ccb_entry);
 #ifdef DEBUG
                ccb->ccb_cookie = NULL;
diff -r 231b6ec74897 -r e2199aab44cb sys/dev/ic/nvmevar.h
--- a/sys/dev/ic/nvmevar.h      Fri Mar 16 17:56:31 2018 +0000
+++ b/sys/dev/ic/nvmevar.h      Fri Mar 16 18:49:18 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: nvmevar.h,v 1.13 2017/04/05 20:15:49 jdolecek Exp $    */
+/*     $NetBSD: nvmevar.h,v 1.14 2018/03/16 18:49:18 jdolecek Exp $    */
 /*     $OpenBSD: nvmevar.h,v 1.8 2016/04/14 11:18:32 dlg Exp $ */
 
 /*
@@ -88,7 +88,6 @@
 
        kmutex_t                q_ccb_mtx;
        uint16_t                q_nccbs;        /* total number of ccbs */
-       uint16_t                q_nccbs_avail;  /* available ccbs */
        struct nvme_ccb         *q_ccbs;
        SIMPLEQ_HEAD(, nvme_ccb) q_ccb_list;
        struct nvme_dmamem      *q_ccb_prpls;