[PATCH v3 0/5] nvme: make core.nvme_multipath configurable

[PATCH v3 0/5] nvme: make core.nvme_multipath configurable

[John Meneghini]

As agreed at LSF/MM here is a v3 patch set which includes the 
new CONFIG_NVME_ENABLE_MULTIPATH_PARAM option.

NOTE: we agreed to call this CONFIG_NVME_DISBALE_MULTIPATH_PARAM.
However, during testing I realized that we needed to make the 
logic positive or else many of the default make 'config' rules
would produce the wrong results. 

For example:

 make localmodconfig
 make allmodconfig 

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

To remove the core.nvme_multipath one needs to 
explicity overide the default setting by saying
CONFIG_NVME_ENABLE_MULTIPATH_PARAM=n.

The current default setting for this option is

  CONFIG_NVME_ENABLE_MULTIPATH_PARAM=y.

To remove the core_nvme.multipath one needs to 
explicity overide the default setting by saying
CONFIG_NVME_ENABLE_MULTIPATH_PARAM=n.

Keith, I suggest that we add these to nvme-6.16 since there are a few
more changes than I had planned for. I'd like these patches to get some
soak time before they merge.

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/ 

Signed-off-by: John Meneghini <jmeneghi@redhat.com>

Changes Since v2

Changed the patch order and added:

  nvme: add NVME_ENABLE_MULTIPATH_PARAM for loongarch
  nvme: add NVME_ENABLE_MULTIPATH_PARAM for powerpc skiroot

I'm not 100% sure these two are needed but it would be
better to be safe than sorry. 

  nvme-multipath: add the NVME_ENABLE_MULTIPATH_PARAM config option

Changed the name of this config option from 
NVME_MULTIPATH_PARAM to NVME_ENABLE_MULTIPATH_PARAM and
Fixed up the Kconfig description.

Tested the following make options:

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

These all provide no change in current behavior.

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 (5):
  nvme: update the multipath warning in nvme_init_ns_head
  nvme: change the NVME_MULTIPATH config option description
  nvme: add the NVME_ENABLE_MULTIPATH_PARAM config option
  nvme: add NVME_ENABLE_MULTIPATH_PARAM for loongarch
  nvme: add NVME_ENABLE_MULTIPATH_PARAM for powerpc skiroot

 arch/loongarch/configs/loongson3_defconfig |  1 +
 arch/powerpc/configs/skiroot_defconfig     |  1 +
 drivers/nvme/host/Kconfig                  | 24 ++++++++++++++++++----
 drivers/nvme/host/core.c                   |  2 +-
 drivers/nvme/host/multipath.c              |  2 ++
 5 files changed, 25 insertions(+), 5 deletions(-)

-- 
2.48.1

[PATCH v3 1/5] 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

[PATCH v3 2/5] nvme: change the NVME_MULTIPATH config option description

[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 v3 3/5] nvme: add the NVME_ENABLE_MULTIPATH_PARAM config option

[John Meneghini]

The CONFIG_NVME_ENABLE_MULTIPATH_PARAM option controls the
core_nvme.multipath parameter. When CONFIG_NVME_ENABLE_MULTIPATH_PARAM=n
the multipath parameter is removed from the kernel and nvme multipathing
is permanently enabled.  When CONFIG_NVME_ENABLE_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_ENABLE_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     | 11 +++++++++++
 drivers/nvme/host/multipath.c |  2 ++
 2 files changed, 13 insertions(+)

diff --git a/drivers/nvme/host/Kconfig b/drivers/nvme/host/Kconfig
index d47dfa80fb95..8c04b6b93982 100644
--- a/drivers/nvme/host/Kconfig
+++ b/drivers/nvme/host/Kconfig
@@ -28,6 +28,17 @@ config NVME_MULTIPATH
 
 	  If unsure, say Y.
 
+config NVME_ENABLE_MULTIPATH_PARAM
+	bool "NVMe enable multipath param"
+	depends on NVME_CORE && NVME_MULTIPATH
+	default y
+	help
+	  This option enables the core_nvme.multipath parameter.
+	  If this option is disabled the core_nvme.multipath parameter
+	  is excluded from 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..83084093e8db 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -10,9 +10,11 @@
 #include "nvme.h"
 
 bool multipath = true;
+#ifdef CONFIG_NVME_ENABLE_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",
-- 
2.48.1

[PATCH v3 4/5] nvme: add NVME_ENABLE_MULTIPATH_PARAM for loongarch

[John Meneghini]

The add the new CONFIG_NVME_ENABLE_MULTIPATH_PARAM to ensure there is no
change in behavior when CONFIG_NVME_MULTIPATH is true with loongson3
configs.

Signed-off-by: John Meneghini <jmeneghi@redhat.com>
---
 arch/loongarch/configs/loongson3_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/loongarch/configs/loongson3_defconfig b/arch/loongarch/configs/loongson3_defconfig
index 73c77500ac46..1ba9ef835fd4 100644
--- a/arch/loongarch/configs/loongson3_defconfig
+++ b/arch/loongarch/configs/loongson3_defconfig
@@ -449,6 +449,7 @@ CONFIG_VIRTIO_BLK=y
 CONFIG_BLK_DEV_RBD=m
 CONFIG_BLK_DEV_NVME=y
 CONFIG_NVME_MULTIPATH=y
+CONFIG_NVME_ENABLE_MULTIPATH_PARAM=y
 CONFIG_NVME_RDMA=m
 CONFIG_NVME_FC=m
 CONFIG_NVME_TCP=m
-- 
2.48.1

[PATCH v3 5/5] nvme: add NVME_ENABLE_MULTIPATH_PARAM for powerpc skiroot

[John Meneghini]

The add the new CONFIG_NVME_ENABLE_MULTIPATH_PARAM
to ensure there is no change in behavior when
CONFIG_NVME_MULTIPATH is true with skiroot_defconfig.

Signed-off-by: John Meneghini <jmeneghi@redhat.com>
---
 arch/powerpc/configs/skiroot_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/configs/skiroot_defconfig b/arch/powerpc/configs/skiroot_defconfig
index 9d44e6630908..85d58429a244 100644
--- a/arch/powerpc/configs/skiroot_defconfig
+++ b/arch/powerpc/configs/skiroot_defconfig
@@ -77,6 +77,7 @@ CONFIG_BLK_DEV_RAM_SIZE=65536
 CONFIG_VIRTIO_BLK=m
 CONFIG_BLK_DEV_NVME=m
 CONFIG_NVME_MULTIPATH=y
+CONFIG_NVME_ENABLE_MULTIPATH_PARAM=y
 CONFIG_EEPROM_AT24=m
 # CONFIG_CXL is not set
 # CONFIG_OCXL is not set
-- 
2.48.1

Re: [PATCH v3 3/5] nvme: add the NVME_ENABLE_MULTIPATH_PARAM config option

[Caleb Sander Mateos]

On Fri, Mar 28, 2025 at 12:42 PM John Meneghini <jmeneghi@redhat.com> wrote:
>
> The CONFIG_NVME_ENABLE_MULTIPATH_PARAM option controls the
> core_nvme.multipath parameter. When CONFIG_NVME_ENABLE_MULTIPATH_PARAM=n
> the multipath parameter is removed from the kernel and nvme multipathing
> is permanently enabled.  When CONFIG_NVME_ENABLE_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_ENABLE_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     | 11 +++++++++++
>  drivers/nvme/host/multipath.c |  2 ++
>  2 files changed, 13 insertions(+)
>
> diff --git a/drivers/nvme/host/Kconfig b/drivers/nvme/host/Kconfig
> index d47dfa80fb95..8c04b6b93982 100644
> --- a/drivers/nvme/host/Kconfig
> +++ b/drivers/nvme/host/Kconfig
> @@ -28,6 +28,17 @@ config NVME_MULTIPATH
>
>           If unsure, say Y.
>
> +config NVME_ENABLE_MULTIPATH_PARAM
> +       bool "NVMe enable multipath param"
> +       depends on NVME_CORE && NVME_MULTIPATH
> +       default y
> +       help
> +         This option enables the core_nvme.multipath parameter.
> +         If this option is disabled the core_nvme.multipath parameter
> +         is excluded from 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..83084093e8db 100644
> --- a/drivers/nvme/host/multipath.c
> +++ b/drivers/nvme/host/multipath.c
> @@ -10,9 +10,11 @@
>  #include "nvme.h"
>
>  bool multipath = true;
> +#ifdef CONFIG_NVME_ENABLE_MULTIPATH_PARAM
>  module_param(multipath, bool, 0444);
>  MODULE_PARM_DESC(multipath,
>         "turn on native support for multiple controllers per subsystem");
> +#endif

If CONFIG_NVME_ENABLE_MULTIPATH_PARAM is disabled, could you #define
multipath false in place of the global variable? That would allow the
compiler to optimize out the multipath checks and the resulting dead
code.

Best,
Caleb

Re: [PATCH v3 3/5] nvme: add the NVME_ENABLE_MULTIPATH_PARAM config option

[John Meneghini]

On 3/28/25 3:49 PM, Caleb Sander Mateos wrote:
>>   bool multipath = true;
>> +#ifdef CONFIG_NVME_ENABLE_MULTIPATH_PARAM
>>   module_param(multipath, bool, 0444);
>>   MODULE_PARM_DESC(multipath,
>>          "turn on native support for multiple controllers per subsystem");
>> +#endif
> If CONFIG_NVME_ENABLE_MULTIPATH_PARAM is disabled, could you #define
> multipath false in place of the global variable? That would allow the
> compiler to optimize out the multipath checks and the resulting dead
> code.

If we #define multipath to be false that would permanently disable the multipath code
when CONFIG_NVME_ENABLE_MULTIPATH_PARAM=n, and this is not what we want. The purpose
of CONFIG_NVME_ENABLE_MULTIPATH_PARAM=n is to simply remove the core_nvme.multipath
parameter from the kernel so the user no longer has the ability to change
the bool multipath variable.  We want multipath to be true.

There should be no change in the multipathing policy or code by
enabling or disabling CONFIG_NVME_ENABLE_MULTIPATH_PARAM.

If you want to compile out the mulipath code you should set CONFIG_NVME_ENABLE_MULTIPATH=n.

/John

Re: [PATCH v3 3/5] nvme: add the NVME_ENABLE_MULTIPATH_PARAM config option

[Caleb Sander Mateos]

On Mon, Mar 31, 2025 at 7:29 AM John Meneghini <jmeneghi@redhat.com> wrote:
>
> On 3/28/25 3:49 PM, Caleb Sander Mateos wrote:
> >>   bool multipath = true;
> >> +#ifdef CONFIG_NVME_ENABLE_MULTIPATH_PARAM
> >>   module_param(multipath, bool, 0444);
> >>   MODULE_PARM_DESC(multipath,
> >>          "turn on native support for multiple controllers per subsystem");
> >> +#endif
> > If CONFIG_NVME_ENABLE_MULTIPATH_PARAM is disabled, could you #define
> > multipath false in place of the global variable? That would allow the
> > compiler to optimize out the multipath checks and the resulting dead
> > code.
>
> If we #define multipath to be false that would permanently disable the multipath code
> when CONFIG_NVME_ENABLE_MULTIPATH_PARAM=n, and this is not what we want. The purpose
> of CONFIG_NVME_ENABLE_MULTIPATH_PARAM=n is to simply remove the core_nvme.multipath
> parameter from the kernel so the user no longer has the ability to change
> the bool multipath variable.  We want multipath to be true.

Sorry, I just meant to #define multipath to a constant. I mixed up the
boolean value. If CONFIG_NVME_MULTIPATH is set, of course it should be
true, not false. If CONFIG_NVME_MULTIPATH is not set, multipath is
already defined as false.

My point was just that the compiler will pessimistically assume a
global variable with extern visibility could be written from some
other translation unit at any time. But
CONFIG_NVME_ENABLE_MULTIPATH_PARAM removes the only way for the global
variable to change at runtime, so it can just be a constant.

Best,
Caleb