* [PATCH 1/4] nvmet: remove extra variable in smart log nsid
2020-12-10 2:58 [PATCH 0/4] nvmet: admin-cmd related cleanups and a fix Chaitanya Kulkarni
@ 2020-12-10 2:58 ` Chaitanya Kulkarni
2020-12-10 2:58 ` [PATCH 2/4] nvmet: remove extra variable in identify ns Chaitanya Kulkarni
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Chaitanya Kulkarni @ 2020-12-10 2:58 UTC (permalink / raw)
To: linux-nvme; +Cc: sagi, Chaitanya Kulkarni, hch
We remove the extra local variable struct nvmet_ns in
nvmet_get_smart_log_nsid() since req already has ns member that can be
reused, this also eliminates the explicit call to nvmet_put_namespace()
which is already present in the request completion path.
Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
drivers/nvme/target/admin-cmd.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index dca34489a1dc..4241cb072f64 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -74,11 +74,11 @@ static void nvmet_execute_get_log_page_error(struct nvmet_req *req)
static u16 nvmet_get_smart_log_nsid(struct nvmet_req *req,
struct nvme_smart_log *slog)
{
- struct nvmet_ns *ns;
u64 host_reads, host_writes, data_units_read, data_units_written;
- ns = nvmet_find_namespace(req->sq->ctrl, req->cmd->get_log_page.nsid);
- if (!ns) {
+ 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);
@@ -86,14 +86,14 @@ static u16 nvmet_get_smart_log_nsid(struct nvmet_req *req,
}
/* we don't have the right data for file backed ns */
- if (!ns->bdev)
+ if (!req->ns->bdev)
goto out;
- host_reads = part_stat_read(ns->bdev->bd_part, ios[READ]);
- data_units_read = DIV_ROUND_UP(part_stat_read(ns->bdev->bd_part,
+ host_reads = part_stat_read(req->ns->bdev->bd_part, ios[READ]);
+ data_units_read = DIV_ROUND_UP(part_stat_read(req->ns->bdev->bd_part,
sectors[READ]), 1000);
- host_writes = part_stat_read(ns->bdev->bd_part, ios[WRITE]);
- data_units_written = DIV_ROUND_UP(part_stat_read(ns->bdev->bd_part,
+ host_writes = part_stat_read(req->ns->bdev->bd_part, ios[WRITE]);
+ data_units_written = DIV_ROUND_UP(part_stat_read(req->ns->bdev->bd_part,
sectors[WRITE]), 1000);
put_unaligned_le64(host_reads, &slog->host_reads[0]);
@@ -101,7 +101,6 @@ static u16 nvmet_get_smart_log_nsid(struct nvmet_req *req,
put_unaligned_le64(host_writes, &slog->host_writes[0]);
put_unaligned_le64(data_units_written, &slog->data_units_written[0]);
out:
- nvmet_put_namespace(ns);
return NVME_SC_SUCCESS;
}
--
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] 5+ messages in thread
* [PATCH 2/4] nvmet: remove extra variable in identify ns
2020-12-10 2:58 [PATCH 0/4] nvmet: admin-cmd related cleanups and a fix Chaitanya Kulkarni
2020-12-10 2:58 ` [PATCH 1/4] nvmet: remove extra variable in smart log nsid Chaitanya Kulkarni
@ 2020-12-10 2:58 ` Chaitanya Kulkarni
2020-12-10 2:58 ` [PATCH 3/4] nvmet: remove extra variable in id-desclist Chaitanya Kulkarni
2020-12-10 2:58 ` [PATCH 4/4] nvmet: set right status on error in id-ns handler Chaitanya Kulkarni
3 siblings, 0 replies; 5+ messages in thread
From: Chaitanya Kulkarni @ 2020-12-10 2:58 UTC (permalink / raw)
To: linux-nvme; +Cc: sagi, Chaitanya Kulkarni, hch
We remove the extra local variable struct nvmet_ns in
nvmet_execute_identify_ns() since req already has ns member that can be
reused, this also eliminates the explicit call to nvmet_put_namespace()
which is already present in the request completion path.
Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
drivers/nvme/target/admin-cmd.c | 31 +++++++++++++++----------------
1 file changed, 15 insertions(+), 16 deletions(-)
diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index 4241cb072f64..86b6bd9a662a 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -468,7 +468,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 nvmet_ns *ns;
struct nvme_id_ns *id;
u16 status = 0;
@@ -485,18 +484,19 @@ static void nvmet_execute_identify_ns(struct nvmet_req *req)
}
/* return an all zeroed buffer if we can't find an active namespace */
- ns = nvmet_find_namespace(ctrl, req->cmd->identify.nsid);
- if (!ns)
+ req->ns = nvmet_find_namespace(ctrl, req->cmd->identify.nsid);
+ if (!req->ns)
goto done;
- nvmet_ns_revalidate(ns);
+ nvmet_ns_revalidate(req->ns);
/*
* nuse = ncap = nsze isn't always true, but we have no way to find
* that out from the underlying device.
*/
- id->ncap = id->nsze = cpu_to_le64(ns->size >> ns->blksize_shift);
- switch (req->port->ana_state[ns->anagrpid]) {
+ id->ncap = id->nsze =
+ cpu_to_le64(req->ns->size >> req->ns->blksize_shift);
+ switch (req->port->ana_state[req->ns->anagrpid]) {
case NVME_ANA_INACCESSIBLE:
case NVME_ANA_PERSISTENT_LOSS:
break;
@@ -505,8 +505,8 @@ static void nvmet_execute_identify_ns(struct nvmet_req *req)
break;
}
- if (ns->bdev)
- nvmet_bdev_set_limits(ns->bdev, id);
+ if (req->ns->bdev)
+ nvmet_bdev_set_limits(req->ns->bdev, id);
/*
* We just provide a single LBA format that matches what the
@@ -520,25 +520,24 @@ static void nvmet_execute_identify_ns(struct nvmet_req *req)
* controllers, but also with any other user of the block device.
*/
id->nmic = (1 << 0);
- id->anagrpid = cpu_to_le32(ns->anagrpid);
+ id->anagrpid = cpu_to_le32(req->ns->anagrpid);
- memcpy(&id->nguid, &ns->nguid, sizeof(id->nguid));
+ memcpy(&id->nguid, &req->ns->nguid, sizeof(id->nguid));
- id->lbaf[0].ds = ns->blksize_shift;
+ id->lbaf[0].ds = req->ns->blksize_shift;
- if (ctrl->pi_support && nvmet_ns_has_pi(ns)) {
+ if (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;
id->mc = NVME_MC_EXTENDED_LBA;
- id->dps = ns->pi_type;
+ id->dps = req->ns->pi_type;
id->flbas = NVME_NS_FLBAS_META_EXT;
- id->lbaf[0].ms = cpu_to_le16(ns->metadata_size);
+ id->lbaf[0].ms = cpu_to_le16(req->ns->metadata_size);
}
- if (ns->readonly)
+ if (req->ns->readonly)
id->nsattr |= (1 << 0);
- nvmet_put_namespace(ns);
done:
status = nvmet_copy_to_sgl(req, 0, id, sizeof(*id));
kfree(id);
--
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] 5+ messages in thread
* [PATCH 3/4] nvmet: remove extra variable in id-desclist
2020-12-10 2:58 [PATCH 0/4] nvmet: admin-cmd related cleanups and a fix Chaitanya Kulkarni
2020-12-10 2:58 ` [PATCH 1/4] nvmet: remove extra variable in smart log nsid Chaitanya Kulkarni
2020-12-10 2:58 ` [PATCH 2/4] nvmet: remove extra variable in identify ns Chaitanya Kulkarni
@ 2020-12-10 2:58 ` Chaitanya Kulkarni
2020-12-10 2:58 ` [PATCH 4/4] nvmet: set right status on error in id-ns handler Chaitanya Kulkarni
3 siblings, 0 replies; 5+ messages in thread
From: Chaitanya Kulkarni @ 2020-12-10 2:58 UTC (permalink / raw)
To: linux-nvme; +Cc: sagi, Chaitanya Kulkarni, hch
We remove the extra local variable struct nvmet_ns in
nvmet_execute_identify_desclist() since req already has ns member that
can be reused, this also eliminates the explicit call to
nvmet_put_namespace() which is already present in the request
completion path.
Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
drivers/nvme/target/admin-cmd.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index 86b6bd9a662a..29438ecfecae 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -601,37 +601,35 @@ static u16 nvmet_copy_ns_identifier(struct nvmet_req *req, u8 type, u8 len,
static void nvmet_execute_identify_desclist(struct nvmet_req *req)
{
- struct nvmet_ns *ns;
u16 status = 0;
off_t off = 0;
- ns = nvmet_find_namespace(req->sq->ctrl, req->cmd->identify.nsid);
- if (!ns) {
+ 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;
goto out;
}
- if (memchr_inv(&ns->uuid, 0, sizeof(ns->uuid))) {
+ if (memchr_inv(&req->ns->uuid, 0, sizeof(req->ns->uuid))) {
status = nvmet_copy_ns_identifier(req, NVME_NIDT_UUID,
NVME_NIDT_UUID_LEN,
- &ns->uuid, &off);
+ &req->ns->uuid, &off);
if (status)
- goto out_put_ns;
+ goto out;
}
- if (memchr_inv(ns->nguid, 0, sizeof(ns->nguid))) {
+ if (memchr_inv(req->ns->nguid, 0, sizeof(req->ns->nguid))) {
status = nvmet_copy_ns_identifier(req, NVME_NIDT_NGUID,
NVME_NIDT_NGUID_LEN,
- &ns->nguid, &off);
+ &req->ns->nguid, &off);
if (status)
- goto out_put_ns;
+ goto out;
}
if (sg_zero_buffer(req->sg, req->sg_cnt, NVME_IDENTIFY_DATA_SIZE - off,
off) != NVME_IDENTIFY_DATA_SIZE - off)
status = NVME_SC_INTERNAL | NVME_SC_DNR;
-out_put_ns:
- nvmet_put_namespace(ns);
+
out:
nvmet_req_complete(req, status);
}
--
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] 5+ messages in thread
* [PATCH 4/4] nvmet: set right status on error in id-ns handler
2020-12-10 2:58 [PATCH 0/4] nvmet: admin-cmd related cleanups and a fix Chaitanya Kulkarni
` (2 preceding siblings ...)
2020-12-10 2:58 ` [PATCH 3/4] nvmet: remove extra variable in id-desclist Chaitanya Kulkarni
@ 2020-12-10 2:58 ` Chaitanya Kulkarni
3 siblings, 0 replies; 5+ messages in thread
From: Chaitanya Kulkarni @ 2020-12-10 2:58 UTC (permalink / raw)
To: linux-nvme; +Cc: sagi, Chaitanya Kulkarni, hch
The function nvmet_execute_identify_ns() does't set the status
if call to nvmet_find_namespace() fails. In that case we set the
status of the request to the value return by the nvmet_copy_sgl().
Set the status to NVME_SC_INVALID_NS and adjust the code such that
request will have the right status on nvmet_find_namespace() failure.
Without this patch :-
# nvme id-ns /dev/nvme1 -n 3
NVME Identify Namespace 3:
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)
With this patch-series :-
# gitlog -4
3fd4a1736f35 (HEAD -> nvme-5.11) nvmet: set right status on error in id-ns handler
4d8a9ebec05e nvmet: remove extra variable in id-desclist
de9e611ac167 nvmet: remove extra variable in identify ns
54165b5c1ca3 nvmet: remove extra variable in smart log nsid
# nvme id-ns /dev/nvme1 -n 3
NVMe status: INVALID_NS: The namespace or the format of that namespace is invalid(0xb)
Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
drivers/nvme/target/admin-cmd.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index 29438ecfecae..74620240ac47 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -485,8 +485,10 @@ 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)
+ if (!req->ns) {
+ status = NVME_SC_INVALID_NS;
goto done;
+ }
nvmet_ns_revalidate(req->ns);
@@ -539,7 +541,11 @@ static void nvmet_execute_identify_ns(struct nvmet_req *req)
if (req->ns->readonly)
id->nsattr |= (1 << 0);
done:
- status = nvmet_copy_to_sgl(req, 0, id, sizeof(*id));
+ if (nvmet_copy_to_sgl(req, 0, id, sizeof(*id))) {
+ if (!status)
+ status = NVME_SC_SGL_INVALID_DATA | NVME_SC_DNR;
+ }
+
kfree(id);
out:
nvmet_req_complete(req, status);
--
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] 5+ messages in thread