linux-nvme.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V3 00/11] nvmet: fixes and some cleanups
@ 2021-02-10  5:47 Chaitanya Kulkarni
  2021-02-10  5:47 ` [PATCH V3 01/11] nvmet: set status to 0 in case for invalid nsid Chaitanya Kulkarni
                   ` (11 more replies)
  0 siblings, 12 replies; 19+ messages in thread
From: Chaitanya Kulkarni @ 2021-02-10  5:47 UTC (permalink / raw)
  To: linux-nvme; +Cc: hch, Chaitanya Kulkarni, sagi

Hi,

This patch-series fixes commit bffcd507780e
("nvmet: set right status on error in id-ns handler") and does cleanup
for nvmet_find_namespace() to get rid of the duplicate code and remove
the inconsistent error behavior of the host, Also this adds an helper
for unhandled commands to have uniform error message reporting for the
bdev, file and passthru backend. Patch 9/10 fix compilation warnings.
Last patch removes an unnecessary else at the tail of the
nvmet_parse_io_cmd().

This is based on nvme-5.12. All the blktests are seem to pass on this
series.                

Below is the testlog for latest change.

-ck   

Changes from V2:-

1. Set the status to 0 when nvmet_req_find_ns() returns an error
   for nvmet_execute_identify_ns().

Changes from V1:-

1, In nvmet_execute_identify_ns() zeroout buffer for unallocated nsid.
2. Adjust the patch #2 to reflect above behavior.
3. Merge patch #3 and #4 from V1 to change the nvmet_find_namespace()
   prototype to only accept request.
4. Rename nvmet_find_namespace()->nvmet_req_find_namespace().
5. Remove the backened string print from nvmet_report_invalid_opcode() &
   replace pr_err() with pr_debug().
6. Add a new patch that creates a helper nvmet_req_subsys() to replace
   a chain of the structures all over code to get the subsys from
   nvmet_req.
7. Add a patch to remove the unnecessay else at the tail of the
   nvmet_parse_io_cmd().

Chaitanya Kulkarni (11):
  nvmet: set status to 0 in case for invalid nsid
  nvmet: return uniform error for invalid ns
  nvmet: make nvmet_find_namespace() req based
  nvmet: remove extra variable in id-ns handler
  nvmet: add helper to report invalid opcode
  nvmet: use invalid cmd opcode helper
  nvmet: use invalid cmd opcode helper
  nvmet: use min of device_path and disk len
  nvme-loop: rename variable to get rid of the warn
  nvmet: add nvmet_req_subsys() helper
  nvmet: remove else at the end of the function

 drivers/nvme/target/admin-cmd.c   | 59 +++++++++++++------------------
 drivers/nvme/target/core.c        | 28 ++++++++-------
 drivers/nvme/target/io-cmd-bdev.c |  5 +--
 drivers/nvme/target/io-cmd-file.c |  5 +--
 drivers/nvme/target/loop.c        |  2 +-
 drivers/nvme/target/nvmet.h       | 18 ++++++++--
 drivers/nvme/target/passthru.c    |  6 ++--
 drivers/nvme/target/trace.h       |  4 ++-
 8 files changed, 64 insertions(+), 63 deletions(-)


# gitlog -11 
2bd1fdd47f27 (HEAD -> nvme-5.12) nvmet: remove else at the end of the function
f3ba4d069b9a nvmet: add nvmet_req_subsys() helper
f72b99a93c10 nvme-loop: rename variable to get rid of the warn
b8caa57f886b nvmet: use min of device_path and disk len
736b56163c13 nvmet: use invalid cmd opcode helper
7ddd60b3d6cf nvmet: use invalid cmd opcode helper
1f83a1024596 nvmet: add helper to report invalid opcode
a3904c9ca583 nvmet: remove extra variable in id-ns handler
c6d99eca6ec9 nvmet: make nvmet_find_namespace() req based
cfe473bf95a2 nvmet: return uniform error for invalid ns
87c302ac8edc nvmet: set status to 0 in case for invalid nsid

# Create 1 subsys with 1 ns
++ FILE=/dev/nvme0n1
++ NN=1
++ NQN=testnqn
++ let NR_DEVICES=NN+1
++ modprobe -r null_blk
++ modprobe null_blk nr_devices=0
++ modprobe nvme
++ modprobe nvme-fabrics
++ modprobe nvmet
++ modprobe nvme-loop
++ dmesg -c
++ sleep 2
++ tree /sys/kernel/config
/sys/kernel/config
├── nullb
│   └── features
└── nvmet
    ├── hosts
    ├── ports
    └── subsystems

5 directories, 1 file
++ mkdir /sys/kernel/config/nvmet/subsystems/testnqn
++ mkdir /sys/kernel/config/nvmet/ports/1/
++ echo -n loop
++ echo -n 1
++ ln -s /sys/kernel/config/nvmet/subsystems/testnqn /sys/kernel/config/nvmet/ports/1/subsystems/
++ sleep 1
++ echo transport=loop,nqn=testnqn
+++ shuf -i 1-1 -n 1
++ for i in '`shuf -i  1-$NN -n $NN`'
++ mkdir config/nullb/nullb1
++ echo 4096
++ echo 20971520
++ echo 1
+++ cat config/nullb/nullb1/index
++ IDX=0
++ mkdir /sys/kernel/config/nvmet/subsystems/testnqn/namespaces/1
++ echo ' ####### /dev/nullb0'
 ####### /dev/nullb0
++ echo -n /dev/nullb0
++ cat /sys/kernel/config/nvmet/subsystems/testnqn/namespaces/1/device_path
/dev/nullb0
++ echo 1
++ dmesg -c
[24762.913991] nvmet: creating controller 1 for subsystem testnqn for NQN nqn.2014-08.org.nvmexpress:uuid:6a4a7451-6274-4d7f-93d3-49724cc1147e.
[24762.914802] nvme nvme1: Please enable CONFIG_NVME_MULTIPATH for full support of multi-port devices.
[24762.915034] nvme nvme1: creating 64 I/O queues.
[24762.932598] nvme nvme1: new ctrl: "testnqn"
[24762.966156] nvmet: adding nsid 1 to subsystem testnqn
[24762.972384] nvme nvme1: rescanning namespaces.
++ sleep 1
++ mount
++ column -t
++ grep nvme
++ '[' 1 ']'
+++ wc -l
+++ ls -l /dev/nvme1 /dev/nvme1n1
++ cnt=2
++ echo 2
2
++ '[' 2 -gt 1 ']'
++ break
++ dmesg -c
# 
# nvme list 
Node             SN                   Model                                    Namespace Usage                      Format           FW Rev  
---------------- -------------------- ---------------------------------------- --------- -------------------------- ---------------- --------
/dev/nvme0n1     foo                  QEMU NVMe Ctrl                           1           1.07  GB /   1.07  GB    512   B +  0 B   1.0     
/dev/nvme1n1     fc13d75abd55eafa     Linux                                    1          21.99  TB /  21.99  TB      4 KiB +  0 B   5.11.0-r
# nvme id-ns /dev/nvme1 -n 1
NVME Identify Namespace 1:
nsze    : 0x140000000
ncap    : 0x140000000
nuse    : 0x140000000
nsfeat  : 0x12
nlbaf   : 0
flbas   : 0
mc      : 0
dpc     : 0
dps     : 0
nmic    : 0x1
rescap  : 0
fpi     : 0
dlfeat  : 0
nawun   : 0
nawupf  : 0
nacwu   : 0
nabsn   : 0
nabo    : 0
nabspf  : 0
noiob   : 0
nvmcap  : 0
npwg    : 0
npwa    : 0
npdg    : 0
npda    : 0
nows    : 0
mssrl   : 0
mcl     : 0
msrc    : 0
nsattr	: 0
nvmsetid: 0
anagrpid: 1
endgid  : 0
nguid   : 00000000000000000000000000000000
eui64   : 0000000000000000
lbaf  0 : ms:0   lbads:12 rp:0 (in use)
# Make sure we get the status 0 for identify namesapce command
# for unallocated nsid
# nvme id-ns /dev/nvme1 -n 2
NVME Identify Namespace 2:
nsze    : 0
ncap    : 0
nuse    : 0
nsfeat  : 0
nlbaf   : 0
flbas   : 0
mc      : 0
dpc     : 0
dps     : 0
nmic    : 0
rescap  : 0
fpi     : 0
dlfeat  : 0
nawun   : 0
nawupf  : 0
nacwu   : 0
nabsn   : 0
nabo    : 0
nabspf  : 0
noiob   : 0
nvmcap  : 0
mssrl   : 0
mcl     : 0
msrc    : 0
nsattr	: 0
nvmsetid: 0
anagrpid: 0
endgid  : 0
nguid   : 00000000000000000000000000000000
eui64   : 0000000000000000
lbaf  0 : ms:0   lbads:0  rp:0 (in use)

-- 
2.22.1


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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH V3 01/11] nvmet: set status to 0 in case for invalid nsid
  2021-02-10  5:47 [PATCH V3 00/11] nvmet: fixes and some cleanups Chaitanya Kulkarni
@ 2021-02-10  5:47 ` Chaitanya Kulkarni
  2021-02-10  5:47 ` [PATCH V3 02/11] nvmet: return uniform error for invalid ns Chaitanya Kulkarni
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: Chaitanya Kulkarni @ 2021-02-10  5:47 UTC (permalink / raw)
  To: linux-nvme; +Cc: hch, Chaitanya Kulkarni, sagi

For unallocated namespace in nvmet_execute_identify_ns() don't set the
status to NVME_SC_INVALID_NS, set it to zero.

Fixes: bffcd507780e ("nvmet: set right status on error in id-ns handler")
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 613a4d8feac1..5070ea5cf260 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -485,7 +485,7 @@ static void nvmet_execute_identify_ns(struct nvmet_req *req)
 	/* return an all zeroed buffer if we can't find an active namespace */
 	req->ns = nvmet_find_namespace(ctrl, req->cmd->identify.nsid);
 	if (!req->ns) {
-		status = NVME_SC_INVALID_NS;
+		status = 0;
 		goto done;
 	}
 
-- 
2.22.1


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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH V3 02/11] nvmet: return uniform error for invalid ns
  2021-02-10  5:47 [PATCH V3 00/11] nvmet: fixes and some cleanups Chaitanya Kulkarni
  2021-02-10  5:47 ` [PATCH V3 01/11] nvmet: set status to 0 in case for invalid nsid Chaitanya Kulkarni
@ 2021-02-10  5:47 ` Chaitanya Kulkarni
  2021-02-10  5:47 ` [PATCH V3 03/11] nvmet: make nvmet_find_namespace() req based Chaitanya Kulkarni
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: Chaitanya Kulkarni @ 2021-02-10  5:47 UTC (permalink / raw)
  To: linux-nvme; +Cc: hch, Chaitanya Kulkarni, sagi

For nvmet_find_namespace() error case we have inconsistent error code
mapping in the function nvmet_get_smart_log_nsid() and
nvmet_set_feat_write_protect().

There is no point in retrying for the invalid namesapce from the host
side. Set the error code to the NVME_SC_INVALID_NS | NVME_SC_DNR which
matches what we have in nvmet_execute_identify_desclist().

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

diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index 5070ea5cf260..e938064254a5 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -82,7 +82,7 @@ static u16 nvmet_get_smart_log_nsid(struct nvmet_req *req,
 		pr_err("Could not find namespace id : %d\n",
 				le32_to_cpu(req->cmd->get_log_page.nsid));
 		req->error_loc = offsetof(struct nvme_rw_command, nsid);
-		return NVME_SC_INVALID_NS;
+		return NVME_SC_INVALID_NS | NVME_SC_DNR;
 	}
 
 	/* we don't have the right data for file backed ns */
@@ -697,7 +697,7 @@ static u16 nvmet_set_feat_write_protect(struct nvmet_req *req)
 	req->ns = nvmet_find_namespace(req->sq->ctrl, req->cmd->rw.nsid);
 	if (unlikely(!req->ns)) {
 		req->error_loc = offsetof(struct nvme_common_command, nsid);
-		return status;
+		return status = NVME_SC_INVALID_NS | NVME_SC_DNR;
 	}
 
 	mutex_lock(&subsys->lock);
-- 
2.22.1


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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH V3 03/11] nvmet: make nvmet_find_namespace() req based
  2021-02-10  5:47 [PATCH V3 00/11] nvmet: fixes and some cleanups Chaitanya Kulkarni
  2021-02-10  5:47 ` [PATCH V3 01/11] nvmet: set status to 0 in case for invalid nsid Chaitanya Kulkarni
  2021-02-10  5:47 ` [PATCH V3 02/11] nvmet: return uniform error for invalid ns Chaitanya Kulkarni
@ 2021-02-10  5:47 ` Chaitanya Kulkarni
  2021-02-10  5:47 ` [PATCH V3 04/11] nvmet: remove extra variable in id-ns handler Chaitanya Kulkarni
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: Chaitanya Kulkarni @ 2021-02-10  5:47 UTC (permalink / raw)
  To: linux-nvme; +Cc: hch, Chaitanya Kulkarni, sagi

The six callers of nvmet_find_namespace() duplicate the error log page
update and status setting code for each call on failure.

All callers are nvmet requests based functions, so we can pass req
to the nvmet_find_namesapce() & derive ctrl from req, that'll allow us
to update the error log page in nvmet_find_namespace(). Now that we
pass the request we can also get rid of the local variable in
nvmet_find_namespace() and use the req->ns and return the error code.

Replace the ctrl parameter with nvmet_req for nvmet_find_namespace(),
centralize the error log page update for non allocated namesapces, and
return uniform error for non-allocated namespace.

The nvmet_find_namespace() takes nsid parameter which is from NVMe
commands structures such as get_log_page, identify, rw and common. All
these commands have same offset for the nsid field.

Derive nsid from req->cmd->common.nsid) & remove the extra parameter
from the nvmet_find_namespace().

Lastly now we associate the ns to the req parameter that we pass to the
nvmet_find_namespace(), rename the nvmet_find_namespace() to
nvmet_req_find_ns() i.e. :-

Old:-
struct nvmet_ns *nvmet_find_namespace(struct nvmet_ctrl *ctrl,
				      __le32 nsid)
New :-
u16 nvmet_req_find_ns(struct nvmet_req *req);

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

diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index e938064254a5..f32533480e66 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -75,15 +75,11 @@ static u16 nvmet_get_smart_log_nsid(struct nvmet_req *req,
 		struct nvme_smart_log *slog)
 {
 	u64 host_reads, host_writes, data_units_read, data_units_written;
+	u16 status;
 
-	req->ns = nvmet_find_namespace(req->sq->ctrl,
-				       req->cmd->get_log_page.nsid);
-	if (!req->ns) {
-		pr_err("Could not find namespace id : %d\n",
-				le32_to_cpu(req->cmd->get_log_page.nsid));
-		req->error_loc = offsetof(struct nvme_rw_command, nsid);
-		return NVME_SC_INVALID_NS | NVME_SC_DNR;
-	}
+	status = nvmet_req_find_ns(req);
+	if (status)
+		return status;
 
 	/* we don't have the right data for file backed ns */
 	if (!req->ns->bdev)
@@ -468,7 +464,7 @@ static void nvmet_execute_identify_ns(struct nvmet_req *req)
 {
 	struct nvmet_ctrl *ctrl = req->sq->ctrl;
 	struct nvme_id_ns *id;
-	u16 status = 0;
+	u16 status;
 
 	if (le32_to_cpu(req->cmd->identify.nsid) == NVME_NSID_ALL) {
 		req->error_loc = offsetof(struct nvme_identify, nsid);
@@ -483,8 +479,8 @@ static void nvmet_execute_identify_ns(struct nvmet_req *req)
 	}
 
 	/* return an all zeroed buffer if we can't find an active namespace */
-	req->ns = nvmet_find_namespace(ctrl, req->cmd->identify.nsid);
-	if (!req->ns) {
+	status = nvmet_req_find_ns(req);
+	if (status) {
 		status = 0;
 		goto done;
 	}
@@ -604,15 +600,12 @@ static u16 nvmet_copy_ns_identifier(struct nvmet_req *req, u8 type, u8 len,
 
 static void nvmet_execute_identify_desclist(struct nvmet_req *req)
 {
-	u16 status = 0;
 	off_t off = 0;
+	u16 status;
 
-	req->ns = nvmet_find_namespace(req->sq->ctrl, req->cmd->identify.nsid);
-	if (!req->ns) {
-		req->error_loc = offsetof(struct nvme_identify, nsid);
-		status = NVME_SC_INVALID_NS | NVME_SC_DNR;
+	status = nvmet_req_find_ns(req);
+	if (status)
 		goto out;
-	}
 
 	if (memchr_inv(&req->ns->uuid, 0, sizeof(req->ns->uuid))) {
 		status = nvmet_copy_ns_identifier(req, NVME_NIDT_UUID,
@@ -692,13 +685,11 @@ static u16 nvmet_set_feat_write_protect(struct nvmet_req *req)
 {
 	u32 write_protect = le32_to_cpu(req->cmd->common.cdw11);
 	struct nvmet_subsys *subsys = req->sq->ctrl->subsys;
-	u16 status = NVME_SC_FEATURE_NOT_CHANGEABLE;
+	u16 status;
 
-	req->ns = nvmet_find_namespace(req->sq->ctrl, req->cmd->rw.nsid);
-	if (unlikely(!req->ns)) {
-		req->error_loc = offsetof(struct nvme_common_command, nsid);
-		return status = NVME_SC_INVALID_NS | NVME_SC_DNR;
-	}
+	status = nvmet_req_find_ns(req);
+	if (status)
+		return status;
 
 	mutex_lock(&subsys->lock);
 	switch (write_protect) {
@@ -799,11 +790,10 @@ static u16 nvmet_get_feat_write_protect(struct nvmet_req *req)
 	struct nvmet_subsys *subsys = req->sq->ctrl->subsys;
 	u32 result;
 
-	req->ns = nvmet_find_namespace(req->sq->ctrl, req->cmd->common.nsid);
-	if (!req->ns)  {
-		req->error_loc = offsetof(struct nvme_common_command, nsid);
-		return NVME_SC_INVALID_NS | NVME_SC_DNR;
-	}
+	result = nvmet_req_find_ns(req);
+	if (result)
+		return result;
+
 	mutex_lock(&subsys->lock);
 	if (req->ns->readonly == true)
 		result = NVME_NS_WRITE_PROTECT;
diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index 8ce4d59cc9e7..95b58d4b1af2 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -417,15 +417,18 @@ void nvmet_stop_keep_alive_timer(struct nvmet_ctrl *ctrl)
 	cancel_delayed_work_sync(&ctrl->ka_work);
 }
 
-struct nvmet_ns *nvmet_find_namespace(struct nvmet_ctrl *ctrl, __le32 nsid)
+u16 nvmet_req_find_ns(struct nvmet_req *req)
 {
-	struct nvmet_ns *ns;
+	u32 nsid = le32_to_cpu(req->cmd->common.nsid);
 
-	ns = xa_load(&ctrl->subsys->namespaces, le32_to_cpu(nsid));
-	if (ns)
-		percpu_ref_get(&ns->ref);
+	req->ns = xa_load(&req->sq->ctrl->subsys->namespaces, nsid);
+	if (unlikely(!req->ns)) {
+		req->error_loc = offsetof(struct nvme_common_command, nsid);
+		return NVME_SC_INVALID_NS | NVME_SC_DNR;
+	}
 
-	return ns;
+	percpu_ref_get(&req->ns->ref);
+	return NVME_SC_SUCCESS;
 }
 
 static void nvmet_destroy_namespace(struct percpu_ref *ref)
@@ -862,11 +865,10 @@ static u16 nvmet_parse_io_cmd(struct nvmet_req *req)
 	if (nvmet_req_passthru_ctrl(req))
 		return nvmet_parse_passthru_io_cmd(req);
 
-	req->ns = nvmet_find_namespace(req->sq->ctrl, cmd->rw.nsid);
-	if (unlikely(!req->ns)) {
-		req->error_loc = offsetof(struct nvme_common_command, nsid);
-		return NVME_SC_INVALID_NS | NVME_SC_DNR;
-	}
+	ret = nvmet_req_find_ns(req);
+	if (unlikely(ret))
+		return ret;
+
 	ret = nvmet_check_ana_state(req->port, req->ns);
 	if (unlikely(ret)) {
 		req->error_loc = offsetof(struct nvme_common_command, nsid);
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index 8776dd1a0490..954b3d8451f5 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -443,7 +443,7 @@ struct nvmet_subsys *nvmet_subsys_alloc(const char *subsysnqn,
 void nvmet_subsys_put(struct nvmet_subsys *subsys);
 void nvmet_subsys_del_ctrls(struct nvmet_subsys *subsys);
 
-struct nvmet_ns *nvmet_find_namespace(struct nvmet_ctrl *ctrl, __le32 nsid);
+u16 nvmet_req_find_ns(struct nvmet_req *req);
 void nvmet_put_namespace(struct nvmet_ns *ns);
 int nvmet_ns_enable(struct nvmet_ns *ns);
 void nvmet_ns_disable(struct nvmet_ns *ns);
-- 
2.22.1


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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH V3 04/11] nvmet: remove extra variable in id-ns handler
  2021-02-10  5:47 [PATCH V3 00/11] nvmet: fixes and some cleanups Chaitanya Kulkarni
                   ` (2 preceding siblings ...)
  2021-02-10  5:47 ` [PATCH V3 03/11] nvmet: make nvmet_find_namespace() req based Chaitanya Kulkarni
@ 2021-02-10  5:47 ` Chaitanya Kulkarni
  2021-02-10  5:47 ` [PATCH V3 05/11] nvmet: add helper to report invalid opcode Chaitanya Kulkarni
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: Chaitanya Kulkarni @ 2021-02-10  5:47 UTC (permalink / raw)
  To: linux-nvme; +Cc: hch, Chaitanya Kulkarni, sagi

In nvmet_execute_identify_ns() local variable ctrl is accessed only in
one place, remove that and directly use it from nvmet_req->sq->ctrl.

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

diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index f32533480e66..552da813da18 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -462,7 +462,6 @@ static void nvmet_execute_identify_ctrl(struct nvmet_req *req)
 
 static void nvmet_execute_identify_ns(struct nvmet_req *req)
 {
-	struct nvmet_ctrl *ctrl = req->sq->ctrl;
 	struct nvme_id_ns *id;
 	u16 status;
 
@@ -523,7 +522,7 @@ static void nvmet_execute_identify_ns(struct nvmet_req *req)
 
 	id->lbaf[0].ds = req->ns->blksize_shift;
 
-	if (ctrl->pi_support && nvmet_ns_has_pi(req->ns)) {
+	if (req->sq->ctrl->pi_support && nvmet_ns_has_pi(req->ns)) {
 		id->dpc = NVME_NS_DPC_PI_FIRST | NVME_NS_DPC_PI_LAST |
 			  NVME_NS_DPC_PI_TYPE1 | NVME_NS_DPC_PI_TYPE2 |
 			  NVME_NS_DPC_PI_TYPE3;
-- 
2.22.1


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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH V3 05/11] nvmet: add helper to report invalid opcode
  2021-02-10  5:47 [PATCH V3 00/11] nvmet: fixes and some cleanups Chaitanya Kulkarni
                   ` (3 preceding siblings ...)
  2021-02-10  5:47 ` [PATCH V3 04/11] nvmet: remove extra variable in id-ns handler Chaitanya Kulkarni
@ 2021-02-10  5:47 ` Chaitanya Kulkarni
  2021-02-10  8:12   ` Christoph Hellwig
  2021-02-10  5:47 ` [PATCH V3 06/11] nvmet: use invalid cmd opcode helper Chaitanya Kulkarni
                   ` (6 subsequent siblings)
  11 siblings, 1 reply; 19+ messages in thread
From: Chaitanya Kulkarni @ 2021-02-10  5:47 UTC (permalink / raw)
  To: linux-nvme; +Cc: hch, Chaitanya Kulkarni, sagi

In the NVMeOF block device backend, file backend, and passthru backend
we reject and report the commands if opcode is not handled.

Add an helper and use it in block device backend to keep the code
and error message uniform.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 drivers/nvme/target/io-cmd-bdev.c | 5 +----
 drivers/nvme/target/nvmet.h       | 9 +++++++++
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/nvme/target/io-cmd-bdev.c b/drivers/nvme/target/io-cmd-bdev.c
index 23095bdfce06..105ef2b125cf 100644
--- a/drivers/nvme/target/io-cmd-bdev.c
+++ b/drivers/nvme/target/io-cmd-bdev.c
@@ -449,9 +449,6 @@ u16 nvmet_bdev_parse_io_cmd(struct nvmet_req *req)
 		req->execute = nvmet_bdev_execute_write_zeroes;
 		return 0;
 	default:
-		pr_err("unhandled cmd %d on qid %d\n", cmd->common.opcode,
-		       req->sq->qid);
-		req->error_loc = offsetof(struct nvme_common_command, opcode);
-		return NVME_SC_INVALID_OPCODE | NVME_SC_DNR;
+		return nvmet_report_invalid_opcode(req);
 	}
 }
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index 954b3d8451f5..6b5f1b60cf50 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -613,4 +613,13 @@ static inline sector_t nvmet_lba_to_sect(struct nvmet_ns *ns, __le64 lba)
 	return le64_to_cpu(lba) << (ns->blksize_shift - SECTOR_SHIFT);
 }
 
+static inline u16 nvmet_report_invalid_opcode(struct nvmet_req *req)
+{
+	pr_debug("unhandled cmd %d on qid %d\n", req->cmd->common.opcode,
+		 req->sq->qid);
+
+	req->error_loc = offsetof(struct nvme_common_command, opcode);
+	return NVME_SC_INVALID_OPCODE | NVME_SC_DNR;
+}
+
 #endif /* _NVMET_H */
-- 
2.22.1


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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH V3 06/11] nvmet: use invalid cmd opcode helper
  2021-02-10  5:47 [PATCH V3 00/11] nvmet: fixes and some cleanups Chaitanya Kulkarni
                   ` (4 preceding siblings ...)
  2021-02-10  5:47 ` [PATCH V3 05/11] nvmet: add helper to report invalid opcode Chaitanya Kulkarni
@ 2021-02-10  5:47 ` Chaitanya Kulkarni
  2021-02-10  5:47 ` [PATCH V3 07/11] " Chaitanya Kulkarni
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: Chaitanya Kulkarni @ 2021-02-10  5:47 UTC (permalink / raw)
  To: linux-nvme; +Cc: hch, Chaitanya Kulkarni, sagi

In the NVMeOF block device backend, file backend, and passthru backend
we reject and report the commands if opcode is not handled.

Use the previously introduced helper in file backend to reduce the
duplicate code and make the error message uniform.

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

diff --git a/drivers/nvme/target/io-cmd-file.c b/drivers/nvme/target/io-cmd-file.c
index 0abbefd9925e..715d4376c997 100644
--- a/drivers/nvme/target/io-cmd-file.c
+++ b/drivers/nvme/target/io-cmd-file.c
@@ -400,9 +400,6 @@ u16 nvmet_file_parse_io_cmd(struct nvmet_req *req)
 		req->execute = nvmet_file_execute_write_zeroes;
 		return 0;
 	default:
-		pr_err("unhandled cmd for file ns %d on qid %d\n",
-				cmd->common.opcode, req->sq->qid);
-		req->error_loc = offsetof(struct nvme_common_command, opcode);
-		return NVME_SC_INVALID_OPCODE | NVME_SC_DNR;
+		return nvmet_report_invalid_opcode(req);
 	}
 }
-- 
2.22.1


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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH V3 07/11] nvmet: use invalid cmd opcode helper
  2021-02-10  5:47 [PATCH V3 00/11] nvmet: fixes and some cleanups Chaitanya Kulkarni
                   ` (5 preceding siblings ...)
  2021-02-10  5:47 ` [PATCH V3 06/11] nvmet: use invalid cmd opcode helper Chaitanya Kulkarni
@ 2021-02-10  5:47 ` Chaitanya Kulkarni
  2021-02-10  5:47 ` [PATCH V3 08/11] nvmet: use min of device_path and disk len Chaitanya Kulkarni
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: Chaitanya Kulkarni @ 2021-02-10  5:47 UTC (permalink / raw)
  To: linux-nvme; +Cc: hch, Chaitanya Kulkarni, sagi

In the NVMeOF block device backend, file backend, and passthru backend
we reject and report the commands if opcode is not handled.

Use the previously introduced helper in the passthru backend to make the
error message uniform.

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

diff --git a/drivers/nvme/target/passthru.c b/drivers/nvme/target/passthru.c
index cbc88acdd233..3b22f4a868f4 100644
--- a/drivers/nvme/target/passthru.c
+++ b/drivers/nvme/target/passthru.c
@@ -494,7 +494,7 @@ u16 nvmet_parse_passthru_admin_cmd(struct nvmet_req *req)
 		return nvmet_setup_passthru_command(req);
 	default:
 		/* Reject commands not in the allowlist above */
-		return NVME_SC_INVALID_OPCODE | NVME_SC_DNR;
+		return nvmet_report_invalid_opcode(req);
 	}
 }
 
-- 
2.22.1


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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH V3 08/11] nvmet: use min of device_path and disk len
  2021-02-10  5:47 [PATCH V3 00/11] nvmet: fixes and some cleanups Chaitanya Kulkarni
                   ` (6 preceding siblings ...)
  2021-02-10  5:47 ` [PATCH V3 07/11] " Chaitanya Kulkarni
@ 2021-02-10  5:47 ` Chaitanya Kulkarni
  2021-02-10  8:13   ` Christoph Hellwig
  2021-02-10  5:48 ` [PATCH V3 09/11] nvme-loop: rename variable to get rid of the warn Chaitanya Kulkarni
                   ` (3 subsequent siblings)
  11 siblings, 1 reply; 19+ messages in thread
From: Chaitanya Kulkarni @ 2021-02-10  5:47 UTC (permalink / raw)
  To: linux-nvme; +Cc: hch, Chaitanya Kulkarni, sagi

In function __assign_req_name() instead of using the DEVICE_NAME_LEN in
strncpy() use min of DISK_NAME_LEN and strlen(req->ns->device_path).

This is needed to turn off the following warnings:-

In file included from drivers/nvme/target/core.c:14:
In function ‘__assign_req_name’,
    inlined from ‘trace_event_raw_event_nvmet_req_init’ at drivers/nvme/target/./trace.h:58:1:
drivers/nvme/target/trace.h:52:3: warning: ‘strncpy’ specified bound 32 equals destination size [-Wstringop-truncation]
   strncpy(name, req->ns->device_path, DISK_NAME_LEN);
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In function ‘__assign_req_name’,
    inlined from ‘perf_trace_nvmet_req_complete’ at drivers/nvme/target/./trace.h:100:1:
drivers/nvme/target/trace.h:52:3: warning: ‘strncpy’ specified bound 32 equals destination size [-Wstringop-truncation]
   strncpy(name, req->ns->device_path, DISK_NAME_LEN);
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In function ‘__assign_req_name’,
    inlined from ‘perf_trace_nvmet_req_init’ at drivers/nvme/target/./trace.h:58:1:
drivers/nvme/target/trace.h:52:3: warning: ‘strncpy’ specified bound 32 equals destination size [-Wstringop-truncation]
   strncpy(name, req->ns->device_path, DISK_NAME_LEN);
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In function ‘__assign_req_name’,
    inlined from ‘trace_event_raw_event_nvmet_req_complete’ at drivers/nvme/target/./trace.h:100:1:
drivers/nvme/target/trace.h:52:3: warning: ‘strncpy’ specified bound 32 equals destination size [-Wstringop-truncation]
   strncpy(name, req->ns->device_path, DISK_NAME_LEN);
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 drivers/nvme/target/trace.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/target/trace.h b/drivers/nvme/target/trace.h
index c14e3249a14d..958dca7bcfc7 100644
--- a/drivers/nvme/target/trace.h
+++ b/drivers/nvme/target/trace.h
@@ -48,8 +48,10 @@ static inline struct nvmet_ctrl *nvmet_req_to_ctrl(struct nvmet_req *req)
 
 static inline void __assign_req_name(char *name, struct nvmet_req *req)
 {
+	size_t len = min_t(size_t, DISK_NAME_LEN, strlen(req->ns->device_path));
+
 	if (req->ns)
-		strncpy(name, req->ns->device_path, DISK_NAME_LEN);
+		strncpy(name, req->ns->device_path, len);
 	else
 		memset(name, 0, DISK_NAME_LEN);
 }
-- 
2.22.1


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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH V3 09/11] nvme-loop: rename variable to get rid of the warn
  2021-02-10  5:47 [PATCH V3 00/11] nvmet: fixes and some cleanups Chaitanya Kulkarni
                   ` (7 preceding siblings ...)
  2021-02-10  5:47 ` [PATCH V3 08/11] nvmet: use min of device_path and disk len Chaitanya Kulkarni
@ 2021-02-10  5:48 ` Chaitanya Kulkarni
  2021-02-10  8:13   ` Christoph Hellwig
  2021-02-10  5:48 ` [PATCH V3 10/11] nvmet: add nvmet_req_subsys() helper Chaitanya Kulkarni
                   ` (2 subsequent siblings)
  11 siblings, 1 reply; 19+ messages in thread
From: Chaitanya Kulkarni @ 2021-02-10  5:48 UTC (permalink / raw)
  To: linux-nvme; +Cc: hch, Chaitanya Kulkarni, sagi

Rename the nvme_loop_init_request() numa_node parameter to
hw_queue_numa_node so that we can get rid of the following warning :-

drivers/nvme/target/loop.c: In function ‘nvme_loop_init_request’:
drivers/nvme/target/loop.c:205:16: warning: declaration of ‘numa_node’ shadows a global declaration [-Wshadow]
   unsigned int numa_node)

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

diff --git a/drivers/nvme/target/loop.c b/drivers/nvme/target/loop.c
index cb6f86572b24..88d0656ad4a5 100644
--- a/drivers/nvme/target/loop.c
+++ b/drivers/nvme/target/loop.c
@@ -202,7 +202,7 @@ static int nvme_loop_init_iod(struct nvme_loop_ctrl *ctrl,
 
 static int nvme_loop_init_request(struct blk_mq_tag_set *set,
 		struct request *req, unsigned int hctx_idx,
-		unsigned int numa_node)
+		unsigned int hw_queue_numa_node)
 {
 	struct nvme_loop_ctrl *ctrl = set->driver_data;
 
-- 
2.22.1


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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH V3 10/11] nvmet: add nvmet_req_subsys() helper
  2021-02-10  5:47 [PATCH V3 00/11] nvmet: fixes and some cleanups Chaitanya Kulkarni
                   ` (8 preceding siblings ...)
  2021-02-10  5:48 ` [PATCH V3 09/11] nvme-loop: rename variable to get rid of the warn Chaitanya Kulkarni
@ 2021-02-10  5:48 ` Chaitanya Kulkarni
  2021-02-10  5:48 ` [PATCH V3 11/11] nvmet: remove else at the end of the function Chaitanya Kulkarni
  2021-02-10  8:11 ` [PATCH V3 00/11] nvmet: fixes and some cleanups Christoph Hellwig
  11 siblings, 0 replies; 19+ messages in thread
From: Chaitanya Kulkarni @ 2021-02-10  5:48 UTC (permalink / raw)
  To: linux-nvme; +Cc: hch, Chaitanya Kulkarni, sagi

Just like what we have to get the passthru ctrl from the req, add an
helper to get the subsystem associated with the nvmet_req() instead
of open coding the chain of structures.

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

diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index 552da813da18..bc6a774f2124 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -683,7 +683,7 @@ static u16 nvmet_write_protect_flush_sync(struct nvmet_req *req)
 static u16 nvmet_set_feat_write_protect(struct nvmet_req *req)
 {
 	u32 write_protect = le32_to_cpu(req->cmd->common.cdw11);
-	struct nvmet_subsys *subsys = req->sq->ctrl->subsys;
+	struct nvmet_subsys *subsys = nvmet_req_subsys(req);
 	u16 status;
 
 	status = nvmet_req_find_ns(req);
@@ -742,7 +742,7 @@ u16 nvmet_set_feat_async_event(struct nvmet_req *req, u32 mask)
 
 void nvmet_execute_set_features(struct nvmet_req *req)
 {
-	struct nvmet_subsys *subsys = req->sq->ctrl->subsys;
+	struct nvmet_subsys *subsys = nvmet_req_subsys(req);
 	u32 cdw10 = le32_to_cpu(req->cmd->common.cdw10);
 	u32 cdw11 = le32_to_cpu(req->cmd->common.cdw11);
 	u16 status = 0;
@@ -786,7 +786,7 @@ void nvmet_execute_set_features(struct nvmet_req *req)
 
 static u16 nvmet_get_feat_write_protect(struct nvmet_req *req)
 {
-	struct nvmet_subsys *subsys = req->sq->ctrl->subsys;
+	struct nvmet_subsys *subsys = nvmet_req_subsys(req);
 	u32 result;
 
 	result = nvmet_req_find_ns(req);
@@ -816,7 +816,7 @@ void nvmet_get_feat_async_event(struct nvmet_req *req)
 
 void nvmet_execute_get_features(struct nvmet_req *req)
 {
-	struct nvmet_subsys *subsys = req->sq->ctrl->subsys;
+	struct nvmet_subsys *subsys = nvmet_req_subsys(req);
 	u32 cdw10 = le32_to_cpu(req->cmd->common.cdw10);
 	u16 status = 0;
 
@@ -923,7 +923,7 @@ u16 nvmet_parse_admin_cmd(struct nvmet_req *req)
 
 	if (nvme_is_fabrics(cmd))
 		return nvmet_parse_fabrics_cmd(req);
-	if (req->sq->ctrl->subsys->type == NVME_NQN_DISC)
+	if (nvmet_req_subsys(req)->type == NVME_NQN_DISC)
 		return nvmet_parse_discovery_cmd(req);
 
 	ret = nvmet_check_ctrl_status(req, cmd);
diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index 95b58d4b1af2..32221f98f1cc 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -421,7 +421,7 @@ u16 nvmet_req_find_ns(struct nvmet_req *req)
 {
 	u32 nsid = le32_to_cpu(req->cmd->common.nsid);
 
-	req->ns = xa_load(&req->sq->ctrl->subsys->namespaces, nsid);
+	req->ns = xa_load(&nvmet_req_subsys(req)->namespaces, nsid);
 	if (unlikely(!req->ns)) {
 		req->error_loc = offsetof(struct nvme_common_command, nsid);
 		return NVME_SC_INVALID_NS | NVME_SC_DNR;
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index 6b5f1b60cf50..50535563e8c5 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -551,6 +551,11 @@ static inline u32 nvmet_dsm_len(struct nvmet_req *req)
 		sizeof(struct nvme_dsm_range);
 }
 
+static inline struct nvmet_subsys *nvmet_req_subsys(struct nvmet_req *req)
+{
+	return req->sq->ctrl->subsys;
+}
+
 #ifdef CONFIG_NVME_TARGET_PASSTHRU
 void nvmet_passthru_subsys_free(struct nvmet_subsys *subsys);
 int nvmet_passthru_ctrl_enable(struct nvmet_subsys *subsys);
@@ -585,7 +590,7 @@ static inline struct nvme_ctrl *nvmet_passthru_ctrl(struct nvmet_subsys *subsys)
 static inline struct nvme_ctrl *
 nvmet_req_passthru_ctrl(struct nvmet_req *req)
 {
-	return nvmet_passthru_ctrl(req->sq->ctrl->subsys);
+	return nvmet_passthru_ctrl(nvmet_req_subsys(req));
 }
 
 u16 errno_to_nvme_status(struct nvmet_req *req, int errno);
diff --git a/drivers/nvme/target/passthru.c b/drivers/nvme/target/passthru.c
index 3b22f4a868f4..f50c7b2bf21c 100644
--- a/drivers/nvme/target/passthru.c
+++ b/drivers/nvme/target/passthru.c
@@ -239,9 +239,9 @@ static void nvmet_passthru_execute_cmd(struct nvmet_req *req)
 		}
 
 		q = ns->queue;
-		timeout = req->sq->ctrl->subsys->io_timeout;
+		timeout = nvmet_req_subsys(req)->io_timeout;
 	} else {
-		timeout = req->sq->ctrl->subsys->admin_timeout;
+		timeout = nvmet_req_subsys(req)->admin_timeout;
 	}
 
 	rq = nvme_alloc_request(q, req->cmd, 0);
-- 
2.22.1


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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH V3 11/11] nvmet: remove else at the end of the function
  2021-02-10  5:47 [PATCH V3 00/11] nvmet: fixes and some cleanups Chaitanya Kulkarni
                   ` (9 preceding siblings ...)
  2021-02-10  5:48 ` [PATCH V3 10/11] nvmet: add nvmet_req_subsys() helper Chaitanya Kulkarni
@ 2021-02-10  5:48 ` Chaitanya Kulkarni
  2021-02-10  8:11 ` [PATCH V3 00/11] nvmet: fixes and some cleanups Christoph Hellwig
  11 siblings, 0 replies; 19+ messages in thread
From: Chaitanya Kulkarni @ 2021-02-10  5:48 UTC (permalink / raw)
  To: linux-nvme; +Cc: hch, Chaitanya Kulkarni, sagi

The function nvmet_parse_io_cmd() returns value from
nvmet_file_parse_io_cmd() or nvmet_bdev_parse_io_cmd() based on which
backend is set for the request. Remove the else and just return the
value from nvmet_bdev_parse_io_cmd().

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

diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index 32221f98f1cc..1d5516272530 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -882,8 +882,8 @@ static u16 nvmet_parse_io_cmd(struct nvmet_req *req)
 
 	if (req->ns->file)
 		return nvmet_file_parse_io_cmd(req);
-	else
-		return nvmet_bdev_parse_io_cmd(req);
+
+	return nvmet_bdev_parse_io_cmd(req);
 }
 
 bool nvmet_req_init(struct nvmet_req *req, struct nvmet_cq *cq,
-- 
2.22.1


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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH V3 00/11] nvmet: fixes and some cleanups
  2021-02-10  5:47 [PATCH V3 00/11] nvmet: fixes and some cleanups Chaitanya Kulkarni
                   ` (10 preceding siblings ...)
  2021-02-10  5:48 ` [PATCH V3 11/11] nvmet: remove else at the end of the function Chaitanya Kulkarni
@ 2021-02-10  8:11 ` Christoph Hellwig
  2021-02-10  8:52   ` Chaitanya Kulkarni
  11 siblings, 1 reply; 19+ messages in thread
From: Christoph Hellwig @ 2021-02-10  8:11 UTC (permalink / raw)
  To: Chaitanya Kulkarni; +Cc: hch, linux-nvme, sagi

I've applied this to nvme-5.12 with one exception and a few tweaks.
More in individual replies.

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH V3 05/11] nvmet: add helper to report invalid opcode
  2021-02-10  5:47 ` [PATCH V3 05/11] nvmet: add helper to report invalid opcode Chaitanya Kulkarni
@ 2021-02-10  8:12   ` Christoph Hellwig
  0 siblings, 0 replies; 19+ messages in thread
From: Christoph Hellwig @ 2021-02-10  8:12 UTC (permalink / raw)
  To: Chaitanya Kulkarni; +Cc: hch, linux-nvme, sagi

No real need to have this slow path function inline, so I've moved it
out of line.

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH V3 08/11] nvmet: use min of device_path and disk len
  2021-02-10  5:47 ` [PATCH V3 08/11] nvmet: use min of device_path and disk len Chaitanya Kulkarni
@ 2021-02-10  8:13   ` Christoph Hellwig
  0 siblings, 0 replies; 19+ messages in thread
From: Christoph Hellwig @ 2021-02-10  8:13 UTC (permalink / raw)
  To: Chaitanya Kulkarni; +Cc: hch, linux-nvme, sagi

>  static inline void __assign_req_name(char *name, struct nvmet_req *req)
>  {
> +	size_t len = min_t(size_t, DISK_NAME_LEN, strlen(req->ns->device_path));
> +
>  	if (req->ns)
> -		strncpy(name, req->ns->device_path, DISK_NAME_LEN);
> +		strncpy(name, req->ns->device_path, len);
>  	else
>  		memset(name, 0, DISK_NAME_LEN);

This could dereference a NULL req->ns.  I've changed the patch
to eliminate the local variable and to return early for the NULL
req->ns case.

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH V3 09/11] nvme-loop: rename variable to get rid of the warn
  2021-02-10  5:48 ` [PATCH V3 09/11] nvme-loop: rename variable to get rid of the warn Chaitanya Kulkarni
@ 2021-02-10  8:13   ` Christoph Hellwig
  2021-02-10  8:18     ` Chaitanya Kulkarni
  0 siblings, 1 reply; 19+ messages in thread
From: Christoph Hellwig @ 2021-02-10  8:13 UTC (permalink / raw)
  To: Chaitanya Kulkarni; +Cc: hch, linux-nvme, sagi

numa_node is a perfectly fine name for a local variable.  Please find
the global one and rename it instead.

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH V3 09/11] nvme-loop: rename variable to get rid of the warn
  2021-02-10  8:13   ` Christoph Hellwig
@ 2021-02-10  8:18     ` Chaitanya Kulkarni
  2021-02-10  8:23       ` Christoph Hellwig
  0 siblings, 1 reply; 19+ messages in thread
From: Chaitanya Kulkarni @ 2021-02-10  8:18 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: sagi, linux-nvme

On 2/10/21 00:13, Christoph Hellwig wrote:
> numa_node is a perfectly fine name for a local variable.  Please find
> the global one and rename it instead.
>
I know, the warning is really annoying breaks the automated script for
clean compilation.

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH V3 09/11] nvme-loop: rename variable to get rid of the warn
  2021-02-10  8:18     ` Chaitanya Kulkarni
@ 2021-02-10  8:23       ` Christoph Hellwig
  0 siblings, 0 replies; 19+ messages in thread
From: Christoph Hellwig @ 2021-02-10  8:23 UTC (permalink / raw)
  To: Chaitanya Kulkarni; +Cc: Christoph Hellwig, linux-nvme, sagi

On Wed, Feb 10, 2021 at 08:18:16AM +0000, Chaitanya Kulkarni wrote:
> On 2/10/21 00:13, Christoph Hellwig wrote:
> > numa_node is a perfectly fine name for a local variable.  Please find
> > the global one and rename it instead.
> >
> I know, the warning is really annoying breaks the automated script for
> clean compilation.

So fix the global variable, which is indeed very badly named for global
variable.

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH V3 00/11] nvmet: fixes and some cleanups
  2021-02-10  8:11 ` [PATCH V3 00/11] nvmet: fixes and some cleanups Christoph Hellwig
@ 2021-02-10  8:52   ` Chaitanya Kulkarni
  0 siblings, 0 replies; 19+ messages in thread
From: Chaitanya Kulkarni @ 2021-02-10  8:52 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: sagi, linux-nvme

On 2/10/21 00:12, Christoph Hellwig wrote:
> I've applied this to nvme-5.12 with one exception and a few tweaks.
> More in individual replies.
>
I've pulled the changes from nvme-5.12 and ran blocktests. It seems to
work fine.

# gitlog -12
26bb85e126d0 (HEAD -> nvme-5.12) Merge branch 'nvme-5.12' of
git://git.infradead.org/nvme into nvme-5.12
523bb6d79f0e (origin/nvme-5.12) nvme-hwmon: rework to avoid devm allocation
4da2cba48943 nvmet: remove else at the end of the function
7684ff0eac09 nvmet: add nvmet_req_subsys() helper
d93178da21dd nvmet: use min of device_path and disk len
ec3dc8c50add nvmet: use invalid cmd opcode helper
ff887268ac4e nvmet: use invalid cmd opcode helper
022b300c9227 nvmet: add helper to report invalid opcode
1be5d3ef5976 nvmet: remove extra variable in id-ns handler
214bffc29d79 nvmet: make nvmet_find_namespace() req based
617e7b18cc7e nvmet: return uniform error for invalid ns
275f63fa4ddb nvmet: set status to 0 in case for invalid nsid
#

# ./check tests/nvme
nvme/002 (create many subsystems and test discovery)         [passed]
    runtime  19.845s  ...  28.439s
nvme/003 (test if we're sending keep-alives to a discovery controller)
[passed]
    runtime    ...  10.131s
nvme/004 (test nvme and nvmet UUID NS descriptors)           [passed]
    runtime  1.595s  ...  1.704s
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.078s  ...  0.112s
nvme/007 (create an NVMeOF target with a file-backed ns)     [passed]
    runtime  0.073s  ...  0.073s
nvme/008 (create an NVMeOF host with a block device-backed ns) [passed]
    runtime  1.619s  ...  1.707s
nvme/009 (create an NVMeOF host with a file-backed ns)       [passed]
    runtime    ...  1.663s
nvme/010 (run data verification fio job on NVMeOF block device-backed
ns) [passed]
    runtime  24.800s  ...  26.836s
nvme/011 (run data verification fio job on NVMeOF file-backed ns) [passed]
    runtime  262.535s  ...  284.324s
nvme/012 (run mkfs and data verification fio job on NVMeOF block
device-backed ns) [passed]
    runtime  46.843s  ...  45.088s
nvme/013 (run mkfs and data verification fio job on NVMeOF file-backed
ns) [passed]
    runtime  280.317s  ...  324.476s
nvme/014 (flush a NVMeOF block device-backed ns)             [passed]
    runtime  21.985s  ...  21.937s
nvme/015 (unit test for NVMe flush for file backed ns)       [passed]
    runtime  22.981s  ...  22.106s
nvme/016 (create/delete many NVMeOF block device-backed ns and test
discovery) [passed]
    runtime  15.777s  ...  16.286s
nvme/017 (create/delete many file-ns and test discovery)     [passed]
    runtime  15.995s  ...  16.244s
nvme/018 (unit test NVMe-oF out of range access on a file backend) [passed]
    runtime  1.650s  ...  1.664s
nvme/019 (test NVMe DSM Discard command on NVMeOF block-device ns) [passed]
    runtime  1.677s  ...  1.726s
nvme/020 (test NVMe DSM Discard command on NVMeOF file-backed ns) [passed]
    runtime  1.660s  ...  1.668s
nvme/021 (test NVMe list command on NVMeOF file-backed ns)   [passed]
    runtime  1.652s  ...  1.669s
nvme/022 (test NVMe reset command on NVMeOF file-backed ns)  [passed]
    runtime  2.087s  ...  2.098s
nvme/023 (test NVMe smart-log command on NVMeOF block-device ns) [passed]
    runtime  1.697s  ...  1.735s
nvme/024 (test NVMe smart-log command on NVMeOF file-backed ns) [passed]
    runtime  1.794s  ...  1.682s
nvme/025 (test NVMe effects-log command on NVMeOF file-backed ns) [passed]
    runtime  1.658s  ...  1.655s
nvme/026 (test NVMe ns-descs command on NVMeOF file-backed ns) [passed]
    runtime  1.651s  ...  1.694s
nvme/027 (test NVMe ns-rescan command on NVMeOF file-backed ns) [passed]
    runtime  1.654s  ...  1.763s
nvme/028 (test NVMe list-subsys command on NVMeOF file-backed ns) [passed]
    runtime  1.659s  ...  1.654s
nvme/029 (test userspace IO via nvme-cli read/write interface) [passed]
    runtime  2.007s  ...  2.051s
nvme/030 (ensure the discovery generation counter is updated
appropriately) [passed]
    runtime  0.321s  ...  0.316s
nvme/031 (test deletion of NVMeOF controllers immediately after setup)
[passed]
    runtime  5.528s  ...  5.587s
nvme/038 (test deletion of NVMeOF subsystem without enabling) [passed]
    runtime  0.046s  ...  0.044s

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2021-02-10  8:52 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-10  5:47 [PATCH V3 00/11] nvmet: fixes and some cleanups Chaitanya Kulkarni
2021-02-10  5:47 ` [PATCH V3 01/11] nvmet: set status to 0 in case for invalid nsid Chaitanya Kulkarni
2021-02-10  5:47 ` [PATCH V3 02/11] nvmet: return uniform error for invalid ns Chaitanya Kulkarni
2021-02-10  5:47 ` [PATCH V3 03/11] nvmet: make nvmet_find_namespace() req based Chaitanya Kulkarni
2021-02-10  5:47 ` [PATCH V3 04/11] nvmet: remove extra variable in id-ns handler Chaitanya Kulkarni
2021-02-10  5:47 ` [PATCH V3 05/11] nvmet: add helper to report invalid opcode Chaitanya Kulkarni
2021-02-10  8:12   ` Christoph Hellwig
2021-02-10  5:47 ` [PATCH V3 06/11] nvmet: use invalid cmd opcode helper Chaitanya Kulkarni
2021-02-10  5:47 ` [PATCH V3 07/11] " Chaitanya Kulkarni
2021-02-10  5:47 ` [PATCH V3 08/11] nvmet: use min of device_path and disk len Chaitanya Kulkarni
2021-02-10  8:13   ` Christoph Hellwig
2021-02-10  5:48 ` [PATCH V3 09/11] nvme-loop: rename variable to get rid of the warn Chaitanya Kulkarni
2021-02-10  8:13   ` Christoph Hellwig
2021-02-10  8:18     ` Chaitanya Kulkarni
2021-02-10  8:23       ` Christoph Hellwig
2021-02-10  5:48 ` [PATCH V3 10/11] nvmet: add nvmet_req_subsys() helper Chaitanya Kulkarni
2021-02-10  5:48 ` [PATCH V3 11/11] nvmet: remove else at the end of the function Chaitanya Kulkarni
2021-02-10  8:11 ` [PATCH V3 00/11] nvmet: fixes and some cleanups Christoph Hellwig
2021-02-10  8:52   ` Chaitanya Kulkarni

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).