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

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

[John Meneghini]

These patches propose an alternative to outright removing the 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
NVME_MULTIPATH_PARAM=n.

Hopefully this compromise gives everyone what they want.

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

John Meneghini (4):
  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
  nvme: add mulitipath warning to nvme_alloc_ns

 drivers/nvme/host/Kconfig     | 28 ++++++++++++++++++++++++----
 drivers/nvme/host/core.c      | 19 ++++++++++++++++++-
 drivers/nvme/host/multipath.c |  3 ++-
 3 files changed, 44 insertions(+), 6 deletions(-)

-- 
2.48.1

[PATCH 1/4] 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>
---
 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 486afe598184..91b0346ce65a 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 2/4] nvme-multipath: add the NVME_MULTIPATH_PARAM config option

[John Meneghini]

The NVME_MULTIPATH_PARAM option controls the core.nvme_multipath module
parameter. When NVME_MULTIPATH_PARAM=n the multipath parameter is removed
and core nvme multipathing is enabled. When NVME_MULTIPATH_PARAM=y
the multipath parameter is added and multipath support becomes
configurable with the core.nvme_multipath parameter.

By default NVME_MULTIPATH_PARAM=y

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

diff --git a/drivers/nvme/host/Kconfig b/drivers/nvme/host/Kconfig
index 91b0346ce65a..c4251504f201 100644
--- a/drivers/nvme/host/Kconfig
+++ b/drivers/nvme/host/Kconfig
@@ -28,6 +28,21 @@ config NVME_MULTIPATH
 
 	  If unsure, say Y.
 
+config NVME_MULTIPATH_PARAM
+	bool "NVMe multipath param"
+	depends on NVME_CORE && NVME_MULTIPATH
+	help
+	  This option enables configurable support for multipath access with
+	  NVMe subsystems. If this option is enabled NVMe multipath support is
+	  configured by the nvme core module parameter named "multipath". If
+	  this option is disabled the nvme core module "multipath" parameter
+	  is removed and support for NVMe multipath access can not be
+	  configured. When this option is disabled a single /dev/nvmeXnY
+	  device entry will be seen for each NVMe namespace, even if the
+	  namespace is accessible through multiple controllers.
+
+	  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 2a7635565083..4536ad5fbb82 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -10,10 +10,11 @@
 #include "nvme.h"
 
 bool multipath = true;
+#ifdef 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

[PATCH 3/4] 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(). Remove
the old warning message and add new ones.

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

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 818d4e49aab5..c2b7e6834535 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3823,8 +3823,16 @@ static int nvme_init_ns_head(struct nvme_ns *ns, struct nvme_ns_info *info)
 			dev_warn(ctrl->device,
 				"Found shared namespace %d, but multipathing not supported.\n",
 				info->nsid);
+#ifdef CONFIG_NVME_MULTIPATH
+#ifdef CONFIG_NVME_MULTIPATH_PARAM
+			dev_warn_once(ctrl->device,
+				"Shared namespace support requires core.nvme_multipath=Y.\n");
+
+#endif
+#else
 			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 CONFIG_NVME_MULTIPATH.\n");
+#endif
 		}
 	}
 
-- 
2.48.1

[PATCH 4/4] nvme: add mulitipath warning to nvme_alloc_ns

[John Meneghini]

When CONFIG_NVME_MULTIPATH is disabled, add a warning if
we discover a multipath enabled controller with an attached
shared namespace.

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

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index c2b7e6834535..465069c0f6a8 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3938,6 +3938,15 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, struct nvme_ns_info *info)
 	} else {
 		sprintf(disk->disk_name, "nvme%dn%d", ctrl->instance,
 			ns->head->instance);
+#ifndef CONFIG_NVME_MULTIPATH
+		if ((ctrl->subsys->cmic & NVME_CTRL_CMIC_MULTI_CTRL) && info->is_shared) {
+			dev_warn(ctrl->device,
+				"Found shared namespace %d but multipathing not supported.\n",
+				info->nsid);
+			dev_warn_once(ctrl->device,
+				"Shared namespace support requires CONFIG_NVME_MULTIPATH.\n");
+		}
+#endif
 	}
 
 	if (nvme_update_ns_info(ns, info))
-- 
2.48.1

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

[Nilay Shroff]

On 2/28/25 8:55 AM, John Meneghini wrote:
> The NVME_MULTIPATH_PARAM option controls the core.nvme_multipath module
> parameter. When NVME_MULTIPATH_PARAM=n the multipath parameter is removed
> and core nvme multipathing is enabled. When NVME_MULTIPATH_PARAM=y
> the multipath parameter is added and multipath support becomes
> configurable with the core.nvme_multipath parameter.
> 
> By default NVME_MULTIPATH_PARAM=y
> 
> Signed-off-by: John Meneghini <jmeneghi@redhat.com>
> ---
>  drivers/nvme/host/Kconfig     | 15 +++++++++++++++
>  drivers/nvme/host/multipath.c |  3 ++-
>  2 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/nvme/host/Kconfig b/drivers/nvme/host/Kconfig
> index 91b0346ce65a..c4251504f201 100644
> --- a/drivers/nvme/host/Kconfig
> +++ b/drivers/nvme/host/Kconfig
> @@ -28,6 +28,21 @@ config NVME_MULTIPATH
>  
>  	  If unsure, say Y.
>  
> +config NVME_MULTIPATH_PARAM
> +	bool "NVMe multipath param"
> +	depends on NVME_CORE && NVME_MULTIPATH
> +	help
> +	  This option enables configurable support for multipath access with
> +	  NVMe subsystems. If this option is enabled NVMe multipath support is
> +	  configured by the nvme core module parameter named "multipath". If
> +	  this option is disabled the nvme core module "multipath" parameter
> +	  is removed and support for NVMe multipath access can not be
> +	  configured. When this option is disabled a single /dev/nvmeXnY
> +	  device entry will be seen for each NVMe namespace, even if the
> +	  namespace is accessible through multiple controllers.
> +
> +	  If unsure, say Y.
> +
If we want to make NVME_MULTIPATH_PARAM default on then I think we need to add 
"default y" under config NVME_MULTIPATH_PARAM.

>  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 2a7635565083..4536ad5fbb82 100644
> --- a/drivers/nvme/host/multipath.c
> +++ b/drivers/nvme/host/multipath.c
> @@ -10,10 +10,11 @@
>  #include "nvme.h"
>  
>  bool multipath = true;
> +#ifdef NVME_MULTIPATH_PARAM

Shouldn't it be CONFIG_NVME_MULTIPATH_PARAM instead of NVME_MULTIPATH_PARAM?

Thanks,
--Nilay

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

[Nilay Shroff]

On 2/28/25 8:55 AM, John Meneghini wrote:
> The new NVME_MULTIPATH_PARAM config option requires updates
> to the warning message in nvme_init_ns_head(). Remove
> the old warning message and add new ones.
> 
> Signed-off-by: John Meneghini <jmeneghi@redhat.com>
> ---
>  drivers/nvme/host/core.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 818d4e49aab5..c2b7e6834535 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -3823,8 +3823,16 @@ static int nvme_init_ns_head(struct nvme_ns *ns, struct nvme_ns_info *info)
>  			dev_warn(ctrl->device,
>  				"Found shared namespace %d, but multipathing not supported.\n",
>  				info->nsid);
> +#ifdef CONFIG_NVME_MULTIPATH
> +#ifdef CONFIG_NVME_MULTIPATH_PARAM
> +			dev_warn_once(ctrl->device,
> +				"Shared namespace support requires core.nvme_multipath=Y.\n");
> +
> +#endif
> +#else
>  			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 CONFIG_NVME_MULTIPATH.\n");
> +#endif
>  		}
>  	}
>  

As NVME_MULTIPATH_PARAM depends on NVME_MULTIPATH, it implicitly implies 
that if NVME_MULTIPATH_PARAM is enabled then NVME_MULTIPATH has to be on.
So above logic could be simplified.

However on another note, I really don't understand why do we need to add 
new warning here as there's already a warning present just above your
changes.

Thanks,
--Nilay

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

[John Meneghini]

On 2/28/25 1:28 AM, Nilay Shroff wrote: 
> On 2/28/25 8:55 AM, John Meneghini wrote:
>> diff --git a/drivers/nvme/host/Kconfig b/drivers/nvme/host/Kconfig
>> index 91b0346ce65a..c4251504f201 100644
>> --- a/drivers/nvme/host/Kconfig
>> +++ b/drivers/nvme/host/Kconfig
>> @@ -28,6 +28,21 @@ config NVME_MULTIPATH
>>   
>>   	  If unsure, say Y.
>>   
>> +config NVME_MULTIPATH_PARAM
>> +	bool "NVMe multipath param"
>> +	depends on NVME_CORE && NVME_MULTIPATH
>> +	help
>> +	  This option enables configurable support for multipath access with
>> +	  NVMe subsystems. If this option is enabled NVMe multipath support is
>> +	  configured by the nvme core module parameter named "multipath". If
>> +	  this option is disabled the nvme core module "multipath" parameter
>> +	  is removed and support for NVMe multipath access can not be
>> +	  configured. When this option is disabled a single /dev/nvmeXnY
>> +	  device entry will be seen for each NVMe namespace, even if the
>> +	  namespace is accessible through multiple controllers.
>> +
>> +	  If unsure, say Y.
>> +
> If we want to make NVME_MULTIPATH_PARAM default on then I think we need to add
> "default y" under config NVME_MULTIPATH_PARAM.

OK. I've tested all of the config options.

make mod2noconfig
make allyesconfig
make allmodconfig
make oldconfig

And is all seems to be working correctly, but I'll add the "default y"
as you've suggested.

>>   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 2a7635565083..4536ad5fbb82 100644
>> --- a/drivers/nvme/host/multipath.c
>> +++ b/drivers/nvme/host/multipath.c
>> @@ -10,10 +10,11 @@
>>   #include "nvme.h"
>>   
>>   bool multipath = true;
>> +#ifdef NVME_MULTIPATH_PARAM
> 
> Shouldn't it be CONFIG_NVME_MULTIPATH_PARAM instead of NVME_MULTIPATH_PARAM?

Oops.  As you can tell, I haven't tested this yet.  I'll fix this up and test these
changes before sending a version 2 patch.

/John
  
> Thanks,
> --Nilay
> 

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

[John Meneghini]

On 2/28/25 1:28 AM, Nilay Shroff wrote:
> On 2/28/25 8:55 AM, John Meneghini wrote:
>> The new NVME_MULTIPATH_PARAM config option requires updates
>> to the warning message in nvme_init_ns_head(). Remove
>> the old warning message and add new ones.
>>
>> Signed-off-by: John Meneghini <jmeneghi@redhat.com>
>> ---
>>   drivers/nvme/host/core.c | 10 +++++++++-
>>   1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>> index 818d4e49aab5..c2b7e6834535 100644
>> --- a/drivers/nvme/host/core.c
>> +++ b/drivers/nvme/host/core.c
>> @@ -3823,8 +3823,16 @@ static int nvme_init_ns_head(struct nvme_ns *ns, struct nvme_ns_info *info)
>>   			dev_warn(ctrl->device,
>>   				"Found shared namespace %d, but multipathing not supported.\n",
>>   				info->nsid);
>> +#ifdef CONFIG_NVME_MULTIPATH
>> +#ifdef CONFIG_NVME_MULTIPATH_PARAM
>> +			dev_warn_once(ctrl->device,
>> +				"Shared namespace support requires core.nvme_multipath=Y.\n");
>> +
>> +#endif
>> +#else
>>   			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 CONFIG_NVME_MULTIPATH.\n");
>> +#endif
>>   		}
>>   	}
>>   
> 
> As NVME_MULTIPATH_PARAM depends on NVME_MULTIPATH, it implicitly implies
> that if NVME_MULTIPATH_PARAM is enabled then NVME_MULTIPATH has to be on.
> So above logic could be simplified.
> 
> However on another note, I really don't understand why do we need to add
> new warning here as there's already a warning present just above your
> changes.

Agreed.  How about if we just remove the

    	dev_warn_once(ctrl->device,
		"Support for shared namespaces without CONFIG_NVME_MULTIPATH is deprecated and will be removed in Linux 6.0.\n");
  
This is real problem.  This is a confusing message since there is no now plan to remove any of these config options from the kernel.

/John

> Thanks,
> --Nilay
> 
> 

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

[Nilay Shroff]

On 2/28/25 6:44 PM, John Meneghini wrote:
> On 2/28/25 1:28 AM, Nilay Shroff wrote:
>> On 2/28/25 8:55 AM, John Meneghini wrote:
>>> The new NVME_MULTIPATH_PARAM config option requires updates
>>> to the warning message in nvme_init_ns_head(). Remove
>>> the old warning message and add new ones.
>>>
>>> Signed-off-by: John Meneghini <jmeneghi@redhat.com>
>>> ---
>>>   drivers/nvme/host/core.c | 10 +++++++++-
>>>   1 file changed, 9 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>>> index 818d4e49aab5..c2b7e6834535 100644
>>> --- a/drivers/nvme/host/core.c
>>> +++ b/drivers/nvme/host/core.c
>>> @@ -3823,8 +3823,16 @@ static int nvme_init_ns_head(struct nvme_ns *ns, struct nvme_ns_info *info)
>>>               dev_warn(ctrl->device,
>>>                   "Found shared namespace %d, but multipathing not supported.\n",
>>>                   info->nsid);
>>> +#ifdef CONFIG_NVME_MULTIPATH
>>> +#ifdef CONFIG_NVME_MULTIPATH_PARAM
>>> +            dev_warn_once(ctrl->device,
>>> +                "Shared namespace support requires core.nvme_multipath=Y.\n");
>>> +
>>> +#endif
>>> +#else
>>>               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 CONFIG_NVME_MULTIPATH.\n");
>>> +#endif
>>>           }
>>>       }
>>>   
>>
>> As NVME_MULTIPATH_PARAM depends on NVME_MULTIPATH, it implicitly implies
>> that if NVME_MULTIPATH_PARAM is enabled then NVME_MULTIPATH has to be on.
>> So above logic could be simplified.
>>
>> However on another note, I really don't understand why do we need to add
>> new warning here as there's already a warning present just above your
>> changes.
> 
> Agreed.  How about if we just remove the
> 
>        dev_warn_once(ctrl->device,
>         "Support for shared namespaces without CONFIG_NVME_MULTIPATH is deprecated and will be removed in Linux 6.0.\n");
>  
> This is real problem.  This is a confusing message since there is no now plan to remove any of these config options from the kernel.
I read it as "NVMe (native) multipathing is only supported when CONFIG_NVME_MULTIPATH
is configured". 

So personally, I find existing warning messages (we print two warning messages when 
multipath is false but shared namespace is detected) sufficient, however lets see 
what others suggest.

Thanks,
--Nilay 

Re: [PATCH 1/4] nvme-multipath: change the NVME_MULTIPATH config option

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

[Christoph Hellwig]

On Thu, Feb 27, 2025 at 10:25:39PM -0500, John Meneghini wrote:
> The NVME_MULTIPATH_PARAM option controls the core.nvme_multipath module
> parameter. When NVME_MULTIPATH_PARAM=n the multipath parameter is removed
> and core nvme multipathing is enabled. When NVME_MULTIPATH_PARAM=y
> the multipath parameter is added and multipath support becomes
> configurable with the core.nvme_multipath parameter.

What's the point of adding yet another confusing option?

Re: [PATCH 4/4] nvme: add mulitipath warning to nvme_alloc_ns

[Christoph Hellwig]

On Thu, Feb 27, 2025 at 10:25:41PM -0500, John Meneghini wrote:
> When CONFIG_NVME_MULTIPATH is disabled, add a warning if
> we discover a multipath enabled controller with an attached
> shared namespace.
> 
> Signed-off-by: John Meneghini <jmeneghi@redhat.com>
> ---
>  drivers/nvme/host/core.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index c2b7e6834535..465069c0f6a8 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -3938,6 +3938,15 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, struct nvme_ns_info *info)
>  	} else {
>  		sprintf(disk->disk_name, "nvme%dn%d", ctrl->instance,
>  			ns->head->instance);
> +#ifndef CONFIG_NVME_MULTIPATH

This can use IS_ENABLED to clean the code up a bit.

> +		if ((ctrl->subsys->cmic & NVME_CTRL_CMIC_MULTI_CTRL) && info->is_shared) {

overly long line.

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

[John Meneghini]

On 3/5/25 9:33 AM, Christoph Hellwig wrote:
> On Thu, Feb 27, 2025 at 10:25:39PM -0500, John Meneghini wrote:
>> The NVME_MULTIPATH_PARAM option controls the core.nvme_multipath module
>> parameter. When NVME_MULTIPATH_PARAM=n the multipath parameter is removed
>> and core nvme multipathing is enabled. When NVME_MULTIPATH_PARAM=y
>> the multipath parameter is added and multipath support becomes
>> configurable with the core.nvme_multipath parameter.
> 
> What's the point of adding yet another confusing option?

If you'll read the kConfig description, hopefully it's not confusing.

The whole point of this patch series is to remove the core.nvme_mulipath parameter.

This is what the patch at: https://lore.kernel.org/linux-nvme/20250204211158.43126-1-bgurney@redhat.com/
does, and this is what this patch does. Since people didn't want to remove core.nvme_multipath parameter
in  https://lore.kernel.org/linux-nvme/20250204211158.43126-1-bgurney@redhat.com/ I've proposed this
patch as an alternative.

It provides a kConfig option to remove the core.nvme_multipath parameter so those who want it
can keep it, and those who don't and compile it out.

/John

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

[Christoph Hellwig]

On Tue, Mar 11, 2025 at 10:35:40PM -0400, John Meneghini wrote:
>> What's the point of adding yet another confusing option?
>
> If you'll read the kConfig description, hopefully it's not confusing.

It still is.

> The whole point of this patch series is to remove the core.nvme_mulipath parameter.

Why would a compile time option be preferable over a runtime one?

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

[John Meneghini]

On 3/12/25 1:19 AM, Christoph Hellwig wrote:
> On Tue, Mar 11, 2025 at 10:35:40PM -0400, John Meneghini wrote:
>>> What's the point of adding yet another confusing option?
>>
>> If you'll read the kConfig description, hopefully it's not confusing.
> 
> It still is.

OK. I can fix that.

>> The whole point of this patch series is to remove the core.nvme_mulipath parameter.
> 
> Why would a compile time option be preferable over a runtime one?

Because distos like RHEL don't allow customers to recompile their kernel. And including this runtime
switch in the kernel let's users turn nvme-multipath off w/out recompiling the kernel.

Our RHEL customers do this all of the time to enable DMMP with their NVMe devices. This has caused tremendous
confusion and turmoil with our RHEL customers who keep on turning nvme-multipath off even when we ship
RHEL with nvme-multipath on and we tell them turning it off is not supported.

I finally decided that there is no good way to control this situation w/out removing the
core.nvme_multipath parameter while CONFIG_NVME_MULTIPATH is enabled.

This patch enables distros like RHEL simply config out the parameter w/out impacting all of the people out
there who currently want and use the run time switch when CONFIG_NVME_MULTIPATH is enabled.

Please let me know what I need to do to make these patches less confusing and get them accepted upstream.

/John

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

[Keith Busch]

On Thu, Mar 13, 2025 at 05:46:25PM -0400, John Meneghini wrote:
> Our RHEL customers do this all of the time to enable DMMP with their NVMe devices. This has caused tremendous
> confusion and turmoil with our RHEL customers who keep on turning nvme-multipath off even when we ship
> RHEL with nvme-multipath on and we tell them turning it off is not supported.
> 
> I finally decided that there is no good way to control this situation w/out removing the
> core.nvme_multipath parameter while CONFIG_NVME_MULTIPATH is enabled.
> 
> This patch enables distros like RHEL simply config out the parameter w/out impacting all of the people out
> there who currently want and use the run time switch when CONFIG_NVME_MULTIPATH is enabled.
> 
> Please let me know what I need to do to make these patches less confusing and get them accepted upstream.

It'd be one thing if it was just me lamenting the loss of the parameter,
but Hannes has customers with a very real and obvious use case for it,
even if Christoph hasn't encountered many (or any?) devices that fall
into that trap. So, from my perspective, I really think pcie hotplug
handling needs to be sorted out before we can just force these to
subscribe to the native mulitpath virtual device hierarchy.

In the meantime, I know RHEL has done out of tree patches for many
reasons, so it shouldn't be a big deal to change the module_param type
from "bool" to "bool_enable_only" in your kernel releases.