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

[Sagi Grimberg <sagi@grimberg.me>]

>> @@ -3605,16 +3608,26 @@ static int nvme_init_ns_head(struct nvme_ns *ns, unsigned nsid,
>>   		head->shared = is_shared;
>>   	} else {
>>   		ret = -EINVAL;
>> -		if (!is_shared || !head->shared) {
>> -			dev_err(ctrl->device,
>> -				"Duplicate unshared namespace %d\n", nsid);
>> -			goto out_put_ns_head;
>> -		}
>> -		if (!nvme_ns_ids_equal(&head->ids, ids)) {
>> -			dev_err(ctrl->device,
>> -				"IDs don't match for shared namespace %d\n",
>> +		/*
>> +		 * 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)) {
>> +			if (!is_shared || !head->shared) {
>> +				dev_err(ctrl->device,
>> +					"Duplicate unshared namespace %d\n", nsid);
>> +				goto out_put_ns_head;
>> +			}
> 
> If not multipath, then it is not shared. The above will fail attaching
> single-path namespaces to a known head.
> 
> The rest is similar to something I was working on too, though, so I
> think it's the right direction.
> 
> 
>> +			if (!nvme_ns_ids_equal(&head->ids, ids)) {
>> +				dev_err(ctrl->device,
>> +					"IDs don't match for shared namespace %d\n",
>>   					nsid);
>> -			goto out_put_ns_head;
>> +				goto out_put_ns_head;
>> +			}
>> +		} else {
>> +			/* But the ids might have changed, so reset them */
>> +			head->ids = *ids;
>>   		}
>>   	}
>>   
>> @@ -3764,8 +3777,6 @@ static void nvme_ns_remove(struct nvme_ns *ns)
>>   
>>   	mutex_lock(&ns->ctrl->subsys->lock);
>>   	list_del_rcu(&ns->siblings);
>> -	if (list_empty(&ns->head->list))
>> -		list_del_init(&ns->head->entry);

Hannes, you sent a patch like this before, my comment was that a nshead
should be removed before the final reference (which who knows when it
will ever arrive) as if a nsid were to be reused by the controller for
a different namespace then we'd reject it so I'm not sure how this helps?

I think that for the raid issue we should solve that one directly, this
patch may have other implications in my mind...

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