mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v3] nvme-multipath: add fail_if_no_path sysfs attribute
@ 2026-09-23  0:49 Krishna Iyer
  2026-09-23 12:06 ` Hannes Reinecke
  2026-09-23 21:48 ` Keith Busch
  0 siblings, 2 replies; 6+ messages in thread
From: Krishna Iyer @ 2026-09-23  0:49 UTC (permalink / raw)
  To: kbusch, axboe, hch, sagi
  Cc: linux-nvme, linux-kernel, nilay, sjpark, saravanand, Krishna Iyer

When no usable path exists, I/O on a multipath namespace is queued
until a path returns. With ctrl_loss_tmo=-1 that can be forever:
during a long fabric outage any process waiting on the I/O is stuck in
D state. We hit this on virtualization hosts, where a SIGKILLed VM
process cannot exit while draining I/O to an unreachable NVMe/TCP
target.

Nothing can fail this I/O without tearing something down: controller
deletion takes every namespace on the controller with it.

Add a fail_if_no_path attribute on the ns-head disk: a persistent
per-namespace policy to fail parked and newly arriving I/O instead of
queueing it when no usable path exists. It is enforced where a path is
known to be unusable: CONNECTING controllers and LIVE controllers with
the path ANA inaccessible or persistent-loss stop counting as
available, RESETTING and ANA change keep queueing, and with no
controllers left the policy overrides the delayed_removal_secs
queueing window. Controller state is untouched and reconnects
continue. Like dm's fail_if_no_path, the policy is transport agnostic.

Assisted-by: Claude:claude-fable-5
Signed-off-by: Krishna Iyer <kiyer@crusoe.ai>
---
Changes since v2 [1]:
- use the nvme_state_is_live() helper for the ANA state check, keeping
  the explicit NVME_ANA_CHANGE carve-out so transient ANA transitions
  still queue (Nilay)
- return early when the stored value matches the current setting,
  skipping synchronize_srcu() and the requeue kick (Nilay)

Validated on hardware with a 6.17 backport of this v3: parked I/O on a
SIGKILLed VM process failed within a second of enabling the policy and
the process was reaped, new I/O failed fast during the outage, the
policy persisted across path recovery, and disabling it restored
queueing.

[1] https://lore.kernel.org/linux-nvme/20260917231647.79956-1-kiyer@crusoe.ai/
 Documentation/ABI/stable/sysfs-nvme | 13 +++++
 drivers/nvme/host/multipath.c       | 76 ++++++++++++++++++++++++++---
 drivers/nvme/host/nvme.h            |  2 +
 drivers/nvme/host/sysfs.c           |  4 +-
 4 files changed, 88 insertions(+), 7 deletions(-)

diff --git a/Documentation/ABI/stable/sysfs-nvme b/Documentation/ABI/stable/sysfs-nvme
index a2f5d0710db4..57a827235995 100644
--- a/Documentation/ABI/stable/sysfs-nvme
+++ b/Documentation/ABI/stable/sysfs-nvme
@@ -337,6 +337,19 @@ Description:
 		is deferred. Only visible on multipath head devices.
 		Requires CONFIG_NVME_MULTIPATH.
 
+What:		/sys/block/nvmeXnY/fail_if_no_path
+Date:		September 2026
+KernelVersion:	7.3
+Contact:	Krishna Iyer <kiyer@crusoe.ai>
+Description:
+		Shows or sets the fail-if-no-path policy of the multipath
+		head device ("on" or "off", default "off"). When on, I/O
+		queued or arriving while no usable path exists is failed
+		immediately instead of being queued, including during the
+		delayed_removal_secs window. Reconnect attempts are not
+		affected. Only visible on multipath head devices.
+		Requires CONFIG_NVME_MULTIPATH.
+
 What:		/sys/block/nvmeXnY/csi
 What:		/sys/block/nvmeXnY/metadata_bytes
 What:		/sys/block/nvmeXnY/nuse
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 99ecc73393e5..3736ba709b6c 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -496,9 +496,16 @@ inline struct nvme_ns *nvme_find_path(struct nvme_ns_head *head)
 	}
 }
 
+static inline bool nvme_state_is_live(enum nvme_ana_state state)
+{
+	return state == NVME_ANA_OPTIMIZED || state == NVME_ANA_NONOPTIMIZED;
+}
+
 static bool nvme_available_path(struct nvme_ns_head *head)
 	__must_hold_shared(&head->srcu)
 {
+	bool fail_if_no_path = test_bit(NVME_NSHEAD_FAIL_IF_NO_PATH,
+					&head->flags);
 	struct nvme_ns *ns;
 
 	if (!test_bit(NVME_NSHEAD_DISK_LIVE, &head->flags))
@@ -510,14 +517,29 @@ static bool nvme_available_path(struct nvme_ns_head *head)
 			continue;
 		switch (nvme_ctrl_state(ns->ctrl)) {
 		case NVME_CTRL_LIVE:
+			/*
+			 * ANA change is transient and bounded by ANATT, so
+			 * keep queueing while it resolves.
+			 */
+			if (fail_if_no_path &&
+			    !nvme_state_is_live(ns->ana_state) &&
+			    ns->ana_state != NVME_ANA_CHANGE)
+				continue;
+			return true;
 		case NVME_CTRL_RESETTING:
-		case NVME_CTRL_CONNECTING:
 			return true;
+		case NVME_CTRL_CONNECTING:
+			if (!fail_if_no_path)
+				return true;
+			continue;
 		default:
 			break;
 		}
 	}
 
+	if (fail_if_no_path)
+		return false;
+
 	/*
 	 * If "head->delayed_removal_secs" is configured (i.e., non-zero), do
 	 * not immediately fail I/O. Instead, requeue the I/O for the configured
@@ -871,11 +893,6 @@ static int nvme_parse_ana_log(struct nvme_ctrl *ctrl, void *data,
 	return 0;
 }
 
-static inline bool nvme_state_is_live(enum nvme_ana_state state)
-{
-	return state == NVME_ANA_OPTIMIZED || state == NVME_ANA_NONOPTIMIZED;
-}
-
 static void nvme_update_ns_ana_state(struct nvme_ana_group_desc *desc,
 		struct nvme_ns *ns)
 {
@@ -1180,6 +1197,53 @@ static ssize_t delayed_removal_secs_store(struct device *dev,
 
 DEVICE_ATTR_RW(delayed_removal_secs);
 
+static ssize_t fail_if_no_path_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct gendisk *disk = dev_to_disk(dev);
+	struct nvme_ns_head *head = disk->private_data;
+
+	return sysfs_emit(buf, test_bit(NVME_NSHEAD_FAIL_IF_NO_PATH,
+			&head->flags) ? "on\n" : "off\n");
+}
+
+static ssize_t fail_if_no_path_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct gendisk *disk = dev_to_disk(dev);
+	struct nvme_ns_head *head = disk->private_data;
+	bool enable;
+	int ret;
+
+	ret = kstrtobool(buf, &enable);
+	if (ret < 0)
+		return ret;
+
+	/* No-op if the stored value matches the current setting. */
+	if (enable) {
+		if (test_and_set_bit(NVME_NSHEAD_FAIL_IF_NO_PATH, &head->flags))
+			return count;
+	} else {
+		if (!test_and_clear_bit(NVME_NSHEAD_FAIL_IF_NO_PATH,
+					&head->flags))
+			return count;
+	}
+
+	/*
+	 * Ensure that update to NVME_NSHEAD_FAIL_IF_NO_PATH is seen
+	 * by its reader.
+	 */
+	synchronize_srcu(&head->srcu);
+
+	/* Make already-queued I/O re-evaluate path availability. */
+	if (enable)
+		kblockd_schedule_work(&head->requeue_work);
+
+	return count;
+}
+
+DEVICE_ATTR_RW(fail_if_no_path);
+
 static ssize_t multipath_failover_count_show(struct device *dev,
 		struct device_attribute *attr, char *buf)
 {
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index a643c4723a69..9773538101ca 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -590,6 +590,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_FAIL_IF_NO_PATH	3
 	struct nvme_ns __rcu_guarded	*current_path[];
 #endif
 };
@@ -1097,6 +1098,7 @@ extern struct device_attribute dev_attr_ana_state;
 extern struct device_attribute dev_attr_queue_depth;
 extern struct device_attribute dev_attr_numa_nodes;
 extern struct device_attribute dev_attr_delayed_removal_secs;
+extern struct device_attribute dev_attr_fail_if_no_path;
 extern struct device_attribute dev_attr_multipath_failover_count;
 extern struct device_attribute dev_attr_io_requeue_no_usable_path_count;
 extern struct device_attribute dev_attr_io_fail_no_available_path_count;
diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c
index e1e3dcfd084b..56e0ce1c9d8a 100644
--- a/drivers/nvme/host/sysfs.c
+++ b/drivers/nvme/host/sysfs.c
@@ -264,6 +264,7 @@ static struct attribute *nvme_ns_attrs[] = {
 	&dev_attr_queue_depth.attr,
 	&dev_attr_numa_nodes.attr,
 	&dev_attr_delayed_removal_secs.attr,
+	&dev_attr_fail_if_no_path.attr,
 #endif
 	&dev_attr_io_passthru_err_log_enabled.attr,
 	NULL,
@@ -300,7 +301,8 @@ static umode_t nvme_ns_attrs_are_visible(struct kobject *kobj,
 		if (nvme_disk_is_ns_head(dev_to_disk(dev)))
 			return 0;
 	}
-	if (a == &dev_attr_delayed_removal_secs.attr) {
+	if (a == &dev_attr_delayed_removal_secs.attr ||
+	    a == &dev_attr_fail_if_no_path.attr) {
 		struct gendisk *disk = dev_to_disk(dev);
 
 		if (!nvme_disk_is_ns_head(disk))
-- 
2.54.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] nvme-multipath: add fail_if_no_path sysfs attribute
  2026-09-23  0:49 [PATCH v3] nvme-multipath: add fail_if_no_path sysfs attribute Krishna Iyer
@ 2026-09-23 12:06 ` Hannes Reinecke
  2026-09-23 21:48 ` Keith Busch
  1 sibling, 0 replies; 6+ messages in thread
From: Hannes Reinecke @ 2026-09-23 12:06 UTC (permalink / raw)
  To: Krishna Iyer, kbusch, axboe, hch, sagi
  Cc: linux-nvme, linux-kernel, nilay, sjpark, saravanand

On 9/23/26 2:49 AM, Krishna Iyer wrote:
> When no usable path exists, I/O on a multipath namespace is queued
> until a path returns. With ctrl_loss_tmo=-1 that can be forever:
> during a long fabric outage any process waiting on the I/O is stuck in
> D state. We hit this on virtualization hosts, where a SIGKILLed VM
> process cannot exit while draining I/O to an unreachable NVMe/TCP
> target.
> 
> Nothing can fail this I/O without tearing something down: controller
> deletion takes every namespace on the controller with it.
> 
> Add a fail_if_no_path attribute on the ns-head disk: a persistent
> per-namespace policy to fail parked and newly arriving I/O instead of
> queueing it when no usable path exists. It is enforced where a path is
> known to be unusable: CONNECTING controllers and LIVE controllers with
> the path ANA inaccessible or persistent-loss stop counting as
> available, RESETTING and ANA change keep queueing, and with no
> controllers left the policy overrides the delayed_removal_secs
> queueing window. Controller state is untouched and reconnects
> continue. Like dm's fail_if_no_path, the policy is transport agnostic.
> 
> Assisted-by: Claude:claude-fable-5
> Signed-off-by: Krishna Iyer <kiyer@crusoe.ai>
> ---
> Changes since v2 [1]:
> - use the nvme_state_is_live() helper for the ANA state check, keeping
>    the explicit NVME_ANA_CHANGE carve-out so transient ANA transitions
>    still queue (Nilay)
> - return early when the stored value matches the current setting,
>    skipping synchronize_srcu() and the requeue kick (Nilay)
> 
[ .. ]
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index a643c4723a69..9773538101ca 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -590,6 +590,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_FAIL_IF_NO_PATH	3
>   	struct nvme_ns __rcu_guarded	*current_path[];
>   #endif
>   };

Remind me again: what is the meaning of 'NVME_NSHEAD_QUEUE_IF_NO_PATH'?
Isn't this the exact opposite of the proposed
'NVME_NSHEAD_FAIL_IF_NO_PATH'?
IOW can't you rewrite this patch to use the existing 
'NVME_NSHEAD_QUEUE_IF_NO_PATH' flag instead of adding another?
(And if you have to introduce 'FAIL_IF_NO_PATH' you'd have to explain
what exactly should happen if QUEUE_IF_NO_PATH _and_ FAIL_IF_NO_PATH
are set ...)

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] nvme-multipath: add fail_if_no_path sysfs attribute
  2026-09-23  0:49 [PATCH v3] nvme-multipath: add fail_if_no_path sysfs attribute Krishna Iyer
  2026-09-23 12:06 ` Hannes Reinecke
@ 2026-09-23 21:48 ` Keith Busch
  2026-09-23 22:57   ` Krishna Iyer
  1 sibling, 1 reply; 6+ messages in thread
From: Keith Busch @ 2026-09-23 21:48 UTC (permalink / raw)
  To: Krishna Iyer
  Cc: axboe, hch, sagi, linux-nvme, linux-kernel, nilay, sjpark, saravanand

On Tue, Sep 22, 2026 at 05:49:59PM -0700, Krishna Iyer wrote:
> When no usable path exists, I/O on a multipath namespace is queued
> until a path returns. With ctrl_loss_tmo=-1 that can be forever:
> during a long fabric outage any process waiting on the I/O is stuck in
> D state. We hit this on virtualization hosts, where a SIGKILLed VM
> process cannot exit while draining I/O to an unreachable NVMe/TCP
> target.
> 
> Nothing can fail this I/O without tearing something down: controller
> deletion takes every namespace on the controller with it.

I agree with Hannes that we shouldn't need a new flag for this. The
current behavior is just broken in a few ways.

We just need to restrict queue_if_no_path to only if there really are no
paths so that we actually respect the failfast_tmo.

And the ANA inaccessible and persistent-loss handling you introduced
here should just be the default without requiring a flag. But I think
you need to add a requeue_list kick in nvme_update_ns_ana_state() too in
case the ANA transition leaves the namespace inaccessible.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] nvme-multipath: add fail_if_no_path sysfs attribute
  2026-09-23 21:48 ` Keith Busch
@ 2026-09-23 22:57   ` Krishna Iyer
  2026-09-24  1:07     ` Keith Busch
  0 siblings, 1 reply; 6+ messages in thread
From: Krishna Iyer @ 2026-09-23 22:57 UTC (permalink / raw)
  To: kbusch
  Cc: hare, axboe, hch, sagi, linux-nvme, linux-kernel, nilay, sjpark,
	saravanand

On 9/23/26 2:48 PM, Keith Busch wrote:
> I agree with Hannes that we shouldn't need a new flag for this. The
> current behavior is just broken in a few ways.
>
> We just need to restrict queue_if_no_path to only if there really are no
> paths so that we actually respect the failfast_tmo.
>
> And the ANA inaccessible and persistent-loss handling you introduced
> here should just be the default without requiring a flag. But I think
> you need to add a requeue_list kick in nvme_update_ns_ana_state() too in
> case the ANA transition leaves the namespace inaccessible.

Agreed those are real problems, but fixing them still leaves no way
to cover this case. Subsystems commonly expose many volumes as
namespaces behind the same controllers, and failfast is controller
scoped and time based: a timeout fails every namespace or none, and
the trigger here is an event, not a duration one can pick up front,
while sibling namespaces should keep queueing and ride out the
outage. delayed_removal_secs does not apply either: it only governs
the window after the last path is removed (here the controllers still
exist and keep reconnecting), and its expiry removes the head disk,
where this case needs the opposite, fail the parked I/O and keep the
device. fail_if_no_path stays the only namespace scoped way to
release parked I/O.

If the direction sounds right to you, I can send a two patch series:
the default fixes you describe first, the namespace scoped attribute
on top.

Thanks,
Krishna

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] nvme-multipath: add fail_if_no_path sysfs attribute
  2026-09-23 22:57   ` Krishna Iyer
@ 2026-09-24  1:07     ` Keith Busch
  2026-09-24  6:35       ` Krishna Iyer
  0 siblings, 1 reply; 6+ messages in thread
From: Keith Busch @ 2026-09-24  1:07 UTC (permalink / raw)
  To: Krishna Iyer
  Cc: hare, axboe, hch, sagi, linux-nvme, linux-kernel, nilay, sjpark,
	saravanand

On Wed, Sep 23, 2026 at 03:57:53PM -0700, Krishna Iyer wrote:
> On 9/23/26 2:48 PM, Keith Busch wrote:
> > I agree with Hannes that we shouldn't need a new flag for this. The
> > current behavior is just broken in a few ways.
> >
> > We just need to restrict queue_if_no_path to only if there really are no
> > paths so that we actually respect the failfast_tmo.
> >
> > And the ANA inaccessible and persistent-loss handling you introduced
> > here should just be the default without requiring a flag. But I think
> > you need to add a requeue_list kick in nvme_update_ns_ana_state() too in
> > case the ANA transition leaves the namespace inaccessible.
> 
> Agreed those are real problems, but fixing them still leaves no way
> to cover this case. Subsystems commonly expose many volumes as
> namespaces behind the same controllers, and failfast is controller
> scoped and time based: a timeout fails every namespace or none, and
> the trigger here is an event, not a duration one can pick up front,
> while sibling namespaces should keep queueing and ride out the
> outage. delayed_removal_secs does not apply either: it only governs
> the window after the last path is removed (here the controllers still
> exist and keep reconnecting), and its expiry removes the head disk,
> where this case needs the opposite, fail the parked I/O and keep the
> device. fail_if_no_path stays the only namespace scoped way to
> release parked I/O.

I get that failfast is at the controller scope, but that seems okay:
when the transport is down, every namespace behind that controller is
equally without a path. When the controller is live, the per-namespace
inaccessible case is handled separately by the ANA state, so namespace
heads that lose access bail and the rest can carry on.

Is your intention to have some namespaces wait for reconnect longer than
others?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] nvme-multipath: add fail_if_no_path sysfs attribute
  2026-09-24  1:07     ` Keith Busch
@ 2026-09-24  6:35       ` Krishna Iyer
  0 siblings, 0 replies; 6+ messages in thread
From: Krishna Iyer @ 2026-09-24  6:35 UTC (permalink / raw)
  To: kbusch
  Cc: hare, axboe, hch, sagi, linux-nvme, linux-kernel, nilay, sjpark,
	saravanand

On 9/23/26 6:07 PM, Keith Busch wrote:
> I get that failfast is at the controller scope, but that seems okay:
> when the transport is down, every namespace behind that controller is
> equally without a path. When the controller is live, the per-namespace
> inaccessible case is handled separately by the ANA state, so namespace
> heads that lose access bail and the rest can carry on.
>
> Is your intention to have some namespaces wait for reconnect longer than
> others?

Yes, exactly. Some namespaces should keep waiting out the reconnect
while others should stop waiting and fail their parked I/O, decided
by a condition only userspace can see. Here that condition is a
killed process stuck in D state on parked I/O that nothing will ever
consume (for us a SIGKILLed VM the host must reap), and it could be
any consumer known to be gone. Failing that namespace lets the
process exit, while namespaces with live users behind the same
controllers keep queueing and carry on when paths return. A
controller wide timeout cannot separate the two, and there is no
duration to pick up front since the trigger is userspace observing
the event. Reconnect itself is untouched either way. So the kernel
provides the per namespace switch and the decision stays in
userspace.

Thanks,
Krishna

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-09-24  6:35 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-23  0:49 [PATCH v3] nvme-multipath: add fail_if_no_path sysfs attribute Krishna Iyer
2026-09-23 12:06 ` Hannes Reinecke
2026-09-23 21:48 ` Keith Busch
2026-09-23 22:57   ` Krishna Iyer
2026-09-24  1:07     ` Keith Busch
2026-09-24  6:35       ` Krishna Iyer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®