enable bs > ps device testing
cover enable bs > ps device testing
As we move forward with bs > ps block device support, add blktests support for these devices. This leverages min-io so to also ensure we make the IOs optimal and make a uniform way to get this min-io from a file or block device.
Luis Chamberlain (4):
common: add and use min io for fio common/xfs: use min io for fs blocksize
tests: use test device min io to support bs > ps nvme/053: provide time extension alternative
common/fio | 6 ++++-- common/rc | 10 ++++++++++ common/xfs | 8 +++++++- tests/block/003 | 4 +++- tests/block/007 | 3 ++- tests/nvme/049 | 5 +++-- tests/nvme/053 | 11 ++++++----- 7 files changed, 35 insertions(+), 12 deletions(-)
Signature · 1 line
2.43.0
PATCH 1/4 common: add and use min io for fio
When using fio we should not issue IOs smaller than the device supports. Today a lot of places have in place 4k, but soon we will have devices which support bs > ps. For those devices we should check the minimum supported IO.
However, since we also have a min optimal IO, we might as well use that as well. By using this we can also leverage the same lookup with stat whether or not the target file is a block device or a file.
--- common/fio | 6 ++++-- common/rc | 10 ++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-)
common/fio +4 -2
@@ -192,12 +192,14 @@ _run_fio() { # Wrapper around _run_fio used if you need some I/O but don't really care much # about the details _run_fio_rand_io() { - _run_fio --bs=4k --rw=randread --norandommap --numjobs="$(nproc)" \ + local test_dev_bs=$(_test_dev_min_io $TEST_DEV) + _run_fio --bs=$test_dev_bs --rw=randread --norandommap --numjobs="$(nproc)" \ --name=reads --direct=1 "$@" } _run_fio_verify_io() { - _run_fio --name=verify --rw=randwrite --direct=1 --ioengine=libaio --bs=4k \ + local test_dev_bs=$(_test_dev_min_io $TEST_DEV) + _run_fio --name=verify --rw=randwrite --direct=1 --ioengine=libaio --bs=$test_dev_bs \ --iodepth=16 --verify=crc32c --verify_state_save=0 "$@" }
common/rc +10 -0
@@ -387,6 +387,16 @@ _test_dev_is_partition() { [[ -n ${TEST_DEV_PART_SYSFS} ]] } +_test_dev_min_io() { + local any_dev=$1 + if [ -c $any_dev ]; then + if [[ "$any_dev" == /dev/ng* ]]; then + any_dev="${any_dev/ng/nvme}" + fi + fi + stat --printf=%o $any_dev +} + # Return max open zones or max active zones of the test target device. # If the device has both, return smaller value. _test_dev_max_open_active_zones() {
Signature · 1 line
2.43.0
Re: [PATCH blktests 1/4] common: add and use min io for fio
Hi Luis, thanks for this patch. Please find my comments in line.
Quoted 20 lines · Luis Chamberlain
When using fio we should not issue IOs smaller than the device supports. Today a lot of places have in place 4k, but soon we will have devices which support bs > ps. For those devices we should check the minimum supported IO.
However, since we also have a min optimal IO, we might as well use that as well. By using this we can also leverage the same lookup with stat whether or not the target file is a block device or a file.
Signed-off-by Luis Chamberlain <[email protected]>--- common/fio | 6 ++++-- common/rc | 10 ++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-)common/fio +0
-0
common/fio _run_fio() { · line 195
Quoted 5 lines
common/fio +1
-1@@ -192,12 +192,14 @@ _run_fio() { # Wrapper around _run_fio used if you need some I/O but don't really care much # about the details _run_fio_rand_io() { - _run_fio --bs=4k --rw=randread --norandommap --numjobs="$(nproc)" \ + local test_dev_bs=$(_test_dev_min_io $TEST_DEV)
Some of the test cases that calls _run_fio_rand_io can not refer to $TEST_DEV e.g., nvme/040. Instead of $TEST_DEV, the device should be extracted from the --filename=X option in the arguments. I suggest to introduce a helper function as follows (_min_io is the function I suggest to rename from _test_dev_min_io).
# If --filename=file option exists in the given options and if the
# specified file is a block device or a character device, try to get its
# minimum IO size. Otherwise return 4096 as the default minimum IO size.
_fio_opts_to_min_io() {
local arg dev
local -i min_io=4096 for arg in "$@"; do
[[ "$arg" =~ ^--filename= ]] || continue
dev="${arg##*=}"
if [[ -b "$dev" || -c "$dev" ]] &&
! min_io=$(_min_io "$dev"); then
echo "Failed to get min_io from fio opts" >> "$FULL"
return 1
fi
# Keep 4K minimum IO size for historical consistency
((min_io < 4096)) && min_io=4096
break
done
echo "$min_io"
}With this, the desired block size 'bs' can be obtained like this:
bs=$(_fio_opts_to_min_io "$@") || return 1
Quoted 16 lines
common/fio +3
-1+ _run_fio --bs=$test_dev_bs --rw=randread --norandommap --numjobs="$(nproc)" \ --name=reads --direct=1 "$@" } _run_fio_verify_io() { - _run_fio --name=verify --rw=randwrite --direct=1 --ioengine=libaio --bs=4k \ + local test_dev_bs=$(_test_dev_min_io $TEST_DEV) + _run_fio --name=verify --rw=randwrite --direct=1 --ioengine=libaio --bs=$test_dev_bs \ --iodepth=16 --verify=crc32c --verify_state_save=0 "$@" }common/rc +0
-0
common/rc _test_dev_is_partition() { · line 390
Quoted 4 lines
common/rc +1
-0@@ -387,6 +387,16 @@ _test_dev_is_partition() { [[ -n ${TEST_DEV_PART_SYSFS} ]] } +_test_dev_min_io() {
I guess the word "test_dev" in this function name implies $TEST_DEV, but this helper function is called not only for $TEST_DEV, but also for other devices. I suggest simpler name _min_IO for this function.
common/rc _test_dev_is_partition() { · line 397
Quoted 7 lines
common/rc +7
-0+ local any_dev=$1 + if [ -c $any_dev ]; then + if [[ "$any_dev" == /dev/ng* ]]; then + any_dev="${any_dev/ng/nvme}" + fi + fi + stat --printf=%o $any_dev
According to "man stat", "%o" prints "optimal I/O transfer size hint". Then it is not super clear for me that it returns minimum I/O size. Instead, how about to refer to the sysfs queue/minimum_io_size attribute? The following script will do that. It relies on the patch attached, which introduces another new helper function _get_sysfs_path.
_min_io() {
local any_dev sysfs_path
any_dev=$(realpath "$1")
if [[ -c "$any_dev" && "$any_dev" =~ ^/dev/ng ]]; then
any_dev="${any_dev/ng/nvme}"
fi
sysfs_path="$(_get_sysfs_path "$any_dev")"
cat "${sysfs_path}/queue/minimum_io_size"
}From 9d91c0528f65d01e20b4d326aab2cc63f3304557 Mon Sep 17 00:00:00 2001
From: Shin'ichiro Kawasaki <[email protected]>
Date: Mon, 23 Dec 2024 15:27:34 +0900
Subject: [PATCH] check: factor out _get_sysfs_path()
The following change requires the code to get sysfs path from the block device. Such code already exists in _find_sysfs_dirs(). Factor out it to the new function _get_sysfs_path().
--- check | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)
check +15 -4
@@ -644,17 +644,28 @@ _run_group() { return $ret } -_find_sysfs_dirs() { - local test_dev="$1" +_get_sysfs_path() { + local dev="$1" local sysfs_path - local major=$((0x$(stat -L -c '%t' "$test_dev"))) - local minor=$((0x$(stat -L -c '%T' "$test_dev"))) + local major=$((0x$(stat -L -c '%t' "$dev"))) + local minor=$((0x$(stat -L -c '%T' "$dev"))) # Get the canonical sysfs path if ! sysfs_path="$(realpath "/sys/dev/block/${major}:${minor}")"; then return 1 fi + echo "$sysfs_path" +} + +_find_sysfs_dirs() { + local test_dev="$1" + local sysfs_path + + if ! sysfs_path=$(_get_sysfs_path "$test_dev"); then + return 1 + fi + if [[ -r "${sysfs_path}"/partition ]]; then # If the device is a partition device, cut the last device name # of the canonical sysfs path to access to the sysfs of its
Signature · 1 line
2.46.2
Re: [PATCH blktests 1/4] common: add and use min io for fio
Quoted 18 lines · Shinichiro Kawasaki
Hi Luis, thanks for this patch. Please find my comments in line.
Quoted 15 lines · Luis Chamberlain
When using fio we should not issue IOs smaller than the device supports. Today a lot of places have in place 4k, but soon we will have devices which support bs > ps. For those devices we should check the minimum supported IO.
However, since we also have a min optimal IO, we might as well use that as well. By using this we can also leverage the same lookup with stat whether or not the target file is a block device or a file.
Signed-off-by Luis Chamberlain <[email protected]>--- common/fio | 6 ++++-- common/rc | 10 ++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-)
Quoted 14 lines
common/fio _run_fio() { · line 195
Quoted 10 lines
common/fio +1
-1@@ -192,12 +192,14 @@ _run_fio() { # Wrapper around _run_fio used if you need some I/O but don't really care much # about the details _run_fio_rand_io() { - _run_fio --bs=4k --rw=randread --norandommap --numjobs="$(nproc)" \ + local test_dev_bs=$(_test_dev_min_io $TEST_DEV)Some of the test cases that calls
_run_fio_rand_iocan not refer to $TEST_DEV e.g., nvme/040. Instead of $TEST_DEV, the device should be extracted from the --filename=X option in the arguments.
Sure, I will fix all that, it will make it clearer its not always TEST_DEV.
Quoted 10 lines
I suggest to introduce a helper function
as follows (_min_iois the function I suggest to rename from_test_dev_min_io).# If --filename=file option exists in the given options and if the
# specified file is a block device or a character device, try to get its # minimum IO size. Otherwise return 4096 as the default minimum IO size. _fio_opts_to_min_io() { local arg dev local -i min_io=4096
Quoted 10 lines
for arg in "$@"; do
[[ "$arg" =~ ^--filename= ]] || continue
dev="${arg##*=}"
if [[ -b "$dev" || -c "$dev" ]] &&
! min_io=$(_min_io "$dev"); then
echo "Failed to get min_io from fio opts" >> "$FULL"
return 1
fi
# Keep 4K minimum IO size for historical consistency
((min_io < 4096)) && min_io=4096But here the file may be a regular file, and if you use 4k as the default on a 16k XFS filesystem it won't work. This can be simplified because using statx block size would be then be the correct thing to do for a file. Then, it so turns out the the min-io can be obtained by the same statx call to a block device as well.
Thoughts?
Luis
Re: [PATCH blktests 1/4] common: add and use min io for fio
Quoted 58 lines · Luis Chamberlain
Quoted 18 lines · Shinichiro Kawasaki
Hi Luis, thanks for this patch. Please find my comments in line.
Quoted 15 lines · Luis Chamberlain
When using fio we should not issue IOs smaller than the device supports. Today a lot of places have in place 4k, but soon we will have devices which support bs > ps. For those devices we should check the minimum supported IO.
However, since we also have a min optimal IO, we might as well use that as well. By using this we can also leverage the same lookup with stat whether or not the target file is a block device or a file.
Signed-off-by Luis Chamberlain <[email protected]>--- common/fio | 6 ++++-- common/rc | 10 ++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-)Quoted 14 lines
common/fio _run_fio() { · line 195
Quoted 10 lines
common/fio +1
-1@@ -192,12 +192,14 @@ _run_fio() { # Wrapper around _run_fio used if you need some I/O but don't really care much # about the details _run_fio_rand_io() { - _run_fio --bs=4k --rw=randread --norandommap --numjobs="$(nproc)" \ + local test_dev_bs=$(_test_dev_min_io $TEST_DEV)Some of the test cases that calls
_run_fio_rand_iocan not refer to $TEST_DEV e.g., nvme/040. Instead of $TEST_DEV, the device should be extracted from the --filename=X option in the arguments.Sure, I will fix all that, it will make it clearer its not always
TEST_DEV.Quoted 10 lines
I suggest to introduce a helper function
as follows (_min_iois the function I suggest to rename from_test_dev_min_io).# If --filename=file option exists in the given options and if the
# specified file is a block device or a character device, try to get its # minimum IO size. Otherwise return 4096 as the default minimum IO size. _fio_opts_to_min_io() { local arg dev local -i min_io=4096Quoted 10 lines
for arg in "$@"; do [[ "$arg" =~ ^--filename= ]] || continue dev="${arg##*=}" if [[ -b "$dev" || -c "$dev" ]] && ! min_io=$(_min_io "$dev"); then echo "Failed to get min_io from fio opts" >> "$FULL" return 1 fi # Keep 4K minimum IO size for historical consistency ((min_io < 4096)) && min_io=4096
Quoted 2 lines
But here the file may be a regular file, and if you use 4k as the default on a 16k XFS filesystem it won't work.
I guessed that the 16k XFS filesystem will allow 4k writes to regular files (In case this is wrong, correct me). Do you mean that 16k writes will be more appropriate blktests condition on 16k XFS filesystem? If this is the case, I agree with it.
Quoted 4 lines
This can be simplified
because using statx block size would be then be the correct thing to do for a file. Then, it so turns out the the min-io can be obtained by the same statx call to a block device as well.
Ah, now I see why you used the "stat --printf=%o" command.
I find that _xfs_run_fio_verify_io() and zbd/009,010 call _run_fio_verify_io() with the --directory fio option. To cover this case, _fio_opts_to_min_io() above will need change to cover both --filename and --directory options. When the --directory option is specified, we need to get the min_io size from the directory. Assuming that the "stat --printf=%o" command works for directories, I agree that this statx call approach will be simpler.
Re: [PATCH blktests 1/4] common: add and use min io for fio
Quoted 59 lines · Shinichiro Kawasaki
Quoted 58 lines · Luis Chamberlain
Quoted 18 lines · Shinichiro Kawasaki
Hi Luis, thanks for this patch. Please find my comments in line.
Quoted 15 lines · Luis Chamberlain
When using fio we should not issue IOs smaller than the device supports. Today a lot of places have in place 4k, but soon we will have devices which support bs > ps. For those devices we should check the minimum supported IO.
However, since we also have a min optimal IO, we might as well use that as well. By using this we can also leverage the same lookup with stat whether or not the target file is a block device or a file.
Signed-off-by Luis Chamberlain <[email protected]>--- common/fio | 6 ++++-- common/rc | 10 ++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-)Quoted 14 lines
common/fio _run_fio() { · line 195
Quoted 10 lines
common/fio +1
-1@@ -192,12 +192,14 @@ _run_fio() { # Wrapper around _run_fio used if you need some I/O but don't really care much # about the details _run_fio_rand_io() { - _run_fio --bs=4k --rw=randread --norandommap --numjobs="$(nproc)" \ + local test_dev_bs=$(_test_dev_min_io $TEST_DEV)Some of the test cases that calls
_run_fio_rand_iocan not refer to $TEST_DEV e.g., nvme/040. Instead of $TEST_DEV, the device should be extracted from the --filename=X option in the arguments.Sure, I will fix all that, it will make it clearer its not always
TEST_DEV.Quoted 20 lines
I suggest to introduce a helper function
as follows (_min_iois the function I suggest to rename from_test_dev_min_io).# If --filename=file option exists in the given options and if the
# specified file is a block device or a character device, try to get its # minimum IO size. Otherwise return 4096 as the default minimum IO size. _fio_opts_to_min_io() { local arg dev local -i min_io=4096for arg in "$@"; do [[ "$arg" =~ ^--filename= ]] || continue dev="${arg##*=}" if [[ -b "$dev" || -c "$dev" ]] && ! min_io=$(_min_io "$dev"); then echo "Failed to get min_io from fio opts" >> "$FULL" return 1 fi # Keep 4K minimum IO size for historical consistency ((min_io < 4096)) && min_io=4096
Quoted 5 lines
Quoted 2 lines
But here the file may be a regular file, and if you use 4k as the default on a 16k XFS filesystem it won't work.
I guessed that the 16k XFS filesystem will allow 4k writes to regular files (In case this is wrong, correct me).
If the filesystem has a 4k sector size it will be allowed, but if both the fs block size and the fs sector size is 16k then no 4k IOs will be allowed.
Quoted 2 lines
Do you mean that 16k writes will be more appropriate blktests condition on 16k XFS filesystem?
And yes this too. Even if you do allow 4k writes with a 16k fs block size but with a 4k fs sector size, you still want 16k IO writes on this fs. One reason for exampe to want this is to avoid RMW which can happen on 4k IO writes.
Quoted 1 line
If this is the case, I agree with it.
Great.
Quoted 5 lines
Quoted 4 lines
This can be simplified
because using statx block size would be then be the correct thing to do for a file. Then, it so turns out the the min-io can be obtained by the same statx call to a block device as well.
Quoted 7 lines
Ah, now I see why you used the "stat --printf=%o" command.
I find that
_xfs_run_fio_verify_io()and zbd/009,010 call_run_fio_verify_io()with the --directory fio option. To cover this case,_fio_opts_to_min_io()above will need change to cover both --filename and --directory options. When the --directory option is specified, we need to get themin_iosize from the directory. Assuming that the "stat --printf=%o" command works for directories, I
Yes it does, this is with a 64k bs XFS fs:
stat --print=%o /mnt/poo
65536
The same fs statx would be used. Right now only XFS will support LBS so its the only fs where we can test this currently on x86_64.
The usage of statx --print=%o for this purpose is summarized in this diagram:
Quoted 1 line
agree that this statx call approach will be simpler.
OK will follow through with this, thanks.
Luis
PATCH 2/4 common/xfs: use min io for fs blocksize
In reply to Luis ChamberlainUse the min io for the target block size. Likewise we need to increase the log size if using a bs > 4096.
--- common/xfs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
common/xfs +7 -1
@@ -13,10 +13,16 @@ _have_xfs() { _xfs_mkfs_and_mount() { local bdev=$1 local mount_dir=$2 + local bs=$(_test_dev_min_io $bdev) + local xfs_logsize="64m" + + if [[ $bs -gt 4096 ]]; then + xfs_logsize="128m" + fi mkdir -p "${mount_dir}" umount "${mount_dir}" - mkfs.xfs -l size=64m -f "${bdev}" || return $? + mkfs.xfs -l size=$xfs_logsize -f "${bdev}" -b size=$bs || return $? mount "${bdev}" "${mount_dir}" }
Signature · 1 line
2.43.0
PATCH 3/4 tests: use test device min io to support bs > ps
In reply to Luis ChamberlainWhen a block device supports a minimum block size > ps we must ensure we don't issue IOs below what is supported. Just leverage the min optimal IO to also ensure we use the optimal IO as well.
--- tests/block/003 | 4 +++- tests/block/007 | 3 ++- tests/nvme/049 | 5 +++-- 3 files changed, 8 insertions(+), 4 deletions(-)
tests/block/003 +3 -1
@@ -18,10 +18,12 @@ device_requires() { } test_device() { + local test_dev_bs=$(_test_dev_min_io $TEST_DEV) + echo "Running ${TEST_NAME}" FIO_PERF_FIELDS=("trim iops") - _fio_perf --bsrange=4k-4g --rw=randtrim --norandommap --name=discards \ + _fio_perf --bsrange=${test_dev_bs}-4g --rw=randtrim --norandommap --name=discards \ --filename="$TEST_DEV" --number_ios=200k echo "Test complete"
tests/block/007 +2 -1
@@ -31,13 +31,14 @@ cleanup_fallback_device() { } run_fio_job() { + local test_dev_bs=$(_test_dev_min_io $TEST_DEV) if _test_dev_is_rotational; then size="32m" else size="1g" fi - _fio_perf --bs=4k --rw=randread --norandommap --name=reads \ + _fio_perf --bs=$test_dev_bs --rw=randread --norandommap --name=reads \ --filename="$TEST_DEV" --size="$size" --direct=1 \ --ioengine=pvsync2 --hipri="$1" }
tests/nvme/049 +3 -2
@@ -19,10 +19,11 @@ test_device() { echo "Running ${TEST_NAME}" local ngdev=${TEST_DEV/nvme/ng} + local test_dev_bs=$(_test_dev_min_io $ngdev) local common_args=( --size=1M --filename="$ngdev" - --bs=4k + --bs=$test_dev_bs --rw=randread --numjobs=1 --iodepth=16
@@ -35,7 +36,7 @@ test_device() { local fio_output # check security permission - if ! fio_output=$(fio --name=check --size=4k --filename="$ngdev" \ + if ! fio_output=$(fio --name=check --bs=$test_dev_bs --size=32k --filename="$ngdev" \ --rw=read --ioengine=io_uring_cmd 2>&1) && grep -q -e "Operation not permitted" \ -e "Permission denied" <<< "$fio_output"; then
Signature · 1 line
2.43.0
PATCH 4/4 nvme/053: provide time extension alternative
In reply to Luis ChamberlainI get this failure when I run this test:
awk: ...rescan.awk:2: warning: The time extension is obsolete. Use the timex extension from gawkextlib
I can't find this extension either, so just use systime() and use system(sleep) for the sleep command.
--- tests/nvme/053 | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
tests/nvme/053 +6 -5
@@ -27,13 +27,14 @@ rescan_controller() { create_rescan_script() { cat >"$TMPDIR/rescan.awk" <<EOF -@load "time" - BEGIN { srand(seed); - finish = gettimeofday() + strtonum(timeout); - while (gettimeofday() < finish) { - sleep(0.1 + 5 * rand()); + start_time = systime(); + finish = start_time + strtonum(timeout); + while (systime() < finish) { + sleep_time = 0.1 + 5 * rand(); + cmd = "sleep " sleep_time; + system(cmd); printf("1\n") > path; close(path); }
Signature · 1 line
2.43.0