* tweak multipath kconfig and options
@ 2018-11-20 16:27 Christoph Hellwig
2018-11-20 16:27 ` [PATCH 1/3] nvme: enable multipathing by default Christoph Hellwig
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Christoph Hellwig @ 2018-11-20 16:27 UTC (permalink / raw)
Based 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.
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 1/3] nvme: enable multipathing by default 2018-11-20 16:27 tweak multipath kconfig and options Christoph Hellwig @ 2018-11-20 16:27 ` Christoph Hellwig 2018-11-20 18:47 ` Keith Busch 2018-11-20 16:27 ` [PATCH 2/3] nvme: warn when finding multi-port subsystems without multipathing enabled Christoph Hellwig 2018-11-20 16:27 ` [PATCH 3/3] nvme: deprecate the multipath=0 module option Christoph Hellwig 2 siblings, 1 reply; 11+ messages in thread From: Christoph Hellwig @ 2018-11-20 16:27 UTC (permalink / raw) 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(+) diff --git a/drivers/nvme/host/Kconfig b/drivers/nvme/host/Kconfig index 88a8b5916624..d2ae6954ce7e 100644 --- a/drivers/nvme/host/Kconfig +++ b/drivers/nvme/host/Kconfig @@ -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 -- 2.19.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 1/3] nvme: enable multipathing by default 2018-11-20 16:27 ` [PATCH 1/3] nvme: enable multipathing by default Christoph Hellwig @ 2018-11-20 18:47 ` Keith Busch 2018-11-21 8:27 ` Christoph Hellwig 0 siblings, 1 reply; 11+ messages in thread From: Keith Busch @ 2018-11-20 18:47 UTC (permalink / raw) On Tue, Nov 20, 2018@05:27:32PM +0100, Christoph Hellwig wrote: > 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(+) > > diff --git a/drivers/nvme/host/Kconfig b/drivers/nvme/host/Kconfig > index 88a8b5916624..d2ae6954ce7e 100644 > --- a/drivers/nvme/host/Kconfig > +++ b/drivers/nvme/host/Kconfig > @@ -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 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... ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/3] nvme: enable multipathing by default 2018-11-20 18:47 ` Keith Busch @ 2018-11-21 8:27 ` Christoph Hellwig 2018-11-22 2:44 ` Sagi Grimberg 0 siblings, 1 reply; 11+ messages in thread From: Christoph Hellwig @ 2018-11-21 8:27 UTC (permalink / raw) On Tue, Nov 20, 2018@11:47:49AM -0700, Keith Busch wrote: > 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... 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? ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/3] nvme: enable multipathing by default 2018-11-21 8:27 ` Christoph Hellwig @ 2018-11-22 2:44 ` Sagi Grimberg 2018-11-26 15:40 ` Keith Busch 0 siblings, 1 reply; 11+ messages in thread From: Sagi Grimberg @ 2018-11-22 2:44 UTC (permalink / raw) >> 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... > > 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? ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/3] nvme: enable multipathing by default 2018-11-22 2:44 ` Sagi Grimberg @ 2018-11-26 15:40 ` Keith Busch 0 siblings, 0 replies; 11+ messages in thread From: Keith Busch @ 2018-11-26 15:40 UTC (permalink / raw) On Wed, Nov 21, 2018@06:44:05PM -0800, Sagi Grimberg wrote: > > >> 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... > > > > 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? 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. Reviewed-by: Keith Busch <keith.busch at intel.com> ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/3] nvme: warn when finding multi-port subsystems without multipathing enabled 2018-11-20 16:27 tweak multipath kconfig and options Christoph Hellwig 2018-11-20 16:27 ` [PATCH 1/3] nvme: enable multipathing by default Christoph Hellwig @ 2018-11-20 16:27 ` Christoph Hellwig 2018-11-22 2:45 ` Sagi Grimberg 2018-11-20 16:27 ` [PATCH 3/3] nvme: deprecate the multipath=0 module option Christoph Hellwig 2 siblings, 1 reply; 11+ messages in thread From: Christoph Hellwig @ 2018-11-20 16:27 UTC (permalink / raw) Without CONFIG_NVME_MULTIPATH enabled a multi-port subsystem might show up as invididual devices and cause problems, warn about it. Signed-off-by: Christoph Hellwig <hch at lst.de> --- drivers/nvme/host/nvme.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h index 27663ce3044e..f2594d468f29 100644 --- a/drivers/nvme/host/nvme.h +++ b/drivers/nvme/host/nvme.h @@ -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) -- 2.19.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/3] nvme: warn when finding multi-port subsystems without multipathing enabled 2018-11-20 16:27 ` [PATCH 2/3] nvme: warn when finding multi-port subsystems without multipathing enabled Christoph Hellwig @ 2018-11-22 2:45 ` Sagi Grimberg 0 siblings, 0 replies; 11+ messages in thread From: Sagi Grimberg @ 2018-11-22 2:45 UTC (permalink / raw) Reviewed-by: Sagi Grimberg <sagi at grimberg.me> ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/3] nvme: deprecate the multipath=0 module option 2018-11-20 16:27 tweak multipath kconfig and options Christoph Hellwig 2018-11-20 16:27 ` [PATCH 1/3] nvme: enable multipathing by default Christoph Hellwig 2018-11-20 16:27 ` [PATCH 2/3] nvme: warn when finding multi-port subsystems without multipathing enabled Christoph Hellwig @ 2018-11-20 16:27 ` Christoph Hellwig 2018-11-22 2:49 ` Sagi Grimberg 2 siblings, 1 reply; 11+ messages in thread From: Christoph Hellwig @ 2018-11-20 16:27 UTC (permalink / raw) 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 and print a warning for each controller found when it is set. Signed-off-by: Christoph Hellwig <hch at lst.de> --- drivers/nvme/host/multipath.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c index eec44e5ec294..3851c09695b5 100644 --- a/drivers/nvme/host/multipath.c +++ b/drivers/nvme/host/multipath.c @@ -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; -- 2.19.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/3] nvme: deprecate the multipath=0 module option 2018-11-20 16:27 ` [PATCH 3/3] nvme: deprecate the multipath=0 module option Christoph Hellwig @ 2018-11-22 2:49 ` Sagi Grimberg 2018-11-27 15:06 ` [PATCH 3/3 v2] " Christoph Hellwig 0 siblings, 1 reply; 11+ messages in thread From: Sagi Grimberg @ 2018-11-22 2:49 UTC (permalink / raw) On 11/20/18 8:27 AM, Christoph Hellwig wrote: > 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/3 v2] nvme: deprecate the multipath=0 module option 2018-11-22 2:49 ` Sagi Grimberg @ 2018-11-27 15:06 ` Christoph Hellwig 0 siblings, 0 replies; 11+ messages in thread From: Christoph Hellwig @ 2018-11-27 15:06 UTC (permalink / raw) 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. Signed-off-by: Christoph Hellwig <hch at lst.de> --- drivers/nvme/host/multipath.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c index 9901afd804ce..5f253839f517 100644 --- a/drivers/nvme/host/multipath.c +++ b/drivers/nvme/host/multipath.c @@ -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; -- 2.19.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-11-27 15:06 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-11-20 16:27 tweak multipath kconfig and options Christoph Hellwig 2018-11-20 16:27 ` [PATCH 1/3] nvme: enable multipathing by default Christoph Hellwig 2018-11-20 18:47 ` Keith Busch 2018-11-21 8:27 ` Christoph Hellwig 2018-11-22 2:44 ` Sagi Grimberg 2018-11-26 15:40 ` Keith Busch 2018-11-20 16:27 ` [PATCH 2/3] nvme: warn when finding multi-port subsystems without multipathing enabled Christoph Hellwig 2018-11-22 2:45 ` Sagi Grimberg 2018-11-20 16:27 ` [PATCH 3/3] nvme: deprecate the multipath=0 module option Christoph Hellwig 2018-11-22 2:49 ` Sagi Grimberg 2018-11-27 15:06 ` [PATCH 3/3 v2] " Christoph Hellwig
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.