[PATCH v2 0/2] ublk: fix infinite loop in ublk server teardown

[PATCH v2 0/2] ublk: fix infinite loop in ublk server teardown

[Uday Shankar]

Fix an infinite loop in ublk server teardown which can arise when a ublk
server dies while recovering a device. Also add a test which
demonstrates the problem and verifies the fix.

Signed-off-by: Uday Shankar <ushankar@purestorage.com>
---
Changes in v2:
- Add support for batched fetch in pre_fetch_io (Ming Lei)
- Link to v1: https://lore.kernel.org/r/20260403-cancel-v1-0-86e5a6b3d3af@purestorage.com

---
Uday Shankar (2):
      ublk: reset per-IO canceled flag on each fetch
      selftests: ublk: test that teardown after incomplete recovery completes

 drivers/block/ublk_drv.c                        | 21 ++++++----
 tools/testing/selftests/ublk/Makefile           |  1 +
 tools/testing/selftests/ublk/fault_inject.c     | 52 +++++++++++++++++++++++--
 tools/testing/selftests/ublk/kublk.c            |  7 ++++
 tools/testing/selftests/ublk/kublk.h            |  3 ++
 tools/testing/selftests/ublk/test_generic_17.sh | 35 +++++++++++++++++
 6 files changed, 108 insertions(+), 11 deletions(-)
---
base-commit: cdd71b7feb3674d14c5edd9c133242ddf9a363f2
change-id: 20260403-cancel-bb6a1292534c

Best regards,
-- 
Uday Shankar <ushankar@purestorage.com>

[PATCH v2 1/2] ublk: reset per-IO canceled flag on each fetch

[Uday Shankar]

If a ublk server starts recovering devices but dies before issuing fetch
commands for all IOs, cancellation of the fetch commands that were
successfully issued may never complete. This is because the per-IO
canceled flag can remain set even after the fetch for that IO has been
submitted - the per-IO canceled flags for all IOs in a queue are reset
together only once all IOs for that queue have been fetched. So if a
nonempty proper subset of the IOs for a queue are fetched when the ublk
server dies, the IOs in that subset will never successfully be canceled,
as their canceled flags remain set, and this prevents ublk_cancel_cmd
from actually calling io_uring_cmd_done on the commands, despite the
fact that they are outstanding.

Fix this by resetting the per-IO cancel flags immediately when each IO
is fetched instead of waiting for all IOs for the queue (which may never
happen).

Signed-off-by: Uday Shankar <ushankar@purestorage.com>
Fixes: 728cbac5fe21 ("ublk: move device reset into ublk_ch_release()")
Reviewed-by: Ming Lei <ming.lei@redhat.com>
Reviewed-by: zhang, the-essence-of-life <zhangweize9@gmail.com>
---
 drivers/block/ublk_drv.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 3ba7da94d31499590a06a8b307ed151919a027cb..92dabeb820344107c9fadfae94396082b933d84e 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -2916,22 +2916,26 @@ static void ublk_stop_dev(struct ublk_device *ub)
 	ublk_cancel_dev(ub);
 }
 
+static void ublk_reset_io_flags(struct ublk_queue *ubq, struct ublk_io *io)
+{
+	/* UBLK_IO_FLAG_CANCELED can be cleared now */
+	spin_lock(&ubq->cancel_lock);
+	io->flags &= ~UBLK_IO_FLAG_CANCELED;
+	spin_unlock(&ubq->cancel_lock);
+}
+
 /* reset per-queue io flags */
 static void ublk_queue_reset_io_flags(struct ublk_queue *ubq)
 {
-	int j;
-
-	/* UBLK_IO_FLAG_CANCELED can be cleared now */
 	spin_lock(&ubq->cancel_lock);
-	for (j = 0; j < ubq->q_depth; j++)
-		ubq->ios[j].flags &= ~UBLK_IO_FLAG_CANCELED;
 	ubq->canceling = false;
 	spin_unlock(&ubq->cancel_lock);
 	ubq->fail_io = false;
 }
 
 /* device can only be started after all IOs are ready */
-static void ublk_mark_io_ready(struct ublk_device *ub, u16 q_id)
+static void ublk_mark_io_ready(struct ublk_device *ub, u16 q_id,
+	struct ublk_io *io)
 	__must_hold(&ub->mutex)
 {
 	struct ublk_queue *ubq = ublk_get_queue(ub, q_id);
@@ -2940,6 +2944,7 @@ static void ublk_mark_io_ready(struct ublk_device *ub, u16 q_id)
 		ub->unprivileged_daemons = true;
 
 	ubq->nr_io_ready++;
+	ublk_reset_io_flags(ubq, io);
 
 	/* Check if this specific queue is now fully ready */
 	if (ublk_queue_ready(ubq)) {
@@ -3202,7 +3207,7 @@ static int ublk_fetch(struct io_uring_cmd *cmd, struct ublk_device *ub,
 	if (!ret)
 		ret = ublk_config_io_buf(ub, io, cmd, buf_addr, NULL);
 	if (!ret)
-		ublk_mark_io_ready(ub, q_id);
+		ublk_mark_io_ready(ub, q_id, io);
 	mutex_unlock(&ub->mutex);
 	return ret;
 }
@@ -3610,7 +3615,7 @@ static int ublk_batch_prep_io(struct ublk_queue *ubq,
 	ublk_io_unlock(io);
 
 	if (!ret)
-		ublk_mark_io_ready(data->ub, ubq->q_id);
+		ublk_mark_io_ready(data->ub, ubq->q_id, io);
 
 	return ret;
 }

-- 
2.34.1

[PATCH v2 2/2] selftests: ublk: test that teardown after incomplete recovery completes

[Uday Shankar]

Before the fix, teardown of a ublk server that was attempting to recover
a device, but died when it had submitted a nonempty proper subset of the
fetch commands to any queue would loop forever. Add a test to verify
that, after the fix, teardown completes. This is done by:

- Adding a new argument to the fault_inject target that causes it die
  after fetching a nonempty proper subset of the IOs to a queue
- Using that argument in a new test while trying to recover an
  already-created device
- Attempting to delete the ublk device at the end of the test; this
  hangs forever if teardown from the fault-injected ublk server never
  completed.

It was manually verified that the test passes with the fix and hangs
without it.

Signed-off-by: Uday Shankar <ushankar@purestorage.com>
---
 tools/testing/selftests/ublk/Makefile           |  1 +
 tools/testing/selftests/ublk/fault_inject.c     | 52 +++++++++++++++++++++++--
 tools/testing/selftests/ublk/kublk.c            |  7 ++++
 tools/testing/selftests/ublk/kublk.h            |  3 ++
 tools/testing/selftests/ublk/test_generic_17.sh | 35 +++++++++++++++++
 5 files changed, 95 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/ublk/Makefile b/tools/testing/selftests/ublk/Makefile
index 8ac2d4a682a1768fb1eb9d2dd2a5d01294a67a03..d338668c5a5fbd73f6d70165455a3551ab13e894 100644
--- a/tools/testing/selftests/ublk/Makefile
+++ b/tools/testing/selftests/ublk/Makefile
@@ -18,6 +18,7 @@ TEST_PROGS += test_generic_10.sh
 TEST_PROGS += test_generic_12.sh
 TEST_PROGS += test_generic_13.sh
 TEST_PROGS += test_generic_16.sh
+TEST_PROGS += test_generic_17.sh
 
 TEST_PROGS += test_batch_01.sh
 TEST_PROGS += test_batch_02.sh
diff --git a/tools/testing/selftests/ublk/fault_inject.c b/tools/testing/selftests/ublk/fault_inject.c
index 3b897f69c014cc73b4b469d816e80284dd21b577..150896e02ff8b3747fdc45eb2a6df7b52e134485 100644
--- a/tools/testing/selftests/ublk/fault_inject.c
+++ b/tools/testing/selftests/ublk/fault_inject.c
@@ -10,11 +10,17 @@
 
 #include "kublk.h"
 
+struct fi_opts {
+	long long delay_ns;
+	bool die_during_fetch;
+};
+
 static int ublk_fault_inject_tgt_init(const struct dev_ctx *ctx,
 				      struct ublk_dev *dev)
 {
 	const struct ublksrv_ctrl_dev_info *info = &dev->dev_info;
 	unsigned long dev_size = 250UL << 30;
+	struct fi_opts *opts = NULL;
 
 	if (ctx->auto_zc_fallback) {
 		ublk_err("%s: not support auto_zc_fallback\n", __func__);
@@ -35,17 +41,52 @@ static int ublk_fault_inject_tgt_init(const struct dev_ctx *ctx,
 	};
 	ublk_set_integrity_params(ctx, &dev->tgt.params);
 
-	dev->private_data = (void *)(unsigned long)(ctx->fault_inject.delay_us * 1000);
+	opts = calloc(1, sizeof(*opts));
+	if (!opts) {
+		ublk_err("%s: couldn't allocate memory for opts\n", __func__);
+		return -ENOMEM;
+	}
+
+	opts->delay_ns = ctx->fault_inject.delay_us * 1000;
+	opts->die_during_fetch = ctx->fault_inject.die_during_fetch;
+	dev->private_data = opts;
+
 	return 0;
 }
 
+static void ublk_fault_inject_pre_fetch_io(struct ublk_thread *t,
+					   struct ublk_queue *q, int tag,
+					   bool batch)
+{
+	struct fi_opts *opts = q->dev->private_data;
+
+	if (!opts->die_during_fetch)
+		return;
+
+	/*
+	 * Each queue fetches its IOs in increasing order of tags, so
+	 * dying just before we're about to fetch tag 1 (regardless of
+	 * what queue we're on) guarantees that we've fetched a nonempty
+	 * proper subset of the tags on that queue.
+	 */
+	if (tag == 1) {
+		/*
+		 * Ensure our commands are actually live in the kernel
+		 * before we die.
+		 */
+		io_uring_submit(&t->ring);
+		raise(SIGKILL);
+	}
+}
+
 static int ublk_fault_inject_queue_io(struct ublk_thread *t,
 				      struct ublk_queue *q, int tag)
 {
 	const struct ublksrv_io_desc *iod = ublk_get_iod(q, tag);
 	struct io_uring_sqe *sqe;
+	struct fi_opts *opts = q->dev->private_data;
 	struct __kernel_timespec ts = {
-		.tv_nsec = (long long)q->dev->private_data,
+		.tv_nsec = opts->delay_ns,
 	};
 
 	ublk_io_alloc_sqes(t, &sqe, 1);
@@ -77,29 +118,34 @@ static void ublk_fault_inject_cmd_line(struct dev_ctx *ctx, int argc, char *argv
 {
 	static const struct option longopts[] = {
 		{ "delay_us", 	1,	NULL,  0  },
+		{ "die_during_fetch", 1, NULL, 0  },
 		{ 0, 0, 0, 0 }
 	};
 	int option_idx, opt;
 
 	ctx->fault_inject.delay_us = 0;
+	ctx->fault_inject.die_during_fetch = false;
 	while ((opt = getopt_long(argc, argv, "",
 				  longopts, &option_idx)) != -1) {
 		switch (opt) {
 		case 0:
 			if (!strcmp(longopts[option_idx].name, "delay_us"))
 				ctx->fault_inject.delay_us = strtoll(optarg, NULL, 10);
+			if (!strcmp(longopts[option_idx].name, "die_during_fetch"))
+				ctx->fault_inject.die_during_fetch = strtoll(optarg, NULL, 10);
 		}
 	}
 }
 
 static void ublk_fault_inject_usage(const struct ublk_tgt_ops *ops)
 {
-	printf("\tfault_inject: [--delay_us us (default 0)]\n");
+	printf("\tfault_inject: [--delay_us us (default 0)] [--die_during_fetch 1]\n");
 }
 
 const struct ublk_tgt_ops fault_inject_tgt_ops = {
 	.name = "fault_inject",
 	.init_tgt = ublk_fault_inject_tgt_init,
+	.pre_fetch_io = ublk_fault_inject_pre_fetch_io,
 	.queue_io = ublk_fault_inject_queue_io,
 	.tgt_io_done = ublk_fault_inject_tgt_io_done,
 	.parse_cmd_line = ublk_fault_inject_cmd_line,
diff --git a/tools/testing/selftests/ublk/kublk.c b/tools/testing/selftests/ublk/kublk.c
index e1c3b3c55e565c8cad6b6fe9b9b764cd244818c0..e5b787ba21754719247bb17e1874313f52c35ed1 100644
--- a/tools/testing/selftests/ublk/kublk.c
+++ b/tools/testing/selftests/ublk/kublk.c
@@ -796,6 +796,8 @@ static void ublk_submit_fetch_commands(struct ublk_thread *t)
 			q = &t->dev->q[q_id];
 			io = &q->ios[tag];
 			io->buf_index = j++;
+			if (q->tgt_ops->pre_fetch_io)
+				q->tgt_ops->pre_fetch_io(t, q, tag, false);
 			ublk_queue_io_cmd(t, io);
 		}
 	} else {
@@ -807,6 +809,8 @@ static void ublk_submit_fetch_commands(struct ublk_thread *t)
 		for (i = 0; i < q->q_depth; i++) {
 			io = &q->ios[i];
 			io->buf_index = i;
+			if (q->tgt_ops->pre_fetch_io)
+				q->tgt_ops->pre_fetch_io(t, q, i, false);
 			ublk_queue_io_cmd(t, io);
 		}
 	}
@@ -983,6 +987,9 @@ static void ublk_batch_setup_queues(struct ublk_thread *t)
 		if (t->q_map[i] == 0)
 			continue;
 
+		if (q->tgt_ops->pre_fetch_io)
+			q->tgt_ops->pre_fetch_io(t, q, 0, true);
+
 		ret = ublk_batch_queue_prep_io_cmds(t, q);
 		ublk_assert(ret >= 0);
 	}
diff --git a/tools/testing/selftests/ublk/kublk.h b/tools/testing/selftests/ublk/kublk.h
index 02f0c55d006b4c791fea4456687d0d8757be7be2..6d1762aa30dfc8e72ff92aa86c8faba12169f644 100644
--- a/tools/testing/selftests/ublk/kublk.h
+++ b/tools/testing/selftests/ublk/kublk.h
@@ -60,6 +60,7 @@ struct stripe_ctx {
 struct fault_inject_ctx {
 	/* fault_inject */
 	unsigned long   delay_us;
+	bool die_during_fetch;
 };
 
 struct dev_ctx {
@@ -138,6 +139,8 @@ struct ublk_tgt_ops {
 	int (*init_tgt)(const struct dev_ctx *ctx, struct ublk_dev *);
 	void (*deinit_tgt)(struct ublk_dev *);
 
+	void (*pre_fetch_io)(struct ublk_thread *t, struct ublk_queue *q,
+			     int tag, bool batch);
 	int (*queue_io)(struct ublk_thread *, struct ublk_queue *, int tag);
 	void (*tgt_io_done)(struct ublk_thread *, struct ublk_queue *,
 			    const struct io_uring_cqe *);
diff --git a/tools/testing/selftests/ublk/test_generic_17.sh b/tools/testing/selftests/ublk/test_generic_17.sh
new file mode 100755
index 0000000000000000000000000000000000000000..2278b5fc9dba523bb1bf8411d22f4f95a28da905
--- /dev/null
+++ b/tools/testing/selftests/ublk/test_generic_17.sh
@@ -0,0 +1,35 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+. "$(cd "$(dirname "$0")" && pwd)"/test_common.sh
+
+ERR_CODE=0
+
+_prep_test "fault_inject" "teardown after incomplete recovery"
+
+# First start and stop a ublk server with device configured for recovery
+dev_id=$(_add_ublk_dev -t fault_inject -r 1)
+_check_add_dev $TID $?
+state=$(__ublk_kill_daemon "${dev_id}" "QUIESCED")
+if [ "$state" != "QUIESCED" ]; then
+        echo "device isn't quiesced($state) after $action"
+        ERR_CODE=255
+fi
+
+# Then recover the device, but use --die_during_fetch to have the ublk
+# server die while a queue has some (but not all) I/Os fetched
+${UBLK_PROG} recover -n "${dev_id}" --foreground -t fault_inject --die_during_fetch 1
+RECOVER_RES=$?
+# 137 is the result when dying of SIGKILL
+if (( RECOVER_RES != 137 )); then
+        echo "recover command exited with unexpected code ${RECOVER_RES}!"
+        ERR_CODE=255
+fi
+
+# Clean up the device. This can only succeed once teardown of the above
+# exited ublk server completes. So if teardown never completes, we will
+# time out here
+_ublk_del_dev "${dev_id}"
+
+_cleanup_test "fault_inject"
+_show_result $TID $ERR_CODE

-- 
2.34.1

Re: [PATCH v2 1/2] ublk: reset per-IO canceled flag on each fetch

[Hannes Reinecke]

On 4/6/26 06:25, Uday Shankar wrote:
> If a ublk server starts recovering devices but dies before issuing fetch
> commands for all IOs, cancellation of the fetch commands that were
> successfully issued may never complete. This is because the per-IO
> canceled flag can remain set even after the fetch for that IO has been
> submitted - the per-IO canceled flags for all IOs in a queue are reset
> together only once all IOs for that queue have been fetched. So if a
> nonempty proper subset of the IOs for a queue are fetched when the ublk
> server dies, the IOs in that subset will never successfully be canceled,
> as their canceled flags remain set, and this prevents ublk_cancel_cmd
> from actually calling io_uring_cmd_done on the commands, despite the
> fact that they are outstanding.
> 
> Fix this by resetting the per-IO cancel flags immediately when each IO
> is fetched instead of waiting for all IOs for the queue (which may never
> happen).
> 
> Signed-off-by: Uday Shankar <ushankar@purestorage.com>
> Fixes: 728cbac5fe21 ("ublk: move device reset into ublk_ch_release()")
> Reviewed-by: Ming Lei <ming.lei@redhat.com>
> Reviewed-by: zhang, the-essence-of-life <zhangweize9@gmail.com>
> ---
>   drivers/block/ublk_drv.c | 21 +++++++++++++--------
>   1 file changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index 3ba7da94d31499590a06a8b307ed151919a027cb..92dabeb820344107c9fadfae94396082b933d84e 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -2916,22 +2916,26 @@ static void ublk_stop_dev(struct ublk_device *ub)
>   	ublk_cancel_dev(ub);
>   }
>   
> +static void ublk_reset_io_flags(struct ublk_queue *ubq, struct ublk_io *io)
> +{
> +	/* UBLK_IO_FLAG_CANCELED can be cleared now */
> +	spin_lock(&ubq->cancel_lock);
> +	io->flags &= ~UBLK_IO_FLAG_CANCELED;
> +	spin_unlock(&ubq->cancel_lock);
> +}
> +
One wonders why we can't use 'set_bit' here, or, rather,
convert 'flags' usage to set_bit().
The spinlock feels a bit silly as it's now per-io, and one would think
that we don't have concurrent accesses to the same io...

>   /* reset per-queue io flags */
>   static void ublk_queue_reset_io_flags(struct ublk_queue *ubq)
>   {
> -	int j;
> -
> -	/* UBLK_IO_FLAG_CANCELED can be cleared now */
>   	spin_lock(&ubq->cancel_lock);
> -	for (j = 0; j < ubq->q_depth; j++)
> -		ubq->ios[j].flags &= ~UBLK_IO_FLAG_CANCELED;
>   	ubq->canceling = false;
>   	spin_unlock(&ubq->cancel_lock);
>   	ubq->fail_io = false;
>   }
Similar here; as we don't loop anymore, why do we need the spinlock?
Isn't WRITE_ONCE() sufficient here?

>   
>   /* device can only be started after all IOs are ready */
> -static void ublk_mark_io_ready(struct ublk_device *ub, u16 q_id)
> +static void ublk_mark_io_ready(struct ublk_device *ub, u16 q_id,
> +	struct ublk_io *io)
>   	__must_hold(&ub->mutex)
>   {
>   	struct ublk_queue *ubq = ublk_get_queue(ub, q_id);
> @@ -2940,6 +2944,7 @@ static void ublk_mark_io_ready(struct ublk_device *ub, u16 q_id)
>   		ub->unprivileged_daemons = true;
>   
>   	ubq->nr_io_ready++;
> +	ublk_reset_io_flags(ubq, io);
>   
>   	/* Check if this specific queue is now fully ready */
>   	if (ublk_queue_ready(ubq)) {
> @@ -3202,7 +3207,7 @@ static int ublk_fetch(struct io_uring_cmd *cmd, struct ublk_device *ub,
>   	if (!ret)
>   		ret = ublk_config_io_buf(ub, io, cmd, buf_addr, NULL);
>   	if (!ret)
> -		ublk_mark_io_ready(ub, q_id);
> +		ublk_mark_io_ready(ub, q_id, io);
>   	mutex_unlock(&ub->mutex);
>   	return ret;
>   }
> @@ -3610,7 +3615,7 @@ static int ublk_batch_prep_io(struct ublk_queue *ubq,
>   	ublk_io_unlock(io);
>   
>   	if (!ret)
> -		ublk_mark_io_ready(data->ub, ubq->q_id);
> +		ublk_mark_io_ready(data->ub, ubq->q_id, io);
>   
>   	return ret;
>   }
> 

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

Re: [PATCH v2 1/2] ublk: reset per-IO canceled flag on each fetch

[Ming Lei]

On Mon, Apr 06, 2026 at 09:22:13AM +0200, Hannes Reinecke wrote:
> On 4/6/26 06:25, Uday Shankar wrote:
> > If a ublk server starts recovering devices but dies before issuing fetch
> > commands for all IOs, cancellation of the fetch commands that were
> > successfully issued may never complete. This is because the per-IO
> > canceled flag can remain set even after the fetch for that IO has been
> > submitted - the per-IO canceled flags for all IOs in a queue are reset
> > together only once all IOs for that queue have been fetched. So if a
> > nonempty proper subset of the IOs for a queue are fetched when the ublk
> > server dies, the IOs in that subset will never successfully be canceled,
> > as their canceled flags remain set, and this prevents ublk_cancel_cmd
> > from actually calling io_uring_cmd_done on the commands, despite the
> > fact that they are outstanding.
> > 
> > Fix this by resetting the per-IO cancel flags immediately when each IO
> > is fetched instead of waiting for all IOs for the queue (which may never
> > happen).
> > 
> > Signed-off-by: Uday Shankar <ushankar@purestorage.com>
> > Fixes: 728cbac5fe21 ("ublk: move device reset into ublk_ch_release()")
> > Reviewed-by: Ming Lei <ming.lei@redhat.com>
> > Reviewed-by: zhang, the-essence-of-life <zhangweize9@gmail.com>
> > ---
> >   drivers/block/ublk_drv.c | 21 +++++++++++++--------
> >   1 file changed, 13 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> > index 3ba7da94d31499590a06a8b307ed151919a027cb..92dabeb820344107c9fadfae94396082b933d84e 100644
> > --- a/drivers/block/ublk_drv.c
> > +++ b/drivers/block/ublk_drv.c
> > @@ -2916,22 +2916,26 @@ static void ublk_stop_dev(struct ublk_device *ub)
> >   	ublk_cancel_dev(ub);
> >   }
> > +static void ublk_reset_io_flags(struct ublk_queue *ubq, struct ublk_io *io)
> > +{
> > +	/* UBLK_IO_FLAG_CANCELED can be cleared now */
> > +	spin_lock(&ubq->cancel_lock);
> > +	io->flags &= ~UBLK_IO_FLAG_CANCELED;
> > +	spin_unlock(&ubq->cancel_lock);
> > +}
> > +
> One wonders why we can't use 'set_bit' here, or, rather,
> convert 'flags' usage to set_bit().

It isn't necessary, because UBLK_F_PER_IO_DAEMON is enabled.

> The spinlock feels a bit silly as it's now per-io, and one would think
> that we don't have concurrent accesses to the same io...

UBLK_IO_FLAG_CANCELED is only used in slow path, yes, it is supposed to be
accessed concurrently.

It could be moved out of io->flags, but we do want to make `struct ublk_io`
held in single cache line.

> 
> >   /* reset per-queue io flags */
> >   static void ublk_queue_reset_io_flags(struct ublk_queue *ubq)
> >   {
> > -	int j;
> > -
> > -	/* UBLK_IO_FLAG_CANCELED can be cleared now */
> >   	spin_lock(&ubq->cancel_lock);
> > -	for (j = 0; j < ubq->q_depth; j++)
> > -		ubq->ios[j].flags &= ~UBLK_IO_FLAG_CANCELED;
> >   	ubq->canceling = false;
> >   	spin_unlock(&ubq->cancel_lock);
> >   	ubq->fail_io = false;
> >   }
> Similar here; as we don't loop anymore, why do we need the spinlock?
> Isn't WRITE_ONCE() sufficient here?

WRITE_ONCE() isn't enough, because we have to make sure that io->cmd is
only completed once, please see ublk_cancel_cmd().

Anyway, all these comments should belong to improvement or new issue,
not a blocker for current bug fix.


Thanks,
Ming

Re: [PATCH v2 2/2] selftests: ublk: test that teardown after incomplete recovery completes

[Ming Lei]

On Sun, Apr 05, 2026 at 10:25:31PM -0600, Uday Shankar wrote:
> Before the fix, teardown of a ublk server that was attempting to recover
> a device, but died when it had submitted a nonempty proper subset of the
> fetch commands to any queue would loop forever. Add a test to verify
> that, after the fix, teardown completes. This is done by:
> 
> - Adding a new argument to the fault_inject target that causes it die
>   after fetching a nonempty proper subset of the IOs to a queue
> - Using that argument in a new test while trying to recover an
>   already-created device
> - Attempting to delete the ublk device at the end of the test; this
>   hangs forever if teardown from the fault-injected ublk server never
>   completed.
> 
> It was manually verified that the test passes with the fix and hangs
> without it.
> 
> Signed-off-by: Uday Shankar <ushankar@purestorage.com>

Reviewed-by: Ming Lei <ming.lei@redhat.com>

Thanks,
Ming

Re: [PATCH v2 0/2] ublk: fix infinite loop in ublk server teardown

[Jens Axboe]

On Sun, 05 Apr 2026 22:25:29 -0600, Uday Shankar wrote:
> Fix an infinite loop in ublk server teardown which can arise when a ublk
> server dies while recovering a device. Also add a test which
> demonstrates the problem and verifies the fix.

Applied, thanks!

[1/2] ublk: reset per-IO canceled flag on each fetch
      commit: 0842186d2c4e67d2f8c8c2d1d779e8acffd41b5b
[2/2] selftests: ublk: test that teardown after incomplete recovery completes
      commit: 320f9b1c6a942a73c26be56742ee1da04f893a4f

Best regards,
-- 
Jens Axboe