mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] nvme-multipath: add fail_io_now sysfs attribute to fail queued I/O
@ 2026-09-04  3:26 Krishna Iyer
  2026-09-05 22:36 ` Sagi Grimberg
  2026-09-07 13:15 ` Nilay Shroff
  0 siblings, 2 replies; 9+ messages in thread
From: Krishna Iyer @ 2026-09-04  3:26 UTC (permalink / raw)
  To: kbusch, axboe, hch, sagi
  Cc: linux-nvme, linux-kernel, sj, saravanand, Krishna Iyer

When all paths to a multipath namespace are down, I/O is held on the
head requeue list until a path returns. With ctrl_loss_tmo=-1 the
controllers reconnect forever, so during a long fabric outage the I/O
is held indefinitely and any process waiting on it sleeps in D state
until the fabric heals or the host is rebooted. We hit this on
virtualization hosts, where a SIGKILLed VM process cannot exit because
it is still draining I/O to an unreachable NVMe/TCP target.

There is currently no way to fail this I/O without tearing something
down. Deleting the controller (or letting ctrl_loss_tmo expire) works
but takes every namespace on the controller with it and requires a
manual reconnect afterwards. fast_io_fail_tmo only arms on the
RESETTING -> CONNECTING transition, so it cannot be set once the
outage has started. delayed_removal_secs only matters after all
controllers are gone, which never happens with ctrl_loss_tmo=-1.
dm-multipath has had "dmsetup message <dev> 0 fail_if_no_path" for
this for decades; nvme multipath has no equivalent.

Add a fail_io_now attribute on the ns-head disk. Writing a true value
sets NVME_NSHEAD_FAIL_IO_NOW, synchronizes SRCU so submitters see it,
and kicks the requeue work. nvme_available_path() treats the flag as
no path available, so the existing bio_io_error() branch fails the
parked and any newly arriving I/O, for that namespace only. Controller
state is not touched: reconnect attempts continue and other namespaces
on the controller keep queueing. The flag is cleared in
nvme_mpath_set_live() when a path comes back, like
NVME_CTRL_FAILFAST_EXPIRED.

Locking, SRCU usage and sysfs visibility follow the neighboring
delayed_removal_secs attribute; input parsing follows
io_passthru_err_log_enabled (kstrtobool, shows on/off). Validated on
real hardware with a 6.17 backport of this change.

Assisted-by: Claude:claude-fable-5
Signed-off-by: Krishna Iyer <kiyer@crusoe.ai>
---
Testing notes: the 6.17 backport was exercised on a virtualization
host with a two-path NVMe/TCP namespace connected with
ctrl_loss_tmo=-1. With both target portals firewalled off and a
SIGKILLed VM process stuck in D state on the parked I/O, the process
stayed unreapable for over six minutes; delayed_removal_secs=60,
armed before the outage, never triggered since the controllers were
CONNECTING throughout. Writing fail_io_now released the process in
about two seconds, the reconnect loop was undisturbed, and once the
firewall was removed the paths came back live and the attribute read
back off on its own. A namespace on a second subsystem kept the
default queueing behavior throughout. This posting is compile-tested
(including W=1) on nvme-next.

 drivers/nvme/host/multipath.c | 62 +++++++++++++++++++++++++++++++++++
 drivers/nvme/host/nvme.h      |  2 ++
 drivers/nvme/host/sysfs.c     |  4 ++-
 3 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index fc6800a9f7f9..a026bdfb9d7f 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -482,6 +482,15 @@ static bool nvme_available_path(struct nvme_ns_head *head)
 	if (!test_bit(NVME_NSHEAD_DISK_LIVE, &head->flags))
 		return false;
 
+	/*
+	 * The user requested any I/O queued or arriving while no path is
+	 * usable to be failed immediately (e.g. to release I/O held for a
+	 * fabric that retries reconnection indefinitely). The flag is
+	 * cleared when a path becomes live again.
+	 */
+	if (test_bit(NVME_NSHEAD_FAIL_IO_NOW, &head->flags))
+		return false;
+
 	list_for_each_entry_srcu(ns, &head->list, siblings,
 				 srcu_read_lock_held(&head->srcu)) {
 		if (test_bit(NVME_CTRL_FAILFAST_EXPIRED, &ns->ctrl->flags))
@@ -780,6 +789,12 @@ static void nvme_mpath_set_live(struct nvme_ns *ns)
 	if (!head->disk)
 		return;
 
+	/*
+	 * A path is usable again, restore the default queue-if-no-path
+	 * behavior in case fail_io_now was set during a fabric outage.
+	 */
+	clear_bit(NVME_NSHEAD_FAIL_IO_NOW, &head->flags);
+
 	/*
 	 * test_and_set_bit() is used because it is protecting against two nvme
 	 * paths simultaneously calling device_add_disk() on the same namespace
@@ -1168,6 +1183,53 @@ static ssize_t delayed_removal_secs_store(struct device *dev,
 
 DEVICE_ATTR_RW(delayed_removal_secs);
 
+static ssize_t fail_io_now_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_IO_NOW,
+			&head->flags) ? "on\n" : "off\n");
+}
+
+static ssize_t fail_io_now_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;
+
+	mutex_lock(&head->subsys->lock);
+	if (enable)
+		set_bit(NVME_NSHEAD_FAIL_IO_NOW, &head->flags);
+	else
+		clear_bit(NVME_NSHEAD_FAIL_IO_NOW, &head->flags);
+	mutex_unlock(&head->subsys->lock);
+
+	/*
+	 * Ensure that update to NVME_NSHEAD_FAIL_IO_NOW is seen
+	 * by its reader.
+	 */
+	synchronize_srcu(&head->srcu);
+
+	/*
+	 * Kick the requeue list so already-queued I/O re-evaluates path
+	 * availability and fails immediately.
+	 */
+	if (enable)
+		kblockd_schedule_work(&head->requeue_work);
+
+	return count;
+}
+
+DEVICE_ATTR_RW(fail_io_now);
+
 static int nvme_lookup_ana_group_desc(struct nvme_ctrl *ctrl,
 		struct nvme_ana_group_desc *desc, void *data)
 {
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index eeabc72863d8..ca93a8934123 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -566,6 +566,7 @@ struct nvme_ns_head {
 	unsigned int		delayed_removal_secs;
 #define NVME_NSHEAD_DISK_LIVE		0
 #define NVME_NSHEAD_QUEUE_IF_NO_PATH	1
+#define NVME_NSHEAD_FAIL_IO_NOW		2
 	struct nvme_ns __rcu	*current_path[];
 #endif
 };
@@ -1067,6 +1068,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_io_now;
 extern struct device_attribute subsys_attr_iopolicy;
 
 static inline bool nvme_disk_is_ns_head(struct gendisk *disk)
diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c
index 93513c17ad5f..c154cc78c290 100644
--- a/drivers/nvme/host/sysfs.c
+++ b/drivers/nvme/host/sysfs.c
@@ -261,6 +261,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_io_now.attr,
 #endif
 	&dev_attr_io_passthru_err_log_enabled.attr,
 	NULL,
@@ -297,7 +298,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_io_now.attr) {
 		struct gendisk *disk = dev_to_disk(dev);
 
 		if (!nvme_disk_is_ns_head(disk))

base-commit: 011e0880d366be065d273c22ad1638934748d3e0
-- 
2.54.0


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

* Re: [PATCH] nvme-multipath: add fail_io_now sysfs attribute to fail queued I/O
  2026-09-04  3:26 [PATCH] nvme-multipath: add fail_io_now sysfs attribute to fail queued I/O Krishna Iyer
@ 2026-09-05 22:36 ` Sagi Grimberg
  2026-09-06  4:22   ` Krishna Iyer
  2026-09-07 13:15 ` Nilay Shroff
  1 sibling, 1 reply; 9+ messages in thread
From: Sagi Grimberg @ 2026-09-05 22:36 UTC (permalink / raw)
  To: Krishna Iyer, kbusch, axboe, hch; +Cc: linux-nvme, linux-kernel, sj, saravanand



On 04/09/2026 6:26, Krishna Iyer wrote:
> When all paths to a multipath namespace are down, I/O is held on the
> head requeue list until a path returns. With ctrl_loss_tmo=-1 the
> controllers reconnect forever, so during a long fabric outage the I/O
> is held indefinitely and any process waiting on it sleeps in D state
> until the fabric heals or the host is rebooted. We hit this on
> virtualization hosts, where a SIGKILLed VM process cannot exit because
> it is still draining I/O to an unreachable NVMe/TCP target.
>
> There is currently no way to fail this I/O without tearing something
> down. Deleting the controller (or letting ctrl_loss_tmo expire) works
> but takes every namespace on the controller with it and requires a
> manual reconnect afterwards. fast_io_fail_tmo only arms on the
> RESETTING -> CONNECTING transition, so it cannot be set once the
> outage has started. delayed_removal_secs only matters after all
> controllers are gone, which never happens with ctrl_loss_tmo=-1.
> dm-multipath has had "dmsetup message <dev> 0 fail_if_no_path" for
> this for decades; nvme multipath has no equivalent.

I don't understand what is not sufficient with fast_io_fail_tmo? It would
determine the time that IO will fail when all paths are down.

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

* Re: [PATCH] nvme-multipath: add fail_io_now sysfs attribute to fail queued I/O
  2026-09-05 22:36 ` Sagi Grimberg
@ 2026-09-06  4:22   ` Krishna Iyer
  2026-09-11 21:37     ` Sagi Grimberg
  0 siblings, 1 reply; 9+ messages in thread
From: Krishna Iyer @ 2026-09-06  4:22 UTC (permalink / raw)
  To: sagi
  Cc: kbusch, axboe, hch, linux-nvme, linux-kernel, sj, saravanand,
	Krishna Iyer

On 06/09/2026 1:36, Sagi Grimberg wrote:
> I don't understand what is not sufficient with fast_io_fail_tmo? It would
> determine the time that IO will fail when all paths are down.

Thanks for taking a look, Sagi. fast_io_fail_tmo is indeed the closest
existing knob. Like this patch, it fails the parked I/O without
touching controller state. Three things make it unsuitable for this
case though:

1. It cannot be engaged after the fact. nvme_start_failfast_work() has
a single call site, the RESETTING -> CONNECTING transition in
nvme_change_ctrl_state(), and it returns without scheduling if
fast_io_fail_tmo is -1 at that instant. A controller in the reconnect
loop stays in CONNECTING, and the sysfs store only updates opts, so
once an outage has begun with failfast disabled there is no way to
make the timer fire for that outage. That is exactly the situation in
which the need for this arises.

2. It decides based on time, and this decision is not about time. For
deployments like ours the right policy for an outage of any length is
to keep queueing; that is why we connect with ctrl_loss_tmo=-1 to
begin with. The exception is not "the outage got long" but "this
particular submitter was just killed and will never consume its
completions", which host software learns at a moment no timer can
anticipate. Any timeout short enough to release such I/O promptly
would also fail I/O for every healthy workload whenever an ordinary
outage outlasts it.

3. It is scoped to the controller. FAILFAST_EXPIRED marks the whole
controller, so with all paths down it releases I/O for every namespace
in the subsystem. The process being reaped owns exactly one namespace;
the others belong to VMs that are still running and should keep
queueing until the fabric heals.

So fail_io_now is the failfast expiry made available on demand and per
namespace: the same nvme_available_path() mechanism, and it likewise
clears itself on reconnect, but it is triggered by the admin, for one
volume, at the moment it is actually needed.

Happy to work any of this into the changelog if that would help.

Thanks,
Krishna

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

* Re: [PATCH] nvme-multipath: add fail_io_now sysfs attribute to fail queued I/O
  2026-09-04  3:26 [PATCH] nvme-multipath: add fail_io_now sysfs attribute to fail queued I/O Krishna Iyer
  2026-09-05 22:36 ` Sagi Grimberg
@ 2026-09-07 13:15 ` Nilay Shroff
  2026-09-10  1:54   ` Krishna Iyer
  1 sibling, 1 reply; 9+ messages in thread
From: Nilay Shroff @ 2026-09-07 13:15 UTC (permalink / raw)
  To: Krishna Iyer, kbusch, axboe, hch, sagi
  Cc: linux-nvme, linux-kernel, sj, saravanand

On 9/4/26 8:56 AM, Krishna Iyer wrote:
> When all paths to a multipath namespace are down, I/O is held on the
> head requeue list until a path returns. With ctrl_loss_tmo=-1 the
> controllers reconnect forever, so during a long fabric outage the I/O
> is held indefinitely and any process waiting on it sleeps in D state
> until the fabric heals or the host is rebooted. We hit this on
> virtualization hosts, where a SIGKILLed VM process cannot exit because
> it is still draining I/O to an unreachable NVMe/TCP target.
> 
> There is currently no way to fail this I/O without tearing something
> down. Deleting the controller (or letting ctrl_loss_tmo expire) works
> but takes every namespace on the controller with it and requires a
> manual reconnect afterwards. fast_io_fail_tmo only arms on the
> RESETTING -> CONNECTING transition, so it cannot be set once the
> outage has started. delayed_removal_secs only matters after all
> controllers are gone, which never happens with ctrl_loss_tmo=-1.
> dm-multipath has had "dmsetup message <dev> 0 fail_if_no_path" for
> this for decades; nvme multipath has no equivalent.
> 
> Add a fail_io_now attribute on the ns-head disk. Writing a true value
> sets NVME_NSHEAD_FAIL_IO_NOW, synchronizes SRCU so submitters see it,
> and kicks the requeue work. nvme_available_path() treats the flag as
> no path available, so the existing bio_io_error() branch fails the
> parked and any newly arriving I/O, for that namespace only. Controller
> state is not touched: reconnect attempts continue and other namespaces
> on the controller keep queueing. The flag is cleared in
> nvme_mpath_set_live() when a path comes back, like
> NVME_CTRL_FAILFAST_EXPIRED.
> 
> Locking, SRCU usage and sysfs visibility follow the neighboring
> delayed_removal_secs attribute; input parsing follows
> io_passthru_err_log_enabled (kstrtobool, shows on/off). Validated on
> real hardware with a 6.17 backport of this change.
> 
> Assisted-by: Claude:claude-fable-5
> Signed-off-by: Krishna Iyer <kiyer@crusoe.ai>
> ---
> Testing notes: the 6.17 backport was exercised on a virtualization
> host with a two-path NVMe/TCP namespace connected with
> ctrl_loss_tmo=-1. With both target portals firewalled off and a
> SIGKILLed VM process stuck in D state on the parked I/O, the process
> stayed unreapable for over six minutes; delayed_removal_secs=60,
> armed before the outage, never triggered since the controllers were
> CONNECTING throughout. Writing fail_io_now released the process in
> about two seconds, the reconnect loop was undisturbed, and once the
> firewall was removed the paths came back live and the attribute read
> back off on its own. A namespace on a second subsystem kept the
> default queueing behavior throughout. This posting is compile-tested
> (including W=1) on nvme-next.
> 
>   drivers/nvme/host/multipath.c | 62 +++++++++++++++++++++++++++++++++++
>   drivers/nvme/host/nvme.h      |  2 ++
>   drivers/nvme/host/sysfs.c     |  4 ++-
>   3 files changed, 67 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
> index fc6800a9f7f9..a026bdfb9d7f 100644
> --- a/drivers/nvme/host/multipath.c
> +++ b/drivers/nvme/host/multipath.c
> @@ -482,6 +482,15 @@ static bool nvme_available_path(struct nvme_ns_head *head)
>   	if (!test_bit(NVME_NSHEAD_DISK_LIVE, &head->flags))
>   		return false;
>   
> +	/*
> +	 * The user requested any I/O queued or arriving while no path is
> +	 * usable to be failed immediately (e.g. to release I/O held for a
> +	 * fabric that retries reconnection indefinitely). The flag is
> +	 * cleared when a path becomes live again.
> +	 */
> +	if (test_bit(NVME_NSHEAD_FAIL_IO_NOW, &head->flags))
> +		return false;
> +
Does the intention here is to force I/O to fail irrespective of the
controller state, or the intention here's to fail I/O only when no
usable path exist? If it's latter then I believe this is not the right
place to enforce this policy as since this check makes nvme_available_path()
return false unconditionally when NVME_NSHEAD_FAIL_IO_NOW is set, without
considering whether a usable path exists.

>   	list_for_each_entry_srcu(ns, &head->list, siblings,
>   				 srcu_read_lock_held(&head->srcu)) {
>   		if (test_bit(NVME_CTRL_FAILFAST_EXPIRED, &ns->ctrl->flags))
> @@ -780,6 +789,12 @@ static void nvme_mpath_set_live(struct nvme_ns *ns)
>   	if (!head->disk)
>   		return;
>   
> +	/*
> +	 * A path is usable again, restore the default queue-if-no-path
> +	 * behavior in case fail_io_now was set during a fabric outage.
> +	 */
> +	clear_bit(NVME_NSHEAD_FAIL_IO_NOW, &head->flags);
> +
>   	/*
>   	 * test_and_set_bit() is used because it is protecting against two nvme
>   	 * paths simultaneously calling device_add_disk() on the same namespace
> @@ -1168,6 +1183,53 @@ static ssize_t delayed_removal_secs_store(struct device *dev,
>   
>   DEVICE_ATTR_RW(delayed_removal_secs);
>   
> +static ssize_t fail_io_now_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_IO_NOW,
> +			&head->flags) ? "on\n" : "off\n");
> +}
> +
> +static ssize_t fail_io_now_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;
> +
> +	mutex_lock(&head->subsys->lock);
> +	if (enable)
> +		set_bit(NVME_NSHEAD_FAIL_IO_NOW, &head->flags);
> +	else
> +		clear_bit(NVME_NSHEAD_FAIL_IO_NOW, &head->flags);
> +	mutex_unlock(&head->subsys->lock);
> +
> +	/*
> +	 * Ensure that update to NVME_NSHEAD_FAIL_IO_NOW is seen
> +	 * by its reader.
> +	 */
> +	synchronize_srcu(&head->srcu);
> +
> +	/*
> +	 * Kick the requeue list so already-queued I/O re-evaluates path
> +	 * availability and fails immediately.
> +	 */
> +	if (enable)
> +		kblockd_schedule_work(&head->requeue_work);
> +
> +	return count;
> +}
> +
> +DEVICE_ATTR_RW(fail_io_now);
> +
>   static int nvme_lookup_ana_group_desc(struct nvme_ctrl *ctrl,
>   		struct nvme_ana_group_desc *desc, void *data)
>   {
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index eeabc72863d8..ca93a8934123 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -566,6 +566,7 @@ struct nvme_ns_head {
>   	unsigned int		delayed_removal_secs;
>   #define NVME_NSHEAD_DISK_LIVE		0
>   #define NVME_NSHEAD_QUEUE_IF_NO_PATH	1
> +#define NVME_NSHEAD_FAIL_IO_NOW		2
>   	struct nvme_ns __rcu	*current_path[];
>   #endif
>   };
> @@ -1067,6 +1068,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_io_now;
>   extern struct device_attribute subsys_attr_iopolicy;
>   
>   static inline bool nvme_disk_is_ns_head(struct gendisk *disk)
> diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c
> index 93513c17ad5f..c154cc78c290 100644
> --- a/drivers/nvme/host/sysfs.c
> +++ b/drivers/nvme/host/sysfs.c
> @@ -261,6 +261,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_io_now.attr,
>   #endif
>   	&dev_attr_io_passthru_err_log_enabled.attr,
>   	NULL,
> @@ -297,7 +298,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_io_now.attr) {

This attribute should be only exposed for fabric controller.
It looks this is being exported for non-fabric controller
as well. Moreover, I like attribute fail_if_no_path better
than fail_io_now, since it describes the actual policy being
enabled: when no usable path exists, fail I/O instead of
queueing it.

>   		struct gendisk *disk = dev_to_disk(dev);
>   
>   		if (!nvme_disk_is_ns_head(disk))
> 
> base-commit: 011e0880d366be065d273c22ad1638934748d3e0

This patch appears to be based off older kernel branch. Please
rebase it against the nvme-7.3 branch.

Thanks,
--Nilay


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

* Re: [PATCH] nvme-multipath: add fail_io_now sysfs attribute to fail queued I/O
  2026-09-07 13:15 ` Nilay Shroff
@ 2026-09-10  1:54   ` Krishna Iyer
  2026-09-10  9:00     ` Nilay Shroff
  0 siblings, 1 reply; 9+ messages in thread
From: Krishna Iyer @ 2026-09-10  1:54 UTC (permalink / raw)
  To: Nilay Shroff
  Cc: kbusch, axboe, hch, sagi, linux-nvme, linux-kernel, sj, saravanand

On 9/7/26 6:15 AM, Nilay Shroff wrote:

> Does the intention here is to force I/O to fail irrespective of the
> controller state, or the intention here's to fail I/O only when no
> usable path exist? If it's latter then I believe this is not the right
> place to enforce this policy as since this check makes nvme_available_path()
> return false unconditionally when NVME_NSHEAD_FAIL_IO_NOW is set, without
> considering whether a usable path exists.

The latter. nvme_available_path() is only called once nvme_find_path()
has come up empty, so the flag only decides whether pathless I/O is
queued (default) or failed.

Agreed on the rename. For v2 I'll make fail_if_no_path a plain
per-namespace policy: admin-set, no self-clearing, toggleable at any
time including mid-outage.

> This attribute should be only exposed for fabric controller.
> It looks this is being exported for non-fabric controller
> as well. Moreover, I like attribute fail_if_no_path better
> than fail_io_now, since it describes the actual policy being
> enabled: when no usable path exists, fail I/O instead of
> queueing it.

Right, v1 exposes it everywhere. Since v2 makes this a plain
fail_if_no_path policy though, is fabric-only still what you'd want?
The queue-vs-fail choice isn't fabric-specific: a PCIe head kept
alive by delayed_removal_secs parks pathless I/O the same way, and
dm's fail_if_no_path is transport-agnostic. If you'd still prefer
fabric-only, I'll have the visibility callback check a sibling for
NVME_F_FABRICS under head->srcu.

> This patch appears to be based off older kernel branch. Please
> rebase it against the nvme-7.3 branch.

Will do; v2 will be based on nvme-7.3.

Thanks for the review. Unless there are further comments, v2 will
have the fail_if_no_path rename with the persistent semantics above,
visibility per your call on the PCIe question, the nvme-7.3 rebase,
and the fast_io_fail_tmo rationale from Sagi's branch of the thread
folded into the commit message.

Thanks,
Krishna

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

* Re: [PATCH] nvme-multipath: add fail_io_now sysfs attribute to fail queued I/O
  2026-09-10  1:54   ` Krishna Iyer
@ 2026-09-10  9:00     ` Nilay Shroff
  2026-09-10 17:13       ` Krishna Iyer
  0 siblings, 1 reply; 9+ messages in thread
From: Nilay Shroff @ 2026-09-10  9:00 UTC (permalink / raw)
  To: Krishna Iyer
  Cc: kbusch, axboe, hch, sagi, linux-nvme, linux-kernel, sj, saravanand

On 9/10/26 7:24 AM, Krishna Iyer wrote:
> On 9/7/26 6:15 AM, Nilay Shroff wrote:
> 
>> Does the intention here is to force I/O to fail irrespective of the
>> controller state, or the intention here's to fail I/O only when no
>> usable path exist? If it's latter then I believe this is not the right
>> place to enforce this policy as since this check makes nvme_available_path()
>> return false unconditionally when NVME_NSHEAD_FAIL_IO_NOW is set, without
>> considering whether a usable path exists.
> 
> The latter. nvme_available_path() is only called once nvme_find_path()
> has come up empty, so the flag only decides whether pathless I/O is
> queued (default) or failed.
> 
Not always, nvme_available_path() could be also called in case
controller is resetting or controller is live but the ns/path ana
state is neither optimized nor non-optimized. So I think the policy
would be better enforced at the point where we have actually
determined that there is no usable path, rather than making
nvme_available_path() return false unconditionally when
fail_if_no_path is set.

> Agreed on the rename. For v2 I'll make fail_if_no_path a plain
> per-namespace policy: admin-set, no self-clearing, toggleable at any
> time including mid-outage.
> 
>> This attribute should be only exposed for fabric controller.
>> It looks this is being exported for non-fabric controller
>> as well. Moreover, I like attribute fail_if_no_path better
>> than fail_io_now, since it describes the actual policy being
>> enabled: when no usable path exists, fail I/O instead of
>> queueing it.
> 
> Right, v1 exposes it everywhere. Since v2 makes this a plain
> fail_if_no_path policy though, is fabric-only still what you'd want?
> The queue-vs-fail choice isn't fabric-specific: a PCIe head kept
> alive by delayed_removal_secs parks pathless I/O the same way, and
> dm's fail_if_no_path is transport-agnostic. If you'd still prefer
> fabric-only, I'll have the visibility callback check a sibling for
> NVME_F_FABRICS under head->srcu.
> 
For PCIe controllers, we don't have the same ctrl_loss_tmo/max_reconnects
semantics as fabrics, so initially I thought supporting fail_if_no_path only
for fabric controllers might make sense. However, looking at this again,
we already queue I/O when no usable path is available irrespective of the
transport. So I'm okay with keeping fail_if_no_path generic and transport-
agnostic. The policy is essentially about what to do when there is no
usable path i.e. fail the I/O or queue it— and that behavior isn't inherently
specific to fabrics.

Thanks,
--Nilay

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

* Re: [PATCH] nvme-multipath: add fail_io_now sysfs attribute to fail queued I/O
  2026-09-10  9:00     ` Nilay Shroff
@ 2026-09-10 17:13       ` Krishna Iyer
  0 siblings, 0 replies; 9+ messages in thread
From: Krishna Iyer @ 2026-09-10 17:13 UTC (permalink / raw)
  To: Nilay Shroff
  Cc: kbusch, axboe, hch, sagi, linux-nvme, linux-kernel, sj, saravanand

On 9/10/26 2:00 AM, Nilay Shroff wrote:

> [...] So I think the policy would be better enforced at the point
> where we have actually determined that there is no usable path,
> rather than making nvme_available_path() return false
> unconditionally when fail_if_no_path is set.

Agreed. In v2 the check moves into the loop. With the policy set, a
path no longer counts as available if its controller is:

- CONNECTING: with ctrl_loss_tmo=-1 that is the entire outage. Same
  effect as FAILFAST_EXPIRED, policy-driven instead of timer-driven.

- LIVE with ANA inaccessible or persistent-loss: the target says no
  usable path, and today only controller deletion releases that I/O.

Everything else (RESETTING, ANA transitions) queues as before.

One interaction to call out: when all controllers are gone,
fail_if_no_path wins over the delayed_removal_secs queueing window --
an explicit fail policy beats a removal grace period. I'll document
that in the changelog.

> So I'm okay with keeping fail_if_no_path generic and transport-
> agnostic. The policy is essentially about what to do when there is
> no usable path i.e. fail the I/O or queue it— and that behavior
> isn't inherently specific to fabrics.

Ack, will keep it transport-agnostic in v2.

Thanks,
Krishna

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

* Re: [PATCH] nvme-multipath: add fail_io_now sysfs attribute to fail queued I/O
  2026-09-06  4:22   ` Krishna Iyer
@ 2026-09-11 21:37     ` Sagi Grimberg
  2026-09-11 23:20       ` Krishna Iyer
  0 siblings, 1 reply; 9+ messages in thread
From: Sagi Grimberg @ 2026-09-11 21:37 UTC (permalink / raw)
  To: Krishna Iyer; +Cc: kbusch, axboe, hch, linux-nvme, linux-kernel, sj, saravanand



On 06/09/2026 7:22, Krishna Iyer wrote:
> On 06/09/2026 1:36, Sagi Grimberg wrote:
>> I don't understand what is not sufficient with fast_io_fail_tmo? It would
>> determine the time that IO will fail when all paths are down.
> Thanks for taking a look, Sagi. fast_io_fail_tmo is indeed the closest
> existing knob. Like this patch, it fails the parked I/O without
> touching controller state. Three things make it unsuitable for this
> case though:
>
> 1. It cannot be engaged after the fact. nvme_start_failfast_work() has
> a single call site, the RESETTING -> CONNECTING transition in
> nvme_change_ctrl_state(), and it returns without scheduling if
> fast_io_fail_tmo is -1 at that instant. A controller in the reconnect
> loop stays in CONNECTING, and the sysfs store only updates opts, so
> once an outage has begun with failfast disabled there is no way to
> make the timer fire for that outage. That is exactly the situation in
> which the need for this arises.

At that point you can disconnect the controllers, which will teardown
the inflight blocked IO.

>
> 2. It decides based on time, and this decision is not about time. For
> deployments like ours the right policy for an outage of any length is
> to keep queueing; that is why we connect with ctrl_loss_tmo=-1 to
> begin with. The exception is not "the outage got long" but "this
> particular submitter was just killed and will never consume its
> completions", which host software learns at a moment no timer can
> anticipate. Any timeout short enough to release such I/O promptly
> would also fail I/O for every healthy workload whenever an ordinary
> outage outlasts it.
>
> 3. It is scoped to the controller. FAILFAST_EXPIRED marks the whole
> controller, so with all paths down it releases I/O for every namespace
> in the subsystem. The process being reaped owns exactly one namespace;
> the others belong to VMs that are still running and should keep
> queueing until the fabric heals.
>
> So fail_io_now is the failfast expiry made available on demand and per
> namespace: the same nvme_available_path() mechanism, and it likewise
> clears itself on reconnect, but it is triggered by the admin, for one
> volume, at the moment it is actually needed.
>
> Happy to work any of this into the changelog if that would help.

I am not sure I am following.

If you have a namespace that is attached to controllers say X,Y,Z,W
Now all of these controllers are unavailable, aren't all of the namespaces
on these paths also unavailable?

Or is it that the paths are online, but the ANA group got into a state where
there is no optimized path?

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

* Re: [PATCH] nvme-multipath: add fail_io_now sysfs attribute to fail queued I/O
  2026-09-11 21:37     ` Sagi Grimberg
@ 2026-09-11 23:20       ` Krishna Iyer
  0 siblings, 0 replies; 9+ messages in thread
From: Krishna Iyer @ 2026-09-11 23:20 UTC (permalink / raw)
  To: Sagi Grimberg
  Cc: Krishna Iyer, kbusch, axboe, hch, nilay, linux-nvme,
	linux-kernel, sj, saravanand

On 9/11/26 2:37 PM, Sagi Grimberg wrote:

> At that point you can disconnect the controllers, which will teardown
> the inflight blocked IO.

That works, but it tears down every namespace on the controller and
someone has to reconnect afterwards. Our controllers back multiple
namespaces and we only want to fail I/O for one of them.

> If you have a namespace that is attached to controllers say X,Y,Z,W
> Now all of these controllers are unavailable, aren't all of the namespaces
> on these paths also unavailable?

Yes, they are all pathless. The difference between them is what to do
with the queued I/O. Holding it until a path returns is right for a
namespace whose user is still around, and wrong for one whose user is
gone. In our case a SIGKILLed VMM will never collect its completions,
while the neighboring namespaces belong to running VMs that should
keep queueing and resume once the fabric heals. The kernel cannot tell
these apart, only host software knows. That is why this needs to be a
per-namespace policy set by the admin, the same model as
fail_if_no_path in dm-multipath.

> Or is it that the paths are online, but the ANA group got into a state where
> there is no optimized path?

In our case no, the controllers sit in CONNECTING for the whole
outage, so there is no LIVE path. But v2 will cover that ANA case too:
with the policy set, a LIVE path with ANA inaccessible or
persistent-loss also does not count as usable.

Either way the no-path condition may be controller-wide, but the
remedy is not: anything controller-scoped releases I/O for every
namespace behind it, and we only want to release one. Hence a
per-namespace policy, which per Nilay's review v2 will rename to
fail_if_no_path and make persistent instead of self-clearing.

Thanks,
Krishna

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

end of thread, other threads:[~2026-09-11 23:21 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-04  3:26 [PATCH] nvme-multipath: add fail_io_now sysfs attribute to fail queued I/O Krishna Iyer
2026-09-05 22:36 ` Sagi Grimberg
2026-09-06  4:22   ` Krishna Iyer
2026-09-11 21:37     ` Sagi Grimberg
2026-09-11 23:20       ` Krishna Iyer
2026-09-07 13:15 ` Nilay Shroff
2026-09-10  1:54   ` Krishna Iyer
2026-09-10  9:00     ` Nilay Shroff
2026-09-10 17:13       ` 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®