[PATCH] nvme: cleanup how disk->disk_name is assigned
Christoph Hellwig
hch at lst.de
Tue Mar 15 07:51:25 PDT 2022
They way how assigning the disk name and commenting on why it is done
is split over core.c and multipath.c seems to be rather confusing.
Now that ns_head->disk always exists we can do all the work in core.c
and have a single big comment explaining the issues.
Signed-off-by: Christoph Hellwig <hch at lst.de>
---
drivers/nvme/host/core.c | 22 ++++++++++++++++++----
drivers/nvme/host/multipath.c | 24 +-----------------------
drivers/nvme/host/nvme.h | 8 ++------
3 files changed, 21 insertions(+), 33 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index f8084ded69e50..fd9878671d734 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3967,13 +3967,27 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid,
goto out_cleanup_disk;
/*
- * Without the multipath code enabled, multiple controller per
- * subsystems are visible as devices and thus we cannot use the
- * subsystem instance.
+ * If multipathing is enabled, the device name for all disks and not
+ * just those that represent shared namespaces needs to be based on the
+ * subsystem instance. Using the controller instance for private
+ * namespaces could lead to naming collisions between shared and private
+ * namespaces if they don't use a common numbering scheme.
+ *
+ * If multipathing is not enabled, disk names must use the controller
+ * instance as shared namespaces will show up as multiple block
+ * devices.
*/
- if (!nvme_mpath_set_disk_name(ns, disk->disk_name, &disk->flags))
+ if (ns->head->disk) {
+ sprintf(disk->disk_name, "nvme%dc%dn%d", ctrl->subsys->instance,
+ ctrl->instance, ns->head->instance);
+ disk->flags |= GENHD_FL_HIDDEN;
+ } else if (multipath) {
+ sprintf(disk->disk_name, "nvme%dn%d", ctrl->subsys->instance,
+ ns->head->instance);
+ } else {
sprintf(disk->disk_name, "nvme%dn%d", ctrl->instance,
ns->head->instance);
+ }
if (nvme_update_ns_info(ns, id))
goto out_unlink_ns;
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index d13b81cd6225c..c97d7f843977c 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -9,7 +9,7 @@
#include <trace/events/block.h>
#include "nvme.h"
-static bool multipath = true;
+bool multipath = true;
module_param(multipath, bool, 0444);
MODULE_PARM_DESC(multipath,
"turn on native support for multiple controllers per subsystem");
@@ -80,28 +80,6 @@ void nvme_mpath_start_freeze(struct nvme_subsystem *subsys)
blk_freeze_queue_start(h->disk->queue);
}
-/*
- * If multipathing is enabled we need to always use the subsystem instance
- * number for numbering our devices to avoid conflicts between subsystems that
- * have multiple controllers and thus use the multipath-aware subsystem node
- * and those that have a single controller and use the controller node
- * directly.
- */
-bool nvme_mpath_set_disk_name(struct nvme_ns *ns, char *disk_name, int *flags)
-{
- if (!multipath)
- return false;
- if (!ns->head->disk) {
- sprintf(disk_name, "nvme%dn%d", ns->ctrl->subsys->instance,
- ns->head->instance);
- return true;
- }
- sprintf(disk_name, "nvme%dc%dn%d", ns->ctrl->subsys->instance,
- ns->ctrl->instance, ns->head->instance);
- *flags = GENHD_FL_HIDDEN;
- return true;
-}
-
void nvme_failover_req(struct request *req)
{
struct nvme_ns *ns = req->q->queuedata;
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 587d92df118b7..47badb76c654c 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -770,7 +770,6 @@ void nvme_mpath_unfreeze(struct nvme_subsystem *subsys);
void nvme_mpath_wait_freeze(struct nvme_subsystem *subsys);
void nvme_mpath_start_freeze(struct nvme_subsystem *subsys);
void nvme_mpath_default_iopolicy(struct nvme_subsystem *subsys);
-bool nvme_mpath_set_disk_name(struct nvme_ns *ns, char *disk_name, int *flags);
void nvme_failover_req(struct request *req);
void nvme_kick_requeue_lists(struct nvme_ctrl *ctrl);
int nvme_mpath_alloc_disk(struct nvme_ctrl *ctrl,struct nvme_ns_head *head);
@@ -793,20 +792,17 @@ static inline void nvme_trace_bio_complete(struct request *req)
trace_block_bio_complete(ns->head->disk->queue, req->bio);
}
+extern bool multipath;
extern struct device_attribute dev_attr_ana_grpid;
extern struct device_attribute dev_attr_ana_state;
extern struct device_attribute subsys_attr_iopolicy;
#else
+#define multipath false
static inline bool nvme_ctrl_use_ana(struct nvme_ctrl *ctrl)
{
return false;
}
-static inline bool nvme_mpath_set_disk_name(struct nvme_ns *ns, char *disk_name,
- int *flags)
-{
- return false;
-}
static inline void nvme_failover_req(struct request *req)
{
}
--
2.30.2
More information about the Linux-nvme
mailing list