Skip to contentSkip to conversation

nvme: Update node paths after adding new path

PATCH New Keith Busch 3 participants 5 messages Oct 5, 2018 – Oct 8, 2018 NVMe

lore
Share & tools
lore

Keith Busch

author
#0 lore raw

PATCH 1/1 nvme: Update node paths after adding new path

The nvme namespace paths were being updated only when the current path was not set or nonoptimized. If a new path comes online that is a better path for its NUMA node, the multipath selector may continue using the previously set path on a potentially further node.

This patch re-runs the path assignment after successfully adding a new optimized path.

Signed-off-by Keith Busch <keith.busch at intel.com>
---
 drivers/nvme/host/multipath.c | 8 ++++++++
 1 file changed, 8 insertions(+)
drivers/nvme/host/multipath.c +8 -0
index 52987052b7fc..fc3c5c2b3eca 100644
@@ -321,6 +321,14 @@ static void nvme_mpath_set_live(struct nvme_ns *ns)
 		device_add_disk(&head->subsys->dev, head->disk,
 				nvme_ns_id_attr_groups);
 
+	if (nvme_path_is_optimized(ns)) {
+		int node, srcu_idx;
+
+		srcu_idx = srcu_read_lock(&head->srcu);
+		for_each_node(node)
+			__nvme_find_path(head, node);
+		srcu_read_unlock(&head->srcu, srcu_idx);
+	}
 	kblockd_schedule_work(&ns->head->requeue_work);
 }
Signature · 1 line
2.14.4
maintainer
#1 lore raw

PATCH 1/1 nvme: Update node paths after adding new path

Quoted 20 lines

The nvme namespace paths were being updated only when the current path was not set or nonoptimized. If a new path comes online that is a better path for its NUMA node, the multipath selector may continue using the previously set path on a potentially further node.

This patch re-runs the path assignment after successfully adding a new optimized path.

Signed-off-by Keith Busch <keith.busch at intel.com>
---
  drivers/nvme/host/multipath.c | 8 ++++++++
  1 file changed, 8 insertions(+)
drivers/nvme/host/multipath.c +0 -0
index 52987052b7fc..fc3c5c2b3eca 100644
@@ -321,6 +321,14 @@ static void nvme_mpath_set_live(struct nvme_ns *ns)
  		device_add_disk(&head->subsys->dev, head->disk,

drivers/nvme/host/multipath.c static void nvme_mpath_set_live(struct nvme_ns *ns) · line 331

Quoted 10 lines
drivers/nvme/host/multipath.c +8 -0
  				nvme_ns_id_attr_groups);
 
+	if (nvme_path_is_optimized(ns)) {
+		int node, srcu_idx;
+
+		srcu_idx = srcu_read_lock(&head->srcu);
+		for_each_node(node)
+			__nvme_find_path(head, node);
+		srcu_read_unlock(&head->srcu, srcu_idx);
+	}

Any reason for interfering with all the numa nodes?

Why not simple call __nvme_find_path(head, ns->ctrl->node_id) ?

author
#2 lore raw

PATCH 1/1 nvme: Update node paths after adding new path

Quoted 32 lines · Sagi Grimberg
Quoted 19 lines

The nvme namespace paths were being updated only when the current path was not set or nonoptimized. If a new path comes online that is a better path for its NUMA node, the multipath selector may continue using the previously set path on a potentially further node.

This patch re-runs the path assignment after successfully adding a new optimized path.

Signed-off-by Keith Busch <keith.busch at intel.com>
---
  drivers/nvme/host/multipath.c | 8 ++++++++
  1 file changed, 8 insertions(+)
drivers/nvme/host/multipath.c +0 -0
index 52987052b7fc..fc3c5c2b3eca 100644

drivers/nvme/host/multipath.c static void nvme_mpath_set_live(struct nvme_ns *ns) · line 330

Quoted 10 lines
drivers/nvme/host/multipath.c +8 -0
@@ -321,6 +321,14 @@ static void nvme_mpath_set_live(struct nvme_ns *ns)
  		device_add_disk(&head->subsys->dev, head->disk,
  				nvme_ns_id_attr_groups);
+	if (nvme_path_is_optimized(ns)) {
+		int node, srcu_idx;
+
+		srcu_idx = srcu_read_lock(&head->srcu);
+		for_each_node(node)
+			__nvme_find_path(head, node);
+		srcu_read_unlock(&head->srcu, srcu_idx);
+	}

Any reason for interfering with all the numa nodes?

Quoted 1 line

Why not simple call __nvme_find_path(head, ns->ctrl->node_id) ?

The new path's local node may be closer to other nodes who's path is currently set to something else.

Consider the following simplified example of nodes A through D:

  -----    -----    -----    ----- 
  | A |<-->| B |<-->| C |<-->| D |
  -----    -----    -----    -----

Let's say we start with a single optimized path that is local to node "D". That path will initially be the "current_path" for all nodes.

Now attach a new path local to node "A". Updating just node A's current_path misses updating B away from D, even though A is closer than D.

maintainer
#3 lore raw

PATCH 1/1 nvme: Update node paths after adding new path

Quoted 11 lines

The new path's local node may be closer to other nodes who's path is currently set to something else.

Consider the following simplified example of nodes A through D:

   -----    -----    -----    -----
   | A |<-->| B |<-->| C |<-->| D |
   -----    -----    -----    -----
Quoted 7 lines

Let's say we start with a single optimized path that is local to node "D". That path will initially be the "current_path" for all nodes.

Now attach a new path local to node "A". Updating just node A's current_path misses updating B away from D, even though A is closer than D.

Git it,

Reviewed-by Sagi Grimberg <sagi at grimberg.me>