[RFC PATCH 0/2] nvme: nvme_setup_cmd() cleanup

[RFC PATCH 0/2] nvme: nvme_setup_cmd() cleanup

[Chaitanya Kulkarni]

Hi,

This cleans up the nvme_setup_cmd() so that it reduces number calls
to nvme_setup_zone_mgmt_send() and nvme_setup_rw() along with removing
an extra parameter to those functions.

Full this is partially tested code.

-ck

Chaitanya Kulkarni (2):
  nvme: derive zone-mgmt-act from struct request
  nvme: derive nvme cmd opcode from struct request

 drivers/nvme/host/core.c | 27 +++++++++++----------------
 drivers/nvme/host/nvme.h |  6 ++----
 drivers/nvme/host/zns.c  | 13 +++++++++++--
 3 files changed, 24 insertions(+), 22 deletions(-)

-- 
2.29.0

[RFC PATCH 1/2] nvme: derive zone-mgmt-act from struct request

[Chaitanya Kulkarni]

Instead of having 4 calls of nvme_setup_zone_mgmt_send() with
zone-mgmt-act as a separate parameter, have one call from nvme
setup_cmd() and derive zome-mgmt-act from struct request that we are
already passing to nvme_setup_zone_mgmt_send().

This reduces duplicate calls and break statements with removal of extra
function parameter.

Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
---
 drivers/nvme/host/core.c |  8 +-------
 drivers/nvme/host/nvme.h |  6 ++----
 drivers/nvme/host/zns.c  | 13 +++++++++++--
 3 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index f8084ded69e5..b5d36e09e763 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1042,16 +1042,10 @@ blk_status_t nvme_setup_cmd(struct nvme_ns *ns, struct request *req)
 		break;
 	case REQ_OP_ZONE_RESET_ALL:
 	case REQ_OP_ZONE_RESET:
-		ret = nvme_setup_zone_mgmt_send(ns, req, cmd, NVME_ZONE_RESET);
-		break;
 	case REQ_OP_ZONE_OPEN:
-		ret = nvme_setup_zone_mgmt_send(ns, req, cmd, NVME_ZONE_OPEN);
-		break;
 	case REQ_OP_ZONE_CLOSE:
-		ret = nvme_setup_zone_mgmt_send(ns, req, cmd, NVME_ZONE_CLOSE);
-		break;
 	case REQ_OP_ZONE_FINISH:
-		ret = nvme_setup_zone_mgmt_send(ns, req, cmd, NVME_ZONE_FINISH);
+		ret = nvme_setup_zone_mgmt_send(ns, req, cmd);
 		break;
 	case REQ_OP_WRITE_ZEROES:
 		ret = nvme_setup_write_zeroes(ns, req, cmd);
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 587d92df118b..a85dc4e29870 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -878,12 +878,10 @@ int nvme_ns_report_zones(struct nvme_ns *ns, sector_t sector,
 #ifdef CONFIG_BLK_DEV_ZONED
 int nvme_update_zone_info(struct nvme_ns *ns, unsigned lbaf);
 blk_status_t nvme_setup_zone_mgmt_send(struct nvme_ns *ns, struct request *req,
-				       struct nvme_command *cmnd,
-				       enum nvme_zone_mgmt_action action);
+				       struct nvme_command *cmnd);
 #else
 static inline blk_status_t nvme_setup_zone_mgmt_send(struct nvme_ns *ns,
-		struct request *req, struct nvme_command *cmnd,
-		enum nvme_zone_mgmt_action action)
+		struct request *req, struct nvme_command *cmnd)
 {
 	return BLK_STS_NOTSUPP;
 }
diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c
index 9f81beb4df4e..7bbbd24ba6b6 100644
--- a/drivers/nvme/host/zns.c
+++ b/drivers/nvme/host/zns.c
@@ -234,14 +234,23 @@ int nvme_ns_report_zones(struct nvme_ns *ns, sector_t sector,
 }
 
 blk_status_t nvme_setup_zone_mgmt_send(struct nvme_ns *ns, struct request *req,
-		struct nvme_command *c, enum nvme_zone_mgmt_action action)
+		struct nvme_command *c)
 {
+	static enum nvme_zone_mgmt_action req_op_to_nvme_zmgmt_act[] = {
+		[REQ_OP_ZONE_OPEN] = NVME_ZONE_OPEN,
+		[REQ_OP_ZONE_CLOSE] = NVME_ZONE_CLOSE,
+		[REQ_OP_ZONE_FINISH] = NVME_ZONE_FINISH,
+		[REQ_OP_ZONE_RESET] = NVME_ZONE_RESET,
+		[REQ_OP_ZONE_RESET_ALL] = NVME_ZONE_RESET
+	};
+
 	memset(c, 0, sizeof(*c));
 
 	c->zms.opcode = nvme_cmd_zone_mgmt_send;
 	c->zms.nsid = cpu_to_le32(ns->head->ns_id);
 	c->zms.slba = cpu_to_le64(nvme_sect_to_lba(ns, blk_rq_pos(req)));
-	c->zms.zsa = action;
+	/* caller already validates REQ_OP_ZONE_XXX */
+	c->zms.zsa = req_op_to_nvme_zmgmt_act[req_op(req)];
 
 	if (req_op(req) == REQ_OP_ZONE_RESET_ALL)
 		c->zms.select_all = 1;
-- 
2.29.0

[RFC PATCH 2/2] nvme: derive nvme cmd opcode from struct request

[Chaitanya Kulkarni]

Instead of having 3 calls of nvme_setup_rw() with nvme command opcode as
a separate parameter have one call from nvme_setup_cmd() and derive
nvme command opcode from struct request that we are already passing to
nvme_setup_rw().

This reduces duplicate calls and break statements with removal of extra
function parameter.

Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
---
 drivers/nvme/host/core.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index b5d36e09e763..fe6179a4a31e 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -949,12 +949,16 @@ static inline blk_status_t nvme_setup_write_zeroes(struct nvme_ns *ns,
 }
 
 static inline blk_status_t nvme_setup_rw(struct nvme_ns *ns,
-		struct request *req, struct nvme_command *cmnd,
-		enum nvme_opcode op)
+		struct request *req, struct nvme_command *cmnd)
 {
 	struct nvme_ctrl *ctrl = ns->ctrl;
 	u16 control = 0;
 	u32 dsmgmt = 0;
+	static enum nvme_opcode req_op_to_nvme_opcode[] = {
+		[REQ_OP_READ] = nvme_cmd_read,
+		[REQ_OP_WRITE] = nvme_cmd_write,
+		[REQ_OP_ZONE_APPEND] = nvme_cmd_zone_append,
+	};
 
 	if (req->cmd_flags & REQ_FUA)
 		control |= NVME_RW_FUA;
@@ -964,7 +968,8 @@ static inline blk_status_t nvme_setup_rw(struct nvme_ns *ns,
 	if (req->cmd_flags & REQ_RAHEAD)
 		dsmgmt |= NVME_RW_DSM_FREQ_PREFETCH;
 
-	cmnd->rw.opcode = op;
+	/* caller already validates REQ_OP_XXX */
+	cmnd->rw.opcode = req_op_to_nvme_opcode[req_op(req)];
 	cmnd->rw.flags = 0;
 	cmnd->rw.nsid = cpu_to_le32(ns->head->ns_id);
 	cmnd->rw.rsvd2 = 0;
@@ -999,7 +1004,7 @@ static inline blk_status_t nvme_setup_rw(struct nvme_ns *ns,
 		case NVME_NS_DPS_PI_TYPE2:
 			control |= NVME_RW_PRINFO_PRCHK_GUARD |
 					NVME_RW_PRINFO_PRCHK_REF;
-			if (op == nvme_cmd_zone_append)
+			if (req_op(req) == nvme_cmd_zone_append)
 				control |= NVME_RW_APPEND_PIREMAP;
 			cmnd->rw.reftag = cpu_to_le32(t10_pi_ref_tag(req));
 			break;
@@ -1054,13 +1059,9 @@ blk_status_t nvme_setup_cmd(struct nvme_ns *ns, struct request *req)
 		ret = nvme_setup_discard(ns, req, cmd);
 		break;
 	case REQ_OP_READ:
-		ret = nvme_setup_rw(ns, req, cmd, nvme_cmd_read);
-		break;
 	case REQ_OP_WRITE:
-		ret = nvme_setup_rw(ns, req, cmd, nvme_cmd_write);
-		break;
 	case REQ_OP_ZONE_APPEND:
-		ret = nvme_setup_rw(ns, req, cmd, nvme_cmd_zone_append);
+		ret = nvme_setup_rw(ns, req, cmd);
 		break;
 	default:
 		WARN_ON_ONCE(1);
-- 
2.29.0

Re: [RFC PATCH 0/2] nvme: nvme_setup_cmd() cleanup

[Christoph Hellwig]

I don't really see the point of this series.  It adds (very slightly)
more source code, and generates larger binary code.