* [PATCH 1/5] nvme: refine the Qemu Identify CNS quirk
2020-04-06 12:13 a few more namespace scanning cleanups Christoph Hellwig
@ 2020-04-06 12:13 ` Christoph Hellwig
2020-04-06 17:03 ` Sagi Grimberg
2020-04-06 12:13 ` [PATCH 2/5] nvme: clean up nvme_scan_work Christoph Hellwig
` (4 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Christoph Hellwig @ 2020-04-06 12:13 UTC (permalink / raw)
To: Keith Busch, Sagi Grimberg; +Cc: linux-nvme
Add a helper to check if we can use Identify CNS values > 1, and refine
the Qemu quirk to not apply to reported versions larger than 1.1, as the
Qemu implementation had been fixed by then.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/nvme/host/core.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 05aa3525ef06..01889905875b 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1027,6 +1027,19 @@ void nvme_stop_keep_alive(struct nvme_ctrl *ctrl)
}
EXPORT_SYMBOL_GPL(nvme_stop_keep_alive);
+/*
+ * In NVMe 1.0 the CNS field was just a binary controller or namespace
+ * flag, thus sending any new CNS opcodes has a big chance of not working.
+ * Qemu unfortunately had that bug after reporting a 1.1 version compliance
+ * (but not for any later version).
+ */
+static bool nvme_ctrl_limited_cns(struct nvme_ctrl *ctrl)
+{
+ if (ctrl->quirks & NVME_QUIRK_IDENTIFY_CNS)
+ return ctrl->vs < NVME_VS(1, 2, 0);
+ return ctrl->vs < NVME_VS(1, 1, 0);
+}
+
static int nvme_identify_ctrl(struct nvme_ctrl *dev, struct nvme_id_ctrl **id)
{
struct nvme_command c = { };
@@ -3812,8 +3825,7 @@ static void nvme_scan_work(struct work_struct *work)
mutex_lock(&ctrl->scan_lock);
nn = le32_to_cpu(id->nn);
- if (ctrl->vs >= NVME_VS(1, 1, 0) &&
- !(ctrl->quirks & NVME_QUIRK_IDENTIFY_CNS)) {
+ if (!nvme_ctrl_limited_cns(ctrl)) {
if (!nvme_scan_ns_list(ctrl, nn))
goto out_free_id;
}
--
2.25.1
_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 2/5] nvme: clean up nvme_scan_work
2020-04-06 12:13 a few more namespace scanning cleanups Christoph Hellwig
2020-04-06 12:13 ` [PATCH 1/5] nvme: refine the Qemu Identify CNS quirk Christoph Hellwig
@ 2020-04-06 12:13 ` Christoph Hellwig
2020-04-06 17:04 ` Sagi Grimberg
2020-04-06 12:13 ` [PATCH 3/5] nvme: factor out a nvme_ns_remove_by_nsid helper Christoph Hellwig
` (3 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Christoph Hellwig @ 2020-04-06 12:13 UTC (permalink / raw)
To: Keith Busch, Sagi Grimberg; +Cc: linux-nvme
Move the check for the supported CNS value into nvme_scan_ns_list, and
limit the life time of the identify controller allocation.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/nvme/host/core.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 01889905875b..5fcc35f74eac 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3736,6 +3736,9 @@ static int nvme_scan_ns_list(struct nvme_ctrl *ctrl, unsigned nn)
unsigned num_lists = DIV_ROUND_UP_ULL((u64)nn, 1024);
int ret = 0;
+ if (nvme_ctrl_limited_cns(ctrl))
+ return -EOPNOTSUPP;
+
ns_list = kzalloc(NVME_IDENTIFY_DATA_SIZE, GFP_KERNEL);
if (!ns_list)
return -ENOMEM;
@@ -3822,17 +3825,14 @@ static void nvme_scan_work(struct work_struct *work)
if (nvme_identify_ctrl(ctrl, &id))
return;
+ nn = le32_to_cpu(id->nn);
+ kfree(id);
mutex_lock(&ctrl->scan_lock);
- nn = le32_to_cpu(id->nn);
- if (!nvme_ctrl_limited_cns(ctrl)) {
- if (!nvme_scan_ns_list(ctrl, nn))
- goto out_free_id;
- }
- nvme_scan_ns_sequential(ctrl, nn);
-out_free_id:
+ if (nvme_scan_ns_list(ctrl, nn) != 0)
+ nvme_scan_ns_sequential(ctrl, nn);
mutex_unlock(&ctrl->scan_lock);
- kfree(id);
+
down_write(&ctrl->namespaces_rwsem);
list_sort(NULL, &ctrl->namespaces, ns_cmp);
up_write(&ctrl->namespaces_rwsem);
--
2.25.1
_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 3/5] nvme: factor out a nvme_ns_remove_by_nsid helper
2020-04-06 12:13 a few more namespace scanning cleanups Christoph Hellwig
2020-04-06 12:13 ` [PATCH 1/5] nvme: refine the Qemu Identify CNS quirk Christoph Hellwig
2020-04-06 12:13 ` [PATCH 2/5] nvme: clean up nvme_scan_work Christoph Hellwig
@ 2020-04-06 12:13 ` Christoph Hellwig
2020-04-06 16:35 ` Keith Busch
2020-04-06 17:04 ` Sagi Grimberg
2020-04-06 12:13 ` [PATCH 4/5] nvme: avoid an Identify Controller command for each namespace scan Christoph Hellwig
` (2 subsequent siblings)
5 siblings, 2 replies; 12+ messages in thread
From: Christoph Hellwig @ 2020-04-06 12:13 UTC (permalink / raw)
To: Keith Busch, Sagi Grimberg; +Cc: linux-nvme
Factor out a pice of deeply indented and logicaly separate piece of code
from nvme_scan_ns_list.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/nvme/host/core.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 5fcc35f74eac..ffa1d62eaeb1 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3697,6 +3697,16 @@ static void nvme_ns_remove(struct nvme_ns *ns)
nvme_put_ns(ns);
}
+static void nvme_ns_remove_by_nsid(struct nvme_ctrl *ctrl, u32 nsid)
+{
+ struct nvme_ns *ns = nvme_find_get_ns(ctrl, nsid);
+
+ if (ns) {
+ nvme_ns_remove(ns);
+ nvme_put_ns(ns);
+ }
+}
+
static void nvme_validate_ns(struct nvme_ctrl *ctrl, unsigned nsid)
{
struct nvme_ns *ns;
@@ -3730,7 +3740,6 @@ static void nvme_remove_invalid_namespaces(struct nvme_ctrl *ctrl,
static int nvme_scan_ns_list(struct nvme_ctrl *ctrl, unsigned nn)
{
- struct nvme_ns *ns;
__le32 *ns_list;
unsigned i, j, nsid, prev = 0;
unsigned num_lists = DIV_ROUND_UP_ULL((u64)nn, 1024);
@@ -3755,13 +3764,8 @@ static int nvme_scan_ns_list(struct nvme_ctrl *ctrl, unsigned nn)
nvme_validate_ns(ctrl, nsid);
- while (++prev < nsid) {
- ns = nvme_find_get_ns(ctrl, prev);
- if (ns) {
- nvme_ns_remove(ns);
- nvme_put_ns(ns);
- }
- }
+ while (++prev < nsid)
+ nvme_ns_remove_by_nsid(ctrl, prev);
}
nn -= j;
}
--
2.25.1
_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 4/5] nvme: avoid an Identify Controller command for each namespace scan
2020-04-06 12:13 a few more namespace scanning cleanups Christoph Hellwig
` (2 preceding siblings ...)
2020-04-06 12:13 ` [PATCH 3/5] nvme: factor out a nvme_ns_remove_by_nsid helper Christoph Hellwig
@ 2020-04-06 12:13 ` Christoph Hellwig
2020-04-06 12:13 ` [PATCH 5/5] nvme: remove the magic 1024 constant in nvme_scan_ns_list Christoph Hellwig
2020-04-06 16:36 ` a few more namespace scanning cleanups Keith Busch
5 siblings, 0 replies; 12+ messages in thread
From: Christoph Hellwig @ 2020-04-06 12:13 UTC (permalink / raw)
To: Keith Busch, Sagi Grimberg; +Cc: linux-nvme
The namespace lists are 0-terminated, so we don't really need the NN value
execept for the legacy sequential scan.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/nvme/host/core.c | 40 ++++++++++++++++++----------------------
1 file changed, 18 insertions(+), 22 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index ffa1d62eaeb1..d11c462af0f3 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3738,12 +3738,11 @@ static void nvme_remove_invalid_namespaces(struct nvme_ctrl *ctrl,
}
-static int nvme_scan_ns_list(struct nvme_ctrl *ctrl, unsigned nn)
+static int nvme_scan_ns_list(struct nvme_ctrl *ctrl)
{
__le32 *ns_list;
- unsigned i, j, nsid, prev = 0;
- unsigned num_lists = DIV_ROUND_UP_ULL((u64)nn, 1024);
- int ret = 0;
+ u32 prev = 0;
+ int ret = 0, i;
if (nvme_ctrl_limited_cns(ctrl))
return -EOPNOTSUPP;
@@ -3752,22 +3751,20 @@ static int nvme_scan_ns_list(struct nvme_ctrl *ctrl, unsigned nn)
if (!ns_list)
return -ENOMEM;
- for (i = 0; i < num_lists; i++) {
+ for (;;) {
ret = nvme_identify_ns_list(ctrl, prev, ns_list);
if (ret)
goto free;
- for (j = 0; j < min(nn, 1024U); j++) {
- nsid = le32_to_cpu(ns_list[j]);
- if (!nsid)
- goto out;
+ for (i = 0; i < 1024; i++) {
+ u32 nsid = le32_to_cpu(ns_list[i]);
+ if (!nsid) /* end of the list? */
+ goto out;
nvme_validate_ns(ctrl, nsid);
-
while (++prev < nsid)
nvme_ns_remove_by_nsid(ctrl, prev);
}
- nn -= j;
}
out:
nvme_remove_invalid_namespaces(ctrl, prev);
@@ -3776,9 +3773,15 @@ static int nvme_scan_ns_list(struct nvme_ctrl *ctrl, unsigned nn)
return ret;
}
-static void nvme_scan_ns_sequential(struct nvme_ctrl *ctrl, unsigned nn)
+static void nvme_scan_ns_sequential(struct nvme_ctrl *ctrl)
{
- unsigned i;
+ struct nvme_id_ctrl *id;
+ u32 nn, i;
+
+ if (nvme_identify_ctrl(ctrl, &id))
+ return;
+ nn = le32_to_cpu(id->nn);
+ kfree(id);
for (i = 1; i <= nn; i++)
nvme_validate_ns(ctrl, i);
@@ -3815,8 +3818,6 @@ static void nvme_scan_work(struct work_struct *work)
{
struct nvme_ctrl *ctrl =
container_of(work, struct nvme_ctrl, scan_work);
- struct nvme_id_ctrl *id;
- unsigned nn;
/* No tagset on a live ctrl means IO queues could not created */
if (ctrl->state != NVME_CTRL_LIVE || !ctrl->tagset)
@@ -3827,14 +3828,9 @@ static void nvme_scan_work(struct work_struct *work)
nvme_clear_changed_ns_log(ctrl);
}
- if (nvme_identify_ctrl(ctrl, &id))
- return;
- nn = le32_to_cpu(id->nn);
- kfree(id);
-
mutex_lock(&ctrl->scan_lock);
- if (nvme_scan_ns_list(ctrl, nn) != 0)
- nvme_scan_ns_sequential(ctrl, nn);
+ if (nvme_scan_ns_list(ctrl) != 0)
+ nvme_scan_ns_sequential(ctrl);
mutex_unlock(&ctrl->scan_lock);
down_write(&ctrl->namespaces_rwsem);
--
2.25.1
_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 5/5] nvme: remove the magic 1024 constant in nvme_scan_ns_list
2020-04-06 12:13 a few more namespace scanning cleanups Christoph Hellwig
` (3 preceding siblings ...)
2020-04-06 12:13 ` [PATCH 4/5] nvme: avoid an Identify Controller command for each namespace scan Christoph Hellwig
@ 2020-04-06 12:13 ` Christoph Hellwig
2020-04-06 17:07 ` Sagi Grimberg
2020-04-06 16:36 ` a few more namespace scanning cleanups Keith Busch
5 siblings, 1 reply; 12+ messages in thread
From: Christoph Hellwig @ 2020-04-06 12:13 UTC (permalink / raw)
To: Keith Busch, Sagi Grimberg; +Cc: linux-nvme
Replace it with a value derived from the identify data and nsid sizes.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/nvme/host/core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index d11c462af0f3..5de3b993525b 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3740,6 +3740,7 @@ static void nvme_remove_invalid_namespaces(struct nvme_ctrl *ctrl,
static int nvme_scan_ns_list(struct nvme_ctrl *ctrl)
{
+ const int nr_entries = NVME_IDENTIFY_DATA_SIZE / sizeof(__le32);
__le32 *ns_list;
u32 prev = 0;
int ret = 0, i;
@@ -3756,7 +3757,7 @@ static int nvme_scan_ns_list(struct nvme_ctrl *ctrl)
if (ret)
goto free;
- for (i = 0; i < 1024; i++) {
+ for (i = 0; i < nr_entries; i++) {
u32 nsid = le32_to_cpu(ns_list[i]);
if (!nsid) /* end of the list? */
--
2.25.1
_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: a few more namespace scanning cleanups
2020-04-06 12:13 a few more namespace scanning cleanups Christoph Hellwig
` (4 preceding siblings ...)
2020-04-06 12:13 ` [PATCH 5/5] nvme: remove the magic 1024 constant in nvme_scan_ns_list Christoph Hellwig
@ 2020-04-06 16:36 ` Keith Busch
5 siblings, 0 replies; 12+ messages in thread
From: Keith Busch @ 2020-04-06 16:36 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Sagi Grimberg, linux-nvme
Looks good to me. For the reset of the series:
Reviewed-by: Keith Busch <kbusch@kernel.org>
_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 12+ messages in thread