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 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 --- 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 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 --- 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 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 --- 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