[PATCH blktests 0/4] enable bs > ps device testing

[PATCH blktests 0/4] enable bs > ps device testing

[Luis Chamberlain]

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

-- 
2.43.0

[PATCH blktests 1/4] common: add and use min io for fio

[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 <mcgrof@kernel.org>
---
 common/fio |  6 ++++--
 common/rc  | 10 ++++++++++
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/common/fio b/common/fio
index b9ea087fc6c5..5676338d3c97 100644
--- a/common/fio
+++ b/common/fio
@@ -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 "$@"
 }
 
diff --git a/common/rc b/common/rc
index 0c8b51f64291..884677149c9e 100644
--- a/common/rc
+++ b/common/rc
@@ -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() {
-- 
2.43.0

[PATCH blktests 2/4] common/xfs: use min io for fs blocksize

[Luis Chamberlain]

Use the min io for the target block size. Likewise we need to increase
the log size if using a bs > 4096.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 common/xfs | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/common/xfs b/common/xfs
index 569770fecd53..9a13258789e6 100644
--- a/common/xfs
+++ b/common/xfs
@@ -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}"
 }
 
-- 
2.43.0

[PATCH blktests 3/4] tests: use test device min io to support bs > ps

[Luis Chamberlain]

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

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 tests/block/003 | 4 +++-
 tests/block/007 | 3 ++-
 tests/nvme/049  | 5 +++--
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/tests/block/003 b/tests/block/003
index 2af9b89ec3e5..e57d76271b8b 100755
--- a/tests/block/003
+++ b/tests/block/003
@@ -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"
diff --git a/tests/block/007 b/tests/block/007
index 3b68d0deec35..793bb4c45c05 100755
--- a/tests/block/007
+++ b/tests/block/007
@@ -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"
 }
diff --git a/tests/nvme/049 b/tests/nvme/049
index 88d4fb122988..6bb731f84fbf 100755
--- a/tests/nvme/049
+++ b/tests/nvme/049
@@ -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
-- 
2.43.0

[PATCH blktests 4/4] nvme/053: provide time extension alternative

[Luis Chamberlain]

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

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 tests/nvme/053 | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/tests/nvme/053 b/tests/nvme/053
index 3ade8d368be6..65a7b86519bc 100755
--- a/tests/nvme/053
+++ b/tests/nvme/053
@@ -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);
     }
-- 
2.43.0

Re: [PATCH blktests 1/4] common: add and use min io for fio

[Shinichiro Kawasaki]

[-- Attachment #1: Type: text/plain, Size: 4223 bytes --]

Hi Luis, thanks for this patch. Please find my comments in line.

On Dec 18, 2024 / 03:21, Luis Chamberlain wrote:
> 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 <mcgrof@kernel.org>
> ---
>  common/fio |  6 ++++--
>  common/rc  | 10 ++++++++++
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/common/fio b/common/fio
> index b9ea087fc6c5..5676338d3c97 100644
> --- a/common/fio
> +++ b/common/fio
> @@ -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


> +	_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 "$@"
>  }
>  
> diff --git a/common/rc b/common/rc
> index 0c8b51f64291..884677149c9e 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -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.

> +	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"
}

[-- Attachment #2: 0001-check-factor-out-_get_sysfs_path.patch --]
[-- Type: text/plain, Size: 1525 bytes --]

From 9d91c0528f65d01e20b4d326aab2cc63f3304557 Mon Sep 17 00:00:00 2001
From: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
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().

Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
 check | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/check b/check
index dad5e70..6f8010f 100755
--- a/check
+++ b/check
@@ -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
-- 
2.46.2

Re: [PATCH blktests 1/4] common: add and use min io for fio

[Luis Chamberlain]

On Mon, Dec 23, 2024 at 09:37:48AM +0000, Shinichiro Kawasaki wrote:
> Hi Luis, thanks for this patch. Please find my comments in line.
> 
> On Dec 18, 2024 / 03:21, Luis Chamberlain wrote:
> > 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 <mcgrof@kernel.org>
> > ---
> >  common/fio |  6 ++++--
> >  common/rc  | 10 ++++++++++
> >  2 files changed, 14 insertions(+), 2 deletions(-)
> > 
> > diff --git a/common/fio b/common/fio
> > index b9ea087fc6c5..5676338d3c97 100644
> > --- a/common/fio
> > +++ b/common/fio
> > @@ -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.

Sure, I will fix all that, it will make it clearer its not always
TEST_DEV.

> 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

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

[Shinichiro Kawasaki]

On Jan 03, 2025 / 20:10, Luis Chamberlain wrote:
> On Mon, Dec 23, 2024 at 09:37:48AM +0000, Shinichiro Kawasaki wrote:
> > Hi Luis, thanks for this patch. Please find my comments in line.
> > 
> > On Dec 18, 2024 / 03:21, Luis Chamberlain wrote:
> > > 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 <mcgrof@kernel.org>
> > > ---
> > >  common/fio |  6 ++++--
> > >  common/rc  | 10 ++++++++++
> > >  2 files changed, 14 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/common/fio b/common/fio
> > > index b9ea087fc6c5..5676338d3c97 100644
> > > --- a/common/fio
> > > +++ b/common/fio
> > > @@ -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.
> 
> Sure, I will fix all that, it will make it clearer its not always
> TEST_DEV.
> 
> > 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
> 
> 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.

> 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

[Luis Chamberlain]

On Tue, Jan 07, 2025 at 05:19:01AM +0000, Shinichiro Kawasaki wrote:
> On Jan 03, 2025 / 20:10, Luis Chamberlain wrote:
> > On Mon, Dec 23, 2024 at 09:37:48AM +0000, Shinichiro Kawasaki wrote:
> > > Hi Luis, thanks for this patch. Please find my comments in line.
> > > 
> > > On Dec 18, 2024 / 03:21, Luis Chamberlain wrote:
> > > > 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 <mcgrof@kernel.org>
> > > > ---
> > > >  common/fio |  6 ++++--
> > > >  common/rc  | 10 ++++++++++
> > > >  2 files changed, 14 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/common/fio b/common/fio
> > > > index b9ea087fc6c5..5676338d3c97 100644
> > > > --- a/common/fio
> > > > +++ b/common/fio
> > > > @@ -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.
> > 
> > Sure, I will fix all that, it will make it clearer its not always
> > TEST_DEV.
> > 
> > > 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
> > 
> > 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.

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

> If this is the case, I agree with it.

Great.

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

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:

https://docs.google.com/drawings/d/e/2PACX-1vQeZaBq2a0dgg9RDyd_XAJBSH-wbuGCtm95sLp2oFj66oghHWmXunib7tYOTPr84AlQ791VGiaKWvKF/pub?w=1006&h=929

> agree that this statx call approach will be simpler.

OK will follow through with this, thanks.

  Luis