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

[Kleber Souza]

On 08/23/18 07:20, Khaled Elmously wrote:
> to bionic/master-next
> 
> On 2018-08-21 16:22:21 , Khalid Elmously wrote:
>> From: Keith Busch <keith.busch at intel.com>
>>
>> BugLink: https://bugs.launchpad.net/bugs/1785334

I have amended this commit using the following public bug report:

BugLink: https://bugs.launchpad.net/bugs/1789227


Thanks,
Kleber

>>
>> 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
>>
>