On Sun, Apr 27, 2025 at 6:49 AM Ming Lei <ming.lei@xxxxxxxxxx> wrote:
>
> Commit 57e13a2e8cd2 ("selftests: ublk: support user recovery") starts to
> support UBLK_F_NEED_GET_DATA for covering recovery feature, however the
> ublk utility implementation isn't done correctly.
>
> Fix it by supporting UBLK_F_NEED_GET_DATA correctly.
>
> Also add test generic_07 for covering UBLK_F_NEED_GET_DATA.
>
> Reviewed-by: Caleb Sander Mateos <csander@xxxxxxxxxxxxxxx>
> Fixes: 57e13a2e8cd2 ("selftests: ublk: support user recovery")
> Signed-off-by: Ming Lei <ming.lei@xxxxxxxxxx>
> ---
> tools/testing/selftests/ublk/Makefile | 1 +
> tools/testing/selftests/ublk/kublk.c | 20 ++++++++++------
> tools/testing/selftests/ublk/kublk.h | 1 +
> .../testing/selftests/ublk/test_generic_07.sh | 24 +++++++++++++++++++
> .../testing/selftests/ublk/test_stress_05.sh | 8 +++----
> 5 files changed, 43 insertions(+), 11 deletions(-)
> create mode 100755 tools/testing/selftests/ublk/test_generic_07.sh
>
> diff --git a/tools/testing/selftests/ublk/Makefile b/tools/testing/selftests/ublk/Makefile
> index ec4624a283bc..f34ac0bac696 100644
> --- a/tools/testing/selftests/ublk/Makefile
> +++ b/tools/testing/selftests/ublk/Makefile
> @@ -9,6 +9,7 @@ TEST_PROGS += test_generic_03.sh
> TEST_PROGS += test_generic_04.sh
> TEST_PROGS += test_generic_05.sh
> TEST_PROGS += test_generic_06.sh
> +TEST_PROGS += test_generic_07.sh
>
> TEST_PROGS += test_null_01.sh
> TEST_PROGS += test_null_02.sh
> diff --git a/tools/testing/selftests/ublk/kublk.c b/tools/testing/selftests/ublk/kublk.c
> index e57a1486bb48..3afd45d7f989 100644
> --- a/tools/testing/selftests/ublk/kublk.c
> +++ b/tools/testing/selftests/ublk/kublk.c
> @@ -536,12 +536,17 @@ int ublk_queue_io_cmd(struct ublk_queue *q, struct ublk_io *io, unsigned tag)
> if (!(io->flags & UBLKSRV_IO_FREE))
> return 0;
>
> - /* we issue because we need either fetching or committing */
> + /*
> + * we issue because we need either fetching or committing or
> + * getting data
> + */
> if (!(io->flags &
> - (UBLKSRV_NEED_FETCH_RQ | UBLKSRV_NEED_COMMIT_RQ_COMP)))
> + (UBLKSRV_NEED_FETCH_RQ | UBLKSRV_NEED_COMMIT_RQ_COMP | UBLKSRV_NEED_GET_DATA)))
> return 0;
>
> - if (io->flags & UBLKSRV_NEED_COMMIT_RQ_COMP)
> + if (io->flags & UBLKSRV_NEED_GET_DATA)
> + cmd_op = UBLK_U_IO_NEED_GET_DATA;
> + else if (io->flags & UBLKSRV_NEED_COMMIT_RQ_COMP)
> cmd_op = UBLK_U_IO_COMMIT_AND_FETCH_REQ;
> else if (io->flags & UBLKSRV_NEED_FETCH_RQ)
> cmd_op = UBLK_U_IO_FETCH_REQ;
> @@ -658,6 +663,9 @@ static void ublk_handle_cqe(struct io_uring *r,
> assert(tag < q->q_depth);
> if (q->tgt_ops->queue_io)
> q->tgt_ops->queue_io(q, tag);
> + } else if (cqe->res == UBLK_IO_RES_NEED_GET_DATA) {
> + io->flags |= UBLKSRV_NEED_GET_DATA | UBLKSRV_IO_FREE;
> + ublk_queue_io_cmd(q, io, tag);
> } else {
> /*
> * COMMIT_REQ will be completed immediately since no fetching
> @@ -1313,7 +1321,7 @@ int main(int argc, char *argv[])
>
> opterr = 0;
> optind = 2;
> - while ((opt = getopt_long(argc, argv, "t:n:d:q:r:e:i:az",
> + while ((opt = getopt_long(argc, argv, "t:n:d:q:r:e:i:gaz",
> longopts, &option_idx)) != -1) {
> switch (opt) {
> case 'a':
> @@ -1351,9 +1359,7 @@ int main(int argc, char *argv[])
> ctx.flags |= UBLK_F_USER_RECOVERY | UBLK_F_USER_RECOVERY_REISSUE;
> break;
> case 'g':
> - value = strtol(optarg, NULL, 10);
> - if (value)
> - ctx.flags |= UBLK_F_NEED_GET_DATA;
> + ctx.flags |= UBLK_F_NEED_GET_DATA;
The help text in __cmd_create_help() should be updated accordingly.
> break;
> case 0:
> if (!strcmp(longopts[option_idx].name, "debug_mask"))
> diff --git a/tools/testing/selftests/ublk/kublk.h b/tools/testing/selftests/ublk/kublk.h
> index 918db5cd633f..44ee1e4ac55b 100644
> --- a/tools/testing/selftests/ublk/kublk.h
> +++ b/tools/testing/selftests/ublk/kublk.h
> @@ -115,6 +115,7 @@ struct ublk_io {
> #define UBLKSRV_NEED_FETCH_RQ (1UL << 0)
> #define UBLKSRV_NEED_COMMIT_RQ_COMP (1UL << 1)
> #define UBLKSRV_IO_FREE (1UL << 2)
> +#define UBLKSRV_NEED_GET_DATA (1UL << 3)
> unsigned short flags;
> unsigned short refs; /* used by target code only */
>
> diff --git a/tools/testing/selftests/ublk/test_generic_07.sh b/tools/testing/selftests/ublk/test_generic_07.sh
> new file mode 100755
> index 000000000000..e3ad36ef7b9a
> --- /dev/null
> +++ b/tools/testing/selftests/ublk/test_generic_07.sh
> @@ -0,0 +1,24 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +
> +. "$(cd "$(dirname "$0")" && pwd)"/test_common.sh
> +
> +TID="generic_07"
> +ERR_CODE=0
> +
> +_prep_test "generic" "test UBLK_F_NEED_GET_DATA"
> +
> +_create_backfile 0 256M
> +dev_id=$(_add_ublk_dev -t loop -q 2 -g "${UBLK_BACKFILES[0]}")
> +_check_add_dev $TID $?
> +
> +# run fio over the ublk disk
> +_run_fio_verify_io --filename=/dev/ublkb"${dev_id}" --size=256M
I thought you were planning to add a _have_program fio check?
Best,
Caleb
> +ERR_CODE=$?
> +if [ "$ERR_CODE" -eq 0 ]; then
> + _mkfs_mount_test /dev/ublkb"${dev_id}"
> + ERR_CODE=$?
> +fi
> +
> +_cleanup_test "generic"
> +_show_result $TID $ERR_CODE
> diff --git a/tools/testing/selftests/ublk/test_stress_05.sh b/tools/testing/selftests/ublk/test_stress_05.sh
> index a7071b10224d..88601b48f1cd 100755
> --- a/tools/testing/selftests/ublk/test_stress_05.sh
> +++ b/tools/testing/selftests/ublk/test_stress_05.sh
> @@ -47,15 +47,15 @@ _create_backfile 0 256M
> _create_backfile 1 256M
>
> for reissue in $(seq 0 1); do
> - ublk_io_and_remove 8G -t null -q 4 -g 1 -r 1 -i "$reissue" &
> - ublk_io_and_remove 256M -t loop -q 4 -g 1 -r 1 -i "$reissue" "${UBLK_BACKFILES[0]}" &
> + ublk_io_and_remove 8G -t null -q 4 -g -r 1 -i "$reissue" &
> + ublk_io_and_remove 256M -t loop -q 4 -g -r 1 -i "$reissue" "${UBLK_BACKFILES[0]}" &
> wait
> done
>
> if _have_feature "ZERO_COPY"; then
> for reissue in $(seq 0 1); do
> - ublk_io_and_remove 8G -t null -q 4 -g 1 -z -r 1 -i "$reissue" &
> - ublk_io_and_remove 256M -t loop -q 4 -g 1 -z -r 1 -i "$reissue" "${UBLK_BACKFILES[1]}" &
> + ublk_io_and_remove 8G -t null -q 4 -g -z -r 1 -i "$reissue" &
> + ublk_io_and_remove 256M -t loop -q 4 -g -z -r 1 -i "$reissue" "${UBLK_BACKFILES[1]}" &
> wait
> done
> fi
> --
> 2.47.0
>
