[PATCH] ublk: refuse to go live after an io command was canceled

[PATCH] ublk: refuse to go live after an io command was canceled

[Josef Bacik]

Since commit "ublk: keep a canceled FETCH round canceling until the
server is gone", a device whose FETCH round saw a cancel keeps its
queues canceling until the server goes away, but START_DEV and
END_USER_RECOVERY still bring it up. Without UBLK_F_USER_RECOVERY, or
with UBLK_F_USER_RECOVERY_FAIL_IO, every request of the new disk fails.
With UBLK_F_USER_RECOVERY, requests are requeued and never kicked: after
START_DEV the partition scan hangs under disk->open_mutex, and after
END_USER_RECOVERY every read parks while the command returned 0. The
server cannot fetch the canceled commands again, so the device can't
serve I/O until it restarts anyway.

Return -ENODEV from START_DEV and END_USER_RECOVERY while ub->canceling
is set. In ublk_ctrl_start_dev() check it and publish ub->ub_disk in one
cancel_mutex section, and have ublk_start_cancel() read the disk in its
cancel_mutex section. Today ublk_start_cancel() samples the disk before
taking the mutex, so a server dying during its own START_DEV can mark
the queues without quiescing a disk START_DEV published in between, with
its first I/O past the canceling check. Now either START_DEV sees the
cancel, or the cancel sees the disk and quiesces it before marking. The
END_USER_RECOVERY check is best effort: the disk exists there, and a
cancel after it is the ordinary death of the new server, which
ublk_start_cancel() handles by quiescing and marking.

Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
This applies on top of Ming's "[PATCH 0/8] ublk: don't dispatch to
canceled io commands" and needs patch 1 of it for ub->canceling to stay
set for the whole FETCH round. generic_18 still passes with it.

 Documentation/block/ublk.rst | 10 ++++++++--
 drivers/block/ublk_drv.c     | 38 ++++++++++++++++++++++++++++++++++--
 2 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/Documentation/block/ublk.rst b/Documentation/block/ublk.rst
index 28300fee22bf..b7875a3cf3fc 100644
--- a/Documentation/block/ublk.rst
+++ b/Documentation/block/ublk.rst
@@ -118,7 +118,11 @@ managing and controlling ublk devices with help of several control commands:
   After the server prepares userspace resources (such as creating I/O handler
   threads & io_uring for handling ublk IO), this command is sent to the
   driver for allocating & exposing ``/dev/ublkb*``. Parameters set via
-  ``UBLK_CMD_SET_PARAMS`` are applied for creating the device.
+  ``UBLK_CMD_SET_PARAMS`` are applied for creating the device. The command
+  fails with ``-ENODEV`` if an I/O command fetched by the current server
+  was canceled, because its io_uring is gone. The server can't fetch it
+  again, and the device can be started again once the server has closed
+  ``/dev/ublkc*``.
 
 - ``UBLK_CMD_STOP_DEV``
 
@@ -195,7 +199,9 @@ managing and controlling ublk devices with help of several control commands:
   command is accepted after ublk device is quiesced and a new process has
   opened ``/dev/ublkc*`` and get all ublk queues be ready. When this command
   returns, ublk device is unquiesced and new I/O requests are passed to the
-  new process.
+  new process. It fails with ``-ENODEV`` if an I/O command of the new
+  process was canceled already. The recovery can be started over once the
+  new process has closed ``/dev/ublkc*``.
 
 - user recovery feature description
 
diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index f57d544c1da2..39eb7775a351 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -2759,9 +2759,11 @@ static void ublk_abort_queue(struct ublk_device *ub, struct ublk_queue *ubq)
 
 static void ublk_start_cancel(struct ublk_device *ub)
 {
-	struct gendisk *disk = ublk_get_disk(ub);
+	struct gendisk *disk;
 
+	/* sync with ublk_ctrl_start_dev() publishing the disk */
 	mutex_lock(&ub->cancel_mutex);
+	disk = ublk_get_disk(ub);
 	if (ub->canceling)
 		goto out;
 
@@ -4575,6 +4577,7 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub,
 		.dma_alignment		= 3,
 	};
 	struct gendisk *disk;
+	bool canceled;
 	int ret = -EINVAL;
 
 	if (ublksrv_pid <= 0)
@@ -4665,8 +4668,24 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub,
 	disk->fops = &ub_fops;
 	disk->private_data = ub;
 
+	/*
+	 * A command of this FETCH round was canceled and can't be fetched
+	 * again, don't bring up a disk over it.  Check and publish the disk
+	 * in one cancel_mutex section: either this sees ub->canceling, or
+	 * ublk_start_cancel() sees the disk and quiesces it before marking
+	 * the queues.
+	 */
+	mutex_lock(&ub->cancel_mutex);
+	canceled = ub->canceling;
+	if (!canceled)
+		ub->ub_disk = disk;
+	mutex_unlock(&ub->cancel_mutex);
+	if (canceled) {
+		put_disk(disk);
+		ret = -ENODEV;
+		goto out_unlock;
+	}
 	ub->dev_info.ublksrv_pid = ub->ublksrv_tgid;
-	ub->ub_disk = disk;
 
 	ublk_apply_params(ub);
 
@@ -5238,6 +5257,7 @@ static int ublk_ctrl_end_recovery(struct ublk_device *ub,
 		const struct ublksrv_ctrl_cmd *header)
 {
 	int ublksrv_pid = (int)header->data[0];
+	bool canceled;
 	int ret = -EINVAL;
 
 	pr_devel("%s: Waiting for all FETCH_REQs, dev id %d...\n", __func__,
@@ -5261,6 +5281,20 @@ static int ublk_ctrl_end_recovery(struct ublk_device *ub,
 		ret = -EBUSY;
 		goto out_unlock;
 	}
+
+	/*
+	 * As in ublk_ctrl_start_dev(), a canceled command can't be fetched
+	 * again.  Best effort: the disk exists here, and a cancel after this
+	 * check is the ordinary death of the new server, which
+	 * ublk_start_cancel() handles by quiescing and marking.
+	 */
+	mutex_lock(&ub->cancel_mutex);
+	canceled = ub->canceling;
+	mutex_unlock(&ub->cancel_mutex);
+	if (canceled) {
+		ret = -ENODEV;
+		goto out_unlock;
+	}
 	ub->dev_info.ublksrv_pid = ub->ublksrv_tgid;
 	ub->dev_info.state = UBLK_S_DEV_LIVE;
 	pr_devel("%s: new ublksrv_pid %d, dev id %d\n",
-- 
2.55.0

[PATCH v2] ublk: refuse to go live after an io command was canceled

[Josef Bacik]

Since commit "ublk: keep a canceled FETCH round canceling until the
server is gone", a device whose FETCH round saw a cancel keeps its
queues canceling until the server goes away, but START_DEV and
END_USER_RECOVERY still bring it up. Without UBLK_F_USER_RECOVERY, or
with UBLK_F_USER_RECOVERY_FAIL_IO, every request of the new disk fails.
With UBLK_F_USER_RECOVERY, requests are requeued and never kicked: after
START_DEV the partition scan hangs under disk->open_mutex, and after
END_USER_RECOVERY every read parks while the command returned 0. The
server cannot fetch the canceled commands again, so the device can't
serve I/O until it restarts anyway.

Return -EBUSY from START_DEV and END_USER_RECOVERY while ub->canceling
is set. In ublk_ctrl_start_dev() check it and publish ub->ub_disk in one
cancel_mutex section, and have ublk_start_cancel() read the disk in its
cancel_mutex section. Today ublk_start_cancel() samples the disk before
taking the mutex, so a server dying during its own START_DEV can mark
the queues without quiescing a disk START_DEV published in between, with
its first I/O past the canceling check. Now either START_DEV sees the
cancel, or the cancel sees the disk and quiesces it before marking. The
END_USER_RECOVERY check is best effort: the disk exists there, and a
cancel after it is the ordinary death of the new server, which
ublk_start_cancel() handles by quiescing and marking.

Assisted-by: LLM
Reviewed-by: Ming Lei <tom.leiming@gmail.com>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
v2: -EBUSY instead of -ENODEV, the device isn't gone (Ming)

This applies on top of Ming's "[PATCH 0/8] ublk: don't dispatch to
canceled io commands" and needs patch 1 of it for ub->canceling to stay
set for the whole FETCH round.

 Documentation/block/ublk.rst | 10 ++++++++--
 drivers/block/ublk_drv.c     | 38 ++++++++++++++++++++++++++++++++++--
 2 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/Documentation/block/ublk.rst b/Documentation/block/ublk.rst
index 28300fee22bf..6c5536ebd987 100644
--- a/Documentation/block/ublk.rst
+++ b/Documentation/block/ublk.rst
@@ -118,7 +118,11 @@ managing and controlling ublk devices with help of several control commands:
   After the server prepares userspace resources (such as creating I/O handler
   threads & io_uring for handling ublk IO), this command is sent to the
   driver for allocating & exposing ``/dev/ublkb*``. Parameters set via
-  ``UBLK_CMD_SET_PARAMS`` are applied for creating the device.
+  ``UBLK_CMD_SET_PARAMS`` are applied for creating the device. The command
+  fails with ``-EBUSY`` if an I/O command fetched by the current server
+  was canceled, because its io_uring is gone. The server can't fetch it
+  again, and the device can be started again once the server has closed
+  ``/dev/ublkc*``.
 
 - ``UBLK_CMD_STOP_DEV``
 
@@ -195,7 +199,9 @@ managing and controlling ublk devices with help of several control commands:
   command is accepted after ublk device is quiesced and a new process has
   opened ``/dev/ublkc*`` and get all ublk queues be ready. When this command
   returns, ublk device is unquiesced and new I/O requests are passed to the
-  new process.
+  new process. It fails with ``-EBUSY`` if an I/O command of the new
+  process was canceled already. The recovery can be started over once the
+  new process has closed ``/dev/ublkc*``.
 
 - user recovery feature description
 
diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index f57d544c1da2..015aff07703c 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -2759,9 +2759,11 @@ static void ublk_abort_queue(struct ublk_device *ub, struct ublk_queue *ubq)
 
 static void ublk_start_cancel(struct ublk_device *ub)
 {
-	struct gendisk *disk = ublk_get_disk(ub);
+	struct gendisk *disk;
 
+	/* sync with ublk_ctrl_start_dev() publishing the disk */
 	mutex_lock(&ub->cancel_mutex);
+	disk = ublk_get_disk(ub);
 	if (ub->canceling)
 		goto out;
 
@@ -4575,6 +4577,7 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub,
 		.dma_alignment		= 3,
 	};
 	struct gendisk *disk;
+	bool canceled;
 	int ret = -EINVAL;
 
 	if (ublksrv_pid <= 0)
@@ -4665,8 +4668,24 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub,
 	disk->fops = &ub_fops;
 	disk->private_data = ub;
 
+	/*
+	 * A command of this FETCH round was canceled and can't be fetched
+	 * again, don't bring up a disk over it.  Check and publish the disk
+	 * in one cancel_mutex section: either this sees ub->canceling, or
+	 * ublk_start_cancel() sees the disk and quiesces it before marking
+	 * the queues.
+	 */
+	mutex_lock(&ub->cancel_mutex);
+	canceled = ub->canceling;
+	if (!canceled)
+		ub->ub_disk = disk;
+	mutex_unlock(&ub->cancel_mutex);
+	if (canceled) {
+		put_disk(disk);
+		ret = -EBUSY;
+		goto out_unlock;
+	}
 	ub->dev_info.ublksrv_pid = ub->ublksrv_tgid;
-	ub->ub_disk = disk;
 
 	ublk_apply_params(ub);
 
@@ -5238,6 +5257,7 @@ static int ublk_ctrl_end_recovery(struct ublk_device *ub,
 		const struct ublksrv_ctrl_cmd *header)
 {
 	int ublksrv_pid = (int)header->data[0];
+	bool canceled;
 	int ret = -EINVAL;
 
 	pr_devel("%s: Waiting for all FETCH_REQs, dev id %d...\n", __func__,
@@ -5261,6 +5281,20 @@ static int ublk_ctrl_end_recovery(struct ublk_device *ub,
 		ret = -EBUSY;
 		goto out_unlock;
 	}
+
+	/*
+	 * As in ublk_ctrl_start_dev(), a canceled command can't be fetched
+	 * again.  Best effort: the disk exists here, and a cancel after this
+	 * check is the ordinary death of the new server, which
+	 * ublk_start_cancel() handles by quiescing and marking.
+	 */
+	mutex_lock(&ub->cancel_mutex);
+	canceled = ub->canceling;
+	mutex_unlock(&ub->cancel_mutex);
+	if (canceled) {
+		ret = -EBUSY;
+		goto out_unlock;
+	}
 	ub->dev_info.ublksrv_pid = ub->ublksrv_tgid;
 	ub->dev_info.state = UBLK_S_DEV_LIVE;
 	pr_devel("%s: new ublksrv_pid %d, dev id %d\n",
-- 
2.55.0

[PATCH 1/4] ublk: don't cancel commands in QUIESCE_DEV on a device that isn't live

[Josef Bacik]

QUIESCE_DEV on a device which is not LIVE returns 0 as "already in
expected state", but ublk_cancel_dev() still runs after ub->mutex is
dropped. A device which is QUIESCED because its server died may have a
new server fetching its commands for recovery at that point, and the
cancel completes them without marking anything:

    new server                       QUIESCE_DEV
    START_USER_RECOVERY
    FETCH part of the queue
                                     device is QUIESCED, ret = 0
                                     ublk_cancel_dev()
                                       io->cmd = NULL, command done
    FETCH the rest of the queue
    END_USER_RECOVERY
      device goes LIVE
                                     ublk_queue_rq()
                                       NULL io->cmd

Skip the cancel when the device was not LIVE. Its server is gone, so
there is nothing of the server QUIESCE_DEV is meant for left to cancel.

Fixes: b465ae7b2524 ("ublk: add feature UBLK_F_QUIESCE")
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 drivers/block/ublk_drv.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 39eb7775a351..0bd0b95b3217 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -5413,6 +5413,7 @@ static int ublk_ctrl_quiesce_dev(struct ublk_device *ub,
 	/* zero means wait forever */
 	u64 timeout_ms = header->data[0];
 	struct gendisk *disk;
+	bool live = true;
 	int ret = -ENODEV;
 
 	if (!(ub->dev_info.flags & UBLK_F_QUIESCE))
@@ -5427,8 +5428,15 @@ static int ublk_ctrl_quiesce_dev(struct ublk_device *ub,
 
 	ret = 0;
 	/* already in expected state */
-	if (ub->dev_info.state != UBLK_S_DEV_LIVE)
+	if (ub->dev_info.state != UBLK_S_DEV_LIVE) {
+		/*
+		 * Nothing to cancel either: the server this was meant for is
+		 * gone, and the commands of the next one, which may be fetching
+		 * them for recovery right now, are not ours to cancel.
+		 */
+		live = false;
 		goto put_disk;
+	}
 
 	/* Mark the device as canceling */
 	mutex_lock(&ub->cancel_mutex);
@@ -5447,7 +5455,7 @@ static int ublk_ctrl_quiesce_dev(struct ublk_device *ub,
 	mutex_unlock(&ub->mutex);
 
 	/* Cancel pending uring_cmd */
-	if (!ret)
+	if (!ret && live)
 		ublk_cancel_dev(ub);
 	return ret;
 }
-- 
2.55.0

Re: [PATCH] ublk: refuse to go live after an io command was canceled

[Ming Lei]

On Mon, Oct 05, 2026 at 04:23:51PM +0000, Josef Bacik wrote:
> Since commit "ublk: keep a canceled FETCH round canceling until the
> server is gone", a device whose FETCH round saw a cancel keeps its
> queues canceling until the server goes away, but START_DEV and
> END_USER_RECOVERY still bring it up. Without UBLK_F_USER_RECOVERY, or
> with UBLK_F_USER_RECOVERY_FAIL_IO, every request of the new disk fails.
> With UBLK_F_USER_RECOVERY, requests are requeued and never kicked: after
> START_DEV the partition scan hangs under disk->open_mutex, and after
> END_USER_RECOVERY every read parks while the command returned 0. The
> server cannot fetch the canceled commands again, so the device can't
> serve I/O until it restarts anyway.
> 
> Return -ENODEV from START_DEV and END_USER_RECOVERY while ub->canceling
> is set. In ublk_ctrl_start_dev() check it and publish ub->ub_disk in one
> cancel_mutex section, and have ublk_start_cancel() read the disk in its
> cancel_mutex section. Today ublk_start_cancel() samples the disk before
> taking the mutex, so a server dying during its own START_DEV can mark
> the queues without quiescing a disk START_DEV published in between, with
> its first I/O past the canceling check. Now either START_DEV sees the
> cancel, or the cancel sees the disk and quiesces it before marking. The
> END_USER_RECOVERY check is best effort: the disk exists there, and a
> cancel after it is the ordinary death of the new server, which
> ublk_start_cancel() handles by quiescing and marking.
> 
> Assisted-by: LLM
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
> This applies on top of Ming's "[PATCH 0/8] ublk: don't dispatch to
> canceled io commands" and needs patch 1 of it for ub->canceling to stay
> set for the whole FETCH round. generic_18 still passes with it.
> 
>  Documentation/block/ublk.rst | 10 ++++++++--
>  drivers/block/ublk_drv.c     | 38 ++++++++++++++++++++++++++++++++++--
>  2 files changed, 44 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/block/ublk.rst b/Documentation/block/ublk.rst
> index 28300fee22bf..b7875a3cf3fc 100644
> --- a/Documentation/block/ublk.rst
> +++ b/Documentation/block/ublk.rst
> @@ -118,7 +118,11 @@ managing and controlling ublk devices with help of several control commands:
>    After the server prepares userspace resources (such as creating I/O handler
>    threads & io_uring for handling ublk IO), this command is sent to the
>    driver for allocating & exposing ``/dev/ublkb*``. Parameters set via
> -  ``UBLK_CMD_SET_PARAMS`` are applied for creating the device.
> +  ``UBLK_CMD_SET_PARAMS`` are applied for creating the device. The command
> +  fails with ``-ENODEV`` if an I/O command fetched by the current server
> +  was canceled, because its io_uring is gone. The server can't fetch it
> +  again, and the device can be started again once the server has closed
> +  ``/dev/ublkc*``.
>  
>  - ``UBLK_CMD_STOP_DEV``
>  
> @@ -195,7 +199,9 @@ managing and controlling ublk devices with help of several control commands:
>    command is accepted after ublk device is quiesced and a new process has
>    opened ``/dev/ublkc*`` and get all ublk queues be ready. When this command
>    returns, ublk device is unquiesced and new I/O requests are passed to the
> -  new process.
> +  new process. It fails with ``-ENODEV`` if an I/O command of the new
> +  process was canceled already. The recovery can be started over once the
> +  new process has closed ``/dev/ublkc*``.
>  
>  - user recovery feature description
>  
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index f57d544c1da2..39eb7775a351 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -2759,9 +2759,11 @@ static void ublk_abort_queue(struct ublk_device *ub, struct ublk_queue *ubq)
>  
>  static void ublk_start_cancel(struct ublk_device *ub)
>  {
> -	struct gendisk *disk = ublk_get_disk(ub);
> +	struct gendisk *disk;
>  
> +	/* sync with ublk_ctrl_start_dev() publishing the disk */
>  	mutex_lock(&ub->cancel_mutex);
> +	disk = ublk_get_disk(ub);
>  	if (ub->canceling)
>  		goto out;
>  
> @@ -4575,6 +4577,7 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub,
>  		.dma_alignment		= 3,
>  	};
>  	struct gendisk *disk;
> +	bool canceled;
>  	int ret = -EINVAL;
>  
>  	if (ublksrv_pid <= 0)
> @@ -4665,8 +4668,24 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub,
>  	disk->fops = &ub_fops;
>  	disk->private_data = ub;
>  
> +	/*
> +	 * A command of this FETCH round was canceled and can't be fetched
> +	 * again, don't bring up a disk over it.  Check and publish the disk
> +	 * in one cancel_mutex section: either this sees ub->canceling, or
> +	 * ublk_start_cancel() sees the disk and quiesces it before marking
> +	 * the queues.
> +	 */
> +	mutex_lock(&ub->cancel_mutex);
> +	canceled = ub->canceling;
> +	if (!canceled)
> +		ub->ub_disk = disk;
> +	mutex_unlock(&ub->cancel_mutex);
> +	if (canceled) {
> +		put_disk(disk);
> +		ret = -ENODEV;
> +		goto out_unlock;
> +	}
>  	ub->dev_info.ublksrv_pid = ub->ublksrv_tgid;
> -	ub->ub_disk = disk;
>  
>  	ublk_apply_params(ub);
>  
> @@ -5238,6 +5257,7 @@ static int ublk_ctrl_end_recovery(struct ublk_device *ub,
>  		const struct ublksrv_ctrl_cmd *header)
>  {
>  	int ublksrv_pid = (int)header->data[0];
> +	bool canceled;
>  	int ret = -EINVAL;
>  
>  	pr_devel("%s: Waiting for all FETCH_REQs, dev id %d...\n", __func__,
> @@ -5261,6 +5281,20 @@ static int ublk_ctrl_end_recovery(struct ublk_device *ub,
>  		ret = -EBUSY;
>  		goto out_unlock;
>  	}
> +
> +	/*
> +	 * As in ublk_ctrl_start_dev(), a canceled command can't be fetched
> +	 * again.  Best effort: the disk exists here, and a cancel after this
> +	 * check is the ordinary death of the new server, which
> +	 * ublk_start_cancel() handles by quiescing and marking.
> +	 */
> +	mutex_lock(&ub->cancel_mutex);
> +	canceled = ub->canceling;
> +	mutex_unlock(&ub->cancel_mutex);
> +	if (canceled) {
> +		ret = -ENODEV;
> +		goto out_unlock;
> +	}
>  	ub->dev_info.ublksrv_pid = ub->ublksrv_tgid;
>  	ub->dev_info.state = UBLK_S_DEV_LIVE;
>  	pr_devel("%s: new ublksrv_pid %d, dev id %d\n",

Hi Josef,

Thanks for the follow-up. The check and the cancel_mutex ordering
look right to me, but I'd suggest -EBUSY instead of -ENODEV.

-ENODEV means the ublk device is gone, and it isn't true in the
START_DEV/END_RECOVERY cases.

With -EBUSY:

Reviewed-by: Ming Lei <tom.leiming@gmail.com>


thanks,
Ming

[PATCH 2/4] ublk: drop QUIESCE_DEV's wait for an idle command

[Josef Bacik]

ublk_wait_for_idle_io() is meant to wait until every queue has a command
whose request is not with the server, so that canceling it tells the
server about the quiesce. It never waits: blk_mq_tagset_busy_iter()
only calls ublk_count_busy_req() for started requests, and the callback
counts a request only when it is not started, so nr_busy is always 0
and every queue looks idle.

Making it count would not help either. It runs with ub->mutex held, so
a server which keeps every tag busy would hold up STOP_DEV, recovery
and its own release work for as long as the QUIESCE_DEV timeout, which
is forever by default. Drop it; nothing changes, since it never waited,
and a later patch has QUIESCE_DEV keep canceling until the server has
been told, without ub->mutex held.

The QUIESCE_DEV timeout in data[0] is unused until then.

Fixes: b465ae7b2524 ("ublk: add feature UBLK_F_QUIESCE")
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 drivers/block/ublk_drv.c | 75 ----------------------------------------
 1 file changed, 75 deletions(-)

diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 0bd0b95b3217..bd7126dcf92f 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -5338,80 +5338,9 @@ static int ublk_ctrl_set_size(struct ublk_device *ub, const struct ublksrv_ctrl_
 	return ret;
 }
 
-struct count_busy {
-	const struct ublk_queue *ubq;
-	u16 nr_busy;
-};
-
-static bool ublk_count_busy_req(struct request *rq, void *data)
-{
-	struct count_busy *idle = data;
-
-	if (!blk_mq_request_started(rq) && rq->mq_hctx->driver_data == idle->ubq)
-		idle->nr_busy += 1;
-	return true;
-}
-
-/* uring_cmd is guaranteed to be active if the associated request is idle */
-static bool ubq_has_idle_io(const struct ublk_queue *ubq)
-{
-	struct count_busy data = {
-		.ubq = ubq,
-	};
-
-	blk_mq_tagset_busy_iter(&ubq->dev->tag_set, ublk_count_busy_req, &data);
-	return data.nr_busy < ubq->q_depth;
-}
-
-/* Wait until each hw queue has at least one idle IO */
-static int ublk_wait_for_idle_io(struct ublk_device *ub,
-				 unsigned int timeout_ms)
-{
-	unsigned int elapsed = 0;
-	int ret;
-
-	/*
-	 * For UBLK_F_BATCH_IO ublk server can get notified with existing
-	 * or new fetch command, so needn't wait any more
-	 */
-	if (ublk_dev_support_batch_io(ub))
-		return 0;
-
-	while (elapsed < timeout_ms && !signal_pending(current)) {
-		u16 i, queues_cancelable = 0;
-
-		for (i = 0; i < ub->dev_info.nr_hw_queues; i++) {
-			struct ublk_queue *ubq = ublk_get_queue(ub, i);
-
-			queues_cancelable += !!ubq_has_idle_io(ubq);
-		}
-
-		/*
-		 * Each queue needs at least one active command for
-		 * notifying ublk server
-		 */
-		if (queues_cancelable == ub->dev_info.nr_hw_queues)
-			break;
-
-		msleep(UBLK_REQUEUE_DELAY_MS);
-		elapsed += UBLK_REQUEUE_DELAY_MS;
-	}
-
-	if (signal_pending(current))
-		ret = -EINTR;
-	else if (elapsed >= timeout_ms)
-		ret = -EBUSY;
-	else
-		ret = 0;
-
-	return ret;
-}
-
 static int ublk_ctrl_quiesce_dev(struct ublk_device *ub,
 				 const struct ublksrv_ctrl_cmd *header)
 {
-	/* zero means wait forever */
-	u64 timeout_ms = header->data[0];
 	struct gendisk *disk;
 	bool live = true;
 	int ret = -ENODEV;
@@ -5445,10 +5374,6 @@ static int ublk_ctrl_quiesce_dev(struct ublk_device *ub,
 	blk_mq_unquiesce_queue(disk->queue);
 	mutex_unlock(&ub->cancel_mutex);
 
-	if (!timeout_ms)
-		timeout_ms = UINT_MAX;
-	ret = ublk_wait_for_idle_io(ub, timeout_ms);
-
 put_disk:
 	ublk_put_disk(disk);
 unlock:
-- 
2.55.0

[PATCH 3/4] ublk: give the command back from COMMIT_AND_FETCH on a canceling queue

[Josef Bacik]

COMMIT_AND_FETCH and NEED_GET_DATA publish the server's next command in
io->cmd without a lock. A cancel which runs at the same time can see
the io active and still read the request pointer io->cmd shares its
storage with, or miss the command altogether. QUIESCE_DEV cancels while
the server still commits, and the command published right after its
cancel pass is never completed, which is what leaves the server waiting
for it forever.

Have the issuer decide instead: read ->canceling before publishing, and
on a canceling queue complete the committed request as usual, but give
the new command back with UBLK_IO_RES_ABORT instead of publishing it,
leaving the io canceled the way ublk_cancel_cmd() does. NEED_GET_DATA
sends its request back the way ublk_queue_rq() does on a canceling
queue.

The read and the publish are one RCU read section, so a cancel which
marks the queue and then calls synchronize_rcu() knows every command
published without seeing the mark is in place before it looks, and
every later one comes back from its issuer. That keeps the commit path
free of locks and barriers. ->canceling is read locklessly now, so write
it with WRITE_ONCE().

Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 drivers/block/ublk_drv.c | 66 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 61 insertions(+), 5 deletions(-)

diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index bd7126dcf92f..6717dabf3a23 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -2492,6 +2492,9 @@ static void ublk_partition_scan_work(struct work_struct *work)
  * - there are no concurrent reads of ubq->canceling from the queue_rq
  *   path. This can be done by quiescing the queue, or through other
  *   means.
+ *
+ * ublk_commit_io_cmd() reads ubq->canceling locklessly, so it is written
+ * with WRITE_ONCE().
  */
 static void ublk_set_canceling(struct ublk_device *ub, bool canceling)
 	__must_hold(&ub->cancel_mutex)
@@ -2500,7 +2503,7 @@ static void ublk_set_canceling(struct ublk_device *ub, bool canceling)
 
 	ub->canceling = canceling;
 	for (i = 0; i < ub->dev_info.nr_hw_queues; i++)
-		ublk_get_queue(ub, i)->canceling = canceling;
+		WRITE_ONCE(ublk_get_queue(ub, i)->canceling, canceling);
 }
 
 static bool ublk_check_and_reset_active_ref(struct ublk_device *ub)
@@ -3109,7 +3112,7 @@ static void ublk_queue_reset_io_flags(struct ublk_device *ub,
 	 */
 	mutex_lock(&ub->cancel_mutex);
 	if (!ub->canceling)
-		ubq->canceling = false;
+		WRITE_ONCE(ubq->canceling, false);
 	mutex_unlock(&ub->cancel_mutex);
 	ubq->fail_io = false;
 	ubq->force_abort = false;
@@ -3227,6 +3230,49 @@ ublk_fill_io_cmd(struct ublk_io *io, struct io_uring_cmd *cmd)
 	return req;
 }
 
+/*
+ * Instead of ublk_fill_io_cmd() on a canceling queue: the server's new
+ * command is not published, it goes back with UBLK_IO_RES_ABORT, and the
+ * io is left canceled the way ublk_cancel_cmd() leaves it.  Returns the
+ * request the server owned.
+ */
+static struct request *ublk_cancel_io_cmd(struct ublk_queue *ubq,
+					  struct ublk_io *io)
+{
+	struct request *req = io->req;
+
+	spin_lock(&ubq->cancel_lock);
+	io->flags &= ~(UBLK_IO_FLAG_OWNED_BY_SRV | UBLK_IO_FLAG_NEED_GET_DATA);
+	io->flags |= UBLK_IO_FLAG_CANCELED;
+	spin_unlock(&ubq->cancel_lock);
+
+	return req;
+}
+
+/*
+ * Publish the command a COMMIT_AND_FETCH or NEED_GET_DATA brings, unless
+ * the queue is canceling, see ublk_quiesce_cancel().  The read of
+ * ->canceling and the publish are one RCU read section: a cancel which
+ * marks the queue after the read waits in synchronize_rcu() until the
+ * command is published, and claims it then.  Returns whether the command
+ * goes back to the server instead.
+ */
+static bool ublk_commit_io_cmd(struct ublk_queue *ubq, struct ublk_io *io,
+			       struct io_uring_cmd *cmd, struct request **req)
+{
+	bool canceling;
+
+	rcu_read_lock();
+	canceling = READ_ONCE(ubq->canceling);
+	if (likely(!canceling))
+		*req = ublk_fill_io_cmd(io, cmd);
+	else
+		*req = ublk_cancel_io_cmd(ubq, io);
+	rcu_read_unlock();
+
+	return canceling;
+}
+
 /*
  * Call before ublk_fill_io_cmd() publishes @cmd in io->cmd: a control-path
  * cancel may complete any command found there, and io_uring_cmd_done() only
@@ -3465,6 +3511,7 @@ static int ublk_ch_uring_cmd_local(struct io_uring_cmd *cmd,
 	u64 addr = READ_ONCE(ub_src->addr); /* unioned with zone_append_lba */
 	struct request *req;
 	int ret;
+	bool canceled;
 	bool compl;
 
 	WARN_ON_ONCE(issue_flags & IO_URING_F_UNLOCKED);
@@ -3547,7 +3594,7 @@ static int ublk_ch_uring_cmd_local(struct io_uring_cmd *cmd,
 			goto out;
 		io->res = result;
 		ublk_prep_cancel(cmd, issue_flags, ubq, tag);
-		req = ublk_fill_io_cmd(io, cmd);
+		canceled = ublk_commit_io_cmd(ubq, io, cmd, &req);
 		ublk_apply_io_buf(ub, io, cmd, addr, &auto_buf, &buf_idx);
 		if (buf_idx != UBLK_INVALID_BUF_IDX)
 			io_buffer_unregister(cmd, buf_idx, issue_flags);
@@ -3557,6 +3604,10 @@ static int ublk_ch_uring_cmd_local(struct io_uring_cmd *cmd,
 			req->__sector = addr;
 		if (compl)
 			__ublk_complete_rq(req, io, ublk_dev_need_map_io(ub), NULL);
+		if (unlikely(canceled)) {
+			ret = UBLK_IO_RES_ABORT;
+			goto out_done;
+		}
 		break;
 	}
 	case UBLK_IO_NEED_GET_DATA:
@@ -3566,7 +3617,12 @@ static int ublk_ch_uring_cmd_local(struct io_uring_cmd *cmd,
 		 * request
 		 */
 		ublk_prep_cancel(cmd, issue_flags, ubq, tag);
-		req = ublk_fill_io_cmd(io, cmd);
+		if (unlikely(ublk_commit_io_cmd(ubq, io, cmd, &req))) {
+			/* as ublk_queue_rq() does on a canceling queue */
+			__ublk_abort_rq(ubq, req);
+			ret = UBLK_IO_RES_ABORT;
+			goto out_done;
+		}
 		io->buf.addr = addr;
 		if (likely(ublk_get_data(ubq, io, req))) {
 			__ublk_prep_compl_io_cmd(io, req);
@@ -3765,7 +3821,7 @@ static int ublk_batch_unprep_io(struct ublk_queue *ubq,
 	if (ublk_queue_ready(ubq)) {
 		data->ub->nr_queue_ready--;
 		spin_lock(&ubq->cancel_lock);
-		ubq->canceling = true;
+		WRITE_ONCE(ubq->canceling, true);
 		spin_unlock(&ubq->cancel_lock);
 	}
 	ubq->nr_io_ready--;
-- 
2.55.0

[PATCH 4/4] ublk: keep canceling in QUIESCE_DEV until the server's commands are taken

[Josef Bacik]

UBLK_CMD_QUIESCE_DEV can leave the server with a command nothing ever
completes. The server then never exits and the device stays LIVE:

    COMMIT_AND_FETCH                    QUIESCE_DEV
                                        queues marked canceling
                                        ublk_cancel_dev()
                                          request still with the
                                          server, io left alone
    ublk_fill_io_cmd()
      command armed again
    request completed

The same holds for a command whose request was dispatched before the
mark, and for the active fetch command of a UBLK_F_BATCH_IO queue, which
ublk_batch_cancel_queue() leaves to its dispatcher, and the dispatcher
only lets go of it. The kublk selftest server hangs this way within a
few quiesce and recover cycles under fio, on every kind of queue.

Since the previous patch a COMMIT_AND_FETCH on a canceling queue gives
its command back itself, once synchronize_rcu() has passed after the
mark. Keep taking the armed commands until no io of the server owes one
any more, and an active batch fetch command once its dispatcher has put
it back on the list. Stop on the QUIESCE_DEV timeout or a signal, with
-EBUSY or -EINTR. Leave ->force_abort of a batch queue alone, which
ublk_batch_cancel_queue() sets: requests of a recoverable device are
then requeued through ->canceling until the next server is ready, as on
a queue without UBLK_F_BATCH_IO, instead of failed.

The server may go away and a new one start fetching for recovery while
this runs, and its commands are not QUIESCE_DEV's to cancel. Count the
FETCH rounds in ub->fetch_round, which ublk_reset_ch_dev() bumps under
cancel_mutex when it clears ub->canceling, before a new server can
fetch. Each pass checks the round and takes the commands in one
cancel_mutex hold, and stops once the round has changed. The commands
are completed after cancel_mutex is dropped, since the ring's cancel
callback takes cancel_mutex under uring_lock.

Fixes: b465ae7b2524 ("ublk: add feature UBLK_F_QUIESCE")
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 drivers/block/ublk_drv.c | 187 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 186 insertions(+), 1 deletion(-)

diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 6717dabf3a23..f8a5dd7e9404 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -129,6 +129,8 @@ struct ublk_uring_cmd_pdu {
 	union {
 		struct request *req;
 		struct request *req_list;
+		/* chains commands QUIESCE_DEV took, none has a request */
+		struct io_uring_cmd *next_claimed;
 	};
 
 	/*
@@ -296,6 +298,9 @@ struct ublk_queue {
 
 		/* Currently active fetch command (NULL = none active) */
 		struct ublk_batch_fetch_cmd  *active_fcmd;
+
+		/* fetch commands QUIESCE_DEV took, for it to complete */
+		struct list_head quiesce_fcmds;
 	}____cacheline_aligned_in_smp;
 
 	struct ublk_io ios[] __counted_by(q_depth);
@@ -344,6 +349,12 @@ struct ublk_device {
 	 * it is set, no queue clears its ->canceling.
 	 */
 	bool canceling;
+	/*
+	 * Counts FETCH rounds, bumped by ublk_reset_ch_dev() together with
+	 * clearing ->canceling, protected by cancel_mutex.  A cancel aimed at
+	 * one server checks it to leave the next server's commands alone.
+	 */
+	u32 fetch_round;
 	pid_t 	ublksrv_tgid;
 	struct delayed_work	exit_work;
 	struct work_struct	partition_scan_work;
@@ -2432,6 +2443,7 @@ static void ublk_reset_ch_dev(struct ublk_device *ub)
 	/* a new FETCH round starts, the queues stay canceling until ready */
 	mutex_lock(&ub->cancel_mutex);
 	ub->canceling = false;
+	ub->fetch_round++;
 	mutex_unlock(&ub->cancel_mutex);
 
 	/* set to NULL, otherwise new tasks cannot mmap io_cmd_buf */
@@ -4417,6 +4429,7 @@ static int ublk_init_queue(struct ublk_device *ub, u16 q_id)
 		if (ret)
 			goto fail;
 		INIT_LIST_HEAD(&ubq->fcmd_head);
+		INIT_LIST_HEAD(&ubq->quiesce_fcmds);
 	}
 	ub->queues[q_id] = ubq;
 	ubq->dev = ub;
@@ -5394,11 +5407,180 @@ static int ublk_ctrl_set_size(struct ublk_device *ub, const struct ublksrv_ctrl_
 	return ret;
 }
 
+/*
+ * Take the armed commands of a queue off their ios the way ublk_cancel_cmd()
+ * does, and chain them on @claimed.  Only after synchronize_rcu() in
+ * ublk_quiesce_cancel(): no COMMIT_AND_FETCH publishes a command on the
+ * canceling queue any more, see ublk_commit_io_cmd(), so an armed io is
+ * stable here.  Sets @left when an io still owes a command: its request is
+ * with the server, or its dispatch is pending and hands the request to the
+ * server, and either way the server's COMMIT_AND_FETCH gives the command
+ * back.
+ */
+static struct io_uring_cmd *ublk_quiesce_claim_queue(struct ublk_queue *ubq,
+						     struct io_uring_cmd *claimed,
+						     bool *left)
+	__must_hold(&ubq->dev->cancel_mutex)
+{
+	struct ublk_device *ub = ubq->dev;
+	u16 tag;
+
+	for (tag = 0; tag < ubq->q_depth; tag++) {
+		struct ublk_io *io = &ubq->ios[tag];
+		struct io_uring_cmd *cmd = NULL;
+		struct request *req;
+		bool started;
+
+		/* see ublk_cancel_cmd() */
+		req = blk_mq_tag_to_rq(ub->tag_set.tags[ubq->q_id], tag);
+		started = req && blk_mq_request_started(req) && req->tag == tag;
+
+		spin_lock(&ubq->cancel_lock);
+		if (!(io->flags & UBLK_IO_FLAG_CANCELED)) {
+			if ((io->flags & UBLK_IO_FLAG_ACTIVE) && !started) {
+				io->flags |= UBLK_IO_FLAG_CANCELED;
+				cmd = READ_ONCE(io->cmd);
+				io->cmd = NULL;
+			} else if (io->flags & (UBLK_IO_FLAG_ACTIVE |
+						UBLK_IO_FLAG_OWNED_BY_SRV)) {
+				*left = true;
+			}
+		}
+		spin_unlock(&ubq->cancel_lock);
+
+		if (cmd) {
+			ublk_get_uring_cmd_pdu(cmd)->next_claimed = claimed;
+			claimed = cmd;
+		}
+	}
+	return claimed;
+}
+
+/*
+ * The same for a UBLK_F_BATCH_IO queue: move its parked fetch commands to
+ * ->quiesce_fcmds.  The active one is left to its dispatcher, which puts it
+ * back on the list once it is done, and the next pass takes it.  Unlike
+ * ublk_batch_cancel_queue() this leaves ->force_abort alone: requests of a
+ * recoverable device are requeued through ->canceling until the next server
+ * is ready, as on a queue without UBLK_F_BATCH_IO.
+ */
+static void ublk_quiesce_claim_fcmds(struct ublk_queue *ubq, bool *left)
+	__must_hold(&ubq->dev->cancel_mutex)
+{
+	struct ublk_batch_fetch_cmd *fcmd;
+
+	spin_lock(&ubq->evts_lock);
+	list_splice_tail_init(&ubq->fcmd_head, &ubq->quiesce_fcmds);
+	fcmd = READ_ONCE(ubq->active_fcmd);
+	if (fcmd) {
+		list_move(&fcmd->node, &ubq->fcmd_head);
+		*left = true;
+	}
+	spin_unlock(&ubq->evts_lock);
+}
+
+/*
+ * The fetch commands stay linked, and the cancel callback of their ring may
+ * take one off the list first: whoever unlinks one under evts_lock
+ * completes it.
+ */
+static void ublk_quiesce_complete_fcmds(struct ublk_queue *ubq)
+{
+	struct ublk_batch_fetch_cmd *fcmd;
+
+	for (;;) {
+		spin_lock(&ubq->evts_lock);
+		fcmd = list_first_entry_or_null(&ubq->quiesce_fcmds,
+						struct ublk_batch_fetch_cmd, node);
+		if (fcmd)
+			list_del_init(&fcmd->node);
+		spin_unlock(&ubq->evts_lock);
+		if (!fcmd)
+			break;
+
+		io_uring_cmd_done(fcmd->cmd, UBLK_IO_RES_ABORT,
+				  IO_URING_F_UNLOCKED);
+		ublk_batch_free_fcmd(fcmd);
+	}
+}
+
+/*
+ * Cancel the server's commands for QUIESCE_DEV until none of the FETCH
+ * round it marked is left.  One pass is not enough: it has to skip a command
+ * whose request is with the server or whose dispatch is pending, and the
+ * server waits for every command before it exits.  After synchronize_rcu()
+ * a COMMIT_AND_FETCH gives its command back itself, and no new request is
+ * dispatched on a canceling queue, so each remaining command is either
+ * given back by its issuer or armed and taken by a later pass.
+ *
+ * Check the round and take the commands in one cancel_mutex hold, and stop
+ * once the round is over: ublk_reset_ch_dev() bumps it, under cancel_mutex,
+ * before the next server can fetch, and those commands are not ours to
+ * cancel.  Complete what was taken after cancel_mutex is dropped, the
+ * ring's cancel callback takes it under uring_lock.
+ */
+static int ublk_quiesce_cancel(struct ublk_device *ub, u32 round,
+			       unsigned int timeout_ms)
+{
+	unsigned int elapsed = 0;
+
+	/* see ublk_commit_io_cmd() */
+	synchronize_rcu();
+
+	for (;;) {
+		struct io_uring_cmd *claimed = NULL;
+		bool left = false;
+		u16 i;
+
+		mutex_lock(&ub->cancel_mutex);
+		if (ub->fetch_round != round) {
+			mutex_unlock(&ub->cancel_mutex);
+			return 0;
+		}
+		for (i = 0; i < ub->dev_info.nr_hw_queues; i++) {
+			struct ublk_queue *ubq = ublk_get_queue(ub, i);
+
+			if (ublk_support_batch_io(ubq))
+				ublk_quiesce_claim_fcmds(ubq, &left);
+			else
+				claimed = ublk_quiesce_claim_queue(ubq, claimed,
+								   &left);
+		}
+		mutex_unlock(&ub->cancel_mutex);
+
+		while (claimed) {
+			struct io_uring_cmd *cmd = claimed;
+
+			claimed = ublk_get_uring_cmd_pdu(cmd)->next_claimed;
+			io_uring_cmd_done(cmd, UBLK_IO_RES_ABORT,
+					  IO_URING_F_UNLOCKED);
+		}
+		for (i = 0; i < ub->dev_info.nr_hw_queues; i++) {
+			struct ublk_queue *ubq = ublk_get_queue(ub, i);
+
+			if (ublk_support_batch_io(ubq))
+				ublk_quiesce_complete_fcmds(ubq);
+		}
+
+		if (!left)
+			return 0;
+		if (signal_pending(current))
+			return -EINTR;
+		if (elapsed >= timeout_ms)
+			return -EBUSY;
+		msleep(UBLK_REQUEUE_DELAY_MS);
+		elapsed += UBLK_REQUEUE_DELAY_MS;
+	}
+}
+
 static int ublk_ctrl_quiesce_dev(struct ublk_device *ub,
 				 const struct ublksrv_ctrl_cmd *header)
 {
+	/* zero means wait forever */
+	u64 timeout_ms = header->data[0];
 	struct gendisk *disk;
 	bool live = true;
+	u32 round;
 	int ret = -ENODEV;
 
 	if (!(ub->dev_info.flags & UBLK_F_QUIESCE))
@@ -5427,6 +5609,7 @@ static int ublk_ctrl_quiesce_dev(struct ublk_device *ub,
 	mutex_lock(&ub->cancel_mutex);
 	blk_mq_quiesce_queue(disk->queue);
 	ublk_set_canceling(ub, true);
+	round = ub->fetch_round;
 	blk_mq_unquiesce_queue(disk->queue);
 	mutex_unlock(&ub->cancel_mutex);
 
@@ -5437,7 +5620,9 @@ static int ublk_ctrl_quiesce_dev(struct ublk_device *ub,
 
 	/* Cancel pending uring_cmd */
 	if (!ret && live)
-		ublk_cancel_dev(ub);
+		ret = ublk_quiesce_cancel(ub, round,
+					  timeout_ms ? min_t(u64, timeout_ms, UINT_MAX) :
+					  UINT_MAX);
 	return ret;
 }
 
-- 
2.55.0

[PATCH 0/4] ublk: fix UBLK_CMD_QUIESCE_DEV leaving commands behind

[Josef Bacik]

UBLK_CMD_QUIESCE_DEV has two problems the fixes for STOP_DEV and the
FETCH rounds don't touch.

Sent to a device that is not LIVE, it still cancels after it returns 0.
A device whose server died is QUIESCED, and a new server may be
fetching its commands for recovery at that point, so the cancel takes
them without marking anything and END_USER_RECOVERY brings the device
up over NULL io->cmd. Patch 1 makes it cancel nothing then.

On a LIVE device it cancels in one pass, which skips every command
whose request is with the server. The server's COMMIT_AND_FETCH arms
the command again right after, nothing ever completes it, and the
server, which waits for all its commands, never exits. The device stays
LIVE. Same for the active fetch command of a UBLK_F_BATCH_IO queue. The
kublk selftest server hangs this way within a few quiesce and recover
cycles under fio, on every kind of queue.

Patch 2 drops ublk_wait_for_idle_io(), which never waited and would
hold ub->mutex against a stalled server if it did. Patch 3 has
COMMIT_AND_FETCH and NEED_GET_DATA give their new command back on a
canceling queue instead of publishing it, deciding inside an RCU read
section, so the I/O path gains no lock or barrier. Patch 4 has
QUIESCE_DEV wait for that with synchronize_rcu() and then keep taking
the armed commands until the server owes none, bounded by its timeout,
and stop once the server's FETCH round is over, so the next server's
commands are left alone.

QUIESCE_DEV now returns -EBUSY or -EINTR when its timeout or a signal
ends that wait with commands still owed, where it returned 0 after one
pass before.

This applies on top of Ming's "[PATCH 0/8] ublk: don't dispatch to
canceled io commands" [1] and my "ublk: refuse to go live after an io
command was canceled" [2].

Tested under QEMU with KASAN and lockdep. Without the series, 20
quiesce and recover cycles under fio hang in every round on getdata,
zero copy and user copy devices and in some on batch ones, and the
quiesce-twice reproducer oopses. With it, 3 rounds of 20 cycles on each
kind of device pass, the reproducer is fine, and the ublk selftests
including generic_18 pass.

[1] https://lore.kernel.org/linux-block/20261001125422.1364260-1-tom.leiming@gmail.com/
[2] https://lore.kernel.org/linux-block/9b876f2c061abc401ec4b9b3c2529eda.josef@toxicpanda.com/

Thanks,
Josef

Josef Bacik (4):
  ublk: don't cancel commands in QUIESCE_DEV on a device that isn't live
  ublk: drop QUIESCE_DEV's wait for an idle command
  ublk: give the command back from COMMIT_AND_FETCH on a canceling queue
  ublk: keep canceling in QUIESCE_DEV until the server's commands are
    taken

 drivers/block/ublk_drv.c | 286 +++++++++++++++++++++++++++++++--------
 1 file changed, 230 insertions(+), 56 deletions(-)

-- 
2.55.0