Re: [PATCH v2 1/2] nvme-multipath: fail queued I/O when delayed_removal_secs is cleared

[Nilay Shroff]

On 11/11/26 6:40 PM, Qiliang Yuan wrote:
> With delayed_removal_secs set, bios submitted after the last path goes
> away are queued on head->requeue_list, in case a path comes back before
> the delayed removal of the head.
>
> Writing 0 to delayed_removal_secs clears NVME_NSHEAD_QUEUE_IF_NO_PATH
> but doesn't kick the requeue work, so the bios already queued stay
> there until the removal work runs, up to the old delay later, instead
> of failing right away.
>
> Kick the requeue work when the attribute is cleared, as
> fail_on_ctrl_loss_store() does when it is set, so that the queued bios
> are resubmitted and fail.
>
> With delayed_removal_secs at 30 and no path left, a direct read is
> queued and delayed_removal_secs cleared two seconds later. The read
> then fails after (mean of two runs, min-max in parentheses):
>
>    before (ms)            after (ms)   change
>    26234 (26209-26258)    3.5 (3-4)    -99.99%
>
> Fixes: 62188639ec160 ("nvme-multipath: introduce delayed removal of the multipath head node")
> Signed-off-by: Qiliang Yuan <qiliang.yuan@xxxxxxxxx>
> ---
>   drivers/nvme/host/multipath.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
> index ae8059366b57d..a92a05b1f925a 100644
> --- a/drivers/nvme/host/multipath.c
> +++ b/drivers/nvme/host/multipath.c
> @@ -1249,6 +1249,10 @@ static ssize_t delayed_removal_secs_store(struct device *dev,
>   	 */
>   	synchronize_srcu(&head->srcu);
>   
> +	/* Fail the I/O queued while waiting for a path to come back */
> +	if (!sec)
> +		kblockd_schedule_work(&head->requeue_work);
> +
>   	return count;
>   }
>   
I think the original design was to apply an updated delayed_removal_secs value to
future namespace head removals, while allowing an already pending removal to
proceed using the previously configured delay.

When the delayed-removal timer expires, the removal work clears disk-live, waits
for the SRCU grace period, and then schedules requeue_work to fail I/O that was
queued while waiting for a path to return.

Given this, I'm wondering why we only kick requeue_work when delayed_removal_secs
is set to zero. If a delayed-removal work item is already pending, wouldn't it be
more consistent to rearm that work based on the updated timeout, rather than only
scheduling requeue_work? This would also ensure that changes to the configured delay
are reflected in an already pending removal.

Thanks,
--Nilay