Skip to content

Commit b739e13

Browse files
author
Christoph Hellwig
committed
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>
1 parent da78373 commit b739e13

3 files changed

Lines changed: 21 additions & 33 deletions

File tree

drivers/nvme/host/core.c

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3967,13 +3967,27 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid,
39673967
goto out_cleanup_disk;
39683968

39693969
/*
3970-
* Without the multipath code enabled, multiple controller per
3971-
* subsystems are visible as devices and thus we cannot use the
3972-
* subsystem instance.
3970+
* If multipathing is enabled, the device name for all disks and not
3971+
* just those that represent shared namespaces needs to be based on the
3972+
* subsystem instance. Using the controller instance for private
3973+
* namespaces could lead to naming collisions between shared and private
3974+
* namespaces if they don't use a common numbering scheme.
3975+
*
3976+
* If multipathing is not enabled, disk names must use the controller
3977+
* instance as shared namespaces will show up as multiple block
3978+
* devices.
39733979
*/
3974-
if (!nvme_mpath_set_disk_name(ns, disk->disk_name, &disk->flags))
3980+
if (ns->head->disk) {
3981+
sprintf(disk->disk_name, "nvme%dc%dn%d", ctrl->subsys->instance,
3982+
ctrl->instance, ns->head->instance);
3983+
disk->flags |= GENHD_FL_HIDDEN;
3984+
} else if (multipath) {
3985+
sprintf(disk->disk_name, "nvme%dn%d", ctrl->subsys->instance,
3986+
ns->head->instance);
3987+
} else {
39753988
sprintf(disk->disk_name, "nvme%dn%d", ctrl->instance,
39763989
ns->head->instance);
3990+
}
39773991

39783992
if (nvme_update_ns_info(ns, id))
39793993
goto out_unlink_ns;

drivers/nvme/host/multipath.c

Lines changed: 1 addition & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
#include <trace/events/block.h>
1010
#include "nvme.h"
1111

12-
static bool multipath = true;
12+
bool multipath = true;
1313
module_param(multipath, bool, 0444);
1414
MODULE_PARM_DESC(multipath,
1515
"turn on native support for multiple controllers per subsystem");
@@ -80,28 +80,6 @@ void nvme_mpath_start_freeze(struct nvme_subsystem *subsys)
8080
blk_freeze_queue_start(h->disk->queue);
8181
}
8282

83-
/*
84-
* If multipathing is enabled we need to always use the subsystem instance
85-
* number for numbering our devices to avoid conflicts between subsystems that
86-
* have multiple controllers and thus use the multipath-aware subsystem node
87-
* and those that have a single controller and use the controller node
88-
* directly.
89-
*/
90-
bool nvme_mpath_set_disk_name(struct nvme_ns *ns, char *disk_name, int *flags)
91-
{
92-
if (!multipath)
93-
return false;
94-
if (!ns->head->disk) {
95-
sprintf(disk_name, "nvme%dn%d", ns->ctrl->subsys->instance,
96-
ns->head->instance);
97-
return true;
98-
}
99-
sprintf(disk_name, "nvme%dc%dn%d", ns->ctrl->subsys->instance,
100-
ns->ctrl->instance, ns->head->instance);
101-
*flags = GENHD_FL_HIDDEN;
102-
return true;
103-
}
104-
10583
void nvme_failover_req(struct request *req)
10684
{
10785
struct nvme_ns *ns = req->q->queuedata;

drivers/nvme/host/nvme.h

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -770,7 +770,6 @@ void nvme_mpath_unfreeze(struct nvme_subsystem *subsys);
770770
void nvme_mpath_wait_freeze(struct nvme_subsystem *subsys);
771771
void nvme_mpath_start_freeze(struct nvme_subsystem *subsys);
772772
void nvme_mpath_default_iopolicy(struct nvme_subsystem *subsys);
773-
bool nvme_mpath_set_disk_name(struct nvme_ns *ns, char *disk_name, int *flags);
774773
void nvme_failover_req(struct request *req);
775774
void nvme_kick_requeue_lists(struct nvme_ctrl *ctrl);
776775
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)
793792
trace_block_bio_complete(ns->head->disk->queue, req->bio);
794793
}
795794

795+
extern bool multipath;
796796
extern struct device_attribute dev_attr_ana_grpid;
797797
extern struct device_attribute dev_attr_ana_state;
798798
extern struct device_attribute subsys_attr_iopolicy;
799799

800800
#else
801+
#define multipath false
801802
static inline bool nvme_ctrl_use_ana(struct nvme_ctrl *ctrl)
802803
{
803804
return false;
804805
}
805-
static inline bool nvme_mpath_set_disk_name(struct nvme_ns *ns, char *disk_name,
806-
int *flags)
807-
{
808-
return false;
809-
}
810806
static inline void nvme_failover_req(struct request *req)
811807
{
812808
}

0 commit comments

Comments
 (0)