Message-ID: <7c588344-f019-4939-8a93-0a450481d4bc@redhat.com>
Date: Thu, 13 Feb 2025 15:37:28 -0500
From: John Meneghini <jmeneghi@...hat.com>
To: kbusch@...nel.org, hch@....de, sagi@...mberg.me
Cc: bmarzins@...hat.com, Bryan Gurney <bgurney@...hat.com>,
 linux-nvme@...ts.infradead.org, linux-kernel@...r.kernel.org,
 Marco Patalano <mpatalan@...hat.com>, axboe@...nel.dk
Subject: Re: [PATCH] nvme: remove multipath module parameter

Keith, Christoph and Sagi,

This patch has been fully tested and analyzed by Red Hat's QA group and no
unexpected side effects or regressions have been found. Both NVMe/FC and NVMe/TCP
have been tested. Our QE engineer has asked me to report this upstream.

Tested-by: Marco Patalano <mpatalan@...hat.com>

Fixes: 32acab3181c7 ("nvme: implement multipath access to nvme subsystems")

As discussed in previous email threads, the nvme.core_multipath parameter was included
by Christoph in the original implementation of nvme/host/multipath.c back in 2017
at the request of Red Hat. At that time Red Hat was only supporting DMMP multipath
with NVMe and we needed a way to disable core native nvme multipathing without
reconfiging the kernel.

The nvme.core_multipath parameter has been used by Red Hat, together with some additional
out-of-tree changes to nvme/host/core.c, to support DMMP multipath with NVMe since RHEL-8.
However, the plan all along has been to deprecate and remove support for DMMP multipath with NVMe
in RHEL and to remove this parameter. This change has been advertised and communicated to our
customers and partners, beginning with RHEL-9, through release notes and other communications.

In RHEL-9 we changed the default setting of this parameter from "N" to "Y" to match the upstream
kernel and the move our customers to native nvme multipath. DMMP multipath w/NVMe was deprecated
in RHEL-9 but still supported. Customers were still able to optionally enabled DMMP multipath
with NVMe if they wanted, by changing this parameter, and many of them do.

In the (soon to be released) RHEL-10 release DMMP multipath is longer supported with NVMe and
the out-of-tree patches needed to support DMMP multipath with NVMe have been removed.

Red Hat is proposing this patch to remove nvme.core_multipath parameter from the kernel as we believe
leaving this non-supported parameter in the kernel for future releases will only lead to confusion.

We plan to ship this patch with RHEL-10. So it would be really good if we could get this
change accepted and merged upstream, perhaps into v6.15.

/John

On 2/4/25 4:11 PM, Bryan Gurney wrote:
> Since device-mapper multipath will no longer be operating on NVMe
> devices, there is no longer a need for the "multipath" parameter.
> 
> Note that, when compiled with CONFIG_NVME_MULTIPATH off multi-path
> capable controllers and namespaces will continue to present multiple
> device entries - one for each controller/namespace discovered.  This
> could be confusing, as device-mapper multipath relies upon code in
> nvme/host/multipath.c, and running device-mapper multipath with a
> kernel compiled with CONFIG_NVME_MULTIPATH disabled is not supported.
> 
> Closes: https://lore.kernel.org/linux-nvme/20241121220321.40616-1-bgurney@redhat.com/
> 
> Tested-by: John Meneghini <jmeneghi@...hat.com>
> Reviewed-by: John Meneghini <jmeneghi@...hat.com>
> 
> Signed-off-by: Bryan Gurney <bgurney@...hat.com>
> ---
>   drivers/nvme/host/core.c      | 14 ++++----------
>   drivers/nvme/host/multipath.c | 14 ++++++--------
>   drivers/nvme/host/nvme.h      |  2 --
>   3 files changed, 10 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 2bcd9f710cb6..b07cd482fbc1 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -3809,14 +3809,6 @@ static int nvme_init_ns_head(struct nvme_ns *ns, struct nvme_ns_info *info)
>   					info->nsid);
>   			goto out_put_ns_head;
>   		}
> -
> -		if (!multipath) {
> -			dev_warn(ctrl->device,
> -				"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");
> -		}
>   	}
>   
>   	list_add_tail_rcu(&ns->siblings, &head->list);
> @@ -3915,12 +3907,14 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, struct nvme_ns_info *info)
>   		sprintf(disk->disk_name, "nvme%dc%dn%d", ctrl->subsys->instance,
>   			ctrl->instance, ns->head->instance);
>   		disk->flags |= GENHD_FL_HIDDEN;
> -	} else if (multipath) {
> +	} else {
> +#ifdef CONFIG_NVME_MULTIPATH
>   		sprintf(disk->disk_name, "nvme%dn%d", ctrl->subsys->instance,
>   			ns->head->instance);
> -	} else {
> +#else
>   		sprintf(disk->disk_name, "nvme%dn%d", ctrl->instance,
>   			ns->head->instance);
> +#endif
>   	}
>   
>   	if (nvme_update_ns_info(ns, info))
> diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
> index a85d190942bd..28ab868182b2 100644
> --- a/drivers/nvme/host/multipath.c
> +++ b/drivers/nvme/host/multipath.c
> @@ -9,11 +9,6 @@
>   #include <trace/events/block.h>
>   #include "nvme.h"
>   
> -bool multipath = true;
> -module_param(multipath, bool, 0444);
> -MODULE_PARM_DESC(multipath,
> -	"turn on native support for multiple controllers per subsystem");
> -
>   static const char *nvme_iopolicy_names[] = {
>   	[NVME_IOPOLICY_NUMA]	= "numa",
>   	[NVME_IOPOLICY_RR]	= "round-robin",
> @@ -632,9 +627,11 @@ 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 flag
>   	 * could change after a rescan.
>   	 */
> +#ifdef CONFIG_NVME_MULTIPATH
>   	if (!(ctrl->subsys->cmic & NVME_CTRL_CMIC_MULTI_CTRL) ||
> -	    !nvme_is_unique_nsid(ctrl, head) || !multipath)
> +	    !nvme_is_unique_nsid(ctrl, head))
>   		return 0;
> +#endif
>   
>   	blk_set_stacking_limits(&lim);
>   	lim.dma_alignment = 3;
> @@ -1038,10 +1035,11 @@ int nvme_mpath_init_identify(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id)
>   	size_t ana_log_size;
>   	int error = 0;
>   
> +#ifdef CONFIG_NVME_MULTIPATH
>   	/* check if multipath is enabled and we have the capability */
> -	if (!multipath || !ctrl->subsys ||
> -	    !(ctrl->subsys->cmic & NVME_CTRL_CMIC_ANA))
> +	if (!ctrl->subsys || !(ctrl->subsys->cmic & NVME_CTRL_CMIC_ANA))
>   		return 0;
> +#endif
>   
>   	/* initialize this in the identify path to cover controller resets */
>   	atomic_set(&ctrl->nr_active, 0);
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index 2c76afd00390..6dea04f05b59 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -972,7 +972,6 @@ static inline void nvme_trace_bio_complete(struct request *req)
>   		trace_block_bio_complete(ns->head->disk->queue, req->bio);
>   }
>   
> -extern bool multipath;
>   extern struct device_attribute dev_attr_ana_grpid;
>   extern struct device_attribute dev_attr_ana_state;
>   extern struct device_attribute subsys_attr_iopolicy;
> @@ -982,7 +981,6 @@ static inline bool nvme_disk_is_ns_head(struct gendisk *disk)
>   	return disk->fops == &nvme_ns_head_ops;
>   }
>   #else
> -#define multipath false
>   static inline bool nvme_ctrl_use_ana(struct nvme_ctrl *ctrl)
>   {
>   	return false;

Powered by blists - more mailing lists