* [PATCH 1/3] nvme/multipath: fix failure to update ns ana state
2021-09-12 1:07 [PATCH 0/3] nvme/mpath: fix missed namespaces in ana state update Anton Eidelman
@ 2021-09-12 1:07 ` Anton Eidelman
2021-09-12 1:07 ` [PATCH 2/3] nvme/multipath: cosmetic: keep ns nsid locally Anton Eidelman
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Anton Eidelman @ 2021-09-12 1:07 UTC (permalink / raw)
To: linux-nvme, hch, kbusch, sagi, axboe; +Cc: Anton Eidelman
nvme_update_ana_state() has a deficiency that results
in failure to update the ana state for a namespace
in the following case:
nsid's in ctrl->namespaces: 1, 3, 4
nsid's in desc->nsids: 1, 2, 3, 4
Loop iteration 0:
ns index = 0, n = 0, ns->head->ns_id = 1, nsid = 1, MATCH.
Loop iteration 1:
ns index = 1, n = 1, ns->head->ns_id = 3, nsid = 2, NO MATCH.
Loop iteration 2:
ns index = 2, n = 2, ns->head->ns_id = 4, nsid = 4, MATCH.
Result: missed nsid=3 and did not update its ana state.
Solution: when ns->head->ns_id is higher than nsid,
increment n and RETRY with the SAME ns.
Signed-off-by: Anton Eidelman <anton@lightbitslabs.com>
---
drivers/nvme/host/multipath.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 5d7bc58a27bd..e8ccdd398f78 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -600,14 +600,17 @@ static int nvme_update_ana_state(struct nvme_ctrl *ctrl,
down_read(&ctrl->namespaces_rwsem);
list_for_each_entry(ns, &ctrl->namespaces, list) {
- unsigned nsid = le32_to_cpu(desc->nsids[n]);
-
+ unsigned nsid;
+again:
+ nsid = le32_to_cpu(desc->nsids[n]);
if (ns->head->ns_id < nsid)
continue;
if (ns->head->ns_id == nsid)
nvme_update_ns_ana_state(desc, ns);
if (++n == nr_nsids)
break;
+ if (ns->head->ns_id > nsid)
+ goto again;
}
up_read(&ctrl->namespaces_rwsem);
return 0;
--
2.25.1
--
*Lightbits Labs**
*Lead the cloud-native data center
transformation by
delivering *scalable *and *efficient *software
defined storage that is
*easy *to consume.
*This message is sent in confidence for the addressee
only. It
may contain legally privileged information. The contents are not
to be
disclosed to anyone other than the addressee. Unauthorized recipients
are
requested to preserve this confidentiality, advise the sender
immediately of
any error in transmission and delete the email from their
systems.*
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/3] nvme/multipath: cosmetic: keep ns nsid locally
2021-09-12 1:07 [PATCH 0/3] nvme/mpath: fix missed namespaces in ana state update Anton Eidelman
2021-09-12 1:07 ` [PATCH 1/3] nvme/multipath: fix failure to update ns ana state Anton Eidelman
@ 2021-09-12 1:07 ` Anton Eidelman
2021-09-12 1:07 ` [PATCH 3/3] nvme/multipath: fix stale ana state for namespaces just added by scan work Anton Eidelman
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Anton Eidelman @ 2021-09-12 1:07 UTC (permalink / raw)
To: linux-nvme, hch, kbusch, sagi, axboe; +Cc: Anton Eidelman
Keep the nsid of the current namespace in a local variable.
Change the type to unsigned int to make checkpatch happy.
Signed-off-by: Anton Eidelman <anton@lightbitslabs.com>
---
drivers/nvme/host/multipath.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index e8ccdd398f78..a51561d67b93 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -600,16 +600,18 @@ static int nvme_update_ana_state(struct nvme_ctrl *ctrl,
down_read(&ctrl->namespaces_rwsem);
list_for_each_entry(ns, &ctrl->namespaces, list) {
- unsigned nsid;
+ unsigned int nsid;
+ unsigned int ns_nsid = ns->head->ns_id;
+
again:
nsid = le32_to_cpu(desc->nsids[n]);
- if (ns->head->ns_id < nsid)
+ if (ns_nsid < nsid)
continue;
- if (ns->head->ns_id == nsid)
+ if (ns_nsid == nsid)
nvme_update_ns_ana_state(desc, ns);
if (++n == nr_nsids)
break;
- if (ns->head->ns_id > nsid)
+ if (ns_nsid > nsid)
goto again;
}
up_read(&ctrl->namespaces_rwsem);
--
2.25.1
--
*Lightbits Labs**
*Lead the cloud-native data center
transformation by
delivering *scalable *and *efficient *software
defined storage that is
*easy *to consume.
*This message is sent in confidence for the addressee
only. It
may contain legally privileged information. The contents are not
to be
disclosed to anyone other than the addressee. Unauthorized recipients
are
requested to preserve this confidentiality, advise the sender
immediately of
any error in transmission and delete the email from their
systems.*
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] nvme/multipath: fix stale ana state for namespaces just added by scan work
2021-09-12 1:07 [PATCH 0/3] nvme/mpath: fix missed namespaces in ana state update Anton Eidelman
2021-09-12 1:07 ` [PATCH 1/3] nvme/multipath: fix failure to update ns ana state Anton Eidelman
2021-09-12 1:07 ` [PATCH 2/3] nvme/multipath: cosmetic: keep ns nsid locally Anton Eidelman
@ 2021-09-12 1:07 ` Anton Eidelman
2021-09-12 18:49 ` [PATCH 0/3] nvme/mpath: fix missed namespaces in ana state update Anton Eidelman
2021-09-12 18:52 ` PLEASE, IGNORE THIS THREAD - BAD FORMATTING Anton Eidelman
4 siblings, 0 replies; 7+ messages in thread
From: Anton Eidelman @ 2021-09-12 1:07 UTC (permalink / raw)
To: linux-nvme, hch, kbusch, sagi, axboe; +Cc: Anton Eidelman
Scan work initially adds new namespaces to ctrl->namespaces TAIL.
They make the list unordered temporarily until nvme_scan_work()
finally sorts the list.
In case nvme_update_ana_state() runs while the list is unsorted,
the recently added namespaces are missed and their ana state
may remain not updated forever if timing between scan work and ana work
is unfortunate, e.g.
Initial state: namespaces = {2, 3}
scan_work: adds nsid=1: namespaces = {2, 3, 1}
scan_work: finds nsid=1 is still Inaccessible
ana_work: log page has nsids = {1, 2, 3, 4}, all Optimized.
ana_work: updates nsids {2, 3} but fails to find nsid=1 in namespaces.
scan_work: adds nsid=4: namespaces = {2, 3, 1, 4}
scan_work: finds nsid=4 is Optimized: sets it live.
scan_work: completes an sorts namespaces = {1, 2, 3, 4}
Result: nsid=1 will remain in Inaccessible state.
Solution:
In order to preserve the way ctrl->namespaces is updated and sorted,
make nvme_update_ana_state() deal with the case where ctrl->namespaces
is not fully sorted and has new namespaces appended with potentially
lower nsids.
nvme_update_ana_state() keeps track of the nsid seen in the list,
detects the unsorted case (rare), and restarts scanning of desc->nsids.
Signed-off-by: Anton Eidelman <anton@lightbitslabs.com>
---
drivers/nvme/host/multipath.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index a51561d67b93..1ad8dc8adb86 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -587,6 +587,7 @@ static int nvme_update_ana_state(struct nvme_ctrl *ctrl,
u32 nr_nsids = le32_to_cpu(desc->nnsids), n = 0;
unsigned *nr_change_groups = data;
struct nvme_ns *ns;
+ unsigned int last_ns_nsid = 0;
dev_dbg(ctrl->device, "ANA group %d: %s.\n",
le32_to_cpu(desc->grpid),
@@ -603,6 +604,11 @@ static int nvme_update_ana_state(struct nvme_ctrl *ctrl,
unsigned int nsid;
unsigned int ns_nsid = ns->head->ns_id;
+ if (ns_nsid < last_ns_nsid) {
+ /* Detected unsorted ctrl->namespaces: re-scan desc->nsids */
+ last_ns_nsid = ns_nsid;
+ n = 0;
+ }
again:
nsid = le32_to_cpu(desc->nsids[n]);
if (ns_nsid < nsid)
--
2.25.1
--
*Lightbits Labs**
*Lead the cloud-native data center
transformation by
delivering *scalable *and *efficient *software
defined storage that is
*easy *to consume.
*This message is sent in confidence for the addressee
only. It
may contain legally privileged information. The contents are not
to be
disclosed to anyone other than the addressee. Unauthorized recipients
are
requested to preserve this confidentiality, advise the sender
immediately of
any error in transmission and delete the email from their
systems.*
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 7+ messages in thread
* RE: [PATCH 0/3] nvme/mpath: fix missed namespaces in ana state update
2021-09-12 1:07 [PATCH 0/3] nvme/mpath: fix missed namespaces in ana state update Anton Eidelman
` (2 preceding siblings ...)
2021-09-12 1:07 ` [PATCH 3/3] nvme/multipath: fix stale ana state for namespaces just added by scan work Anton Eidelman
@ 2021-09-12 18:49 ` Anton Eidelman
2021-09-12 18:52 ` PLEASE, IGNORE THIS THREAD - BAD FORMATTING Anton Eidelman
4 siblings, 0 replies; 7+ messages in thread
From: Anton Eidelman @ 2021-09-12 18:49 UTC (permalink / raw)
To: linux-nvme; +Cc: Anton Eidelman
Please, ignore this thread - will resend with correct formatting.
--
2.25.1
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 7+ messages in thread
* PLEASE, IGNORE THIS THREAD - BAD FORMATTING
2021-09-12 1:07 [PATCH 0/3] nvme/mpath: fix missed namespaces in ana state update Anton Eidelman
` (3 preceding siblings ...)
2021-09-12 18:49 ` [PATCH 0/3] nvme/mpath: fix missed namespaces in ana state update Anton Eidelman
@ 2021-09-12 18:52 ` Anton Eidelman
4 siblings, 0 replies; 7+ messages in thread
From: Anton Eidelman @ 2021-09-12 18:52 UTC (permalink / raw)
To: linux-nvme; +Cc: Anton Eidelman
Please, ignore this thread - will resend with correct formatting.
--
2.25.1
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/3] nvme/multipath: fix failure to update ns ana state
2021-09-12 18:54 [PATCH RESEND 0/3] nvme/mpath: fix missed namespaces in ana state update Anton Eidelman
@ 2021-09-12 18:54 ` Anton Eidelman
0 siblings, 0 replies; 7+ messages in thread
From: Anton Eidelman @ 2021-09-12 18:54 UTC (permalink / raw)
To: linux-nvme, hch, kbusch, sagi, axboe; +Cc: Anton Eidelman
nvme_update_ana_state() has a deficiency that results
in failure to update the ana state for a namespace
in the following case:
nsid's in ctrl->namespaces: 1, 3, 4
nsid's in desc->nsids: 1, 2, 3, 4
Loop iteration 0:
ns index = 0, n = 0, ns->head->ns_id = 1, nsid = 1, MATCH.
Loop iteration 1:
ns index = 1, n = 1, ns->head->ns_id = 3, nsid = 2, NO MATCH.
Loop iteration 2:
ns index = 2, n = 2, ns->head->ns_id = 4, nsid = 4, MATCH.
Result: missed nsid=3 and did not update its ana state.
Solution: when ns->head->ns_id is higher than nsid,
increment n and RETRY with the SAME ns.
Signed-off-by: Anton Eidelman <anton@lightbitslabs.com>
---
drivers/nvme/host/multipath.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 5d7bc58a27bd..e8ccdd398f78 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -600,14 +600,17 @@ static int nvme_update_ana_state(struct nvme_ctrl *ctrl,
down_read(&ctrl->namespaces_rwsem);
list_for_each_entry(ns, &ctrl->namespaces, list) {
- unsigned nsid = le32_to_cpu(desc->nsids[n]);
-
+ unsigned nsid;
+again:
+ nsid = le32_to_cpu(desc->nsids[n]);
if (ns->head->ns_id < nsid)
continue;
if (ns->head->ns_id == nsid)
nvme_update_ns_ana_state(desc, ns);
if (++n == nr_nsids)
break;
+ if (ns->head->ns_id > nsid)
+ goto again;
}
up_read(&ctrl->namespaces_rwsem);
return 0;
--
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] 7+ messages in thread