[PATCH 0/3] ublk: scan partition in async way

[PATCH 0/3] ublk: scan partition in async way

[Ming Lei]

Hi Guys,

The 1st patch scans partition in async way, so IO hang during partition
scan can be covered by current error handling code.

The 2nd patch adds one test for verifying if hang from scanning partition
can be avoided.

The last patch fixes one selftest/ublk rebuild depending issue.

Ming Lei (3):
  ublk: scan partition in async way
  selftests/ublk: add test for async partition scan
  selftests/ublk: fix Makefile to rebuild on header changes

 drivers/block/ublk_drv.c                      | 32 +++++++-
 tools/testing/selftests/ublk/Makefile         |  7 +-
 tools/testing/selftests/ublk/test_common.sh   | 16 +++-
 .../testing/selftests/ublk/test_generic_15.sh | 80 +++++++++++++++++++
 4 files changed, 126 insertions(+), 9 deletions(-)
 create mode 100755 tools/testing/selftests/ublk/test_generic_15.sh

-- 
2.47.0

[PATCH 1/3] ublk: scan partition in async way

[Ming Lei]

Implement async partition scan to avoid IO hang when reading partition
tables. Similar to nvme_partition_scan_work(), partition scanning is
deferred to a work queue to prevent deadlocks.

When partition scan happens synchronously during add_disk(), IO errors
can cause the partition scan to wait while holding ub->mutex, which
can deadlock with other operations that need the mutex.

Changes:
- Add partition_scan_work to ublk_device structure
- Implement ublk_partition_scan_work() to perform async scan
- Always suppress sync partition scan during add_disk()
- Schedule async work after add_disk() for trusted daemons
- Add flush_work() in ublk_stop_dev() before grabbing ub->mutex

Reported-by: Yoav Cohen <yoav@nvidia.com>
Closes: https://lore.kernel.org/linux-block/DM4PR12MB63280C5637917C071C2F0D65A9A8A@DM4PR12MB6328.namprd12.prod.outlook.com/
Fixes: 71f28f3136af ("ublk_drv: add io_uring based userspace block driver")
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 drivers/block/ublk_drv.c | 32 +++++++++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 49c208457198..21593826ad2d 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -237,6 +237,7 @@ struct ublk_device {
 	bool canceling;
 	pid_t 	ublksrv_tgid;
 	struct delayed_work	exit_work;
+	struct work_struct	partition_scan_work;
 
 	struct ublk_queue       *queues[];
 };
@@ -254,6 +255,20 @@ static inline struct request *__ublk_check_and_get_req(struct ublk_device *ub,
 		u16 q_id, u16 tag, struct ublk_io *io, size_t offset);
 static inline unsigned int ublk_req_build_flags(struct request *req);
 
+static void ublk_partition_scan_work(struct work_struct *work)
+{
+	struct ublk_device *ub =
+		container_of(work, struct ublk_device, partition_scan_work);
+
+	if (WARN_ON_ONCE(!test_and_clear_bit(GD_SUPPRESS_PART_SCAN,
+					     &ub->ub_disk->state)))
+		return;
+
+	mutex_lock(&ub->ub_disk->open_mutex);
+	bdev_disk_changed(ub->ub_disk, false);
+	mutex_unlock(&ub->ub_disk->open_mutex);
+}
+
 static inline struct ublksrv_io_desc *
 ublk_get_iod(const struct ublk_queue *ubq, unsigned tag)
 {
@@ -2026,6 +2041,7 @@ static void ublk_stop_dev(struct ublk_device *ub)
 	mutex_lock(&ub->mutex);
 	ublk_stop_dev_unlocked(ub);
 	mutex_unlock(&ub->mutex);
+	flush_work(&ub->partition_scan_work);
 	ublk_cancel_dev(ub);
 }
 
@@ -2954,9 +2970,14 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub,
 
 	ublk_apply_params(ub);
 
-	/* don't probe partitions if any daemon task is un-trusted */
-	if (ub->unprivileged_daemons)
-		set_bit(GD_SUPPRESS_PART_SCAN, &disk->state);
+	/*
+	 * Suppress partition scan to avoid potential IO hang.
+	 * If a path error occurs during partition scan, the IO may wait
+	 * while holding ub->mutex, which can deadlock with other operations
+	 * that need the mutex. Defer partition scan to async work.
+	 * For unprivileged daemons, keep GD_SUPPRESS_PART_SCAN set permanently.
+	 */
+	set_bit(GD_SUPPRESS_PART_SCAN, &disk->state);
 
 	ublk_get_device(ub);
 	ub->dev_info.state = UBLK_S_DEV_LIVE;
@@ -2973,6 +2994,10 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub,
 
 	set_bit(UB_STATE_USED, &ub->state);
 
+	/* Schedule async partition scan for trusted daemons */
+	if (!ub->unprivileged_daemons)
+		schedule_work(&ub->partition_scan_work);
+
 out_put_cdev:
 	if (ret) {
 		ublk_detach_disk(ub);
@@ -3138,6 +3163,7 @@ static int ublk_ctrl_add_dev(const struct ublksrv_ctrl_cmd *header)
 	mutex_init(&ub->mutex);
 	spin_lock_init(&ub->lock);
 	mutex_init(&ub->cancel_mutex);
+	INIT_WORK(&ub->partition_scan_work, ublk_partition_scan_work);
 
 	ret = ublk_alloc_dev_number(ub, header->dev_id);
 	if (ret < 0)
-- 
2.47.0

[PATCH 2/3] selftests/ublk: add test for async partition scan

[Ming Lei]

Add test_generic_15.sh to verify that async partition scan prevents
IO hang when reading partition tables.

The test creates ublk devices with fault_inject target and very large
delay (60s) to simulate blocked partition table reads, then kills the
daemon to verify proper state transitions without hanging:

1. Without recovery support (-r 0):
   - Create device with fault_inject and 60s delay
   - Kill daemon while partition scan may be blocked
   - Verify device transitions to DEAD state within 10s

2. With recovery support (-r 1):
   - Create device with fault_inject, 60s delay, and recovery
   - Kill daemon while partition scan may be blocked
   - Verify device transitions to QUIESCED state within 10s

Before the async partition scan fix, killing the daemon during
partition scan would cause deadlock as partition scan held ub->mutex
while waiting for IO. With the async fix, partition scan happens in
a work function and flush_work() ensures proper synchronization.

Add _add_ublk_dev_no_settle() helper function to skip udevadm settle,
which would otherwise hang waiting for partition scan events to
complete when partition table read is delayed.

Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 tools/testing/selftests/ublk/Makefile         |  1 +
 tools/testing/selftests/ublk/test_common.sh   | 16 +++-
 .../testing/selftests/ublk/test_generic_15.sh | 80 +++++++++++++++++++
 3 files changed, 93 insertions(+), 4 deletions(-)
 create mode 100755 tools/testing/selftests/ublk/test_generic_15.sh

diff --git a/tools/testing/selftests/ublk/Makefile b/tools/testing/selftests/ublk/Makefile
index 837977b62417..eb0e6cfb00ad 100644
--- a/tools/testing/selftests/ublk/Makefile
+++ b/tools/testing/selftests/ublk/Makefile
@@ -22,6 +22,7 @@ TEST_PROGS += test_generic_11.sh
 TEST_PROGS += test_generic_12.sh
 TEST_PROGS += test_generic_13.sh
 TEST_PROGS += test_generic_14.sh
+TEST_PROGS += test_generic_15.sh
 
 TEST_PROGS += test_null_01.sh
 TEST_PROGS += test_null_02.sh
diff --git a/tools/testing/selftests/ublk/test_common.sh b/tools/testing/selftests/ublk/test_common.sh
index 6f1c042de40e..ea9a5f3eb70a 100755
--- a/tools/testing/selftests/ublk/test_common.sh
+++ b/tools/testing/selftests/ublk/test_common.sh
@@ -178,8 +178,9 @@ _have_feature()
 _create_ublk_dev() {
 	local dev_id;
 	local cmd=$1
+	local settle=$2
 
-	shift 1
+	shift 2
 
 	if [ ! -c /dev/ublk-control ]; then
 		return ${UBLK_SKIP_CODE}
@@ -194,7 +195,10 @@ _create_ublk_dev() {
 		echo "fail to add ublk dev $*"
 		return 255
 	fi
-	udevadm settle
+
+	if [ "$settle" = "yes" ]; then
+		udevadm settle
+	fi
 
 	if [[ "$dev_id" =~ ^[0-9]+$ ]]; then
 		echo "${dev_id}"
@@ -204,14 +208,18 @@ _create_ublk_dev() {
 }
 
 _add_ublk_dev() {
-	_create_ublk_dev "add" "$@"
+	_create_ublk_dev "add" "yes" "$@"
+}
+
+_add_ublk_dev_no_settle() {
+	_create_ublk_dev "add" "no" "$@"
 }
 
 _recover_ublk_dev() {
 	local dev_id
 	local state
 
-	dev_id=$(_create_ublk_dev "recover" "$@")
+	dev_id=$(_create_ublk_dev "recover" "yes" "$@")
 	for ((j=0;j<20;j++)); do
 		state=$(_get_ublk_dev_state "${dev_id}")
 		[ "$state" == "LIVE" ] && break
diff --git a/tools/testing/selftests/ublk/test_generic_15.sh b/tools/testing/selftests/ublk/test_generic_15.sh
new file mode 100755
index 000000000000..c241e641c340
--- /dev/null
+++ b/tools/testing/selftests/ublk/test_generic_15.sh
@@ -0,0 +1,80 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+. "$(cd "$(dirname "$0")" && pwd)"/test_common.sh
+
+TID="generic_15"
+ERR_CODE=0
+
+_test_partition_scan_no_hang()
+{
+	local recovery_flag=$1
+	local expected_state=$2
+	local dev_id
+	local state
+	local daemon_pid
+	local start_time
+	local elapsed
+
+	# Create ublk device with fault_inject target and very large delay
+	# to simulate hang during partition table read
+	# --delay_us 60000000 = 60 seconds delay
+	# Use _add_ublk_dev_no_settle to avoid udevadm settle hang waiting
+	# for partition scan events to complete
+	if [ "$recovery_flag" = "yes" ]; then
+		echo "Testing partition scan with recovery support..."
+		dev_id=$(_add_ublk_dev_no_settle -t fault_inject -q 1 -d 1 --delay_us 60000000 -r 1)
+	else
+		echo "Testing partition scan without recovery..."
+		dev_id=$(_add_ublk_dev_no_settle -t fault_inject -q 1 -d 1 --delay_us 60000000)
+	fi
+
+	_check_add_dev "$TID" $?
+
+	# The add command should return quickly because partition scan is async.
+	# Now sleep briefly to let the async partition scan work start and hit
+	# the delay in the fault_inject handler.
+	sleep 1
+
+	# Kill the ublk daemon while partition scan is potentially blocked
+	start_time=${SECONDS}
+	daemon_pid=$(_get_ublk_daemon_pid "${dev_id}")
+
+	# Kill daemon and check state transitions properly
+	state=$(__ublk_kill_daemon "${dev_id}" "${expected_state}")
+
+	elapsed=$((SECONDS - start_time))
+
+	# Verify the device transitioned to expected state
+	if [ "$state" != "${expected_state}" ]; then
+		echo "FAIL: Device state is $state, expected ${expected_state}"
+		ERR_CODE=255
+		${UBLK_PROG} del -n "${dev_id}" > /dev/null 2>&1
+		return
+	fi
+
+	# Verify state transition happened within reasonable time (< 10s)
+	# This ensures we didn't hang waiting for partition scan
+	if [ $elapsed -ge 10 ]; then
+		echo "FAIL: State transition took $elapsed seconds (>= 10s), likely hung on partition scan"
+		ERR_CODE=255
+		${UBLK_PROG} del -n "${dev_id}" > /dev/null 2>&1
+		return
+	fi
+
+	echo "PASS: Device transitioned to ${expected_state} in ${elapsed}s without hanging"
+
+	# Clean up the device
+	${UBLK_PROG} del -n "${dev_id}" > /dev/null 2>&1
+}
+
+_prep_test "partition_scan" "verify async partition scan prevents IO hang"
+
+# Test 1: Without recovery support - should transition to DEAD
+_test_partition_scan_no_hang "no" "DEAD"
+
+# Test 2: With recovery support - should transition to QUIESCED
+_test_partition_scan_no_hang "yes" "QUIESCED"
+
+_cleanup_test "partition_scan"
+_show_result $TID $ERR_CODE
-- 
2.47.0

[PATCH 3/3] selftests/ublk: fix Makefile to rebuild on header changes

[Ming Lei]

Add header dependencies to kublk build rule so that changes to
kublk.h, ublk_dep.h, or utils.h trigger a rebuild.

Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 tools/testing/selftests/ublk/Makefile | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/ublk/Makefile b/tools/testing/selftests/ublk/Makefile
index eb0e6cfb00ad..fb7b2273e563 100644
--- a/tools/testing/selftests/ublk/Makefile
+++ b/tools/testing/selftests/ublk/Makefile
@@ -53,8 +53,10 @@ TEST_GEN_PROGS_EXTENDED = kublk
 
 include ../lib.mk
 
-$(TEST_GEN_PROGS_EXTENDED): kublk.c null.c file_backed.c common.c stripe.c \
-	fault_inject.c
+LOCAL_HDRS += kublk.h ublk_dep.h utils.h
+
+$(TEST_GEN_PROGS_EXTENDED): $(LOCAL_HDRS) \
+	kublk.c null.c file_backed.c common.c stripe.c fault_inject.c
 
 check:
 	shellcheck -x -f gcc *.sh
-- 
2.47.0

Re: [PATCH 1/3] ublk: scan partition in async way

[Caleb Sander Mateos]

On Sun, Dec 21, 2025 at 11:42 AM Ming Lei <ming.lei@redhat.com> wrote:
>
> Implement async partition scan to avoid IO hang when reading partition
> tables. Similar to nvme_partition_scan_work(), partition scanning is
> deferred to a work queue to prevent deadlocks.
>
> When partition scan happens synchronously during add_disk(), IO errors
> can cause the partition scan to wait while holding ub->mutex, which
> can deadlock with other operations that need the mutex.
>
> Changes:
> - Add partition_scan_work to ublk_device structure
> - Implement ublk_partition_scan_work() to perform async scan
> - Always suppress sync partition scan during add_disk()
> - Schedule async work after add_disk() for trusted daemons
> - Add flush_work() in ublk_stop_dev() before grabbing ub->mutex
>
> Reported-by: Yoav Cohen <yoav@nvidia.com>
> Closes: https://lore.kernel.org/linux-block/DM4PR12MB63280C5637917C071C2F0D65A9A8A@DM4PR12MB6328.namprd12.prod.outlook.com/
> Fixes: 71f28f3136af ("ublk_drv: add io_uring based userspace block driver")
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
>  drivers/block/ublk_drv.c | 32 +++++++++++++++++++++++++++++---
>  1 file changed, 29 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index 49c208457198..21593826ad2d 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -237,6 +237,7 @@ struct ublk_device {
>         bool canceling;
>         pid_t   ublksrv_tgid;
>         struct delayed_work     exit_work;
> +       struct work_struct      partition_scan_work;
>
>         struct ublk_queue       *queues[];
>  };
> @@ -254,6 +255,20 @@ static inline struct request *__ublk_check_and_get_req(struct ublk_device *ub,
>                 u16 q_id, u16 tag, struct ublk_io *io, size_t offset);
>  static inline unsigned int ublk_req_build_flags(struct request *req);
>
> +static void ublk_partition_scan_work(struct work_struct *work)
> +{
> +       struct ublk_device *ub =
> +               container_of(work, struct ublk_device, partition_scan_work);
> +
> +       if (WARN_ON_ONCE(!test_and_clear_bit(GD_SUPPRESS_PART_SCAN,
> +                                            &ub->ub_disk->state)))
> +               return;
> +
> +       mutex_lock(&ub->ub_disk->open_mutex);
> +       bdev_disk_changed(ub->ub_disk, false);
> +       mutex_unlock(&ub->ub_disk->open_mutex);
> +}
> +
>  static inline struct ublksrv_io_desc *
>  ublk_get_iod(const struct ublk_queue *ubq, unsigned tag)
>  {
> @@ -2026,6 +2041,7 @@ static void ublk_stop_dev(struct ublk_device *ub)
>         mutex_lock(&ub->mutex);
>         ublk_stop_dev_unlocked(ub);
>         mutex_unlock(&ub->mutex);
> +       flush_work(&ub->partition_scan_work);
>         ublk_cancel_dev(ub);
>  }
>
> @@ -2954,9 +2970,14 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub,
>
>         ublk_apply_params(ub);
>
> -       /* don't probe partitions if any daemon task is un-trusted */
> -       if (ub->unprivileged_daemons)
> -               set_bit(GD_SUPPRESS_PART_SCAN, &disk->state);
> +       /*
> +        * Suppress partition scan to avoid potential IO hang.
> +        * If a path error occurs during partition scan, the IO may wait

What does a "path error" mean for ublk?

Other than that,
Reviewed-by: Caleb Sander Mateos <csander@purestorage.com>

> +        * while holding ub->mutex, which can deadlock with other operations
> +        * that need the mutex. Defer partition scan to async work.
> +        * For unprivileged daemons, keep GD_SUPPRESS_PART_SCAN set permanently.
> +        */
> +       set_bit(GD_SUPPRESS_PART_SCAN, &disk->state);
>
>         ublk_get_device(ub);
>         ub->dev_info.state = UBLK_S_DEV_LIVE;
> @@ -2973,6 +2994,10 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub,
>
>         set_bit(UB_STATE_USED, &ub->state);
>
> +       /* Schedule async partition scan for trusted daemons */
> +       if (!ub->unprivileged_daemons)
> +               schedule_work(&ub->partition_scan_work);
> +
>  out_put_cdev:
>         if (ret) {
>                 ublk_detach_disk(ub);
> @@ -3138,6 +3163,7 @@ static int ublk_ctrl_add_dev(const struct ublksrv_ctrl_cmd *header)
>         mutex_init(&ub->mutex);
>         spin_lock_init(&ub->lock);
>         mutex_init(&ub->cancel_mutex);
> +       INIT_WORK(&ub->partition_scan_work, ublk_partition_scan_work);
>
>         ret = ublk_alloc_dev_number(ub, header->dev_id);
>         if (ret < 0)
> --
> 2.47.0
>

Re: [PATCH 3/3] selftests/ublk: fix Makefile to rebuild on header changes

[Caleb Sander Mateos]

On Sun, Dec 21, 2025 at 11:42 AM Ming Lei <ming.lei@redhat.com> wrote:
>
> Add header dependencies to kublk build rule so that changes to
> kublk.h, ublk_dep.h, or utils.h trigger a rebuild.
>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
>  tools/testing/selftests/ublk/Makefile | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/ublk/Makefile b/tools/testing/selftests/ublk/Makefile
> index eb0e6cfb00ad..fb7b2273e563 100644
> --- a/tools/testing/selftests/ublk/Makefile
> +++ b/tools/testing/selftests/ublk/Makefile
> @@ -53,8 +53,10 @@ TEST_GEN_PROGS_EXTENDED = kublk
>
>  include ../lib.mk
>
> -$(TEST_GEN_PROGS_EXTENDED): kublk.c null.c file_backed.c common.c stripe.c \
> -       fault_inject.c
> +LOCAL_HDRS += kublk.h ublk_dep.h utils.h
> +
> +$(TEST_GEN_PROGS_EXTENDED): $(LOCAL_HDRS) \
> +       kublk.c null.c file_backed.c common.c stripe.c fault_inject.c

I'm not really familiar with the selftests Makefile magic, but will
this end up passing the header files as source files to the cc command
too? That seems a bit wasteful, but probably won't cause any
compilation failures. Maybe it would be better to explicitly list the
.c files to pass to the cc command, which seems to be what most of the
other selftests do.

Best,
Caleb

Re: [PATCH 3/3] selftests/ublk: fix Makefile to rebuild on header changes

[Ming Lei]

On Mon, Dec 22, 2025 at 11:48:37AM -0500, Caleb Sander Mateos wrote:
> On Sun, Dec 21, 2025 at 11:42 AM Ming Lei <ming.lei@redhat.com> wrote:
> >
> > Add header dependencies to kublk build rule so that changes to
> > kublk.h, ublk_dep.h, or utils.h trigger a rebuild.
> >
> > Signed-off-by: Ming Lei <ming.lei@redhat.com>
> > ---
> >  tools/testing/selftests/ublk/Makefile | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/testing/selftests/ublk/Makefile b/tools/testing/selftests/ublk/Makefile
> > index eb0e6cfb00ad..fb7b2273e563 100644
> > --- a/tools/testing/selftests/ublk/Makefile
> > +++ b/tools/testing/selftests/ublk/Makefile
> > @@ -53,8 +53,10 @@ TEST_GEN_PROGS_EXTENDED = kublk
> >
> >  include ../lib.mk
> >
> > -$(TEST_GEN_PROGS_EXTENDED): kublk.c null.c file_backed.c common.c stripe.c \
> > -       fault_inject.c
> > +LOCAL_HDRS += kublk.h ublk_dep.h utils.h
> > +
> > +$(TEST_GEN_PROGS_EXTENDED): $(LOCAL_HDRS) \
> > +       kublk.c null.c file_backed.c common.c stripe.c fault_inject.c
> 
> I'm not really familiar with the selftests Makefile magic, but will
> this end up passing the header files as source files to the cc command
> too? That seems a bit wasteful, but probably won't cause any

I won't do this way, just setup build dependency on local header, please
see `$(OUTPUT)/%:%.c $(LOCAL_HDRS)` in `../lib.mk`.

> compilation failures. Maybe it would be better to explicitly list the
> .c files to pass to the cc command, which seems to be what most of the
> other selftests do.

It can be done in the following way:

diff --git a/tools/testing/selftests/ublk/Makefile b/tools/testing/selftests/ublk/Makefile
index eb0e6cfb00ad..06ba6fde098d 100644
--- a/tools/testing/selftests/ublk/Makefile
+++ b/tools/testing/selftests/ublk/Makefile
@@ -51,10 +51,10 @@ TEST_PROGS += test_stress_07.sh

 TEST_GEN_PROGS_EXTENDED = kublk

+LOCAL_HDRS += $(wildcard *.h)
 include ../lib.mk

-$(TEST_GEN_PROGS_EXTENDED): kublk.c null.c file_backed.c common.c stripe.c \
-       fault_inject.c
+$(TEST_GEN_PROGS_EXTENDED): $(wildcard *.c)

 check:
        shellcheck -x -f gcc *.sh


Thanks,
Ming