Re: [PATCH] nvme-multipath: add fail_io_now sysfs attribute to fail queued I/O

[Krishna Iyer]

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