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

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

[Hannes Reinecke]

We should only remove the ns head from the list of heads per
subsystem if the reference count drops to zero. Removing it
at the start of nvme_ns_remove() will prevent us from reattaching
the namespace to the correct ns head once a path becomes available
again.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/nvme/host/core.c | 33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 762125f2905f..08ff0fc01f9e 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -546,6 +546,9 @@ static void nvme_free_ns_head(struct kref *ref)
 	struct nvme_ns_head *head =
 		container_of(ref, struct nvme_ns_head, ref);
 
+	mutex_lock(&head->subsys->lock);
+	list_del_init(&head->entry);
+	mutex_unlock(&head->subsys->lock);
 	nvme_mpath_remove_disk(head);
 	ida_simple_remove(&head->subsys->ns_ida, head->instance);
 	cleanup_srcu_struct(&head->srcu);
@@ -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 (!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);
 	mutex_unlock(&ns->ctrl->subsys->lock);
 
 	synchronize_rcu(); /* guarantee not available in head->list */
-- 
2.26.2


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

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

[Keith Busch]

On Mon, May 10, 2021 at 04:49:06PM +0200, Hannes Reinecke wrote:
> @@ -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);
>  	mutex_unlock(&ns->ctrl->subsys->lock);
>  
>  	synchronize_rcu(); /* guarantee not available in head->list */
> -- 
> 2.26.2

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

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

[Sagi Grimberg]

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

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

[Hannes Reinecke]

On 5/10/21 5:53 PM, Keith Busch wrote:
> On Mon, May 10, 2021 at 04:49:06PM +0200, Hannes Reinecke wrote:
>> @@ -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.
> 
Right. Will be fixing it up.

> The rest is similar to something I was working on too, though, so I
> think it's the right direction.
> 
> 
At long last, hope is on the horizon :-)

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

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

[Hannes Reinecke]

On 5/11/21 12:25 AM, Sagi Grimberg wrote:
> 
>>> @@ -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?
> 

But we _need_ to have the nshead available in the linked list if we ever 
want to re-attach a namespace after reconnecting the controller.
And we should be able to catch the 'reuse namespace' issue by checking 
the contents of the nsids field, which we keep attached to the nshead.

I know I need to fixup the patch for that, though.

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

The RAID issue _is_ solved with this patch.

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

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

[Sagi Grimberg]

>>>> @@ -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?
>>
> 
> But we _need_ to have the nshead available in the linked list if we ever 
> want to re-attach a namespace after reconnecting the controller.

Again, this is a queue_if_no_path functionality, this sequence is 
correct for that.

> And we should be able to catch the 'reuse namespace' issue by checking 
> the contents of the nsids field, which we keep attached to the nshead.

Well, you may be right, what I'm concerned about is that a new namespace
may be falsely identified as the same namespace because it is 
re-attaching to a lingering mounted mpath device.

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

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

[Keith Busch]

On Mon, May 10, 2021 at 03:25:04PM -0700, Sagi Grimberg wrote:
> 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?

Hm, you're considering something like if a user does namespace
management commands to delete one that is in-use (mounted, raid, etc),
then creates and attaches a new namespace that happens to be assigned
the same NSID.

The new one may get different EUI/NGUID values, so this approach here
would prevent the namespace from being usable without additional user
intervention. I think we'd want the driver to allocate an entirely new
head in that case if we could detect that's what happened. There is also
a possibility that the controller recycles the same NGUID for the new
namespace, and that could be confusing for the application holding the
open reference on the old namespace.

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

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

[Hannes Reinecke]

On 5/11/21 7:02 PM, Sagi Grimberg wrote:
> 
>>>>> @@ -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?
>>>
>>
>> But we _need_ to have the nshead available in the linked list if we 
>> ever want to re-attach a namespace after reconnecting the controller.
> 
> Again, this is a queue_if_no_path functionality, this sequence is 
> correct for that.
> 

We so need to have a face-to-face meeting for this will all concerned 
parties ...

Why do we need queue_if_no_path for this?

We do _not_ queue I/O in any shape or form; in fact, MD will register 
I/O errors on the failed nshead just fine. And my understanding for 
'queue_if_no_path' is that I/O will be held until the underlying paths 
gets reattached. But this is _not_ what this patch does.

This patch merely deals with the situation when a controller is 
reattached after an all-paths-removed scenario.
And as the nshead still _is_ present we should use it, precisely to 
avoid the situation where the NSID changes underneath us.

>> And we should be able to catch the 'reuse namespace' issue by checking 
>> the contents of the nsids field, which we keep attached to the nshead.
> 
> Well, you may be right, what I'm concerned about is that a new namespace
> may be falsely identified as the same namespace because it is 
> re-attaching to a lingering mounted mpath device.

Please check v3. Your concern should be addressed there.

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

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

[Hannes Reinecke]

On 5/11/21 11:37 PM, Keith Busch wrote:
> On Mon, May 10, 2021 at 03:25:04PM -0700, Sagi Grimberg wrote:
>> 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?
> 
> Hm, you're considering something like if a user does namespace
> management commands to delete one that is in-use (mounted, raid, etc),
> then creates and attaches a new namespace that happens to be assigned
> the same NSID.
> 
> The new one may get different EUI/NGUID values, so this approach here
> would prevent the namespace from being usable without additional user
> intervention. I think we'd want the driver to allocate an entirely new
> head in that case if we could detect that's what happened. There is also
> a possibility that the controller recycles the same NGUID for the new
> namespace, and that could be confusing for the application holding the
> open reference on the old namespace.
> 
... and, of course, none of this would happen if we would _not_ keep the
old nshead around. I'm still not sure _why_ this is considered
important. After all, things work perfectly when the CMIC bit is not
set; even device hot-removal and re-insertion (one of our IHVs tested
the crap out of that one, and I can confidently state the nvme stack did
not cause any issue there. Unlike other subsystems ...).

So I'm really curious which use-case we're trying to support with that;
EIO is EIO, and the application couldn't care less if it's generated due
to the nshead running out of paths or the nshead not existing...

Cheers,

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

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

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

[Sagi Grimberg]

>> On Mon, May 10, 2021 at 03:25:04PM -0700, Sagi Grimberg wrote:
>>> 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?
>>
>> Hm, you're considering something like if a user does namespace
>> management commands to delete one that is in-use (mounted, raid, etc),
>> then creates and attaches a new namespace that happens to be assigned
>> the same NSID.
>>
>> The new one may get different EUI/NGUID values, so this approach here
>> would prevent the namespace from being usable without additional user
>> intervention. I think we'd want the driver to allocate an entirely new
>> head in that case if we could detect that's what happened. There is also
>> a possibility that the controller recycles the same NGUID for the new
>> namespace, and that could be confusing for the application holding the
>> open reference on the old namespace.
>>
> ... and, of course, none of this would happen if we would _not_ keep the
> old nshead around. I'm still not sure _why_ this is considered
> important. After all, things work perfectly when the CMIC bit is not
> set; even device hot-removal and re-insertion (one of our IHVs tested
> the crap out of that one, and I can confidently state the nvme stack did
> not cause any issue there. Unlike other subsystems ...).
> 
> So I'm really curious which use-case we're trying to support with that;
> EIO is EIO, and the application couldn't care less if it's generated due
> to the nshead running out of paths or the nshead not existing...

I don't disagree with you Hannes, hence I think this should be the
default behavior. Christoph wants to see a queue_if_no_path behavior
that would be the existing functionality (although I don't think we
actually provide this type of functionality because we _do_ fail
the I/O as we don't have any paths).

So I'm on the camp of restoring the behavior that matches non-mpath
and then add queue_if_no_path properly as an opt-in.

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