Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 24 Jul 2023 17:33:26 GMT
From:      John Baldwin <jhb@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: 92103adbebd9 - main - nvme: Use a memdesc for the request buffer instead of a bespoke union.
Message-ID:  <202307241733.36OHXQRO062534@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by jhb:

URL: https://cgit.FreeBSD.org/src/commit/?id=92103adbebd9c476c5c9c72318ff1853353e890c

commit 92103adbebd9c476c5c9c72318ff1853353e890c
Author:     John Baldwin <jhb@FreeBSD.org>
AuthorDate: 2023-07-24 17:32:58 +0000
Commit:     John Baldwin <jhb@FreeBSD.org>
CommitDate: 2023-07-24 17:32:58 +0000

    nvme: Use a memdesc for the request buffer instead of a bespoke union.
    
    This avoids encoding CAM-specific knowledge in nvme_qpair.c.
    
    Reviewed by:    chuck, imp, markj
    Sponsored by:   Chelsio Communications
    Differential Revision:  https://reviews.freebsd.org/D41119
---
 sys/dev/nvme/nvme_private.h | 30 +++++++++--------------------
 sys/dev/nvme/nvme_qpair.c   | 46 ++++++++-------------------------------------
 2 files changed, 17 insertions(+), 59 deletions(-)

diff --git a/sys/dev/nvme/nvme_private.h b/sys/dev/nvme/nvme_private.h
index 081f5e984a6c..370bd0dccf20 100644
--- a/sys/dev/nvme/nvme_private.h
+++ b/sys/dev/nvme/nvme_private.h
@@ -37,6 +37,7 @@
 #include <sys/kernel.h>
 #include <sys/lock.h>
 #include <sys/malloc.h>
+#include <sys/memdesc.h>
 #include <sys/module.h>
 #include <sys/mutex.h>
 #include <sys/rman.h>
@@ -114,21 +115,11 @@ struct nvme_completion_poll_status {
 	int			done;
 };
 
-#define NVME_REQUEST_VADDR	1
-#define NVME_REQUEST_NULL	2 /* For requests with no payload. */
-#define NVME_REQUEST_UIO	3
-#define NVME_REQUEST_BIO	4
-#define NVME_REQUEST_CCB        5
-
 struct nvme_request {
 	struct nvme_command		cmd;
 	struct nvme_qpair		*qpair;
-	union {
-		void			*payload;
-		struct bio		*bio;
-	} u;
-	uint32_t			type;
-	uint32_t			payload_size;
+	struct memdesc			payload;
+	bool				payload_valid;
 	bool				timeout;
 	nvme_cb_fn_t			cb_fn;
 	void				*cb_arg;
@@ -521,9 +512,8 @@ nvme_allocate_request_vaddr(void *payload, uint32_t payload_size,
 
 	req = _nvme_allocate_request(cb_fn, cb_arg);
 	if (req != NULL) {
-		req->type = NVME_REQUEST_VADDR;
-		req->u.payload = payload;
-		req->payload_size = payload_size;
+		req->payload = memdesc_vaddr(payload, payload_size);
+		req->payload_valid = true;
 	}
 	return (req);
 }
@@ -534,8 +524,6 @@ nvme_allocate_request_null(nvme_cb_fn_t cb_fn, void *cb_arg)
 	struct nvme_request *req;
 
 	req = _nvme_allocate_request(cb_fn, cb_arg);
-	if (req != NULL)
-		req->type = NVME_REQUEST_NULL;
 	return (req);
 }
 
@@ -546,8 +534,8 @@ nvme_allocate_request_bio(struct bio *bio, nvme_cb_fn_t cb_fn, void *cb_arg)
 
 	req = _nvme_allocate_request(cb_fn, cb_arg);
 	if (req != NULL) {
-		req->type = NVME_REQUEST_BIO;
-		req->u.bio = bio;
+		req->payload = memdesc_bio(bio);
+		req->payload_valid = true;
 	}
 	return (req);
 }
@@ -559,8 +547,8 @@ nvme_allocate_request_ccb(union ccb *ccb, nvme_cb_fn_t cb_fn, void *cb_arg)
 
 	req = _nvme_allocate_request(cb_fn, cb_arg);
 	if (req != NULL) {
-		req->type = NVME_REQUEST_CCB;
-		req->u.payload = ccb;
+		req->payload = memdesc_ccb(ccb);
+		req->payload_valid = true;
 	}
 
 	return (req);
diff --git a/sys/dev/nvme/nvme_qpair.c b/sys/dev/nvme/nvme_qpair.c
index 3dcc6aadc6ca..dffbaba8deca 100644
--- a/sys/dev/nvme/nvme_qpair.c
+++ b/sys/dev/nvme/nvme_qpair.c
@@ -447,7 +447,7 @@ nvme_qpair_complete_tracker(struct nvme_tracker *tr,
 	KASSERT(cpl->cid == req->cmd.cid, ("cpl cid does not match cmd cid\n"));
 
 	if (!retry) {
-		if (req->type != NVME_REQUEST_NULL) {
+		if (req->payload_valid) {
 			bus_dmamap_sync(qpair->dma_tag_payload,
 			    tr->payload_dma_map,
 			    BUS_DMASYNC_POSTREAD | BUS_DMASYNC_POSTWRITE);
@@ -462,7 +462,7 @@ nvme_qpair_complete_tracker(struct nvme_tracker *tr,
 		req->retries++;
 		nvme_qpair_submit_tracker(qpair, tr);
 	} else {
-		if (req->type != NVME_REQUEST_NULL) {
+		if (req->payload_valid) {
 			bus_dmamap_unload(qpair->dma_tag_payload,
 			    tr->payload_dma_map);
 		}
@@ -1179,45 +1179,13 @@ _nvme_qpair_submit_request(struct nvme_qpair *qpair, struct nvme_request *req)
 	tr->deadline = SBT_MAX;
 	tr->req = req;
 
-	switch (req->type) {
-	case NVME_REQUEST_VADDR:
-		KASSERT(req->payload_size <= qpair->ctrlr->max_xfer_size,
-		    ("payload_size (%d) exceeds max_xfer_size (%d)\n",
-		    req->payload_size, qpair->ctrlr->max_xfer_size));
-		err = bus_dmamap_load(tr->qpair->dma_tag_payload,
-		    tr->payload_dma_map, req->u.payload, req->payload_size,
-		    nvme_payload_map, tr, 0);
-		if (err != 0)
-			nvme_printf(qpair->ctrlr,
-			    "bus_dmamap_load returned 0x%x!\n", err);
-		break;
-	case NVME_REQUEST_NULL:
+	if (!req->payload_valid) {
 		nvme_qpair_submit_tracker(tr->qpair, tr);
-		break;
-	case NVME_REQUEST_BIO:
-		KASSERT(req->u.bio->bio_bcount <= qpair->ctrlr->max_xfer_size,
-		    ("bio->bio_bcount (%jd) exceeds max_xfer_size (%d)\n",
-		    (intmax_t)req->u.bio->bio_bcount,
-		    qpair->ctrlr->max_xfer_size));
-		err = bus_dmamap_load_bio(tr->qpair->dma_tag_payload,
-		    tr->payload_dma_map, req->u.bio, nvme_payload_map, tr, 0);
-		if (err != 0)
-			nvme_printf(qpair->ctrlr,
-			    "bus_dmamap_load_bio returned 0x%x!\n", err);
-		break;
-	case NVME_REQUEST_CCB:
-		err = bus_dmamap_load_ccb(tr->qpair->dma_tag_payload,
-		    tr->payload_dma_map, req->u.payload,
-		    nvme_payload_map, tr, 0);
-		if (err != 0)
-			nvme_printf(qpair->ctrlr,
-			    "bus_dmamap_load_ccb returned 0x%x!\n", err);
-		break;
-	default:
-		panic("unknown nvme request type 0x%x\n", req->type);
-		break;
+		return;
 	}
 
+	err = bus_dmamap_load_mem(tr->qpair->dma_tag_payload,
+	    tr->payload_dma_map, &req->payload, nvme_payload_map, tr, 0);
 	if (err != 0) {
 		/*
 		 * The dmamap operation failed, so we manually fail the
@@ -1226,6 +1194,8 @@ _nvme_qpair_submit_request(struct nvme_qpair *qpair, struct nvme_request *req)
 		 * nvme_qpair_manual_complete_tracker must not be called
 		 *  with the qpair lock held.
 		 */
+		nvme_printf(qpair->ctrlr,
+		    "bus_dmamap_load_mem returned 0x%x!\n", err);
 		mtx_unlock(&qpair->lock);
 		nvme_qpair_manual_complete_tracker(tr, NVME_SCT_GENERIC,
 		    NVME_SC_DATA_TRANSFER_ERROR, DO_NOT_RETRY, ERROR_PRINT_ALL);



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