[PATCH V2 0/6] nvmet: cleanup and status, error log fix

[PATCH V2 0/6] nvmet: cleanup and status, error log fix

[Chaitanya Kulkarni]

Hi,

This removes duplicate status value assignments, unnecessary function
parameters and fixes the error log page update. Last patch also
removes the spaces where tabs are expected.

-ck

V1->V2:

1. Remove the patch to set the status variable under if at the error.
2. Move the error log page update into nvmet_alloc_ctrl().
3. Add a patch to replace the spaces with tabs.

Chaitanya Kulkarni (6):
  nvmet: remove duplicate status assignment
  nvmet: update error log page in nvmet_alloc_ctrl()
  nvmet: remove unnecessary function parameters
  nvmet: remove unnecessary function parameter
  nvmet: remove unnecessary function parameters
  nvmet: replace white spaces with tabs

 drivers/nvme/target/admin-cmd.c   |  4 +--
 drivers/nvme/target/core.c        | 58 +++++++++++++++----------------
 drivers/nvme/target/fabrics-cmd.c | 19 ++++------
 drivers/nvme/target/nvmet.h       | 10 +++---
 4 files changed, 43 insertions(+), 48 deletions(-)

# ./check tests/nvme/
nvme/002 (create many subsystems and test discovery)         [passed]
    runtime  36.789s  ...  37.383s
nvme/003 (test if we're sending keep-alives to a discovery controller) [passed]
    runtime  10.177s  ...  10.164s
nvme/004 (test nvme and nvmet UUID NS descriptors)           [passed]
    runtime  1.743s  ...  1.739s
nvme/005 (reset local loopback target)                       [not run]
    nvme_core module does not have parameter multipath
nvme/006 (create an NVMeOF target with a block device-backed ns) [passed]
    runtime  0.141s  ...  0.137s
nvme/007 (create an NVMeOF target with a file-backed ns)     [passed]
    runtime  0.096s  ...  0.092s
nvme/008 (create an NVMeOF host with a block device-backed ns) [passed]
    runtime  1.756s  ...  1.760s
nvme/009 (create an NVMeOF host with a file-backed ns)       [passed]
    runtime  1.709s  ...  1.732s
nvme/010 (run data verification fio job on NVMeOF block device-backed ns) [passed]
    runtime  19.931s  ...  29.821s
nvme/011 (run data verification fio job on NVMeOF file-backed ns) [passed]
    runtime  267.604s  ...  282.232s
nvme/012 (run mkfs and data verification fio job on NVMeOF block device-backed ns) [passed]
    runtime  49.320s  ...  50.911s
nvme/013 (run mkfs and data verification fio job on NVMeOF file-backed ns) [passed]
    runtime  335.511s  ...  292.379s
nvme/014 (flush a NVMeOF block device-backed ns)             [passed]
    runtime  21.308s  ...  20.296s
nvme/015 (unit test for NVMe flush for file backed ns)       [passed]
    runtime  21.888s  ...  18.687s
nvme/016 (create/delete many NVMeOF block device-backed ns and test discovery) [passed]
    runtime  19.097s  ...  19.649s
nvme/017 (create/delete many file-ns and test discovery)     [passed]
    runtime  19.393s  ...  19.435s
nvme/018 (unit test NVMe-oF out of range access on a file backend) [passed]
    runtime  1.718s  ...  1.729s
nvme/019 (test NVMe DSM Discard command on NVMeOF block-device ns) [passed]
    runtime  1.747s  ...  1.786s
nvme/020 (test NVMe DSM Discard command on NVMeOF file-backed ns) [passed]
    runtime  1.700s  ...  1.704s
nvme/021 (test NVMe list command on NVMeOF file-backed ns)   [passed]
    runtime  1.702s  ...  1.745s
nvme/022 (test NVMe reset command on NVMeOF file-backed ns)  [passed]
    runtime  2.131s  ...  2.128s
nvme/023 (test NVMe smart-log command on NVMeOF block-device ns) [passed]
    runtime  1.739s  ...  1.761s
nvme/024 (test NVMe smart-log command on NVMeOF file-backed ns) [passed]
    runtime  1.708s  ...  1.717s
nvme/025 (test NVMe effects-log command on NVMeOF file-backed ns) [passed]
    runtime  1.692s  ...  1.703s
nvme/026 (test NVMe ns-descs command on NVMeOF file-backed ns) [passed]
    runtime  1.798s  ...  1.716s
nvme/027 (test NVMe ns-rescan command on NVMeOF file-backed ns) [passed]
    runtime  1.713s  ...  1.716s
nvme/028 (test NVMe list-subsys command on NVMeOF file-backed ns) [passed]
    runtime  1.695s  ...  1.723s
nvme/029 (test userspace IO via nvme-cli read/write interface) [passed]
    runtime  2.168s  ...  2.148s
nvme/030 (ensure the discovery generation counter is updated appropriately) [passed]
    runtime  0.379s  ...  0.381s
nvme/031 (test deletion of NVMeOF controllers immediately after setup) [passed]
    runtime  5.774s  ...  5.747s
nvme/038 (test deletion of NVMeOF subsystem without enabling) [passed]
    runtime  0.051s  ...  0.059s

-- 
2.22.1


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

[PATCH V2 1/6] nvmet: remove duplicate status assignment

[Chaitanya Kulkarni]

In the function nvmet_alloc_ctrl() we assign status value before we
call nvmet_fine_get_subsys() to :-
status = NVME_SC_CONNECT_INVALID_PARAM | NVME_SC_DNR;

After we successfully find the subsystem we again set the status value
to :-
status = NVME_SC_CONNECT_INVALID_PARAM | NVME_SC_DNR;

Remove the duplicate status assignment value.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 drivers/nvme/target/core.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index be6fcdaf51a7..e3b8ec535eb4 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -1314,7 +1314,6 @@ u16 nvmet_alloc_ctrl(const char *subsysnqn, const char *hostnqn,
 		goto out;
 	}
 
-	status = NVME_SC_CONNECT_INVALID_PARAM | NVME_SC_DNR;
 	down_read(&nvmet_config_sem);
 	if (!nvmet_host_allowed(subsys, hostnqn)) {
 		pr_info("connect by host %s for subsystem %s not allowed\n",
-- 
2.22.1


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

[PATCH V2 2/6] nvmet: update error log page in nvmet_alloc_ctrl()

[Chaitanya Kulkarni]

Instead of updating the error log page in the caller of the
nvmet_alloc_ctrt() update the error log page in the nvmet_alloc_ctrl().

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 drivers/nvme/target/core.c        | 2 ++
 drivers/nvme/target/fabrics-cmd.c | 6 +-----
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index e3b8ec535eb4..c4238c08e912 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -1311,6 +1311,7 @@ u16 nvmet_alloc_ctrl(const char *subsysnqn, const char *hostnqn,
 		pr_warn("connect request for invalid subsystem %s!\n",
 			subsysnqn);
 		req->cqe->result.u32 = IPO_IATTR_CONNECT_DATA(subsysnqn);
+		req->error_loc = offsetof(struct nvme_common_command, dptr);
 		goto out;
 	}
 
@@ -1321,6 +1322,7 @@ u16 nvmet_alloc_ctrl(const char *subsysnqn, const char *hostnqn,
 		req->cqe->result.u32 = IPO_IATTR_CONNECT_DATA(hostnqn);
 		up_read(&nvmet_config_sem);
 		status = NVME_SC_CONNECT_INVALID_HOST | NVME_SC_DNR;
+		req->error_loc = offsetof(struct nvme_common_command, dptr);
 		goto out_put_subsystem;
 	}
 	up_read(&nvmet_config_sem);
diff --git a/drivers/nvme/target/fabrics-cmd.c b/drivers/nvme/target/fabrics-cmd.c
index 42bd12b8bf00..d2289aa26645 100644
--- a/drivers/nvme/target/fabrics-cmd.c
+++ b/drivers/nvme/target/fabrics-cmd.c
@@ -190,12 +190,8 @@ static void nvmet_execute_admin_connect(struct nvmet_req *req)
 
 	status = nvmet_alloc_ctrl(d->subsysnqn, d->hostnqn, req,
 				  le32_to_cpu(c->kato), &ctrl);
-	if (status) {
-		if (status == (NVME_SC_INVALID_FIELD | NVME_SC_DNR))
-			req->error_loc =
-				offsetof(struct nvme_common_command, opcode);
+	if (status)
 		goto out;
-	}
 
 	ctrl->pi_support = ctrl->port->pi_enable && ctrl->subsys->pi_support;
 
-- 
2.22.1


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

[PATCH V2 3/6] nvmet: remove unnecessary function parameters

[Chaitanya Kulkarni]

The function nvmet_ctrl_find_get() accepts subsysnqn, hostnqn, cntlid,
nvmet_req, and out pointer to nvmet_ctrl structure. The parameters
subsysnqn, hostnqn and cntlid can be derived from the caller's
struct nvmf_connect_data.

Replace these parameters with structure pointer nvmf_connect_data *d.

Also, this function returns the same error value from two places. First
on failure to find subsystem from connect data and failure to find ctrl
from cntlid connect data:- NVME_SC_CONNECT_INVALID_PARAM | NVME_SC_DNR.

Move this to the caller to change the return type so we can return ctrl.

Now that we can change the return type instead of taking the pointer to
pointer to the nvmet_ctrl structure remove that function parameter and
return the valid nvmet_ctrl pointer on success and NULL on failure.

Also, add and rename the goto labels for more readability with comment.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 drivers/nvme/target/core.c        | 30 +++++++++++++++---------------
 drivers/nvme/target/fabrics-cmd.c | 10 +++++-----
 drivers/nvme/target/nvmet.h       |  4 ++--
 3 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index c4238c08e912..db55f3a6f140 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -1179,45 +1179,45 @@ static void nvmet_init_cap(struct nvmet_ctrl *ctrl)
 	ctrl->cap |= NVMET_QUEUE_SIZE - 1;
 }
 
-u16 nvmet_ctrl_find_get(const char *subsysnqn, const char *hostnqn, u16 cntlid,
-		struct nvmet_req *req, struct nvmet_ctrl **ret)
+struct nvmet_ctrl *nvmet_ctrl_find_get(struct nvmf_connect_data *d,
+				       struct nvmet_req *req)
 {
+	struct nvmet_ctrl *ctrl = NULL;
 	struct nvmet_subsys *subsys;
-	struct nvmet_ctrl *ctrl;
-	u16 status = 0;
 
-	subsys = nvmet_find_get_subsys(req->port, subsysnqn);
+	subsys = nvmet_find_get_subsys(req->port, d->subsysnqn);
 	if (!subsys) {
 		pr_warn("connect request for invalid subsystem %s!\n",
-			subsysnqn);
+			d->subsysnqn);
 		req->cqe->result.u32 = IPO_IATTR_CONNECT_DATA(subsysnqn);
-		return NVME_SC_CONNECT_INVALID_PARAM | NVME_SC_DNR;
+		goto out;
 	}
 
 	mutex_lock(&subsys->lock);
 	list_for_each_entry(ctrl, &subsys->ctrls, subsys_entry) {
-		if (ctrl->cntlid == cntlid) {
-			if (strncmp(hostnqn, ctrl->hostnqn, NVMF_NQN_SIZE)) {
+		if (ctrl->cntlid == le16_to_cpu(d->cntlid)) {
+			if (strncmp(d->hostnqn, ctrl->hostnqn, NVMF_NQN_SIZE)) {
 				pr_warn("hostnqn mismatch.\n");
 				continue;
 			}
 			if (!kref_get_unless_zero(&ctrl->ref))
 				continue;
 
-			*ret = ctrl;
-			goto out;
+			/* ctrl found */
+			goto found;
 		}
 	}
 
+	ctrl = NULL; /* ctrl not found */
 	pr_warn("could not find controller %d for subsys %s / host %s\n",
-		cntlid, subsysnqn, hostnqn);
+		le16_to_cpu(d->cntlid), d->subsysnqn, d->hostnqn);
 	req->cqe->result.u32 = IPO_IATTR_CONNECT_DATA(cntlid);
-	status = NVME_SC_CONNECT_INVALID_PARAM | NVME_SC_DNR;
 
-out:
+found:
 	mutex_unlock(&subsys->lock);
 	nvmet_subsys_put(subsys);
-	return status;
+out:
+	return ctrl;
 }
 
 u16 nvmet_check_ctrl_status(struct nvmet_req *req, struct nvme_command *cmd)
diff --git a/drivers/nvme/target/fabrics-cmd.c b/drivers/nvme/target/fabrics-cmd.c
index d2289aa26645..acbb8d1c402c 100644
--- a/drivers/nvme/target/fabrics-cmd.c
+++ b/drivers/nvme/target/fabrics-cmd.c
@@ -218,7 +218,7 @@ static void nvmet_execute_io_connect(struct nvmet_req *req)
 {
 	struct nvmf_connect_command *c = &req->cmd->connect;
 	struct nvmf_connect_data *d;
-	struct nvmet_ctrl *ctrl = NULL;
+	struct nvmet_ctrl *ctrl;
 	u16 qid = le16_to_cpu(c->qid);
 	u16 status = 0;
 
@@ -245,11 +245,11 @@ static void nvmet_execute_io_connect(struct nvmet_req *req)
 		goto out;
 	}
 
-	status = nvmet_ctrl_find_get(d->subsysnqn, d->hostnqn,
-				     le16_to_cpu(d->cntlid),
-				     req, &ctrl);
-	if (status)
+	ctrl = nvmet_ctrl_find_get(d, req);
+	if (!ctrl) {
+		status = NVME_SC_CONNECT_INVALID_PARAM | NVME_SC_DNR;
 		goto out;
+	}
 
 	if (unlikely(qid > ctrl->subsys->max_qid)) {
 		pr_warn("invalid queue id (%d)\n", qid);
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index 4b84edb49f22..2938eca5378a 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -428,8 +428,8 @@ void nvmet_ctrl_fatal_error(struct nvmet_ctrl *ctrl);
 void nvmet_update_cc(struct nvmet_ctrl *ctrl, u32 new);
 u16 nvmet_alloc_ctrl(const char *subsysnqn, const char *hostnqn,
 		struct nvmet_req *req, u32 kato, struct nvmet_ctrl **ctrlp);
-u16 nvmet_ctrl_find_get(const char *subsysnqn, const char *hostnqn, u16 cntlid,
-		struct nvmet_req *req, struct nvmet_ctrl **ret);
+struct nvmet_ctrl *nvmet_ctrl_find_get(struct nvmf_connect_data *d,
+				       struct nvmet_req *req);
 void nvmet_ctrl_put(struct nvmet_ctrl *ctrl);
 u16 nvmet_check_ctrl_status(struct nvmet_req *req, struct nvme_command *cmd);
 
-- 
2.22.1


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

[PATCH V2 4/6] nvmet: remove unnecessary function parameter

[Chaitanya Kulkarni]

In nvmet_check_ctrl_status() cmd can be derived from nvmet_req. Remove
the local variable cmd in the nvmet_check_ctrl_status() and function
parameter cmd for nvmet_check_ctrl_status(). Derive the cmd value from
req parameter in the nvmet_check_ctrl_status().

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 drivers/nvme/target/admin-cmd.c | 2 +-
 drivers/nvme/target/core.c      | 9 ++++-----
 drivers/nvme/target/nvmet.h     | 2 +-
 3 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index fe6b8aa90b53..16a3e434f52e 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -940,7 +940,7 @@ u16 nvmet_parse_admin_cmd(struct nvmet_req *req)
 	if (nvmet_req_subsys(req)->type == NVME_NQN_DISC)
 		return nvmet_parse_discovery_cmd(req);
 
-	ret = nvmet_check_ctrl_status(req, cmd);
+	ret = nvmet_check_ctrl_status(req);
 	if (unlikely(ret))
 		return ret;
 
diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index db55f3a6f140..4dce09000b3a 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -864,10 +864,9 @@ static inline u16 nvmet_io_cmd_check_access(struct nvmet_req *req)
 
 static u16 nvmet_parse_io_cmd(struct nvmet_req *req)
 {
-	struct nvme_command *cmd = req->cmd;
 	u16 ret;
 
-	ret = nvmet_check_ctrl_status(req, cmd);
+	ret = nvmet_check_ctrl_status(req);
 	if (unlikely(ret))
 		return ret;
 
@@ -1220,17 +1219,17 @@ struct nvmet_ctrl *nvmet_ctrl_find_get(struct nvmf_connect_data *d,
 	return ctrl;
 }
 
-u16 nvmet_check_ctrl_status(struct nvmet_req *req, struct nvme_command *cmd)
+u16 nvmet_check_ctrl_status(struct nvmet_req *req)
 {
 	if (unlikely(!(req->sq->ctrl->cc & NVME_CC_ENABLE))) {
 		pr_err("got cmd %d while CC.EN == 0 on qid = %d\n",
-		       cmd->common.opcode, req->sq->qid);
+		       req->cmd->common.opcode, req->sq->qid);
 		return NVME_SC_CMD_SEQ_ERROR | NVME_SC_DNR;
 	}
 
 	if (unlikely(!(req->sq->ctrl->csts & NVME_CSTS_RDY))) {
 		pr_err("got cmd %d while CSTS.RDY == 0 on qid = %d\n",
-		       cmd->common.opcode, req->sq->qid);
+		       req->cmd->common.opcode, req->sq->qid);
 		return NVME_SC_CMD_SEQ_ERROR | NVME_SC_DNR;
 	}
 	return 0;
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index 2938eca5378a..9a445fab3547 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -431,7 +431,7 @@ u16 nvmet_alloc_ctrl(const char *subsysnqn, const char *hostnqn,
 struct nvmet_ctrl *nvmet_ctrl_find_get(struct nvmf_connect_data *d,
 				       struct nvmet_req *req);
 void nvmet_ctrl_put(struct nvmet_ctrl *ctrl);
-u16 nvmet_check_ctrl_status(struct nvmet_req *req, struct nvme_command *cmd);
+u16 nvmet_check_ctrl_status(struct nvmet_req *req);
 
 struct nvmet_subsys *nvmet_subsys_alloc(const char *subsysnqn,
 		enum nvme_subsys_type type);
-- 
2.22.1


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

[PATCH V2 5/6] nvmet: remove unnecessary function parameters

[Chaitanya Kulkarni]

The function nvmet_alloc_ctrl() accepts subsysnqn, hostnqn, cntlid,
nvmet_req, and out pointer to nvmet_ctrl structure. The parameters
subsysnqn, hostnqn and cntlid can be derived from the caller's
struct nvmf_connect_data.

Replace these parameters with structure pointer nvmf_connect_data *d.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 drivers/nvme/target/core.c        | 16 ++++++++--------
 drivers/nvme/target/fabrics-cmd.c |  3 +--
 drivers/nvme/target/nvmet.h       |  4 ++--
 3 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index 4dce09000b3a..c64f0322c757 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -1296,8 +1296,8 @@ static void nvmet_fatal_error_handler(struct work_struct *work)
 	ctrl->ops->delete_ctrl(ctrl);
 }
 
-u16 nvmet_alloc_ctrl(const char *subsysnqn, const char *hostnqn,
-		struct nvmet_req *req, u32 kato, struct nvmet_ctrl **ctrlp)
+u16 nvmet_alloc_ctrl(struct nvmf_connect_data *d, struct nvmet_req *req,
+		     u32 kato, struct nvmet_ctrl **ctrlp)
 {
 	struct nvmet_subsys *subsys;
 	struct nvmet_ctrl *ctrl;
@@ -1305,19 +1305,19 @@ u16 nvmet_alloc_ctrl(const char *subsysnqn, const char *hostnqn,
 	u16 status;
 
 	status = NVME_SC_CONNECT_INVALID_PARAM | NVME_SC_DNR;
-	subsys = nvmet_find_get_subsys(req->port, subsysnqn);
+	subsys = nvmet_find_get_subsys(req->port, d->subsysnqn);
 	if (!subsys) {
 		pr_warn("connect request for invalid subsystem %s!\n",
-			subsysnqn);
+			d->subsysnqn);
 		req->cqe->result.u32 = IPO_IATTR_CONNECT_DATA(subsysnqn);
 		req->error_loc = offsetof(struct nvme_common_command, dptr);
 		goto out;
 	}
 
 	down_read(&nvmet_config_sem);
-	if (!nvmet_host_allowed(subsys, hostnqn)) {
+	if (!nvmet_host_allowed(subsys, d->hostnqn)) {
 		pr_info("connect by host %s for subsystem %s not allowed\n",
-			hostnqn, subsysnqn);
+			d->hostnqn, d->subsysnqn);
 		req->cqe->result.u32 = IPO_IATTR_CONNECT_DATA(hostnqn);
 		up_read(&nvmet_config_sem);
 		status = NVME_SC_CONNECT_INVALID_HOST | NVME_SC_DNR;
@@ -1341,8 +1341,8 @@ u16 nvmet_alloc_ctrl(const char *subsysnqn, const char *hostnqn,
 	INIT_RADIX_TREE(&ctrl->p2p_ns_map, GFP_KERNEL);
 	INIT_WORK(&ctrl->fatal_err_work, nvmet_fatal_error_handler);
 
-	memcpy(ctrl->subsysnqn, subsysnqn, NVMF_NQN_SIZE);
-	memcpy(ctrl->hostnqn, hostnqn, NVMF_NQN_SIZE);
+	memcpy(ctrl->subsysnqn, d->subsysnqn, NVMF_NQN_SIZE);
+	memcpy(ctrl->hostnqn, d->hostnqn, NVMF_NQN_SIZE);
 
 	kref_init(&ctrl->ref);
 	ctrl->subsys = subsys;
diff --git a/drivers/nvme/target/fabrics-cmd.c b/drivers/nvme/target/fabrics-cmd.c
index acbb8d1c402c..b41748d8edf3 100644
--- a/drivers/nvme/target/fabrics-cmd.c
+++ b/drivers/nvme/target/fabrics-cmd.c
@@ -188,8 +188,7 @@ static void nvmet_execute_admin_connect(struct nvmet_req *req)
 		goto out;
 	}
 
-	status = nvmet_alloc_ctrl(d->subsysnqn, d->hostnqn, req,
-				  le32_to_cpu(c->kato), &ctrl);
+	status = nvmet_alloc_ctrl(d, req, le32_to_cpu(c->kato), &ctrl);
 	if (status)
 		goto out;
 
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index 9a445fab3547..87a0c528a107 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -426,8 +426,8 @@ int nvmet_sq_init(struct nvmet_sq *sq);
 void nvmet_ctrl_fatal_error(struct nvmet_ctrl *ctrl);
 
 void nvmet_update_cc(struct nvmet_ctrl *ctrl, u32 new);
-u16 nvmet_alloc_ctrl(const char *subsysnqn, const char *hostnqn,
-		struct nvmet_req *req, u32 kato, struct nvmet_ctrl **ctrlp);
+u16 nvmet_alloc_ctrl(struct nvmf_connect_data *d, struct nvmet_req *req,
+		     u32 kato, struct nvmet_ctrl **ctrlp);
 struct nvmet_ctrl *nvmet_ctrl_find_get(struct nvmf_connect_data *d,
 				       struct nvmet_req *req);
 void nvmet_ctrl_put(struct nvmet_ctrl *ctrl);
-- 
2.22.1


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

[PATCH V2 6/6] nvmet: replace white spaces with tabs

[Chaitanya Kulkarni]

Instead of the using the whitespaces use tab spacing in the
nvmet_execute_identify_ns().

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 drivers/nvme/target/admin-cmd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index 16a3e434f52e..f4cc32674edd 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -513,7 +513,7 @@ static void nvmet_execute_identify_ns(struct nvmet_req *req)
 	default:
 		id->nuse = id->nsze;
 		break;
-        }
+	}
 
 	if (req->ns->bdev)
 		nvmet_bdev_set_limits(req->ns->bdev, id);
-- 
2.22.1


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH V2 1/6] nvmet: remove duplicate status assignment

[Sagi Grimberg]

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH V2 2/6] nvmet: update error log page in nvmet_alloc_ctrl()

[Sagi Grimberg]

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH V2 3/6] nvmet: remove unnecessary function parameters

[Sagi Grimberg]

> The function nvmet_ctrl_find_get() accepts subsysnqn, hostnqn, cntlid,
> nvmet_req, and out pointer to nvmet_ctrl structure. The parameters
> subsysnqn, hostnqn and cntlid can be derived from the caller's
> struct nvmf_connect_data.
> 
> Replace these parameters with structure pointer nvmf_connect_data *d.

Usually its preferable to use explicit parameters instead of a wire
transferred data structure. Not sure this particular change is useful.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH V2 4/6] nvmet: remove unnecessary function parameter

[Sagi Grimberg]

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH V2 5/6] nvmet: remove unnecessary function parameters

[Sagi Grimberg]

> The function nvmet_alloc_ctrl() accepts subsysnqn, hostnqn, cntlid,
> nvmet_req, and out pointer to nvmet_ctrl structure. The parameters
> subsysnqn, hostnqn and cntlid can be derived from the caller's
> struct nvmf_connect_data.
> 
> Replace these parameters with structure pointer nvmf_connect_data *d.

Again, not a fan of this. But if others think its fine then go ahead...

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH V2 6/6] nvmet: replace white spaces with tabs

[Sagi Grimberg]

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH V2 3/6] nvmet: remove unnecessary function parameters

[Christoph Hellwig]

On Fri, Mar 05, 2021 at 02:40:34PM -0800, Sagi Grimberg wrote:
>
>> The function nvmet_ctrl_find_get() accepts subsysnqn, hostnqn, cntlid,
>> nvmet_req, and out pointer to nvmet_ctrl structure. The parameters
>> subsysnqn, hostnqn and cntlid can be derived from the caller's
>> struct nvmf_connect_data.
>>
>> Replace these parameters with structure pointer nvmf_connect_data *d.
>
> Usually its preferable to use explicit parameters instead of a wire
> transferred data structure. Not sure this particular change is useful.

Yes, I can see that point.  Especially if we want to eventually add
PCIe/vhost support.

Chaitanya, can you respin a patch to just change the return value to the
nvmet_ctrl?

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH V2 0/6] nvmet: cleanup and status, error log fix

[Christoph Hellwig]

Applied to nvme-5.13 except for the two patches that use the nvmf
on the wire structures.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH V2 3/6] nvmet: remove unnecessary function parameters

[Chaitanya Kulkarni]

On 3/9/21 02:00, Christoph Hellwig wrote:
> Yes, I can see that point.  Especially if we want to eventually add
> PCIe/vhost support.
>
> Chaitanya, can you respin a patch to just change the return value to the
> nvmet_ctrl?
>

Patch is posted [1].

[1] http://lists.infradead.org/pipermail/linux-nvme/2021-March/023513.html

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme