[PATCH RFC] nvme-multipath: don't retry adding head disk

[PATCH RFC] nvme-multipath: don't retry adding head disk

[John Garry]

From: John Garry <john.garry@linux.dev>

If an attempt to add the head disk in nvme_mpath_set_live() ->
device_add_disk() fails, then flag NVME_NSHEAD_DISK_LIVE is cleared and
we bail out.

However, we may later call nvme_mpath_set_live() -> device_add_disk() again
for another NS or from ANA updates for the same NS. This is broken, as we
should not retry adding the disk - it breaks the driver model.

Add a flag NVME_NSHEAD_DISK_BROKEN to stop this happening.

Signed-off-by: John Garry <john.g.garry@oracle.com>
---
I'm not happy with this solution, as we have a DOA disk and it would be
better to remove the NSes in this case. OTOH, this device_add_disk()
failure is very unlikely to happen, so we should not add a complex
solution to handle it.

diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 9b9a657fa330f..2ffc11bc08440 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -789,6 +789,12 @@ static void nvme_mpath_set_live(struct nvme_ns *ns)
 	if (!head->disk)
 		return;
 
+	mutex_lock(&head->lock);
+	if (test_bit(NVME_NSHEAD_DISK_BROKEN, &head->flags)) {
+		mutex_unlock(&head->lock);
+		return;
+	}
+
 	/*
 	 * test_and_set_bit() is used because it is protecting against two nvme
 	 * paths simultaneously calling device_add_disk() on the same namespace
@@ -798,12 +804,17 @@ static void nvme_mpath_set_live(struct nvme_ns *ns)
 		rc = device_add_disk(&head->subsys->dev, head->disk,
 				     nvme_ns_attr_groups);
 		if (rc) {
+			dev_err(disk_to_dev(ns->disk),
+				"Unable to add multipath disk\n");
+			set_bit(NVME_NSHEAD_DISK_BROKEN, &head->flags);
 			clear_bit(NVME_NSHEAD_DISK_LIVE, &head->flags);
+			mutex_unlock(&head->lock);
 			return;
 		}
 		nvme_add_ns_head_cdev(head);
 		queue_work(nvme_wq, &head->partition_scan_work);
 	}
+	mutex_unlock(&head->lock);
 
 	nvme_mpath_add_sysfs_link(ns->head);
 
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 824651cc898db..6e3c0ce09cadd 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -574,6 +574,7 @@ struct nvme_ns_head {
 #define NVME_NSHEAD_DISK_LIVE		0
 #define NVME_NSHEAD_QUEUE_IF_NO_PATH	1
 #define NVME_NSHEAD_CDEV_LIVE		2
+#define NVME_NSHEAD_DISK_BROKEN		3
 	struct nvme_ns __rcu	*current_path[];
 #endif
 };
-- 
2.43.7

Re: [PATCH RFC] nvme-multipath: don't retry adding head disk

[Christoph Hellwig]

On Wed, Jul 29, 2026 at 06:22:35PM +0000, John Garry wrote:
> From: John Garry <john.garry@linux.dev>
> 
> If an attempt to add the head disk in nvme_mpath_set_live() ->
> device_add_disk() fails, then flag NVME_NSHEAD_DISK_LIVE is cleared and
> we bail out.
> 
> However, we may later call nvme_mpath_set_live() -> device_add_disk() again
> for another NS or from ANA updates for the same NS. This is broken, as we
> should not retry adding the disk - it breaks the driver model.
> 
> Add a flag NVME_NSHEAD_DISK_BROKEN to stop this happening.
> 
> Signed-off-by: John Garry <john.g.garry@oracle.com>
> ---
> I'm not happy with this solution, as we have a DOA disk and it would be
> better to remove the NSes in this case. OTOH, this device_add_disk()
> failure is very unlikely to happen, so we should not add a complex
> solution to handle it.

Yeah.  Broken is a bit weird of a name for something that would
either be a logic bug or a really messed up kernel, though.

I guess you arrive there by error injection?

Re: [PATCH RFC] nvme-multipath: don't retry adding head disk

[John Garry]

On 30/07/2026 12:40, Christoph Hellwig wrote:
> On Wed, Jul 29, 2026 at 06:22:35PM +0000, John Garry wrote:
>> From: John Garry <john.garry@linux.dev>
>> 
>> If an attempt to add the head disk in nvme_mpath_set_live() ->
>> device_add_disk() fails, then flag NVME_NSHEAD_DISK_LIVE is cleared and
>> we bail out.
>> 
>> However, we may later call nvme_mpath_set_live() -> device_add_disk() again
>> for another NS or from ANA updates for the same NS. This is broken, as we
>> should not retry adding the disk - it breaks the driver model.
>> 
>> Add a flag NVME_NSHEAD_DISK_BROKEN to stop this happening.
>> 
>> Signed-off-by: John Garry <john.g.garry@oracle.com>
>> ---
>> I'm not happy with this solution, as we have a DOA disk and it would be
>> better to remove the NSes in this case. OTOH, this device_add_disk()
>> failure is very unlikely to happen, so we should not add a complex
>> solution to handle it.
> 
> Yeah.  Broken is a bit weird of a name for something that would
> either be a logic bug or a really messed up kernel, though.

NVME_NSHEAD_DISK_FAILED or NVME_NSHEAD_DISK_FAILED_ADD

> 
> I guess you arrive there by error injection?

I just bodged an error in device_add_disk(). btw, sashiko notified me 
about this issue elsewhere.