tweak multipath kconfig and options
Christoph Hellwig
maintainerBased on all the flames lately I think we need to make the Kconfig description of CONFIG_NVME_MULTIPATH a little more clear, and also warn people if they use unsupported and dangerous setups.
Christoph Hellwig
maintainerPATCH 1/3 nvme: enable multipathing by default
And make it a little more clear that the only reason to disable it is to optimize for size in constrained environment.
--- drivers/nvme/host/Kconfig | 8 ++++++++ 1 file changed, 8 insertions(+)
drivers/nvme/host/Kconfig +8 -0
@@ -16,12 +16,20 @@ config BLK_DEV_NVME config NVME_MULTIPATH bool "NVMe multipath support" depends on NVME_CORE + default y ---help--- This option enables support for multipath access to NVMe subsystems. If this option is enabled only a single /dev/nvmeXnY device will show up for each NVMe namespaces, even if it is accessible through multiple controllers. + Say Y here unless you need to optimize for size in embedded + systems that are never going to see multi-port NVMe devices. + + If you say N multiple ports of the same subsystem will show + up as separate devices, and none of the normal block device + exclusion mechanisms will work. + config NVME_FABRICS tristate
Signature · 1 line
2.19.1
PATCH 1/3 nvme: enable multipathing by default
Quoted 22 lines · Christoph Hellwig
And make it a little more clear that the only reason to disable it is to optimize for size in constrained environment.
Signed-off-by Christoph Hellwig <hch at lst.de>--- drivers/nvme/host/Kconfig | 8 ++++++++ 1 file changed, 8 insertions(+)drivers/nvme/host/Kconfig +1
-0@@ -16,12 +16,20 @@ config BLK_DEV_NVME config NVME_MULTIPATH bool "NVMe multipath support" depends on NVME_CORE + default y ---help--- This option enables support for multipath access to NVMe subsystems. If this option is enabled only a single /dev/nvmeXnY device will show up for each NVMe namespaces, even if it is accessible through multiple controllers.
drivers/nvme/host/Kconfig config BLK_DEV_NVME · line 34
Quoted 10 lines
drivers/nvme/host/Kconfig +7
-0+ Say Y here unless you need to optimize for size in embedded + systems that are never going to see multi-port NVMe devices. + + If you say N multiple ports of the same subsystem will show + up as separate devices, and none of the normal block device + exclusion mechanisms will work. + config NVME_FABRICS tristate
This is okay with me, but I have one little follow-on I'd like to RFC here:
The /dev/nvme naming convention with multiplath has broken people's minds, and making it default will invite a lot of repeat questions on how to match up block to admin handles.
Could we promote /dev/nvme0 to be a subsystem handle instead of a controller handle, and add new per-controller handles, like /dev/nvme0c1, nvme0c2, etc...? The subsystem handle's management interface can just use the first active controller to forward admin commands, and per-controller specific administration can be taught to use the new handles as needed.
Tooling shouldn't care about the names, but I am not sure if this could possibly break productino scripts. The naming disconnect has broken some things, though, so I hope this doesn't end up being a pick your poision suggestion...
Christoph Hellwig
maintainerPATCH 1/3 nvme: enable multipathing by default
Quoted 12 lines · Keith Busch
This is okay with me, but I have one little follow-on I'd like to RFC here:
The
/dev/nvmenaming convention with multiplath has broken people's minds, and making it default will invite a lot of repeat questions on how to match up block to admin handles.Could we promote
/dev/nvme0to be a subsystem handle instead of a controller handle, and add new per-controller handles, like/dev/nvme0c1, nvme0c2, etc...? The subsystem handle's management interface can just use the first active controller to forward admin commands, and per-controller specific administration can be taught to use the new handles as needed.
Quoted 4 lines
Tooling shouldn't care about the names, but I am not sure if this could possibly break productino scripts. The naming disconnect has broken some things, though, so I hope this doesn't end up being a pick your poision suggestion...
The problem is that typical admin command are sensitive to the controller they are sent to, so a per-subsystem handle does not make whole lot sense. But I like the idea of the nvmeXcY names. Maybe we just keep the nvmeX names as an additional alias for the first found controller in a subsystem and otherwise use nvmeXcY?
Sagi Grimberg
maintainerPATCH 1/3 nvme: enable multipathing by default
Quoted 18 lines
Quoted 13 lines
This is okay with me, but I have one little follow-on I'd like to RFC here:
The
/dev/nvmenaming convention with multiplath has broken people's minds, and making it default will invite a lot of repeat questions on how to match up block to admin handles.Could we promote
/dev/nvme0to be a subsystem handle instead of a controller handle, and add new per-controller handles, like/dev/nvme0c1, nvme0c2, etc...? The subsystem handle's management interface can just use the first active controller to forward admin commands, and per-controller specific administration can be taught to use the new handles as needed.Quoted 4 lines
Tooling shouldn't care about the names, but I am not sure if this could possibly break productino scripts. The naming disconnect has broken some things, though, so I hope this doesn't end up being a pick your poision suggestion...
Quoted 5 lines
The problem is that typical admin command are sensitive to the controller they are sent to, so a per-subsystem handle does not make whole lot sense. But I like the idea of the nvmeXcY names. Maybe we just keep the nvmeX names as an additional alias for the first found controller in a subsystem and otherwise use nvmeXcY?
Not sure if different naming conventions between controllers are a good idea..
What is the problem of simply changing nvmeX to nvmeXcY? if nvmeX is a must perhaps we can soft link nvmeX to nvmeXc1?
PATCH 1/3 nvme: enable multipathing by default
Quoted 24 lines · Sagi Grimberg
Quoted 18 lines
Quoted 13 lines
This is okay with me, but I have one little follow-on I'd like to RFC here:
The
/dev/nvmenaming convention with multiplath has broken people's minds, and making it default will invite a lot of repeat questions on how to match up block to admin handles.Could we promote
/dev/nvme0to be a subsystem handle instead of a controller handle, and add new per-controller handles, like/dev/nvme0c1, nvme0c2, etc...? The subsystem handle's management interface can just use the first active controller to forward admin commands, and per-controller specific administration can be taught to use the new handles as needed.Quoted 4 lines
Tooling shouldn't care about the names, but I am not sure if this could possibly break productino scripts. The naming disconnect has broken some things, though, so I hope this doesn't end up being a pick your poision suggestion...
Quoted 5 lines
The problem is that typical admin command are sensitive to the controller they are sent to, so a per-subsystem handle does not make whole lot sense. But I like the idea of the nvmeXcY names. Maybe we just keep the nvmeX names as an additional alias for the first found controller in a subsystem and otherwise use nvmeXcY?
Quoted 5 lines
Not sure if different naming conventions between controllers are a good idea..
What is the problem of simply changing nvmeX to nvmeXcY? if nvmeX is a must perhaps we can soft link nvmeX to nvmeXc1?
I am not aware of anything that has a hard dependency on /dev/nvmeX being present, but I can't say if that applies to everyone. I'll check with some internal folks if this will break there tools/workflows.
It will just take some minor shuffling of the initialization order to do this since we currently assign the controller instance before we know the subsystem instance. Otherwise, it should work.
I'll finish off the code and send separately. As far as this patch series is concerned, looks good to me.
Christoph Hellwig
maintainerPATCH 2/3 nvme: warn when finding multi-port subsystems without multipathing enabled
In reply to Christoph HellwigWithout CONFIG_NVME_MULTIPATH enabled a multi-port subsystem might show up as invididual devices and cause problems, warn about it.
--- drivers/nvme/host/nvme.h | 3 +++ 1 file changed, 3 insertions(+)
drivers/nvme/host/nvme.h +3 -0
@@ -534,6 +534,9 @@ static inline void nvme_mpath_check_last_path(struct nvme_ns *ns)static inline int nvme_mpath_init (struct nvme_ctrl *ctrl,struct nvme_id_ctrl *id) { +if (ctrl->subsys->cmic & (1 <<3 )) +dev_warn (ctrl->device, +"Please enable CONFIG_NVME_MULTIPATH for full support of multi-port devices.\n" );return 0 ; }static inline void nvme_mpath_uninit (struct nvme_ctrl *ctrl)
Signature · 1 line
2.19.1
Sagi Grimberg
maintainerPATCH 2/3 nvme: warn when finding multi-port subsystems without multipathing enabled
Christoph Hellwig
maintainerPATCH 3/3 nvme: deprecate the multipath=0 module option
In reply to Christoph HellwigThis was intended as a bridge for pre-existing setups if there were any, and is not intended to build long-term solutions on top. Explicitly it and print a warning for each controller found when it is set.
--- drivers/nvme/host/multipath.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/nvme/host/multipath.c +7 -1
@@ -273,9 +273,15 @@ int nvme_mpath_alloc_disk(struct nvme_ctrl *ctrl, struct nvme_ns_head *head)* We also do this for private namespaces as the namespace sharing data could * change after a rescan. */ -if (!(ctrl->subsys->cmic & (1 <<1 )) || !multipath) +if (!(ctrl->subsys->cmic & (1 <<1 )))return 0 ; +if (!multipath) { +dev_warn (ctrl->device, +"multipath=0 module option is deprecated and will be removed in 2020.\n" ); +return 0 ; + } + q =blk_alloc_queue_node (GFP_KERNEL , ctrl->numa_node);if (!q) goto out;
Signature · 1 line
2.19.1
Sagi Grimberg
maintainerPATCH 3/3 nvme: deprecate the multipath=0 module option
Quoted 3 lines · Christoph Hellwig
This was intended as a bridge for pre-existing setups if there were any, and is not intended to build long-term solutions on top. Explicitly it
typo
Christoph Hellwig
maintainerPATCH v2 3/3 nvme: deprecate the multipath=0 module option
This was intended as a bridge for pre-existing setups if there were any, and is not intended to build long-term solutions on top. Deprecate it and print a warning for each controller found when it is set.
--- drivers/nvme/host/multipath.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/nvme/host/multipath.c +7 -1
@@ -273,9 +273,15 @@ int nvme_mpath_alloc_disk(struct nvme_ctrl *ctrl, struct nvme_ns_head *head)* We also do this for private namespaces as the namespace sharing data could * change after a rescan. */ -if (!(ctrl->subsys->cmic & (1 <<1 )) || !multipath) +if (!(ctrl->subsys->cmic & (1 <<1 )))return 0 ; +if (!multipath) { +dev_warn (ctrl->device, +"multipath=0 module option is deprecated and will be removed in 2020.\n" ); +return 0 ; + } + q =blk_alloc_queue_node (GFP_KERNEL ,NUMA_NO_NODE ,NULL );if (!q) goto out;
Signature · 1 line
2.19.1