[PATCH v2 0/3] nvme: make core.nvme_multipath configurable

[PATCH v2 0/3] nvme: make core.nvme_multipath configurable

[John Meneghini]

These patches propose an alternative to the outright removal of the nvme
core multipath module parameter. Rather than deleting this module
parameter we control its appearance with a new Kconfig option named
NVME_MULTIPATH_PARAM

Note that the default kernel config settings produce a kernel
with no change in functionality. By default both NVME_MULTIPATH
and NVME_MULTIPATH_PARAM are enabled and there are no user visable
changes.

To remove the core.nvme_multipath parameter simply compile with
CONFIG_NVME_MULTIPATH_PARAM=n.

Note: the current behavior of nvme-v6.15

  CONFIG_NVME_MULTIPATH=y and nvme_core.multipath=N

  [  182.863531] nvme nvme4: Found shared namespace 1, but multipathing not supported.
  [  182.871030] nvme nvme4: Support for shared namespaces without CONFIG_NVME_MULTIPATH is deprecated and will be removed in Linux 6.0.

Closes: https://lore.kernel.org/linux-nvme/20241121220321.40616-1-bgurney@redhat.com/
Closes: https://lore.kernel.org/linux-nvme/20250204211158.43126-1-bgurney@redhat.com/
Closes: https://lore.kernel.org/linux-nvme/20250228032541.369804-1-jmeneghi@redhat.com/ 

Changes Since v1

Rebased patches to nvme-v6.15

  nvme: add mulitipath warning to nvme_alloc_ns

Dropped this patch because the dmesg it produced was redundant and not
needed. Testing multiport devices with CONFIG_NVME_MULTIPATH disabled
show the following console message at boot.

[167523.157284] nvme nvme5: Found shared namespace 1, but multipathing not supported.
[167523.217083] nvme nvme6: Please enable CONFIG_NVME_MULTIPATH for full support of multi-port devices.

  nvme-multipath: add the NVME_MULTIPATH_PARAM config option

Fixed up the Kconfig description for NVME_MULTIPATH_PARAM.
Add default y and tested the follow Kconfig options.

Tested the following:

  make mod2noconfig
  make allyesconfig
  make allmodconfig
  make olddefconfig
  make oldconfig - accept all defaults

No change in current behavior.

  nvme: update the multipath warning in nvme_init_ns_head

Tests ran with CONFIG_NVME_MULTIPATH=y and CONFIG_NVME_MULTIPATH_PARM=n

No change in current behavior except:

  [root@rhel-storage-108 ~]# cat /sys/module/nvme_core/parameters/multipath
  cat: /sys/module/nvme_core/parameters/multipath: No such file or directory
  
The following does nothing, just like it does when CONFIG_NVME_MULTIPATH=n

  [root@rhel-storage-108 ~]# grubby --update-kernel=ALL --args="nvme_core.multipath=N"
  [root@rhel-storage-108 ~]# reboot now
  .
  .
  .
  [root@rhel-storage-108 ~]# cat /proc/cmdline
  BOOT_IMAGE=(hd1,gpt2)/vmlinuz-6.14.0-rc2_config_ana4a+ ...  nvme_core.multipath=N
  [root@rhel-storage-108 ~]# cat /sys/module/nvme_core/parameters/multipath
  cat: /sys/module/nvme_core/parameters/multipath: No such file or directory

Tests ran with CONFIG_NVME_MULTIPATH=y and CONFIG_NVME_MULTIPATH_PARM=y

  There is no change in current behavior:

Tests ran with CONFIG_NVME_MULTIPATH=y, CONFIG_NVME_MULTIPATH_PARM=y
and nvme_core.multipath=N

  [  372.040105] nvme nvme4: Found shared namespace 1, but multipathing not supported.
  [  372.047606] nvme nvme4: Shared namespace support requires core_nvme.multipath=Y.
                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
			                   new message
  [  372.105469] nvme nvme5: creating 2 I/O queues.
  [  372.110383] nvme nvme5: mapped 2/0/0 default/read/poll queues.
  [  372.125480] nvme nvme5: new ctrl: NQN ....
  [  372.166081] nvme nvme5: Found shared namespace 1, but multipathing not supported.
  [  372.235964] nvme nvme6: creating 2 I/O queues.
  [  372.240926] nvme nvme6: mapped 2/0/0 default/read/poll queues.
  [  372.255964] nvme nvme6: new ctrl: NQN ....
  [root@rhel-storage-108 ~]# cat /sys/module/nvme_core/parameters/multipath
  N
  [root@rhel-storage-108 ~]# grubby --remove-args="nvme_core.multipath=N" --update-kernel=ALL
  [root@rhel-storage-108 ~]# grubby --info=ALL | grep nvme_core 
  [root@rhel-storage-108 ~]# reboot now
  .
  .
  .
  [root@rhel-storage-108 ~]# cat /sys/module/nvme_core/parameters/multipath
  Y



John Meneghini (3):
  nvme-multipath: change the NVME_MULTIPATH config option
  nvme-multipath: add the NVME_MULTIPATH_PARAM config option
  nvme: update the multipath warning in nvme_init_ns_head

 drivers/nvme/host/Kconfig     | 26 ++++++++++++++++++++++----
 drivers/nvme/host/core.c      |  2 +-
 drivers/nvme/host/multipath.c |  3 ++-
 3 files changed, 25 insertions(+), 6 deletions(-)

-- 
2.48.1

[PATCH v2 1/3] nvme-multipath: change the NVME_MULTIPATH config option

[John Meneghini]

Fix up the NVME_MULTIPATH config description so that
it accurately describes what it does.

Signed-off-by: John Meneghini <jmeneghi@redhat.com>
Tested-by: John Meneghini <jmeneghi@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 drivers/nvme/host/Kconfig | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/nvme/host/Kconfig b/drivers/nvme/host/Kconfig
index 10e453b2436e..d47dfa80fb95 100644
--- a/drivers/nvme/host/Kconfig
+++ b/drivers/nvme/host/Kconfig
@@ -18,10 +18,15 @@ config NVME_MULTIPATH
 	bool "NVMe multipath support"
 	depends on NVME_CORE
 	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 namespace,
-	   even if it is accessible through multiple controllers.
+	  This option controls support for multipath access to NVMe
+	  subsystems. If this option is enabled support for NVMe multipath
+	  access is included in the kernel. If this option is disabled support
+	  for NVMe multipath access is excluded from the kernel. When this
+	  option is disabled each controller/namespace receives its
+	  own /dev/nvmeXnY device entry and NVMe multipath access is
+	  not supported.
+
+	  If unsure, say Y.
 
 config NVME_VERBOSE_ERRORS
 	bool "NVMe verbose error reporting"
-- 
2.48.1

[PATCH v2 2/3] nvme-multipath: add the NVME_MULTIPATH_PARAM config option

[John Meneghini]

The new CONFIG_NVME_MULTIPATH_PARAM option controls the core_nvme.multipath
parameter. When CONFIG_NVME_MULTIPATH_PARAM=n the multipath parameter is
removed from the kernel and nvme multipathing is permanently enabled.
When NVME_MULTIPATH_PARAM=y the nvme multipath parameter is added to the
kernel and nvme multipath support is controlled by the
core_nvme.multipath parameter.

By default CONFIG_NVME_MULTIPATH_PARAM=y

Signed-off-by: John Meneghini <jmeneghi@redhat.com>
Tested-by: John Meneghini <jmeneghi@redhat.com>
Reviewed-by: Bryan Gurney <bgurney@redhat.com>
---
 drivers/nvme/host/Kconfig     | 13 +++++++++++++
 drivers/nvme/host/multipath.c |  3 ++-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/Kconfig b/drivers/nvme/host/Kconfig
index d47dfa80fb95..c8458b10c83f 100644
--- a/drivers/nvme/host/Kconfig
+++ b/drivers/nvme/host/Kconfig
@@ -28,6 +28,19 @@ config NVME_MULTIPATH
 
 	  If unsure, say Y.
 
+config NVME_MULTIPATH_PARAM
+	bool "NVMe multipath param"
+	depends on NVME_CORE && NVME_MULTIPATH
+	default y
+	help
+	  This option controls the inclusion of the NVMe core module
+	  "multipath" parameter. If this option is disabled the
+	  nvme_core.multipath parameter is excluded from the kernel.
+	  If this option is enabled the nvme_core.multipath parameter
+	  is included in the kernel.
+
+	  If unsure, say Y.
+
 config NVME_VERBOSE_ERRORS
 	bool "NVMe verbose error reporting"
 	depends on NVME_CORE
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 6b12ca80aa27..cad76de2830a 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -10,10 +10,11 @@
 #include "nvme.h"
 
 bool multipath = true;
+#ifdef CONFIG_NVME_MULTIPATH_PARAM
 module_param(multipath, bool, 0444);
 MODULE_PARM_DESC(multipath,
 	"turn on native support for multiple controllers per subsystem");
-
+#endif
 static const char *nvme_iopolicy_names[] = {
 	[NVME_IOPOLICY_NUMA]	= "numa",
 	[NVME_IOPOLICY_RR]	= "round-robin",
-- 
2.48.1

Re: [PATCH v2 2/3] nvme-multipath: add the NVME_MULTIPATH_PARAM config option

[Christoph Hellwig]

On Sat, Mar 22, 2025 at 07:28:47PM -0400, John Meneghini wrote:
> The new CONFIG_NVME_MULTIPATH_PARAM option controls the core_nvme.multipath
> parameter. When CONFIG_NVME_MULTIPATH_PARAM=n the multipath parameter is
> removed from the kernel and nvme multipathing is permanently enabled.
> When NVME_MULTIPATH_PARAM=y the nvme multipath parameter is added to the
> kernel and nvme multipath support is controlled by the
> core_nvme.multipath parameter.

So as stated before I hate these options with passion.  On the other
hand both RH and SuSE stated they'd prefer to ship with the option
disabled, so I'd rather accomodate them rather than having to ship
patches for this which will then confuse folks why they are different
from upstream.

But:

> +config NVME_MULTIPATH_PARAM
> +	bool "NVMe multipath param"

This isn't really a good config options description.

> +	depends on NVME_CORE && NVME_MULTIPATH
> +	default y
> +	help
> +	  This option controls the inclusion of the NVMe core module
> +	  "multipath" parameter. If this option is disabled the
> +	  nvme_core.multipath parameter is excluded from the kernel.
> +	  If this option is enabled the nvme_core.multipath parameter
> +	  is included in the kernel.

So maybe invert the option to 

config NVME_MULTIPATH_DISABLE
	bool "Allow overriding the default nvme-multipath parameter"

	help
	  This option controls the inclusion of the NVMe core module
	  "multipath" parameter. If this option is enabled the
	  nvme_core.multipath parameter is excluded from the kernel.
	  If this option is enabled the nvme_core.multipath parameter

	  See the nvme_core.multipath documentation why disabling
	  multipathing is generally harmful but there might be
	  exception reasons to do so anyway.

(assuming we already have the documentation mentioned, if not we
need to add it)

Re: [PATCH v2 2/3] nvme-multipath: add the NVME_MULTIPATH_PARAM config option

[John Meneghini]

On 4/3/25 12:35 AM, Christoph Hellwig wrote:
> On Sat, Mar 22, 2025 at 07:28:47PM -0400, John Meneghini wrote:

>> +config NVME_MULTIPATH_PARAM
>> +	bool "NVMe multipath param"
> 
> This isn't really a good config options description.
> 
>> +	depends on NVME_CORE && NVME_MULTIPATH
>> +	default y
>> +	help
>> +	  This option controls the inclusion of the NVMe core module
>> +	  "multipath" parameter. If this option is disabled the
>> +	  nvme_core.multipath parameter is excluded from the kernel.
>> +	  If this option is enabled the nvme_core.multipath parameter
>> +	  is included in the kernel.
> 
> So maybe invert the option to
> 
> config NVME_MULTIPATH_DISABLE
> 	bool "Allow overriding the default nvme-multipath parameter"

So the question is: do you want the core_nvme.multipath parameter
to be excluded by default, or included by default?

Keith and I agreed to call this CONFIG_NVME_DISBALE_MULTIPATH_PARAM.
However during testing I realized that many of the default make 'config'
rules would end up with CONFIG_NVME_DISBALE_MULTIPATH_PARAM=y,
even if I set the config rule to "default n".

For example:

  make localmodconfig
  make allmodconfig

would end up with compiling out the core_nvme.multipath parameter and I
don't think this is what we want.

If we want this new config option to provide no net change in the
default behavior, we need this to be the logic to be positive.

> 	help
> 	  This option controls the inclusion of the NVMe core module
> 	  "multipath" parameter. If this option is enabled the
> 	  nvme_core.multipath parameter is excluded from the kernel.
> 	  If this option is enabled the nvme_core.multipath parameter

How about something simple like this:

+config NVME_ENABLE_MULTIPATH_PARAM
+       bool "NVMe enable core_nvme.multipath param"
+       depends on NVME_CORE && NVME_MULTIPATH
+       default y
+       help
+         If this option is N the core_nvme.multipath parameter
+         is excluded from the kernel. If this option is Y the
+         core_nvme.multipath parameter is included in the kernel.
+
+         If unsure, say Y.
+

> 	  See the nvme_core.multipath documentation why disabling
> 	  multipathing is generally harmful but there might be
> 	  exception reasons to do so anyway.
> 
> (assuming we already have the documentation mentioned, if not we
> need to add it)

Yes, I will add some documentation about this.  It looks like we currently
don't have any.

/John

Re: [PATCH v2 2/3] nvme-multipath: add the NVME_MULTIPATH_PARAM config option

[Christoph Hellwig]

On Fri, Apr 04, 2025 at 06:28:10PM -0400, John Meneghini wrote:
>> So maybe invert the option to
>>
>> config NVME_MULTIPATH_DISABLE
>> 	bool "Allow overriding the default nvme-multipath parameter"
>
> So the question is: do you want the core_nvme.multipath parameter
> to be excluded by default, or included by default?

I can live with it either way.

> Keith and I agreed to call this CONFIG_NVME_DISBALE_MULTIPATH_PARAM.
> However during testing I realized that many of the default make 'config'
> rules would end up with CONFIG_NVME_DISBALE_MULTIPATH_PARAM=y,
> even if I set the config rule to "default n".
>
> For example:
>
>  make localmodconfig
>  make allmodconfig
>
> would end up with compiling out the core_nvme.multipath parameter and I
> don't think this is what we want.

Weird, how does that override the explicit default statement?

> How about something simple like this:
>
> +config NVME_ENABLE_MULTIPATH_PARAM
> +       bool "NVMe enable core_nvme.multipath param"
> +       depends on NVME_CORE && NVME_MULTIPATH
> +       default y

"default y" is the default, so it can be skipped.

Re: [PATCH v2 2/3] nvme-multipath: add the NVME_MULTIPATH_PARAM config option

[Keith Busch]

On Sat, Mar 22, 2025 at 07:28:47PM -0400, John Meneghini wrote:
> +config NVME_MULTIPATH_PARAM
> +	bool "NVMe multipath param"
> +	depends on NVME_CORE && NVME_MULTIPATH
> +	default y
> +	help
> +	  This option controls the inclusion of the NVMe core module
> +	  "multipath" parameter. If this option is disabled the
> +	  nvme_core.multipath parameter is excluded from the kernel.
> +	  If this option is enabled the nvme_core.multipath parameter
> +	  is included in the kernel.
> +
> +	  If unsure, say Y.
> +
>  config NVME_VERBOSE_ERRORS
>  	bool "NVMe verbose error reporting"
>  	depends on NVME_CORE
> diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
> index 6b12ca80aa27..cad76de2830a 100644
> --- a/drivers/nvme/host/multipath.c
> +++ b/drivers/nvme/host/multipath.c
> @@ -10,10 +10,11 @@
>  #include "nvme.h"
>  
>  bool multipath = true;
> +#ifdef CONFIG_NVME_MULTIPATH_PARAM
>  module_param(multipath, bool, 0444);
>  MODULE_PARM_DESC(multipath,
>  	"turn on native support for multiple controllers per subsystem");
> -
> +#endif

John,

Is this the logic and wording you want this option to be? You, or at
least somebody, suggested to rename it DISABLE_MULTIPATH", default "n".
The end result is just a matter of setting it accordingly; I just don't
want to stall getting your feature in over a miscommunication. Thanks!

[PATCH v2 3/3] nvme: update the multipath warning in nvme_init_ns_head

[John Meneghini]

The new NVME_MULTIPATH_PARAM config option requires updates
to the warning message in nvme_init_ns_head().

Signed-off-by: John Meneghini <jmeneghi@redhat.com>
Tested-by: John Meneghini <jmeneghi@redhat.com>
---
 drivers/nvme/host/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 870314c52107..b2bd8e1673ac 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3824,7 +3824,7 @@ static int nvme_init_ns_head(struct nvme_ns *ns, struct nvme_ns_info *info)
 				"Found shared namespace %d, but multipathing not supported.\n",
 				info->nsid);
 			dev_warn_once(ctrl->device,
-				"Support for shared namespaces without CONFIG_NVME_MULTIPATH is deprecated and will be removed in Linux 6.0.\n");
+				"Shared namespace support requires core_nvme.multipath=Y.\n");
 		}
 	}
 
-- 
2.48.1

Re: [PATCH v2 0/3] nvme: make core.nvme_multipath configurable

[Keith Busch]

On Sat, Mar 22, 2025 at 07:28:45PM -0400, John Meneghini wrote:
> These patches propose an alternative to the outright removal of the nvme
> core multipath module parameter. Rather than deleting this module
> parameter we control its appearance with a new Kconfig option named
> NVME_MULTIPATH_PARAM
> 
> Note that the default kernel config settings produce a kernel
> with no change in functionality. By default both NVME_MULTIPATH
> and NVME_MULTIPATH_PARAM are enabled and there are no user visable
> changes.
> 
> To remove the core.nvme_multipath parameter simply compile with
> CONFIG_NVME_MULTIPATH_PARAM=n.

Thanks, patches 1 and 3 are applied.

Lacking a better way to handle your situation, we can add patch 2. But I
recall you wanted to invert the logic and option name, so I'll wait for
that.