linux-nvme.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] nvme: delete disk after last connection
@ 2021-03-31 14:53 Hannes Reinecke
  2021-03-31 14:53 ` [PATCH 1/2] nvme-mpath: " Hannes Reinecke
  2021-03-31 14:53 ` [PATCH 2/2] nvme: do not detach nshead when a namespace is removed Hannes Reinecke
  0 siblings, 2 replies; 10+ messages in thread
From: Hannes Reinecke @ 2021-03-31 14:53 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Sagi Grimberg, Keith Busch, linux-nvme, Hannes Reinecke

Hi all,

as there now is a consensus that we should destroy 'struct nshead'
when the last reference is dropped (and not once the last opener is released)
here now is the updated patchset to make it happen.
The first is the original patch from Keith, for correctly dropping the
disk reference.
The second one is for not removing struct nshead from the subsystem
lists once the last path is gone, but rather defer it until the struct
itself is freed.

As usual, comments and reviews are welcome.

Hannes Reinecke (1):
  nvme: do not detach nshead when a namespace is removed

Keith Busch (1):
  nvme-mpath: delete disk after last connection

 drivers/nvme/host/core.c      | 13 ++++++++-----
 drivers/nvme/host/multipath.c |  1 -
 drivers/nvme/host/nvme.h      |  2 +-
 3 files changed, 9 insertions(+), 7 deletions(-)

-- 
2.29.2


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

^ permalink raw reply	[flat|nested] 10+ messages in thread
* [PATCH 1/2] nvme-mpath: delete disk after last connection
  2021-03-31 14:53 [PATCH 0/2] nvme: delete disk after last connection Hannes Reinecke
@ 2021-03-31 14:53 ` Hannes Reinecke
  2021-03-31 23:40   ` Sagi Grimberg
  2021-04-01  8:25   ` Daniel Wagner
  2021-03-31 14:53 ` [PATCH 2/2] nvme: do not detach nshead when a namespace is removed Hannes Reinecke
  1 sibling, 2 replies; 10+ messages in thread
From: Hannes Reinecke @ 2021-03-31 14:53 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Sagi Grimberg, Keith Busch, linux-nvme, Keith Busch, Hannes Reinecke

From: Keith Busch <kbusch@kernel.org>

The multipath code currently deletes the disk only after all references
to it are dropped rather than when the last path to that disk is lost.
This has been reported to cause problems with some usage, like MD RAID.

Delete the disk when the last path is gone. This is the same behavior we
currently have with non-multipathed nvme devices.

The following is just a simple example that demonstrates what is currently
observed using a simple nvme loop back (loop setup file not shown):

 # nvmetcli restore loop.json
 [   31.156452] nvmet: adding nsid 1 to subsystem testnqn1
 [   31.159140] nvmet: adding nsid 1 to subsystem testnqn2

 # nvme connect -t loop -n testnqn1 -q hostnqn
 [   36.866302] nvmet: creating controller 1 for subsystem testnqn1 for NQN hostnqn.
 [   36.872926] nvme nvme3: new ctrl: "testnqn1"

 # nvme connect -t loop -n testnqn1 -q hostnqn
 [   38.227186] nvmet: creating controller 2 for subsystem testnqn1 for NQN hostnqn.
 [   38.234450] nvme nvme4: new ctrl: "testnqn1"

 # nvme connect -t loop -n testnqn2 -q hostnqn
 [   43.902761] nvmet: creating controller 3 for subsystem testnqn2 for NQN hostnqn.
 [   43.907401] nvme nvme5: new ctrl: "testnqn2"

 # nvme connect -t loop -n testnqn2 -q hostnqn
 [   44.627689] nvmet: creating controller 4 for subsystem testnqn2 for NQN hostnqn.
 [   44.641773] nvme nvme6: new ctrl: "testnqn2"

 # mdadm --create /dev/md0 --level=mirror --raid-devices=2 /dev/nvme3n1 /dev/nvme5n1
 [   53.497038] md/raid1:md0: active with 2 out of 2 mirrors
 [   53.501717] md0: detected capacity change from 0 to 66060288

 # cat /proc/mdstat
 Personalities : [raid1]
 md0 : active raid1 nvme5n1[1] nvme3n1[0]
       64512 blocks super 1.2 [2/2] [UU]

Now delete all paths to one of the namespaces:

 # echo 1 > /sys/class/nvme/nvme3/delete_controller
 # echo 1 > /sys/class/nvme/nvme4/delete_controller

We have no path, but mdstat says:

 # cat /proc/mdstat
 Personalities : [raid1]
 md0 : active (auto-read-only) raid1 nvme5n1[1]
       64512 blocks super 1.2 [2/1] [_U]

And this is reported to cause a problem.

With the proposed patch, the following messages appear:

 [  227.516807] md/raid1:md0: Disk failure on nvme3n1, disabling device.
 [  227.516807] md/raid1:md0: Operation continuing on 1 devices.

And mdstat shows only the viable members:

 # cat /proc/mdstat
 Personalities : [raid1]
 md0 : active (auto-read-only) raid1 nvme5n1[1]
       64512 blocks super 1.2 [2/1] [_U]

Reported-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Keith Busch <kbusch@kernel.org>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/nvme/host/core.c      | 5 ++++-
 drivers/nvme/host/multipath.c | 1 -
 drivers/nvme/host/nvme.h      | 2 +-
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 40215a0246e4..ee898c8da786 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -542,7 +542,10 @@ static void nvme_free_ns_head(struct kref *ref)
 	struct nvme_ns_head *head =
 		container_of(ref, struct nvme_ns_head, ref);
 
-	nvme_mpath_remove_disk(head);
+#ifdef CONFIG_NVME_MULTIPATH
+	if (head->disk)
+		put_disk(head->disk);
+#endif
 	ida_simple_remove(&head->subsys->ns_ida, head->instance);
 	cleanup_srcu_struct(&head->srcu);
 	nvme_put_subsystem(head->subsys);
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index a1d476e1ac02..adea060d1470 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -702,7 +702,6 @@ void nvme_mpath_remove_disk(struct nvme_ns_head *head)
 		 */
 		head->disk->queue = NULL;
 	}
-	put_disk(head->disk);
 }
 
 int nvme_mpath_init(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id)
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index b0863c59fac4..6b8992774998 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -680,7 +680,7 @@ static inline void nvme_mpath_check_last_path(struct nvme_ns *ns)
 	struct nvme_ns_head *head = ns->head;
 
 	if (head->disk && list_empty(&head->list))
-		kblockd_schedule_work(&head->requeue_work);
+		nvme_mpath_remove_disk(head);
 }
 
 static inline void nvme_trace_bio_complete(struct request *req)
-- 
2.29.2


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

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH 2/2] nvme: do not detach nshead when a namespace is removed
  2021-03-31 14:53 [PATCH 0/2] nvme: delete disk after last connection Hannes Reinecke
  2021-03-31 14:53 ` [PATCH 1/2] nvme-mpath: " Hannes Reinecke
@ 2021-03-31 14:53 ` Hannes Reinecke
  2021-03-31 23:49   ` Sagi Grimberg
  1 sibling, 1 reply; 10+ messages in thread
From: Hannes Reinecke @ 2021-03-31 14:53 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Sagi Grimberg, Keith Busch, linux-nvme, Hannes Reinecke

'struct nshead' and 'struct ns' have different lifetimes; the nshead
can (and occasionally will) stay around even if no namespaces are
connected, as it will only be finally removed once the last
reference is gone.
But while it is in this state we should still be able to access
it during lookup; a nshead with no controllers is perfectly valid
(at least for fabrics), and the namespace has no business with
removing the nshead from the subsystem lists.

This patch fixes the situation by only removing the nshead from the
subsystem list once it's being finally removed.

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

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index ee898c8da786..05fecbd33fa7 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -542,6 +542,10 @@ 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);
+
 #ifdef CONFIG_NVME_MULTIPATH
 	if (head->disk)
 		put_disk(head->disk);
@@ -3993,8 +3997,6 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid,
  out_unlink_ns:
 	mutex_lock(&ctrl->subsys->lock);
 	list_del_rcu(&ns->siblings);
-	if (list_empty(&ns->head->list))
-		list_del_init(&ns->head->entry);
 	mutex_unlock(&ctrl->subsys->lock);
 	nvme_put_ns_head(ns->head);
  out_free_queue:
@@ -4015,8 +4017,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.29.2


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

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] nvme-mpath: delete disk after last connection
  2021-03-31 14:53 ` [PATCH 1/2] nvme-mpath: " Hannes Reinecke
@ 2021-03-31 23:40   ` Sagi Grimberg
  2021-04-01  8:25   ` Daniel Wagner
  1 sibling, 0 replies; 10+ messages in thread
From: Sagi Grimberg @ 2021-03-31 23:40 UTC (permalink / raw)
  To: Hannes Reinecke, Christoph Hellwig; +Cc: Keith Busch, linux-nvme, Keith Busch



On 3/31/21 7:53 AM, Hannes Reinecke wrote:
> From: Keith Busch <kbusch@kernel.org>
> 
> The multipath code currently deletes the disk only after all references
> to it are dropped rather than when the last path to that disk is lost.
> This has been reported to cause problems with some usage, like MD RAID.
> 
> Delete the disk when the last path is gone. This is the same behavior we
> currently have with non-multipathed nvme devices.
> 
> The following is just a simple example that demonstrates what is currently
> observed using a simple nvme loop back (loop setup file not shown):
> 
>   # nvmetcli restore loop.json
>   [   31.156452] nvmet: adding nsid 1 to subsystem testnqn1
>   [   31.159140] nvmet: adding nsid 1 to subsystem testnqn2
> 
>   # nvme connect -t loop -n testnqn1 -q hostnqn
>   [   36.866302] nvmet: creating controller 1 for subsystem testnqn1 for NQN hostnqn.
>   [   36.872926] nvme nvme3: new ctrl: "testnqn1"
> 
>   # nvme connect -t loop -n testnqn1 -q hostnqn
>   [   38.227186] nvmet: creating controller 2 for subsystem testnqn1 for NQN hostnqn.
>   [   38.234450] nvme nvme4: new ctrl: "testnqn1"
> 
>   # nvme connect -t loop -n testnqn2 -q hostnqn
>   [   43.902761] nvmet: creating controller 3 for subsystem testnqn2 for NQN hostnqn.
>   [   43.907401] nvme nvme5: new ctrl: "testnqn2"
> 
>   # nvme connect -t loop -n testnqn2 -q hostnqn
>   [   44.627689] nvmet: creating controller 4 for subsystem testnqn2 for NQN hostnqn.
>   [   44.641773] nvme nvme6: new ctrl: "testnqn2"
> 
>   # mdadm --create /dev/md0 --level=mirror --raid-devices=2 /dev/nvme3n1 /dev/nvme5n1
>   [   53.497038] md/raid1:md0: active with 2 out of 2 mirrors
>   [   53.501717] md0: detected capacity change from 0 to 66060288
> 
>   # cat /proc/mdstat
>   Personalities : [raid1]
>   md0 : active raid1 nvme5n1[1] nvme3n1[0]
>         64512 blocks super 1.2 [2/2] [UU]
> 
> Now delete all paths to one of the namespaces:
> 
>   # echo 1 > /sys/class/nvme/nvme3/delete_controller
>   # echo 1 > /sys/class/nvme/nvme4/delete_controller
> 
> We have no path, but mdstat says:
> 
>   # cat /proc/mdstat
>   Personalities : [raid1]
>   md0 : active (auto-read-only) raid1 nvme5n1[1]
>         64512 blocks super 1.2 [2/1] [_U]
> 
> And this is reported to cause a problem.
> 
> With the proposed patch, the following messages appear:
> 
>   [  227.516807] md/raid1:md0: Disk failure on nvme3n1, disabling device.
>   [  227.516807] md/raid1:md0: Operation continuing on 1 devices.
> 
> And mdstat shows only the viable members:
> 
>   # cat /proc/mdstat
>   Personalities : [raid1]
>   md0 : active (auto-read-only) raid1 nvme5n1[1]
>         64512 blocks super 1.2 [2/1] [_U]
> 
> Reported-by: Hannes Reinecke <hare@suse.de>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>   drivers/nvme/host/core.c      | 5 ++++-
>   drivers/nvme/host/multipath.c | 1 -
>   drivers/nvme/host/nvme.h      | 2 +-
>   3 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 40215a0246e4..ee898c8da786 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -542,7 +542,10 @@ static void nvme_free_ns_head(struct kref *ref)
>   	struct nvme_ns_head *head =
>   		container_of(ref, struct nvme_ns_head, ref);
>   
> -	nvme_mpath_remove_disk(head);
> +#ifdef CONFIG_NVME_MULTIPATH
> +	if (head->disk)
> +		put_disk(head->disk);
> +#endif

Hannes, maybe make that nvme_mpath_put_disk(head) and place an empty
stab in nvme.h like we do for other mpath variants?

Other than that, patch looks good, so you can add to the respin
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

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

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] nvme: do not detach nshead when a namespace is removed
  2021-03-31 14:53 ` [PATCH 2/2] nvme: do not detach nshead when a namespace is removed Hannes Reinecke
@ 2021-03-31 23:49   ` Sagi Grimberg
  0 siblings, 0 replies; 10+ messages in thread
From: Sagi Grimberg @ 2021-03-31 23:49 UTC (permalink / raw)
  To: Hannes Reinecke, Christoph Hellwig; +Cc: Keith Busch, linux-nvme


> 'struct nshead' and 'struct ns' have different lifetimes; the nshead
> can (and occasionally will) stay around even if no namespaces are
> connected, as it will only be finally removed once the last
> reference is gone.
> But while it is in this state we should still be able to access
> it during lookup; a nshead with no controllers is perfectly valid
> (at least for fabrics), and the namespace has no business with
> removing the nshead from the subsystem lists.
> 
> This patch fixes the situation by only removing the nshead from the
> subsystem list once it's being finally removed.

Hannes, please explain what is the bug you are solving here.

I actually think that the nshead should be removed from the
list as soon as the last the head->list is empty.

What if for example a new ns is allocated on the subsystem
and the nsid (which was already deleted) is reused, it will now
be rejected when we match against existing nsheads which will
contain this stale nshead that happens to have an open reference.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] nvme-mpath: delete disk after last connection
  2021-03-31 14:53 ` [PATCH 1/2] nvme-mpath: " Hannes Reinecke
  2021-03-31 23:40   ` Sagi Grimberg
@ 2021-04-01  8:25   ` Daniel Wagner
  1 sibling, 0 replies; 10+ messages in thread
From: Daniel Wagner @ 2021-04-01  8:25 UTC (permalink / raw)
  To: Hannes Reinecke
  Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, linux-nvme, Keith Busch

On Wed, Mar 31, 2021 at 04:53:50PM +0200, Hannes Reinecke wrote:
> Reported-by: Hannes Reinecke <hare@suse.de>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> Signed-off-by: Hannes Reinecke <hare@suse.de>

Reviewed-by: Daniel Wagner <dwagner@suse.de>

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

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] nvme-mpath: delete disk after last connection
  2020-10-05 12:50   ` Christoph Hellwig
@ 2021-03-05 20:06     ` Sagi Grimberg
  0 siblings, 0 replies; 10+ messages in thread
From: Sagi Grimberg @ 2021-03-05 20:06 UTC (permalink / raw)
  To: Christoph Hellwig, Hannes Reinecke; +Cc: Keith Busch, linux-nvme, Keith Busch


>> -	nvme_mpath_remove_disk(head);
>> +#ifdef CONFIG_NVME_MULTIPATH
>> +	if (head->disk)
>> +		put_disk(head->disk);
>> +#endif
> 
> Please move this into a helper in multipath.c with a stub for the
> non-multipath case.

Other than this comment, it looks good to me.

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

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

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] nvme-mpath: delete disk after last connection
  2020-10-05 12:44 ` [PATCH 1/2] nvme-mpath: delete disk after last connection Hannes Reinecke
  2020-10-05 12:50   ` Christoph Hellwig
@ 2021-03-04 14:34   ` Daniel Wagner
  1 sibling, 0 replies; 10+ messages in thread
From: Daniel Wagner @ 2021-03-04 14:34 UTC (permalink / raw)
  To: Hannes Reinecke
  Cc: Christoph Hellwig, Keith Busch, linux-nvme, Sagi Grimberg, Keith Busch

On Mon, Oct 05, 2020 at 02:44:59PM +0200, Hannes Reinecke wrote:
>  # cat /proc/mdstat
>  Personalities : [raid1]
>  md0 : active raid1 nvme5n1[1] nvme3n1[0]
>        64512 blocks super 1.2 [2/2] [UU]
> 
> Now delete all paths to one of the namespaces:
> 
>  # echo 1 > /sys/class/nvme/nvme3/delete_controller
>  # echo 1 > /sys/class/nvme/nvme4/delete_controller
> 
> We have no path, but mdstat says:
> 
>  # cat /proc/mdstat
>  Personalities : [raid1]
>  md0 : active (auto-read-only) raid1 nvme5n1[1]
>        64512 blocks super 1.2 [2/1] [_U]

Shouldn't there be nvme3n1 still listed?


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

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] nvme-mpath: delete disk after last connection
  2020-10-05 12:44 ` [PATCH 1/2] nvme-mpath: delete disk after last connection Hannes Reinecke
@ 2020-10-05 12:50   ` Christoph Hellwig
  2021-03-05 20:06     ` Sagi Grimberg
  2021-03-04 14:34   ` Daniel Wagner
  1 sibling, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2020-10-05 12:50 UTC (permalink / raw)
  To: Hannes Reinecke
  Cc: Keith Busch, linux-nvme, Christoph Hellwig, Keith Busch, Sagi Grimberg

> -	nvme_mpath_remove_disk(head);
> +#ifdef CONFIG_NVME_MULTIPATH
> +	if (head->disk)
> +		put_disk(head->disk);
> +#endif

Please move this into a helper in multipath.c with a stub for the
non-multipath case.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread
* [PATCH 1/2] nvme-mpath: delete disk after last connection
  2020-10-05 12:44 [RFC PATCHv3 0/2] nvme: queue_if_no_path functionality Hannes Reinecke
@ 2020-10-05 12:44 ` Hannes Reinecke
  2020-10-05 12:50   ` Christoph Hellwig
  2021-03-04 14:34   ` Daniel Wagner
  0 siblings, 2 replies; 10+ messages in thread
From: Hannes Reinecke @ 2020-10-05 12:44 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Keith Busch, linux-nvme, Sagi Grimberg, Keith Busch

From: Keith Busch <kbusch@kernel.org>

The multipath code currently deletes the disk only after all references
to it are dropped rather than when the last path to that disk is lost.
This has been reported to cause problems with some usage, like MD RAID.

Delete the disk when the last path is gone. This is the same behavior we
currently have with non-multipathed nvme devices.

The following is just a simple example that demonstrates what is currently
observed using a simple nvme loop back (loop setup file not shown):

 # nvmetcli restore loop.json
 [   31.156452] nvmet: adding nsid 1 to subsystem testnqn1
 [   31.159140] nvmet: adding nsid 1 to subsystem testnqn2

 # nvme connect -t loop -n testnqn1 -q hostnqn
 [   36.866302] nvmet: creating controller 1 for subsystem testnqn1 for NQN hostnqn.
 [   36.872926] nvme nvme3: new ctrl: "testnqn1"

 # nvme connect -t loop -n testnqn1 -q hostnqn
 [   38.227186] nvmet: creating controller 2 for subsystem testnqn1 for NQN hostnqn.
 [   38.234450] nvme nvme4: new ctrl: "testnqn1"

 # nvme connect -t loop -n testnqn2 -q hostnqn
 [   43.902761] nvmet: creating controller 3 for subsystem testnqn2 for NQN hostnqn.
 [   43.907401] nvme nvme5: new ctrl: "testnqn2"

 # nvme connect -t loop -n testnqn2 -q hostnqn
 [   44.627689] nvmet: creating controller 4 for subsystem testnqn2 for NQN hostnqn.
 [   44.641773] nvme nvme6: new ctrl: "testnqn2"

 # mdadm --create /dev/md0 --level=mirror --raid-devices=2 /dev/nvme3n1 /dev/nvme5n1
 [   53.497038] md/raid1:md0: active with 2 out of 2 mirrors
 [   53.501717] md0: detected capacity change from 0 to 66060288

 # cat /proc/mdstat
 Personalities : [raid1]
 md0 : active raid1 nvme5n1[1] nvme3n1[0]
       64512 blocks super 1.2 [2/2] [UU]

Now delete all paths to one of the namespaces:

 # echo 1 > /sys/class/nvme/nvme3/delete_controller
 # echo 1 > /sys/class/nvme/nvme4/delete_controller

We have no path, but mdstat says:

 # cat /proc/mdstat
 Personalities : [raid1]
 md0 : active (auto-read-only) raid1 nvme5n1[1]
       64512 blocks super 1.2 [2/1] [_U]

And this is reported to cause a problem.

With the proposed patch, the following messages appear:

 [  227.516807] md/raid1:md0: Disk failure on nvme3n1, disabling device.
 [  227.516807] md/raid1:md0: Operation continuing on 1 devices.

And mdstat shows only the viable members:

 # cat /proc/mdstat
 Personalities : [raid1]
 md0 : active (auto-read-only) raid1 nvme5n1[1]
       64512 blocks super 1.2 [2/1] [_U]

Reported-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
 drivers/nvme/host/core.c      | 5 ++++-
 drivers/nvme/host/multipath.c | 1 -
 drivers/nvme/host/nvme.h      | 2 +-
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 385b10317873..4459a40b057c 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -474,7 +474,10 @@ static void nvme_free_ns_head(struct kref *ref)
 	struct nvme_ns_head *head =
 		container_of(ref, struct nvme_ns_head, ref);
 
-	nvme_mpath_remove_disk(head);
+#ifdef CONFIG_NVME_MULTIPATH
+	if (head->disk)
+		put_disk(head->disk);
+#endif
 	ida_simple_remove(&head->subsys->ns_ida, head->instance);
 	cleanup_srcu_struct(&head->srcu);
 	nvme_put_subsystem(head->subsys);
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 74896be40c17..55045291b4de 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -697,7 +697,6 @@ void nvme_mpath_remove_disk(struct nvme_ns_head *head)
 		 */
 		head->disk->queue = NULL;
 	}
-	put_disk(head->disk);
 }
 
 int nvme_mpath_init(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id)
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 566776100126..b6180bb3361d 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -670,7 +670,7 @@ static inline void nvme_mpath_check_last_path(struct nvme_ns *ns)
 	struct nvme_ns_head *head = ns->head;
 
 	if (head->disk && list_empty(&head->list))
-		kblockd_schedule_work(&head->requeue_work);
+		nvme_mpath_remove_disk(head);
 }
 
 static inline void nvme_trace_bio_complete(struct request *req,
-- 
2.16.4


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

^ permalink raw reply related	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-04-01  8:25 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-31 14:53 [PATCH 0/2] nvme: delete disk after last connection Hannes Reinecke
2021-03-31 14:53 ` [PATCH 1/2] nvme-mpath: " Hannes Reinecke
2021-03-31 23:40   ` Sagi Grimberg
2021-04-01  8:25   ` Daniel Wagner
2021-03-31 14:53 ` [PATCH 2/2] nvme: do not detach nshead when a namespace is removed Hannes Reinecke
2021-03-31 23:49   ` Sagi Grimberg
  -- strict thread matches above, loose matches on Subject: below --
2020-10-05 12:44 [RFC PATCHv3 0/2] nvme: queue_if_no_path functionality Hannes Reinecke
2020-10-05 12:44 ` [PATCH 1/2] nvme-mpath: delete disk after last connection Hannes Reinecke
2020-10-05 12:50   ` Christoph Hellwig
2021-03-05 20:06     ` Sagi Grimberg
2021-03-04 14:34   ` Daniel Wagner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).