[PATCH blktests 0/2] Support max_open_zones and max_active_zones

[PATCH blktests 0/2] Support max_open_zones and max_active_zones

[Shin'ichiro Kawasaki]

Linux kernel 5.9 introduced new sysfs attributes max_active_zones and
max_open_zones for zoned block devices. Blktests already handles
max_active_zones. However, max_open_zones handling is missing. Also,
zbd/005 lacks support for the two attributes.

This patch series fills the missing attributes handling. The first patch
modifies the helper function for max_active_zones to support both
max_active_zones and max_open_zones. The second patch modifies zbd/005 to
handle the attributes.

Shin'ichiro Kawasaki (2):
  common/rc: Check both max_active_zones and max_open_zones
  zbd/005: Provide max_active/open_zones limit to fio command

 common/rc       | 21 +++++++++++++++++----
 tests/block/004 |  2 +-
 tests/zbd/003   |  6 +++---
 tests/zbd/005   | 13 ++++++++-----
 4 files changed, 29 insertions(+), 13 deletions(-)

-- 
2.28.0

[PATCH blktests 1/2] common/rc: Check both max_active_zones and max_open_zones

[Shin'ichiro Kawasaki]

Linux kernel 5.9 introduced new sysfs attributes max_active_zones and
max_open_zones for zoned block devices. Max_open_zones is the limit of
number of zones in open status. Max_active_zones is the limit of number
of zones in open or closed status. Currently, the helper function
_test_dev_max_active_zones() checks only max_active_zones, but it is not
enough. When the device has max_open_zones, check for max_active_zones
can not avoid the errors for write operations.

To avoid the error, improve the function _test_dev_max_active_zones() to
check the limits both. Rename it to _test_dev_max_open_active_zones().
When one of the limits is available for the test target device, return
it. If both limits are available, return smaller limit.

Also modify block/004 and zbd/003 to call the renamed helper function
and update comment description.

Fixes: e6981bb2d9ce ("common/rc: Add _test_dev_max_active_zones() helper function")
Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
 common/rc       | 21 +++++++++++++++++----
 tests/block/004 |  2 +-
 tests/zbd/003   |  6 +++---
 3 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/common/rc b/common/rc
index d396fb5..a14f093 100644
--- a/common/rc
+++ b/common/rc
@@ -280,12 +280,25 @@ _test_dev_is_partition() {
 	[[ -n ${TEST_DEV_PART_SYSFS} ]]
 }
 
-_test_dev_max_active_zones() {
+# 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() {
+	local -i ret=0
+	local -i maz=0
+
+	if [[ -r "${TEST_DEV_SYSFS}/queue/max_open_zones" ]]; then
+		ret=$(_test_dev_queue_get max_open_zones)
+	fi
+
 	if [[ -r "${TEST_DEV_SYSFS}/queue/max_active_zones" ]]; then
-		_test_dev_queue_get max_active_zones
-	else
-		echo 0
+		maz=$(_test_dev_queue_get max_active_zones)
 	fi
+
+	if ((!ret)) || ((maz && maz < ret)); then
+		ret="${maz}"
+	fi
+
+	echo "${ret}"
 }
 
 _require_test_dev_is_partition() {
diff --git a/tests/block/004 b/tests/block/004
index 6eff6ce..a7cec95 100755
--- a/tests/block/004
+++ b/tests/block/004
@@ -26,7 +26,7 @@ test_device() {
 	if _test_dev_is_zoned; then
 		_test_dev_queue_set scheduler deadline
 		opts+=("--direct=1" "--zonemode=zbd")
-		opts+=("--max_open_zones=$(_test_dev_max_active_zones)")
+		opts+=("--max_open_zones=$(_test_dev_max_open_active_zones)")
 	fi
 
 	FIO_PERF_FIELDS=("write iops")
diff --git a/tests/zbd/003 b/tests/zbd/003
index 1e92e81..7f4fa2c 100755
--- a/tests/zbd/003
+++ b/tests/zbd/003
@@ -30,10 +30,10 @@ test_device() {
 
 	echo "Running ${TEST_NAME}"
 
-	# When the test device has max_active_zone limit, reset all zones. This
-	# ensures the write operations in this test case do not open zones
+	# When the test device has max_open/active_zones limit, reset all zones.
+	# This ensures the write operations in this test case do not open zones
 	# beyond the limit.
-	(($(_test_dev_max_active_zones))) && blkzone reset "${TEST_DEV}"
+	(($(_test_dev_max_open_active_zones))) && blkzone reset "${TEST_DEV}"
 
 	# Get physical block size as dd block size to meet zoned block device
 	# requirement
-- 
2.28.0

[PATCH blktests 2/2] zbd/005: Provide max_active/open_zones limit to fio command

[Shin'ichiro Kawasaki]

When test target zoned block devices have max_open_zones or
max_active_zones limit, high queue depth sequential write in the test
case zbd/005 may result in parallel writes to number of zones beyond the
limit. This causes I/O errors.

To avoid the errors, specify the limit to fio command in the test case.

Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
 tests/zbd/005 | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/tests/zbd/005 b/tests/zbd/005
index 1e8962c..a7fb175 100755
--- a/tests/zbd/005
+++ b/tests/zbd/005
@@ -28,7 +28,8 @@ cleanup_fallback_device() {
 test_device() {
 	local -i zone_idx
 	local -i offset
-	local zbdmode=""
+	local -i moaz
+	local -a zbdmode=()
 
 	echo "Running ${TEST_NAME}"
 
@@ -36,11 +37,13 @@ test_device() {
 
 	zone_idx=$(_find_first_sequential_zone) || return $?
 	offset=$((ZONE_STARTS[zone_idx] * 512))
+	moaz=$(_test_dev_max_open_active_zones)
 
 	# If the test target zone has smaller zone capacity than zone size,
-	# enable zonemode=zbd to have fio handle the zone capacity limit.
-	if ((ZONE_CAPS[zone_idx] != ZONE_LENGTHS[zone_idx])); then
-		zbdmode="--zonemode=zbd"
+	# or if the test target device has max open/active zones limit, enable
+	# zonemode=zbd and specify the limit to handle the zone restrictions.
+	if ((ZONE_CAPS[zone_idx] != ZONE_LENGTHS[zone_idx])) || ((moaz)); then
+		zbdmode=("--zonemode=zbd" "--max_open_zones=${moaz}")
 	fi
 
 	blkzone reset -o "${ZONE_STARTS[zone_idx]}" "${TEST_DEV}"
@@ -51,7 +54,7 @@ test_device() {
 	FIO_PERF_FIELDS=("write io" "write iops")
 	_fio_perf --filename="${TEST_DEV}" --name zbdwo --rw=write --direct=1 \
 		  --ioengine=libaio --iodepth=128 --bs=256k \
-		  --offset="${offset}" ${zbdmode}
+		  --offset="${offset}" "${zbdmode[@]}"
 
 	_put_blkzone_report
 
-- 
2.28.0

Re: [PATCH blktests 1/2] common/rc: Check both max_active_zones and max_open_zones

[Johannes Thumshirn]

On 03/12/2020 09:22, Shin'ichiro Kawasaki wrote:
> -_test_dev_max_active_zones() {
> +# 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() {
> +	local -i ret=0
> +	local -i maz=0
> +
> +	if [[ -r "${TEST_DEV_SYSFS}/queue/max_open_zones" ]]; then
> +		ret=$(_test_dev_queue_get max_open_zones)
> +	fi
> +
>  	if [[ -r "${TEST_DEV_SYSFS}/queue/max_active_zones" ]]; then
> -		_test_dev_queue_get max_active_zones
> -	else
> -		echo 0
> +		maz=$(_test_dev_queue_get max_active_zones)
>  	fi
> +
> +	if ((!ret)) || ((maz && maz < ret)); then
> +		ret="${maz}"
> +	fi
> +
> +	echo "${ret}"
>  }

Maybe change $ret to $moz and then

if ((!moz)) || ((maz && maz < moz)); then
	echo ${maz}
else
	echo ${moz}
fi

Otherwise looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH blktests 2/2] zbd/005: Provide max_active/open_zones limit to fio command

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH blktests 1/2] common/rc: Check both max_active_zones and max_open_zones

[Shinichiro Kawasaki]

On Dec 03, 2020 / 09:50, Johannes Thumshirn wrote:
> On 03/12/2020 09:22, Shin'ichiro Kawasaki wrote:
> > -_test_dev_max_active_zones() {
> > +# 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() {
> > +	local -i ret=0
> > +	local -i maz=0
> > +
> > +	if [[ -r "${TEST_DEV_SYSFS}/queue/max_open_zones" ]]; then
> > +		ret=$(_test_dev_queue_get max_open_zones)
> > +	fi
> > +
> >  	if [[ -r "${TEST_DEV_SYSFS}/queue/max_active_zones" ]]; then
> > -		_test_dev_queue_get max_active_zones
> > -	else
> > -		echo 0
> > +		maz=$(_test_dev_queue_get max_active_zones)
> >  	fi
> > +
> > +	if ((!ret)) || ((maz && maz < ret)); then
> > +		ret="${maz}"
> > +	fi
> > +
> > +	echo "${ret}"
> >  }
> 
> Maybe change $ret to $moz and then
> 
> if ((!moz)) || ((maz && maz < moz)); then
> 	echo ${maz}
> else
> 	echo ${moz}
> fi

Thanks. I agree that your code is easier to understand.
Will reflect this comment and send out v2.

> 
> Otherwise looks good,
> Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> 

-- 
Best Regards,
Shin'ichiro Kawasaki