Module Name: src
Committed By: jdolecek
Date: Thu Oct 20 19:20:40 UTC 2016
Modified Files:
src/sys/dev/ic: nvme.c nvmevar.h
Log Message:
revert change from rev. 1.12:
"""
slightly optimize memory access - change struct nvme_queue so that the
struct dmamem members are allocated as part of it, instead of separate
kmem_alloc()s
"""
that change quite curiously caused completion queue corruption on MP systems,
regardless of MPSAFE setting for the pci/softintr interrupt
To generate a diff of this commit:
cvs rdiff -u -r1.18 -r1.19 src/sys/dev/ic/nvme.c
cvs rdiff -u -r1.8 -r1.9 src/sys/dev/ic/nvmevar.h
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/dev/ic/nvme.c
diff -u src/sys/dev/ic/nvme.c:1.18 src/sys/dev/ic/nvme.c:1.19
--- src/sys/dev/ic/nvme.c:1.18 Wed Oct 19 19:34:31 2016
+++ src/sys/dev/ic/nvme.c Thu Oct 20 19:20:40 2016
@@ -1,4 +1,4 @@
-/* $NetBSD: nvme.c,v 1.18 2016/10/19 19:34:31 jdolecek Exp $ */
+/* $NetBSD: nvme.c,v 1.19 2016/10/20 19:20:40 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.18 2016/10/19 19:34:31 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: nvme.c,v 1.19 2016/10/20 19:20:40 jdolecek Exp $");
#include <sys/param.h>
#include <sys/systm.h>
@@ -85,9 +85,11 @@ static void nvme_q_submit(struct nvme_so
static int nvme_q_complete(struct nvme_softc *, struct nvme_queue *q);
static void nvme_q_free(struct nvme_softc *, struct nvme_queue *);
-static int nvme_dmamem_alloc(struct nvme_softc *, size_t,
- struct nvme_dmamem *);
+static struct nvme_dmamem *
+ nvme_dmamem_alloc(struct nvme_softc *, size_t);
static void nvme_dmamem_free(struct nvme_softc *, struct nvme_dmamem *);
+static void nvme_dmamem_sync(struct nvme_softc *, struct nvme_dmamem *,
+ int);
static void nvme_ns_io_fill(struct nvme_queue *, struct nvme_ccb *,
void *);
@@ -151,12 +153,8 @@ nvme_write8(struct nvme_softc *sc, bus_s
#endif
}
#endif /* __LP64__ */
-
#define nvme_barrier(_s, _r, _l, _f) \
bus_space_barrier((_s)->sc_iot, (_s)->sc_ioh, (_r), (_l), (_f))
-#define nvme_dmamem_sync(sc, mem, ops) \
- bus_dmamap_sync((sc)->sc_dmat, NVME_DMA_MAP(mem), \
- 0, NVME_DMA_LEN(mem), (ops));
static void
nvme_version(struct nvme_softc *sc, uint32_t ver)
@@ -568,19 +566,19 @@ nvme_ns_identify(struct nvme_softc *sc,
{
struct nvme_sqe sqe;
struct nvm_identify_namespace *identify;
- struct nvme_dmamem mem;
+ struct nvme_dmamem *mem;
struct nvme_ccb *ccb;
struct nvme_namespace *ns;
- int error;
+ int rv;
KASSERT(nsid > 0);
ccb = nvme_ccb_get(sc->sc_admin_q);
KASSERT(ccb != NULL); /* it's a bug if we don't have spare ccb here */
- error = nvme_dmamem_alloc(sc, sizeof(*identify), &mem);
- if (error)
- return error;
+ mem = nvme_dmamem_alloc(sc, sizeof(*identify));
+ if (mem == NULL)
+ return ENOMEM;
memset(&sqe, 0, sizeof(sqe));
sqe.opcode = NVM_ADMIN_IDENTIFY;
@@ -592,30 +590,30 @@ nvme_ns_identify(struct nvme_softc *sc,
ccb->ccb_cookie = &sqe;
nvme_dmamem_sync(sc, mem, BUS_DMASYNC_PREREAD);
- error = nvme_poll(sc, sc->sc_admin_q, ccb, nvme_sqe_fill,
- NVME_TIMO_IDENT);
+ rv = nvme_poll(sc, sc->sc_admin_q, ccb, nvme_sqe_fill, NVME_TIMO_IDENT);
nvme_dmamem_sync(sc, mem, BUS_DMASYNC_POSTREAD);
nvme_ccb_put(sc->sc_admin_q, ccb);
- if (error != 0) {
- error = EIO;
+ if (rv != 0) {
+ rv = EIO;
goto done;
}
/* commit */
identify = kmem_zalloc(sizeof(*identify), KM_SLEEP);
- memcpy(identify, NVME_DMA_KVA(mem), sizeof(*identify));
+ *identify = *((volatile struct nvm_identify_namespace *)NVME_DMA_KVA(mem));
+ //memcpy(identify, NVME_DMA_KVA(mem), sizeof(*identify));
ns = nvme_ns_get(sc, nsid);
KASSERT(ns);
ns->ident = identify;
done:
- nvme_dmamem_free(sc, &mem);
+ nvme_dmamem_free(sc, mem);
- return error;
+ return rv;
}
int
@@ -1108,29 +1106,29 @@ nvme_identify(struct nvme_softc *sc, u_i
{
char sn[41], mn[81], fr[17];
struct nvm_identify_controller *identify;
- struct nvme_dmamem mem;
+ struct nvme_dmamem *mem;
struct nvme_ccb *ccb;
u_int mdts;
- int error;
+ int rv = 1;
ccb = nvme_ccb_get(sc->sc_admin_q);
KASSERT(ccb != NULL); /* it's a bug if we don't have spare ccb here */
- error = nvme_dmamem_alloc(sc, sizeof(*identify), &mem);
- if (error)
- return error;
+ mem = nvme_dmamem_alloc(sc, sizeof(*identify));
+ if (mem == NULL)
+ return 1;
ccb->ccb_done = nvme_empty_done;
- ccb->ccb_cookie = &mem;
+ ccb->ccb_cookie = mem;
nvme_dmamem_sync(sc, mem, BUS_DMASYNC_PREREAD);
- error = nvme_poll(sc, sc->sc_admin_q, ccb, nvme_fill_identify,
+ rv = nvme_poll(sc, sc->sc_admin_q, ccb, nvme_fill_identify,
NVME_TIMO_IDENT);
nvme_dmamem_sync(sc, mem, BUS_DMASYNC_POSTREAD);
nvme_ccb_put(sc->sc_admin_q, ccb);
- if (error != 0)
+ if (rv != 0)
goto done;
identify = NVME_DMA_KVA(mem);
@@ -1155,9 +1153,9 @@ nvme_identify(struct nvme_softc *sc, u_i
memcpy(&sc->sc_identify, identify, sizeof(sc->sc_identify));
done:
- nvme_dmamem_free(sc, &mem);
+ nvme_dmamem_free(sc, mem);
- return error;
+ return rv;
}
static int
@@ -1260,7 +1258,7 @@ nvme_fill_identify(struct nvme_queue *q,
struct nvme_dmamem *mem = ccb->ccb_cookie;
sqe->opcode = NVM_ADMIN_IDENTIFY;
- htolem64(&sqe->entry.prp[0], NVME_DMA_DVA(*mem));
+ htolem64(&sqe->entry.prp[0], NVME_DMA_DVA(mem));
htolem32(&sqe->cdw10, 1);
}
@@ -1272,7 +1270,6 @@ nvme_ccbs_alloc(struct nvme_queue *q, u_
bus_addr_t off;
uint64_t *prpl;
u_int i;
- int error;
mutex_init(&q->q_ccb_mtx, MUTEX_DEFAULT, IPL_BIO);
SIMPLEQ_INIT(&q->q_ccb_list);
@@ -1282,12 +1279,8 @@ nvme_ccbs_alloc(struct nvme_queue *q, u_
return 1;
q->q_nccbs = nccbs;
- error = nvme_dmamem_alloc(sc, sizeof(*prpl) * sc->sc_max_sgl * nccbs,
- &q->q_ccb_prpls);
- if (error) {
- kmem_free(q->q_ccbs, sizeof(*ccb) * q->q_nccbs);
- return error;
- }
+ q->q_ccb_prpls = nvme_dmamem_alloc(sc,
+ sizeof(*prpl) * sc->sc_max_sgl * nccbs);
prpl = NVME_DMA_KVA(q->q_ccb_prpls);
off = 0;
@@ -1362,7 +1355,7 @@ nvme_ccbs_free(struct nvme_queue *q)
}
mutex_exit(&q->q_ccb_mtx);
- nvme_dmamem_free(sc, &q->q_ccb_prpls);
+ nvme_dmamem_free(sc, q->q_ccb_prpls);
kmem_free(q->q_ccbs, sizeof(*ccb) * q->q_nccbs);
q->q_ccbs = NULL;
mutex_destroy(&q->q_ccb_mtx);
@@ -1372,21 +1365,20 @@ static struct nvme_queue *
nvme_q_alloc(struct nvme_softc *sc, uint16_t id, u_int entries, u_int dstrd)
{
struct nvme_queue *q;
- int error;
q = kmem_alloc(sizeof(*q), KM_SLEEP);
if (q == NULL)
return NULL;
q->q_sc = sc;
- error = nvme_dmamem_alloc(sc, sizeof(struct nvme_sqe) * entries,
- &q->q_sq_dmamem);
- if (error)
+ q->q_sq_dmamem = nvme_dmamem_alloc(sc,
+ sizeof(struct nvme_sqe) * entries);
+ if (q->q_sq_dmamem == NULL)
goto free;
- error = nvme_dmamem_alloc(sc, sizeof(struct nvme_cqe) * entries,
- &q->q_cq_dmamem);
- if (error)
+ q->q_cq_dmamem = nvme_dmamem_alloc(sc,
+ sizeof(struct nvme_cqe) * entries);
+ if (q->q_cq_dmamem == NULL)
goto free_sq;
memset(NVME_DMA_KVA(q->q_sq_dmamem), 0, NVME_DMA_LEN(q->q_sq_dmamem));
@@ -1413,9 +1405,9 @@ nvme_q_alloc(struct nvme_softc *sc, uint
return q;
free_cq:
- nvme_dmamem_free(sc, &q->q_cq_dmamem);
+ nvme_dmamem_free(sc, q->q_cq_dmamem);
free_sq:
- nvme_dmamem_free(sc, &q->q_sq_dmamem);
+ nvme_dmamem_free(sc, q->q_sq_dmamem);
free:
kmem_free(q, sizeof(*q));
@@ -1430,8 +1422,8 @@ nvme_q_free(struct nvme_softc *sc, struc
mutex_destroy(&q->q_cq_mtx);
nvme_dmamem_sync(sc, q->q_cq_dmamem, BUS_DMASYNC_POSTREAD);
nvme_dmamem_sync(sc, q->q_sq_dmamem, BUS_DMASYNC_POSTWRITE);
- nvme_dmamem_free(sc, &q->q_cq_dmamem);
- nvme_dmamem_free(sc, &q->q_sq_dmamem);
+ nvme_dmamem_free(sc, q->q_cq_dmamem);
+ nvme_dmamem_free(sc, q->q_sq_dmamem);
kmem_free(q, sizeof(*q));
}
@@ -1499,12 +1491,16 @@ nvme_softintr_msi(void *xq)
nvme_q_complete(sc, q);
}
-static int
-nvme_dmamem_alloc(struct nvme_softc *sc, size_t size, struct nvme_dmamem *ndm)
+static struct nvme_dmamem *
+nvme_dmamem_alloc(struct nvme_softc *sc, size_t size)
{
+ struct nvme_dmamem *ndm;
int nsegs;
- memset(ndm, 0, sizeof(*ndm));
+ ndm = kmem_zalloc(sizeof(*ndm), KM_SLEEP);
+ if (ndm == NULL)
+ return NULL;
+
ndm->ndm_size = size;
if (bus_dmamap_create(sc->sc_dmat, size, 1, size, 0,
@@ -1524,7 +1520,7 @@ nvme_dmamem_alloc(struct nvme_softc *sc,
NULL, BUS_DMA_WAITOK) != 0)
goto unmap;
- return 0;
+ return ndm;
unmap:
bus_dmamem_unmap(sc->sc_dmat, ndm->ndm_kva, size);
@@ -1533,7 +1529,15 @@ free:
destroy:
bus_dmamap_destroy(sc->sc_dmat, ndm->ndm_map);
ndmfree:
- return ENOMEM;
+ kmem_free(ndm, sizeof(*ndm));
+ return NULL;
+}
+
+static void
+nvme_dmamem_sync(struct nvme_softc *sc, struct nvme_dmamem *mem, int ops)
+{
+ bus_dmamap_sync(sc->sc_dmat, NVME_DMA_MAP(mem),
+ 0, NVME_DMA_LEN(mem), ops);
}
void
@@ -1543,6 +1547,7 @@ nvme_dmamem_free(struct nvme_softc *sc,
bus_dmamem_unmap(sc->sc_dmat, ndm->ndm_kva, ndm->ndm_size);
bus_dmamem_free(sc->sc_dmat, &ndm->ndm_seg, 1);
bus_dmamap_destroy(sc->sc_dmat, ndm->ndm_map);
+ kmem_free(ndm, sizeof(*ndm));
}
/*
Index: src/sys/dev/ic/nvmevar.h
diff -u src/sys/dev/ic/nvmevar.h:1.8 src/sys/dev/ic/nvmevar.h:1.9
--- src/sys/dev/ic/nvmevar.h:1.8 Wed Oct 19 19:34:31 2016
+++ src/sys/dev/ic/nvmevar.h Thu Oct 20 19:20:40 2016
@@ -1,4 +1,4 @@
-/* $NetBSD: nvmevar.h,v 1.8 2016/10/19 19:34:31 jdolecek Exp $ */
+/* $NetBSD: nvmevar.h,v 1.9 2016/10/20 19:20:40 jdolecek Exp $ */
/* $OpenBSD: nvmevar.h,v 1.8 2016/04/14 11:18:32 dlg Exp $ */
/*
@@ -30,10 +30,10 @@ struct nvme_dmamem {
size_t ndm_size;
void *ndm_kva;
};
-#define NVME_DMA_MAP(_ndm) ((_ndm).ndm_map)
-#define NVME_DMA_LEN(_ndm) ((_ndm).ndm_map->dm_segs[0].ds_len)
-#define NVME_DMA_DVA(_ndm) ((uint64_t)(_ndm).ndm_map->dm_segs[0].ds_addr)
-#define NVME_DMA_KVA(_ndm) ((void *)(_ndm).ndm_kva)
+#define NVME_DMA_MAP(_ndm) ((_ndm)->ndm_map)
+#define NVME_DMA_LEN(_ndm) ((_ndm)->ndm_map->dm_segs[0].ds_len)
+#define NVME_DMA_DVA(_ndm) ((uint64_t)(_ndm)->ndm_map->dm_segs[0].ds_addr)
+#define NVME_DMA_KVA(_ndm) ((void *)(_ndm)->ndm_kva)
struct nvme_softc;
struct nvme_queue;
@@ -75,8 +75,8 @@ struct nvme_queue {
struct nvme_softc *q_sc;
kmutex_t q_sq_mtx;
kmutex_t q_cq_mtx;
- struct nvme_dmamem q_sq_dmamem;
- struct nvme_dmamem q_cq_dmamem;
+ struct nvme_dmamem *q_sq_dmamem;
+ struct nvme_dmamem *q_cq_dmamem;
bus_size_t q_sqtdbl; /* submission queue tail doorbell */
bus_size_t q_cqhdbl; /* completion queue head doorbell */
uint16_t q_id;
@@ -89,7 +89,7 @@ struct nvme_queue {
u_int q_nccbs;
struct nvme_ccb *q_ccbs;
SIMPLEQ_HEAD(, nvme_ccb) q_ccb_list;
- struct nvme_dmamem q_ccb_prpls;
+ struct nvme_dmamem *q_ccb_prpls;
};
struct nvme_namespace {
