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

[John Meneghini <jmeneghi@redhat.com>]

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