[PATCH v3 0/3] t/zbd: Fix max_open_zones related failures

[PATCH v3 0/3] t/zbd: Fix max_open_zones related failures

[Shin'ichiro Kawasaki]

Recent changes in fio [1] triggered three failures in the test script
t/zbd/test-zoned-support. This series address the failures.

The first failure was caused by difference between max_open_zones value that fio
refers and that test script refers. The first patch modifies the test script to
use the value fio refers.

The second failure was observed when users specify special max_open_zones values
larger than the max_open_zones limit of the test target device. The second patch
adds --ignore_zone_limits option to the fio command only when users specify such
special conditions.

The third failure was caused by write target zone count bug in the test case 31.
Data write to more zones than max_open_zones limit resulted in exceeding
max_open_zones limit. The third patch fixes the write target zone count logic.

[1] https://www.spinics.net/lists/fio/msg09502.html

Changes from v2:
* Improved the comment to explain why the test script gets max_open_zones
* Added Reviewed-by tags

Changes from v1:
* Added comment to explain why the test script gets max_open_zones
* Reflected other comments and added Reviewed-by tags

Shin'ichiro Kawasaki (3):
  t/zbd: Use max_open_zones that fio fetched from device
  t/zbd: Add ignore_zone_limit option to test with special
    max_open_zones
  t/zbd: Fix write target zones counting in test case #31

 t/zbd/functions        | 14 +++++++++++---
 t/zbd/test-zbd-support | 37 +++++++++++++++++--------------------
 2 files changed, 28 insertions(+), 23 deletions(-)

-- 
2.31.1

[PATCH v3 1/3] t/zbd: Use max_open_zones that fio fetched from device

[Shin'ichiro Kawasaki]

Recent commit d2f442bc0bd5 ("ioengines: add get_max_open_zones zoned
block device operation") modified fio to compare --max_open_zones option
value and max_open_zones reported by the device. The device limit is
fetched through sysfs or through an ioengine specific implementation.

The test script currently try to fetch the max open zones limit using
libzbc tools or sg_inq. If either of these fail, default value 128 is
supplied. This default value can be too high when the test script is
run for certain zoned block devices, and can therefore result in fio
error and test case failure.

To avoid the failure, modify the default value used in the test script
from 128 to 0. With this, --max_open_zones=0 is passed to fio, and it
makes fio use the max_open_zones reported by the device. Also add
comments to describe why the test script gets max_open_zones with tools.

Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
 t/zbd/functions | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/t/zbd/functions b/t/zbd/functions
index 40ffe1de..08a2c629 100644
--- a/t/zbd/functions
+++ b/t/zbd/functions
@@ -173,15 +173,23 @@ last_online_zone() {
     fi
 }
 
+# Get max_open_zones of SMR drives using sg_inq or libzbc tools. Two test cases
+# 31 and 32 use this max_open_zones value. The test case 31 uses max_open_zones
+# to decide number of write target zones. The test case 32 passes max_open_zones
+# value to fio with --max_open_zones option. Of note is that fio itself has the
+# feature to get max_open_zones from the device through sysfs or ioengine
+# specific implementation. This max_open_zones fetch by test script is required
+# in case fio is running on an old Linux kernel version which lacks
+# max_open_zones in sysfs, or which lacks zoned block device support completely.
 max_open_zones() {
     local dev=$1
 
     if [ -n "${sg_inq}" ] && [ ! -n "${use_libzbc}" ]; then
 	if ! ${sg_inq} -e --page=0xB6 --len=20 --hex "$dev" \
 		 > /dev/null 2>&1; then
-	    # Non scsi device such as null_blk can not return max open zones.
-	    # Use default value.
-	    echo 128
+	    # When sg_inq can not get max open zones, specify 0 which indicates
+	    # fio to get max open zones limit from the device.
+	    echo 0
 	else
 	    ${sg_inq} -e --page=0xB6 --len=20 --hex "$dev" | tail -1 |
 		{
-- 
2.31.1

Re: [PATCH v3 1/3] t/zbd: Use max_open_zones that fio fetched from device

[Niklas Cassel]

On Fri, Jun 04, 2021 at 08:32:50PM +0900, Shin'ichiro Kawasaki wrote:
> Recent commit d2f442bc0bd5 ("ioengines: add get_max_open_zones zoned
> block device operation") modified fio to compare --max_open_zones option
> value and max_open_zones reported by the device. The device limit is
> fetched through sysfs or through an ioengine specific implementation.
> 
> The test script currently try to fetch the max open zones limit using
> libzbc tools or sg_inq. If either of these fail, default value 128 is
> supplied. This default value can be too high when the test script is
> run for certain zoned block devices, and can therefore result in fio
> error and test case failure.
> 
> To avoid the failure, modify the default value used in the test script
> from 128 to 0. With this, --max_open_zones=0 is passed to fio, and it
> makes fio use the max_open_zones reported by the device. Also add
> comments to describe why the test script gets max_open_zones with tools.
> 
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> ---
>  t/zbd/functions | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/t/zbd/functions b/t/zbd/functions
> index 40ffe1de..08a2c629 100644
> --- a/t/zbd/functions
> +++ b/t/zbd/functions
> @@ -173,15 +173,23 @@ last_online_zone() {
>      fi
>  }
>  
> +# Get max_open_zones of SMR drives using sg_inq or libzbc tools. Two test cases
> +# 31 and 32 use this max_open_zones value. The test case 31 uses max_open_zones
> +# to decide number of write target zones. The test case 32 passes max_open_zones
> +# value to fio with --max_open_zones option. Of note is that fio itself has the
> +# feature to get max_open_zones from the device through sysfs or ioengine
> +# specific implementation. This max_open_zones fetch by test script is required
> +# in case fio is running on an old Linux kernel version which lacks
> +# max_open_zones in sysfs, or which lacks zoned block device support completely.
>  max_open_zones() {
>      local dev=$1
>  
>      if [ -n "${sg_inq}" ] && [ ! -n "${use_libzbc}" ]; then
>  	if ! ${sg_inq} -e --page=0xB6 --len=20 --hex "$dev" \
>  		 > /dev/null 2>&1; then
> -	    # Non scsi device such as null_blk can not return max open zones.
> -	    # Use default value.
> -	    echo 128
> +	    # When sg_inq can not get max open zones, specify 0 which indicates
> +	    # fio to get max open zones limit from the device.
> +	    echo 0
>  	else
>  	    ${sg_inq} -e --page=0xB6 --len=20 --hex "$dev" | tail -1 |
>  		{
> -- 
> 2.31.1
> 

Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com>

[PATCH v3 2/3] t/zbd: Add ignore_zone_limit option to test with special max_open_zones

[Shin'ichiro Kawasaki]

Recent commit d2f442bc0bd5 ("ioengines: add get_max_open_zones zoned
block device operation") modified fio to compare --max_open_zones option
value and max_open_zones reported by the device. When the option
--max_open_zones is larger than the device limit, fio exits with an
error. However, sometimes it is useful to run fio with --max_open_zones
larger than the device limit to check performance impact of implicit
zone open and close by the zoned block devices. The test script
t/zbd/test-zbd-support has an option -o so that users can specify such
larger max_open_zones value. After the commit, such test runs fail with
the fio error.

To avoid the failure, modify the test script to specify another option
--ignore_zone_limits to fio command, which was added by the commit
575686bb85fa (zbd: add a new --ignore_zone_limits option). This option
is added to fio command only when users specify -o option and special
max_open_zones value to the test script. This change does not affect
default test conditions.

Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>
Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com>
---
 t/zbd/test-zbd-support | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/zbd/test-zbd-support b/t/zbd/test-zbd-support
index 26aff373..015fa1dc 100755
--- a/t/zbd/test-zbd-support
+++ b/t/zbd/test-zbd-support
@@ -1348,6 +1348,7 @@ fi
 if [[ -n ${max_open_zones_opt} ]]; then
 	# Override max_open_zones with the script option value
 	max_open_zones="${max_open_zones_opt}"
+	global_var_opts+=("--ignore_zone_limits=1")
 	job_var_opts+=("--max_open_zones=${max_open_zones_opt}")
 fi
 
-- 
2.31.1

[PATCH v3 3/3] t/zbd: Fix write target zones counting in test case #31

[Shin'ichiro Kawasaki]

The test case #31 in t/zbd/test-zbd-support writes 128KB data to
sequential write required zones as the preparation for the following
random read test. The data write leaves the target zones in open status.
The test case refers the variable 'nz', which has max_open_zones value,
to decide how many zones to write the data. However, the end condition
of the write target zone loop has a bug. The disk end offset is used as
the loop end condition, which does not match the last target zone when
number of sequential write required zones divided by nz has remainder.
This results in write to more zones than nz=max_open_zones limit and the
test case failure. To fix the bug and to simplify the script, avoid the
loop and utilize zonemode strided to achieve the same data write
pattern. Also specify size and io_size using nz to reliably count the
write target zones.

Even with the fix above, still the number of open zones may exceed
max_open_zones since other test cases executed before the test case 31
may leave open zones on the test target device. To avoid this failure,
reset all zones before the data write.

The failures were observed with libzbc I/O engine after the commit
e8267436fd7a ("engines/libzbc: add support for the get_max_open_zones io
op"), which changed the max_open_zones value fio refers.

Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>
Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com>
---
 t/zbd/test-zbd-support | 36 ++++++++++++++++--------------------
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/t/zbd/test-zbd-support b/t/zbd/test-zbd-support
index 015fa1dc..a684f988 100755
--- a/t/zbd/test-zbd-support
+++ b/t/zbd/test-zbd-support
@@ -731,32 +731,28 @@ test30() {
 test31() {
     local bs inc nz off opts size
 
-    prep_write
-    # Start with writing 128 KB to max_open_zones sequential zones.
-    bs=128K
+    [ -n "$is_zbd" ] && reset_zone "$dev" -1
+
+    # As preparation, write 128 KB to sequential write required zones. Limit
+    # write target zones up to max_open_zones to keep test time reasonable.
+    # To distribute the write target zones evenly, skip certain zones for every
+    # write. Utilize zonemode strided for such write patterns.
+    bs=$((128 * 1024))
     nz=$((max_open_zones))
     if [[ $nz -eq 0 ]]; then
 	nz=128
     fi
-    # shellcheck disable=SC2017
-    inc=$(((disk_size - (first_sequential_zone_sector * 512)) / (nz * zone_size)
-	   * zone_size))
-    if [ "$inc" -eq 0 ]; then
-	require_seq_zones $nz || return $SKIP_TESTCASE
-    fi
-    opts=()
-    for ((off = first_sequential_zone_sector * 512; off < disk_size;
-	  off += inc)); do
-	opts+=("--name=$dev" "--filename=$dev" "--offset=$off" "--io_size=$bs")
-	opts+=("--bs=$bs" "--size=$zone_size" "$(ioengine "libaio")")
-	opts+=("--rw=write" "--direct=1" "--thread=1" "--stats=0")
-	opts+=("--zonemode=zbd" "--zonesize=${zone_size}")
-	opts+=(${job_var_opts[@]})
-    done
-    "$(dirname "$0")/../../fio" "${opts[@]}" >> "${logfile}.${test_number}" 2>&1
-    # Next, run the test.
     off=$((first_sequential_zone_sector * 512))
     size=$((disk_size - off))
+    inc=$(((size / nz / zone_size) * zone_size))
+    opts=("--name=$dev" "--filename=$dev" "--rw=write" "--bs=${bs}")
+    opts+=("--offset=$off" "--size=$((inc * nz))" "--io_size=$((bs * nz))")
+    opts+=("--zonemode=strided" "--zonesize=${bs}" "--zonerange=${inc}")
+    opts+=("--direct=1")
+    echo "fio ${opts[@]}" >> "${logfile}.${test_number}"
+    "$(dirname "$0")/../../fio" "${opts[@]}" >> "${logfile}.${test_number}" 2>&1
+
+    # Next, run the test.
     opts=("--name=$dev" "--filename=$dev" "--offset=$off" "--size=$size")
     opts+=("--bs=$bs" "$(ioengine "psync")" "--rw=randread" "--direct=1")
     opts+=("--thread=1" "--time_based" "--runtime=30" "--zonemode=zbd")
-- 
2.31.1

Re: [PATCH v3 0/3] t/zbd: Fix max_open_zones related failures

[Jens Axboe]

On 6/4/21 5:32 AM, Shin'ichiro Kawasaki wrote:
> Recent changes in fio [1] triggered three failures in the test script
> t/zbd/test-zoned-support. This series address the failures.
> 
> The first failure was caused by difference between max_open_zones value that fio
> refers and that test script refers. The first patch modifies the test script to
> use the value fio refers.
> 
> The second failure was observed when users specify special max_open_zones values
> larger than the max_open_zones limit of the test target device. The second patch
> adds --ignore_zone_limits option to the fio command only when users specify such
> special conditions.
> 
> The third failure was caused by write target zone count bug in the test case 31.
> Data write to more zones than max_open_zones limit resulted in exceeding
> max_open_zones limit. The third patch fixes the write target zone count logic.
> 
> [1] https://www.spinics.net/lists/fio/msg09502.html

Applied, thanks.

-- 
Jens Axboe