nvme: cleanup how disk->disk_name is assigned

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@lst.de>
Reviewed-by: Keith Busch <kbusch@kernel.org>
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index f8084de..fd98786 100644 (file)
--- 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 d13b81c..c97d7f8 100644 (file)
--- 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 587d92d..47badb7 100644 (file)
--- 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)
 {
 }