Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 18 Oct 2012 00:37:11 +0000 (UTC)
From:      Jim Harris <jimharris@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r241658 - head/sys/dev/nvme
Message-ID:  <201210180037.q9I0bBLQ000511@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: jimharris
Date: Thu Oct 18 00:37:11 2012
New Revision: 241658
URL: http://svn.freebsd.org/changeset/base/241658

Log:
  Merge struct nvme_prp_list into struct nvme_tracker.
  
  This simplifies the driver significantly where it is constructing
  commands to be submitted to hardware.  By reducing the number of
  PRPs (NVMe parlance for SGE) from 128 to 32, it ensures we do not
  allocate too much memory for more common smaller I/O sizes, while
  still supporting up to 128KB I/O sizes.
  
  This also paves the way for pre-allocation of nvme_tracker objects
  for each queue which will simplify the I/O path even further.
  
  Sponsored by:	Intel

Modified:
  head/sys/dev/nvme/nvme.c
  head/sys/dev/nvme/nvme_ctrlr_cmd.c
  head/sys/dev/nvme/nvme_ns_cmd.c
  head/sys/dev/nvme/nvme_private.h
  head/sys/dev/nvme/nvme_qpair.c
  head/sys/dev/nvme/nvme_sysctl.c
  head/sys/dev/nvme/nvme_uio.c

Modified: head/sys/dev/nvme/nvme.c
==============================================================================
--- head/sys/dev/nvme/nvme.c	Thu Oct 18 00:32:07 2012	(r241657)
+++ head/sys/dev/nvme/nvme.c	Thu Oct 18 00:37:11 2012	(r241658)
@@ -211,7 +211,6 @@ nvme_payload_map(void *arg, bus_dma_segm
 {
 	struct nvme_tracker 	*tr;
 	struct nvme_qpair 	*qpair;
-	struct nvme_prp_list	*prp_list;
 	uint32_t		cur_nseg;
 
 	KASSERT(error == 0, ("nvme_payload_map error != 0\n"));
@@ -230,13 +229,10 @@ nvme_payload_map(void *arg, bus_dma_segm
 	if (nseg == 2) {
 		tr->cmd.prp2 = seg[1].ds_addr;
 	} else if (nseg > 2) {
-		KASSERT(tr->prp_list,
-		    ("prp_list needed but not attached to tracker\n"));
 		cur_nseg = 1;
-		prp_list = tr->prp_list;
-		tr->cmd.prp2 = (uint64_t)prp_list->bus_addr;
+		tr->cmd.prp2 = (uint64_t)tr->prp_bus_addr;
 		while (cur_nseg < nseg) {
-			prp_list->prp[cur_nseg-1] =
+			tr->prp[cur_nseg-1] =
 			    (uint64_t)seg[cur_nseg].ds_addr;
 			cur_nseg++;
 		}
@@ -251,8 +247,6 @@ nvme_allocate_tracker(struct nvme_contro
 {
 	struct nvme_tracker 	*tr;
 	struct nvme_qpair	*qpair;
-	uint32_t 		modulo, offset, num_prps;
-	boolean_t		alloc_prp_list = FALSE;
 
 	if (is_admin) {
 		qpair = &ctrlr->adminq;
@@ -263,17 +257,7 @@ nvme_allocate_tracker(struct nvme_contro
 			qpair = &ctrlr->ioq[0];
 	}
 
-	num_prps = payload_size / PAGE_SIZE;
-	modulo = payload_size % PAGE_SIZE;
-	offset = (uint32_t)((uintptr_t)payload % PAGE_SIZE);
-
-	if (modulo || offset)
-		num_prps += 1 + (modulo + offset - 1) / PAGE_SIZE;
-
-	if (num_prps > 2)
-		alloc_prp_list = TRUE;
-
-	tr = nvme_qpair_allocate_tracker(qpair, alloc_prp_list);
+	tr = nvme_qpair_allocate_tracker(qpair);
 
 	if (tr == NULL)
 		return (NULL);

Modified: head/sys/dev/nvme/nvme_ctrlr_cmd.c
==============================================================================
--- head/sys/dev/nvme/nvme_ctrlr_cmd.c	Thu Oct 18 00:32:07 2012	(r241657)
+++ head/sys/dev/nvme/nvme_ctrlr_cmd.c	Thu Oct 18 00:37:11 2012	(r241658)
@@ -49,7 +49,7 @@ nvme_ctrlr_cmd_identify_controller(struc
 	 */
 	cmd->cdw10 = 1;
 
-	err = bus_dmamap_load(tr->qpair->dma_tag, tr->dma_map, payload,
+	err = bus_dmamap_load(tr->qpair->dma_tag, tr->payload_dma_map, payload,
 	    tr->payload_size, nvme_payload_map, tr, 0);
 
 	KASSERT(err == 0, ("bus_dmamap_load returned non-zero!\n"));
@@ -74,7 +74,7 @@ nvme_ctrlr_cmd_identify_namespace(struct
 	 */
 	cmd->nsid = nsid;
 
-	err = bus_dmamap_load(tr->qpair->dma_tag, tr->dma_map, payload,
+	err = bus_dmamap_load(tr->qpair->dma_tag, tr->payload_dma_map, payload,
 	    tr->payload_size, nvme_payload_map, tr, 0);
 
 	KASSERT(err == 0, ("bus_dmamap_load returned non-zero!\n"));
@@ -189,8 +189,8 @@ nvme_ctrlr_cmd_set_feature(struct nvme_c
 	cmd->cdw11 = cdw11;
 
 	if (payload_size > 0) {
-		err = bus_dmamap_load(tr->qpair->dma_tag, tr->dma_map, payload,
-		    payload_size, nvme_payload_map, tr, 0);
+		err = bus_dmamap_load(tr->qpair->dma_tag, tr->payload_dma_map,
+		    payload, payload_size, nvme_payload_map, tr, 0);
 
 		KASSERT(err == 0, ("bus_dmamap_load returned non-zero!\n"));
 	} else
@@ -215,8 +215,8 @@ nvme_ctrlr_cmd_get_feature(struct nvme_c
 	cmd->cdw11 = cdw11;
 
 	if (payload_size > 0) {
-		err = bus_dmamap_load(tr->qpair->dma_tag, tr->dma_map, payload,
-		    payload_size, nvme_payload_map, tr, 0);
+		err = bus_dmamap_load(tr->qpair->dma_tag, tr->payload_dma_map,
+		    payload, payload_size, nvme_payload_map, tr, 0);
 
 		KASSERT(err == 0, ("bus_dmamap_load returned non-zero!\n"));
 	} else
@@ -305,7 +305,7 @@ nvme_ctrlr_cmd_get_health_information_pa
 	cmd->cdw10 = ((sizeof(*payload)/sizeof(uint32_t)) - 1) << 16;
 	cmd->cdw10 |= NVME_LOG_HEALTH_INFORMATION;
 
-	err = bus_dmamap_load(tr->qpair->dma_tag, tr->dma_map, payload,
+	err = bus_dmamap_load(tr->qpair->dma_tag, tr->payload_dma_map, payload,
 	    sizeof(*payload), nvme_payload_map, tr, 0);
 
 	KASSERT(err == 0, ("bus_dmamap_load returned non-zero!\n"));

Modified: head/sys/dev/nvme/nvme_ns_cmd.c
==============================================================================
--- head/sys/dev/nvme/nvme_ns_cmd.c	Thu Oct 18 00:32:07 2012	(r241657)
+++ head/sys/dev/nvme/nvme_ns_cmd.c	Thu Oct 18 00:37:11 2012	(r241658)
@@ -51,7 +51,7 @@ nvme_ns_cmd_read(struct nvme_namespace *
 	*(uint64_t *)&cmd->cdw10 = lba;
 	cmd->cdw12 = lba_count-1;
 
-	err = bus_dmamap_load(tr->qpair->dma_tag, tr->dma_map, payload,
+	err = bus_dmamap_load(tr->qpair->dma_tag, tr->payload_dma_map, payload,
 	    tr->payload_size, nvme_payload_map, tr, 0);
 
 	KASSERT(err == 0, ("bus_dmamap_load returned non-zero!\n"));
@@ -81,7 +81,7 @@ nvme_ns_cmd_write(struct nvme_namespace 
 	*(uint64_t *)&cmd->cdw10 = lba;
 	cmd->cdw12 = lba_count-1;
 
-	err = bus_dmamap_load(tr->qpair->dma_tag, tr->dma_map, payload,
+	err = bus_dmamap_load(tr->qpair->dma_tag, tr->payload_dma_map, payload,
 	    tr->payload_size, nvme_payload_map, tr, 0);
 
 	KASSERT(err == 0, ("bus_dmamap_load returned non-zero!\n"));
@@ -111,7 +111,7 @@ nvme_ns_cmd_deallocate(struct nvme_names
 	cmd->cdw10 = num_ranges;
 	cmd->cdw11 = NVME_DSM_ATTR_DEALLOCATE;
 
-	err = bus_dmamap_load(tr->qpair->dma_tag, tr->dma_map, payload,
+	err = bus_dmamap_load(tr->qpair->dma_tag, tr->payload_dma_map, payload,
 	    tr->payload_size, nvme_payload_map, tr, 0);
 
 	KASSERT(err == 0, ("bus_dmamap_load returned non-zero!\n"));

Modified: head/sys/dev/nvme/nvme_private.h
==============================================================================
--- head/sys/dev/nvme/nvme_private.h	Thu Oct 18 00:32:07 2012	(r241657)
+++ head/sys/dev/nvme/nvme_private.h	Thu Oct 18 00:37:11 2012	(r241658)
@@ -55,14 +55,14 @@ MALLOC_DECLARE(M_NVME);
 
 #define IDT_PCI_ID		0x80d0111d
 
-#define NVME_MAX_PRP_LIST_ENTRIES	(128)
+#define NVME_MAX_PRP_LIST_ENTRIES	(32)
 
 /*
  * For commands requiring more than 2 PRP entries, one PRP will be
  *  embedded in the command (prp1), and the rest of the PRP entries
  *  will be in a list pointed to by the command (prp2).  This means
- *  that real max number of PRP entries we support is 128+1, which
- *  results in a max xfer size of 128*PAGE_SIZE.
+ *  that real max number of PRP entries we support is 32+1, which
+ *  results in a max xfer size of 32*PAGE_SIZE.
  */
 #define NVME_MAX_XFER_SIZE	NVME_MAX_PRP_LIST_ENTRIES * PAGE_SIZE
 
@@ -92,25 +92,21 @@ MALLOC_DECLARE(M_NVME);
 #define CACHE_LINE_SIZE		(64)
 #endif
 
-struct nvme_prp_list {
-	uint64_t			prp[NVME_MAX_PRP_LIST_ENTRIES];
-	SLIST_ENTRY(nvme_prp_list)	slist;
-	bus_addr_t			bus_addr;
-	bus_dmamap_t			dma_map;
-};
-
 struct nvme_tracker {
 
 	SLIST_ENTRY(nvme_tracker)	slist;
 	struct nvme_qpair		*qpair;
 	struct nvme_command		cmd;
 	struct callout			timer;
-	bus_dmamap_t			dma_map;
+	bus_dmamap_t			payload_dma_map;
 	nvme_cb_fn_t			cb_fn;
 	void				*cb_arg;
 	uint32_t			payload_size;
-	struct nvme_prp_list		*prp_list;
 	uint16_t			cid;
+
+	uint64_t			prp[NVME_MAX_PRP_LIST_ENTRIES];
+	bus_addr_t			prp_bus_addr;
+	bus_dmamap_t			prp_dma_map;
 };
 
 struct nvme_qpair {
@@ -148,14 +144,11 @@ struct nvme_qpair {
 	uint64_t		cpl_bus_addr;
 
 	uint32_t		num_tr;
-	uint32_t		num_prp_list;
 
 	SLIST_HEAD(, nvme_tracker)	free_tr;
 
 	struct nvme_tracker	**act_tr;
 
-	SLIST_HEAD(, nvme_prp_list)	free_prp_list;
-
 	struct mtx		lock __aligned(CACHE_LINE_SIZE);
 
 } __aligned(CACHE_LINE_SIZE);
@@ -347,8 +340,7 @@ void	nvme_qpair_construct(struct nvme_qp
 void	nvme_qpair_submit_cmd(struct nvme_qpair *qpair,
 			      struct nvme_tracker *tr);
 void	nvme_qpair_process_completions(struct nvme_qpair *qpair);
-struct nvme_tracker *	nvme_qpair_allocate_tracker(struct nvme_qpair *qpair,
-						    boolean_t alloc_prp_list);
+struct nvme_tracker *	nvme_qpair_allocate_tracker(struct nvme_qpair *qpair);
 
 void	nvme_admin_qpair_destroy(struct nvme_qpair *qpair);
 

Modified: head/sys/dev/nvme/nvme_qpair.c
==============================================================================
--- head/sys/dev/nvme/nvme_qpair.c	Thu Oct 18 00:32:07 2012	(r241657)
+++ head/sys/dev/nvme/nvme_qpair.c	Thu Oct 18 00:37:11 2012	(r241658)
@@ -75,10 +75,9 @@ nvme_completion_check_retry(const struct
 }
 
 struct nvme_tracker *
-nvme_qpair_allocate_tracker(struct nvme_qpair *qpair, boolean_t alloc_prp_list)
+nvme_qpair_allocate_tracker(struct nvme_qpair *qpair)
 {
 	struct nvme_tracker	*tr;
-	struct nvme_prp_list	*prp_list;
 
 	mtx_lock(&qpair->lock);
 
@@ -102,33 +101,17 @@ nvme_qpair_allocate_tracker(struct nvme_
 			return (NULL);
 		}
 
-		bus_dmamap_create(qpair->dma_tag, 0, &tr->dma_map);
+		bus_dmamap_create(qpair->dma_tag, 0, &tr->payload_dma_map);
+		bus_dmamap_create(qpair->dma_tag, 0, &tr->prp_dma_map);
+
+		bus_dmamap_load(qpair->dma_tag, tr->prp_dma_map, tr->prp,
+		    sizeof(tr->prp), nvme_single_map, &tr->prp_bus_addr, 0);
+
 		callout_init_mtx(&tr->timer, &qpair->lock, 0);
 		tr->cid = qpair->num_tr++;
 	} else
 		SLIST_REMOVE_HEAD(&qpair->free_tr, slist);
 
-	if (alloc_prp_list) {
-		prp_list = SLIST_FIRST(&qpair->free_prp_list);
-
-		if (prp_list == NULL) {
-			prp_list = malloc(sizeof(struct nvme_prp_list),
-			    M_NVME, M_ZERO | M_NOWAIT);
-
-			bus_dmamap_create(qpair->dma_tag, 0, &prp_list->dma_map);
-
-			bus_dmamap_load(qpair->dma_tag, prp_list->dma_map,
-			    prp_list->prp, sizeof(struct nvme_prp_list),
-			    nvme_single_map, &prp_list->bus_addr, 0);
-
-			qpair->num_prp_list++;
-		} else {
-			SLIST_REMOVE_HEAD(&qpair->free_prp_list, slist);
-		}
-
-		tr->prp_list = prp_list;
-	}
-
 	return (tr);
 }
 
@@ -176,14 +159,9 @@ nvme_qpair_process_completions(struct nv
 			/* nvme_qpair_submit_cmd() will release the lock. */
 			nvme_qpair_submit_cmd(qpair, tr);
 		else {
-			if (tr->prp_list) {
-				SLIST_INSERT_HEAD(&qpair->free_prp_list,
-				    tr->prp_list, slist);
-				tr->prp_list = NULL;
-			}
-
 			if (tr->payload_size > 0)
-				bus_dmamap_unload(qpair->dma_tag, tr->dma_map);
+				bus_dmamap_unload(qpair->dma_tag,
+				    tr->payload_dma_map);
 
 			SLIST_INSERT_HEAD(&qpair->free_tr, tr, slist);
 
@@ -256,7 +234,6 @@ nvme_qpair_construct(struct nvme_qpair *
 	qpair->num_cmds = 0;
 	qpair->num_intr_handler_calls = 0;
 	qpair->num_tr = 0;
-	qpair->num_prp_list = 0;
 	qpair->sq_head = qpair->sq_tail = qpair->cq_head = 0;
 
 	/* TODO: error checking on contigmalloc, bus_dmamap_load calls */
@@ -281,7 +258,6 @@ nvme_qpair_construct(struct nvme_qpair *
 	qpair->cq_hdbl_off = nvme_mmio_offsetof(doorbell[id].cq_hdbl);
 
 	SLIST_INIT(&qpair->free_tr);
-	SLIST_INIT(&qpair->free_prp_list);
 
 	qpair->act_tr = malloc(sizeof(struct nvme_tracker *) * qpair->num_entries,
 	    M_NVME, M_ZERO | M_NOWAIT);
@@ -291,7 +267,6 @@ static void
 nvme_qpair_destroy(struct nvme_qpair *qpair)
 {
 	struct nvme_tracker *tr;
-	struct nvme_prp_list *prp_list;
 
 	if (qpair->tag)
 		bus_teardown_intr(qpair->ctrlr->dev, qpair->res, qpair->tag);
@@ -309,16 +284,10 @@ nvme_qpair_destroy(struct nvme_qpair *qp
 	while (!SLIST_EMPTY(&qpair->free_tr)) {
 		tr = SLIST_FIRST(&qpair->free_tr);
 		SLIST_REMOVE_HEAD(&qpair->free_tr, slist);
-		bus_dmamap_destroy(qpair->dma_tag, tr->dma_map);
+		bus_dmamap_destroy(qpair->dma_tag, tr->payload_dma_map);
+		bus_dmamap_destroy(qpair->dma_tag, tr->prp_dma_map);
 		free(tr, M_NVME);
 	}
-
-	while (!SLIST_EMPTY(&qpair->free_prp_list)) {
-		prp_list = SLIST_FIRST(&qpair->free_prp_list);
-		SLIST_REMOVE_HEAD(&qpair->free_prp_list, slist);
-		bus_dmamap_destroy(qpair->dma_tag, prp_list->dma_map);
-		free(prp_list, M_NVME);
-	}
 }
 
 void

Modified: head/sys/dev/nvme/nvme_sysctl.c
==============================================================================
--- head/sys/dev/nvme/nvme_sysctl.c	Thu Oct 18 00:32:07 2012	(r241657)
+++ head/sys/dev/nvme/nvme_sysctl.c	Thu Oct 18 00:37:11 2012	(r241658)
@@ -195,9 +195,6 @@ nvme_sysctl_initialize_queue(struct nvme
 	SYSCTL_ADD_UINT(ctrlr_ctx, que_list, OID_AUTO, "num_tr",
 	    CTLFLAG_RD, &qpair->num_tr, 0,
 	    "Number of trackers allocated");
-	SYSCTL_ADD_UINT(ctrlr_ctx, que_list, OID_AUTO, "num_prp_list",
-	    CTLFLAG_RD, &qpair->num_prp_list, 0,
-	    "Number of PRP lists allocated");
 	SYSCTL_ADD_UINT(ctrlr_ctx, que_list, OID_AUTO, "sq_head",
 	    CTLFLAG_RD, &qpair->sq_head, 0,
 	    "Current head of submission queue (as observed by driver)");

Modified: head/sys/dev/nvme/nvme_uio.c
==============================================================================
--- head/sys/dev/nvme/nvme_uio.c	Thu Oct 18 00:32:07 2012	(r241657)
+++ head/sys/dev/nvme/nvme_uio.c	Thu Oct 18 00:37:11 2012	(r241658)
@@ -58,11 +58,7 @@ nvme_allocate_tracker_uio(struct nvme_co
 	else
 		qpair = &ctrlr->ioq[0];
 
-	/*
-	 * For uio, always allocate a PRP list, rather than walking
-	 *  the iovecs.
-	 */
-	tr = nvme_qpair_allocate_tracker(qpair, TRUE /* alloc_prp_list */);
+	tr = nvme_qpair_allocate_tracker(qpair);
 
 	if (tr == NULL)
 		return (NULL);
@@ -109,7 +105,7 @@ nvme_read_uio(struct nvme_namespace *ns,
 
 	cmd->cdw12 = (iosize / nvme_ns_get_sector_size(ns))-1;
 
-	err = bus_dmamap_load_uio(tr->qpair->dma_tag, tr->dma_map, uio,
+	err = bus_dmamap_load_uio(tr->qpair->dma_tag, tr->payload_dma_map, uio,
 	    nvme_payload_map_uio, tr, 0);
 
 	KASSERT(err == 0, ("bus_dmamap_load_uio returned non-zero!\n"));
@@ -143,7 +139,7 @@ nvme_write_uio(struct nvme_namespace *ns
 
 	cmd->cdw12 = (iosize / nvme_ns_get_sector_size(ns))-1;
 
-	err = bus_dmamap_load_uio(tr->qpair->dma_tag, tr->dma_map, uio,
+	err = bus_dmamap_load_uio(tr->qpair->dma_tag, tr->payload_dma_map, uio,
 	    nvme_payload_map_uio, tr, 0);
 
 	KASSERT(err == 0, ("bus_dmamap_load_uio returned non-zero!\n"));



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201210180037.q9I0bBLQ000511>