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

[Hannes Reinecke <hare@suse.de>]

On 6/8/21 6:42 PM, Christoph Hellwig wrote:
> While I do like the look, I don't see how this could actually work:
> 
>>   	} else {
>>   		ret = -EINVAL;
>> -		if (!is_shared || !head->shared) {
>> +		/*
>> +		 * If multipath is enabled we might hit an ns head with no
>> +		 * paths, but that doesn't indicate it's a shared namespace.
>> +		 */
>> +		if (!list_empty(&head->list) && (!is_shared || !head->shared)) {
>>   			dev_err(ctrl->device,
>>   				"Duplicate unshared namespace %d\n", nsid);
>>   			goto out_put_ns_head;
> 
> When we get here with an existing ns_head with no namespaces attached
> to it, we can assume it once had namespaces attached to it.  Because
> of that NVME_NSHEAD_DISK_LIVE will be set, and we'll never add the
> disk for it again.  Even if we did add the disk again what prevents
> adding a disk and removing one from racing with each other?
> 
Hmm. I thought it was resolved by refcount magic.
Will have a closer look to check what needs to be done.

Thanks for the review.

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