[PATCHv2 0/2] nvme-multipath: path selection fixes

[PATCHv2 0/2] nvme-multipath: path selection fixes

[Hannes Reinecke]

Hi all,

during analysing a mysterious failure due to blk_queue_split() (again)
I've found that the multipath path selection might return invalid
namespaces under certain circumstances.
This patchset fixes those scenarios.

As usual, reviews and comments are welcome.

Changes to v1:
- Use common helper nvme_path_is_disabled()

Hannes Reinecke (2):
  nvme-multipath: check singular list in vme_round_robin_path()
  nvme-multipath: do not select namespaces which are about to be removed

 drivers/nvme/host/multipath.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

-- 
2.16.4

[PATCH 1/2] nvme-multipath: check singular list in vme_round_robin_path()

[Hannes Reinecke]

When we have a singular list in nvme_round_robin_path() we still
need to check its validity.

Signed-off-by: Hannes Reinecke <hare at suse.com>
---
 drivers/nvme/host/multipath.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 499acf07d61a..9b6dc11fa559 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -123,14 +123,19 @@ void nvme_mpath_clear_current_path(struct nvme_ns *ns)
 	}
 }
 
+static bool nvme_path_is_disabled(struct nvme_ns *ns)
+{
+	return ns->ctrl->state != NVME_CTRL_LIVE ||
+		test_bit(NVME_NS_ANA_PENDING, &ns->flags);
+}
+
 static struct nvme_ns *__nvme_find_path(struct nvme_ns_head *head, int node)
 {
 	int found_distance = INT_MAX, fallback_distance = INT_MAX, distance;
 	struct nvme_ns *found = NULL, *fallback = NULL, *ns;
 
 	list_for_each_entry_rcu(ns, &head->list, siblings) {
-		if (ns->ctrl->state != NVME_CTRL_LIVE ||
-		    test_bit(NVME_NS_ANA_PENDING, &ns->flags))
+		if (nvme_path_is_disabled(ns))
 			continue;
 
 		if (READ_ONCE(head->subsys->iopolicy) == NVME_IOPOLICY_NUMA)
@@ -178,14 +183,16 @@ static struct nvme_ns *nvme_round_robin_path(struct nvme_ns_head *head,
 {
 	struct nvme_ns *ns, *found, *fallback = NULL;
 
-	if (list_is_singular(&head->list))
+	if (list_is_singular(&head->list)) {
+		if (nvme_path_is_disabled(old))
+			return NULL;
 		return old;
+	}
 
 	for (ns = nvme_next_ns(head, old);
 	     ns != old;
 	     ns = nvme_next_ns(head, ns)) {
-		if (ns->ctrl->state != NVME_CTRL_LIVE ||
-		    test_bit(NVME_NS_ANA_PENDING, &ns->flags))
+		if (nvme_path_is_disabled(ns))
 			continue;
 
 		if (ns->ana_state == NVME_ANA_OPTIMIZED) {
-- 
2.16.4

[PATCH 2/2] nvme-multipath: do not select namespaces which are about to be removed

[Hannes Reinecke]

nvme_ns_remove() will first set the NVME_NS_REMOVING flag before removing
it from the list at the very last step.
So to avoid selecting a namespace in nvme_find_path() which is about to be
removed check the NVME_NS_REMOVING flag, too, when selecting a new path.

Signed-off-by: Hannes Reinecke <hare at suse.com>
---
 drivers/nvme/host/multipath.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 9b6dc11fa559..a9a927677970 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -126,7 +126,8 @@ void nvme_mpath_clear_current_path(struct nvme_ns *ns)
 static bool nvme_path_is_disabled(struct nvme_ns *ns)
 {
 	return ns->ctrl->state != NVME_CTRL_LIVE ||
-		test_bit(NVME_NS_ANA_PENDING, &ns->flags);
+		test_bit(NVME_NS_ANA_PENDING, &ns->flags) ||
+		test_bit(NVME_NS_REMOVING, &ns->flags);
 }
 
 static struct nvme_ns *__nvme_find_path(struct nvme_ns_head *head, int node)
-- 
2.16.4

[PATCHv2 0/2] nvme-multipath: path selection fixes

[Christoph Hellwig]

Thanks,

I've split out the addition of the new nvme_path_is_disabled helper into
a separate patch and then applied there series to the nvme-5.3 branch.

[PATCH 0/2] nvme-multipath: path selection fixes

[Hannes Reinecke]

Hi all,

during analysing a mysterious failure due to blk_queue_split() (again)
I've found that the multipath path selection might return invalid
namespaces under certain circumstances.
This patchset fixes those scenarios.

As usual, reviews and comments are welcome.

Hannes Reinecke (2):
  nvme-multipath: check singular list in vme_round_robin_path()
  nvme-multipath: do not select namespaces which are about to be removed

 drivers/nvme/host/multipath.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

-- 
2.16.4

[PATCH 1/2] nvme-multipath: check singular list in vme_round_robin_path()

[Hannes Reinecke]

When we have a singular list in nvme_round_robin_path() we still
need to check its validity.

Signed-off-by: Hannes Reinecke <hare at suse.com>
---
 drivers/nvme/host/multipath.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 499acf07d61a..c8cc82639327 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -178,8 +178,12 @@ static struct nvme_ns *nvme_round_robin_path(struct nvme_ns_head *head,
 {
 	struct nvme_ns *ns, *found, *fallback = NULL;
 
-	if (list_is_singular(&head->list))
+	if (list_is_singular(&head->list)) {
+		if (old->ctrl->state != NVME_CTRL_LIVE ||
+		    test_bit(NVME_NS_ANA_PENDING, &old->flags))
+			return NULL;
 		return old;
+	}
 
 	for (ns = nvme_next_ns(head, old);
 	     ns != old;
-- 
2.16.4