Re: [PATCHv3] nvme: allow to re-attach namespaces after all paths are down

From: Hannes Reinecke <hare@suse.de>
To: Christoph Hellwig <hch@lst.de>
Cc: Sagi Grimberg <sagi@grimberg.me>,
	Keith Busch <keith.busch@wdc.com>,
	linux-nvme@lists.infradead.org
Subject: Re: [PATCHv3] nvme: allow to re-attach namespaces after all paths are down
Date: Wed, 12 May 2021 07:28:13 +0200	[thread overview]
Message-ID: <45ac2c89-7812-a2a2-07dc-46abd6fd555a@suse.de> (raw)
In-Reply-To: <20210511162611.GA17617@lst.de>

On 5/11/21 6:26 PM, Christoph Hellwig wrote:
>>   	} else {
>>   		ret = -EINVAL;
>> +		/*
>> +		 * If multipath is enabled we might hit an ns head with no
>> +		 * paths, but that doesn't indicate it's a shared namespace.
>> +		 */
>> +		if (nvme_ns_head_multipath(head) &&
>> +		    list_empty(&head->list))
>> +			goto check_ids;
>>   		if (!is_shared || !head->shared) {
>>   			dev_err(ctrl->device,
>>   				"Duplicate unshared namespace %d\n", nsid);
>>   			goto out_put_ns_head;
>>   		}
>> +check_ids:
> 
> I think this should simply be:
> 
> 		if (!list_empty(&head->list) && (!is_shared || !head->shared)) {
>   			dev_err(ctrl->device,
>    				"Duplicate unshared namespace %d\n", nsid);
>    			goto out_put_ns_head;
>    		}
> 
I wasn't sure if this isn't just for multipathing; other setups can't 
queue I/O, so we'll be hitting this scenario only in the unlikely case 
that the device is removed _and re-attached_ without any I/O pending.
Hence the check.
But sure, I can modify it.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme