ACK: [SRU Bionic][PATCH 1/1] nvme/multipath: Fix multipath disabled naming collisions

[Kamal Mostafa]

Acked-by: Kamal Mostafa <kamal at canonical.com>

On Tue, Aug 21, 2018 at 04:22:21PM -0400, Khalid Elmously wrote:
> From: Keith Busch <keith.busch at intel.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1785334
> 
> When CONFIG_NVME_MULTIPATH is set, but we're not using nvme to multipath,
> namespaces with multiple paths were not creating unique names due to
> reusing the same instance number from the namespace's head.
> 
> This patch fixes this by falling back to the non-multipath naming method
> when the parameter disabled using multipath.
> 
> Reported-by: Mike Snitzer <snitzer at redhat.com>
> Signed-off-by: Keith Busch <keith.busch at intel.com>
> Reviewed-by: Christoph Hellwig <hch at lst.de>
> Signed-off-by: Jens Axboe <axboe at kernel.dk>
> (cherry-picked from a785dbccd95c37606c720580714f5a7a8b3255f1)
> Signed-off-by: Khalid Elmously <khalid.elmously at canonical.com>
> ---
>  drivers/nvme/host/core.c      | 26 +-------------------------
>  drivers/nvme/host/multipath.c | 22 ++++++++++++++++++++++
>  drivers/nvme/host/nvme.h      | 12 ++++++++++++
>  3 files changed, 35 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index a7662186c88e..0230b192540c 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -2902,31 +2902,7 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid)
>  	if (nvme_init_ns_head(ns, nsid, id, &new))
>  		goto out_free_id;
>  	nvme_setup_streams_ns(ctrl, ns);
> -	
> -#ifdef CONFIG_NVME_MULTIPATH
> -	/*
> -	 * If multipathing is enabled we need to always use the subsystem
> -	 * instance number for numbering our devices to avoid conflicts
> -	 * between subsystems that have multiple controllers and thus use
> -	 * the multipath-aware subsystem node and those that have a single
> -	 * controller and use the controller node directly.
> -	 */
> -	if (ns->head->disk) {
> -		sprintf(disk_name, "nvme%dc%dn%d", ctrl->subsys->instance,
> -				ctrl->cntlid, ns->head->instance);
> -		flags = GENHD_FL_HIDDEN;
> -	} else {
> -		sprintf(disk_name, "nvme%dn%d", ctrl->subsys->instance,
> -				ns->head->instance);
> -	}
> -#else
> -	/*
> -	 * But without the multipath code enabled, multiple controller per
> -	 * subsystems are visible as devices and thus we cannot use the
> -	 * subsystem instance.
> -	 */
> -	sprintf(disk_name, "nvme%dn%d", ctrl->instance, ns->head->instance);
> -#endif
> +	nvme_set_disk_name(disk_name, ns, ctrl, &flags);
>  
>  	if ((ctrl->quirks & NVME_QUIRK_LIGHTNVM) && id->vs[0] == 0x1) {
>  		if (nvme_nvm_register(ns, disk_name, node)) {
> diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
> index cf16905d25e2..3f3143205071 100644
> --- a/drivers/nvme/host/multipath.c
> +++ b/drivers/nvme/host/multipath.c
> @@ -19,6 +19,28 @@ module_param(multipath, bool, 0644);
>  MODULE_PARM_DESC(multipath,
>  	"turn on native support for multiple controllers per subsystem");
>  
> +/*
> + * If multipathing is enabled we need to always use the subsystem instance
> + * number for numbering our devices to avoid conflicts between subsystems that
> + * have multiple controllers and thus use the multipath-aware subsystem node
> + * and those that have a single controller and use the controller node
> + * directly.
> + */
> +void nvme_set_disk_name(char *disk_name, struct nvme_ns *ns,
> +			struct nvme_ctrl *ctrl, int *flags)
> +{
> +	if (!multipath) {
> +		sprintf(disk_name, "nvme%dn%d", ctrl->instance, ns->head->instance);
> +	} else if (ns->head->disk) {
> +		sprintf(disk_name, "nvme%dc%dn%d", ctrl->subsys->instance,
> +				ctrl->cntlid, ns->head->instance);
> +		*flags = GENHD_FL_HIDDEN;
> +	} else {
> +		sprintf(disk_name, "nvme%dn%d", ctrl->subsys->instance,
> +				ns->head->instance);
> +	}
> +}
> +
>  void nvme_failover_req(struct request *req)
>  {
>  	struct nvme_ns *ns = req->q->queuedata;
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index f82365cc99fd..a9ec71cce3cc 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -405,6 +405,8 @@ extern const struct attribute_group nvme_ns_id_attr_group;
>  extern const struct block_device_operations nvme_ns_head_ops;
>  
>  #ifdef CONFIG_NVME_MULTIPATH
> +void nvme_set_disk_name(char *disk_name, struct nvme_ns *ns,
> +			struct nvme_ctrl *ctrl, int *flags);
>  void nvme_failover_req(struct request *req);
>  bool nvme_req_needs_failover(struct request *req);
>  void nvme_kick_requeue_lists(struct nvme_ctrl *ctrl);
> @@ -430,6 +432,16 @@ static inline void nvme_mpath_check_last_path(struct nvme_ns *ns)
>  }
>  
>  #else
> +/*
> + * Without the multipath code enabled, multiple controller per subsystems are
> + * visible as devices and thus we cannot use the subsystem instance.
> + */
> +static inline void nvme_set_disk_name(char *disk_name, struct nvme_ns *ns,
> +				      struct nvme_ctrl *ctrl, int *flags)
> +{
> +	sprintf(disk_name, "nvme%dn%d", ctrl->instance, ns->head->instance);
> +}
> +
>  static inline void nvme_failover_req(struct request *req)
>  {
>  }
> -- 
> 2.17.1
> 
> 
> -- 
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team