[PATCH v3 0/2] zbd: fix zone reset for file reset during asynchronous IOs

[PATCH v3 0/2] zbd: fix zone reset for file reset during asynchronous IOs

[Shin'ichiro Kawasaki]

Repeated random write jobs with block size unaligned to zone size and libaio
engine trigger unaligned command errors because of zone reset during
asynchronous writes in-flight. The first patch of this series addresses the
problem by removing the zone reset. The second patch adds a new test case to
confirm the problem fix and to prevent future regression.

Changes from v2:
* Removed unnecessary numjob option from the test case workload
* Added a Reviewd-by tag

Changes from v1:
* Changed fix approach from waiting IO completion to removing zone reset
* Simplified the added test case not to refer sysfs

Shin'ichiro Kawasaki (2):
  zbd: avoid zone reset during asynchronous IOs in-flight
  t/zbd: test repeated async write with block size unaligned to zone
    size

 t/zbd/test-zbd-support | 18 ++++++++++++++++++
 zbd.c                  | 23 +++++++----------------
 2 files changed, 25 insertions(+), 16 deletions(-)

-- 
2.29.2

[PATCH v3 1/2] zbd: avoid zone reset during asynchronous IOs in-flight

[Shin'ichiro Kawasaki]

When fio repeats same workload on zoned block devices, zbd_file_reset()
is called for each repetition. This function resets target zones when
one of two conditions are met: 1) the write pointer of the zone has
offset from the device start unaligned to block size, or 2) the workload
is verify and verification is not in process. When the workload runs
with block size not a divisor of the zone size, the offsets of write
pointers from device start (not from zone start) become unaligned to
block size, then zbd_file_reset() resets target zones. This zone reset
happens even when the asynchronous IOs are in-flight and causes
unexpected IO results. Especially if write requests are in-flight, they
fail with unaligned write command error. A single thread may do both the
zone reset and the write request submit, recursive zone locks can not
prevent the zone reset during the writes.

The write pointer check for block size alignment is not working as
intended. It should have checked offset not from device start but from
zone start. Having said that, regardless of this write pointer check
correctness, the zone reset is not required since the zones are reset in
zbd_adjust_block() anyway when remainder of the zone between write
pointer and zone end is smaller than block size.

To avoid the zone reset during asynchronous IOs, do not reset zones in
zbd_file_reset() when the write pointer offset from the device start is
unaligned to block size. Modify zbd_file_reset() to call the helper
function zbd_reset_zones() only when the workload is verify and
verification is not in process. The function zbd_reset_zones() had an
argument 'all_zones' to inform that the zones should be reset when its
write pointer is unaligned to block size. This argument is not required.
Remove it and simplify the function accordingly.

The zone reset for verify workloads is still required. It does not
conflict with asynchronous IOs, since fio waits for IO completion at
verification end, then IOs are not in-flight when zbd_file_reset() is
called for repetition after verification.

Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 zbd.c | 23 +++++++----------------
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/zbd.c b/zbd.c
index d16b890f..eed796b3 100644
--- a/zbd.c
+++ b/zbd.c
@@ -842,16 +842,13 @@ static void zbd_close_zone(struct thread_data *td, const struct fio_file *f,
  * @f: fio file for which to reset zones
  * @zb: first zone to reset.
  * @ze: first zone not to reset.
- * @all_zones: whether to reset all zones or only those zones for which the
- *	write pointer is not a multiple of td->o.min_bs[DDIR_WRITE].
  */
 static int zbd_reset_zones(struct thread_data *td, struct fio_file *f,
 			   struct fio_zone_info *const zb,
-			   struct fio_zone_info *const ze, bool all_zones)
+			   struct fio_zone_info *const ze)
 {
 	struct fio_zone_info *z;
 	const uint32_t min_bs = td->o.min_bs[DDIR_WRITE];
-	bool reset_wp;
 	int res = 0;
 
 	assert(min_bs);
@@ -864,16 +861,10 @@ static int zbd_reset_zones(struct thread_data *td, struct fio_file *f,
 		if (!z->has_wp)
 			continue;
 		zone_lock(td, f, z);
-		if (all_zones) {
-			pthread_mutex_lock(&f->zbd_info->mutex);
-			zbd_close_zone(td, f, nz);
-			pthread_mutex_unlock(&f->zbd_info->mutex);
-
-			reset_wp = z->wp != z->start;
-		} else {
-			reset_wp = z->wp % min_bs != 0;
-		}
-		if (reset_wp) {
+		pthread_mutex_lock(&f->zbd_info->mutex);
+		zbd_close_zone(td, f, nz);
+		pthread_mutex_unlock(&f->zbd_info->mutex);
+		if (z->wp != z->start) {
 			dprint(FD_ZBD, "%s: resetting zone %u\n",
 			       f->file_name, zbd_zone_nr(f, z));
 			if (zbd_reset_zone(td, f, z) < 0)
@@ -996,8 +987,8 @@ void zbd_file_reset(struct thread_data *td, struct fio_file *f)
 	 * writing any data to avoid that a zone reset has to be issued while
 	 * writing data, which causes data loss.
 	 */
-	zbd_reset_zones(td, f, zb, ze, td->o.verify != VERIFY_NONE &&
-			td->runstate != TD_VERIFYING);
+	if (td->o.verify != VERIFY_NONE && td->runstate != TD_VERIFYING)
+		zbd_reset_zones(td, f, zb, ze);
 	zbd_reset_write_cnt(td, f);
 }
 
-- 
2.29.2

[PATCH v3 2/2] t/zbd: test repeated async write with block size unaligned to zone size

[Shin'ichiro Kawasaki]

A recently fixed bug was caused by zone reset during asynchronous IOs
in-flight. The bug symptom was unaligned command error which was
observed using random write workload with libaio engine and block size
not a divisor of zone size. To confirm the bug fix and to prevent future
regression, add a test case which runs the workload.

Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
 t/zbd/test-zbd-support | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/t/zbd/test-zbd-support b/t/zbd/test-zbd-support
index be129615..26aff373 100755
--- a/t/zbd/test-zbd-support
+++ b/t/zbd/test-zbd-support
@@ -1201,6 +1201,24 @@ test56() {
 		>> "${logfile}.${test_number}" 2>&1 || return $?
 }
 
+# Test that repeated async write job does not cause zone reset during writes
+# in-flight, when the block size is not a divisor of the zone size.
+test57() {
+	local bs off
+
+	require_zbd || return $SKIP_TESTCASE
+
+	bs=$((4096 * 7))
+	off=$((first_sequential_zone_sector * 512))
+
+	run_fio --name=job --filename="${dev}" --rw=randwrite --bs="${bs}" \
+		--offset="${off}" --size=$((4 * zone_size)) --iodepth=256 \
+		"$(ioengine "libaio")" --time_based=1 --runtime=30s \
+		--zonemode=zbd --direct=1 --zonesize="${zone_size}" \
+		${job_var_opts[@]} \
+		>> "${logfile}.${test_number}" 2>&1 || return $?
+}
+
 SECONDS=0
 tests=()
 dynamic_analyzer=()
-- 
2.29.2

Re: [PATCH v3 2/2] t/zbd: test repeated async write with block size unaligned to zone size

[Damien Le Moal]

On 2021/03/26 13:59, Shin'ichiro Kawasaki wrote:
> A recently fixed bug was caused by zone reset during asynchronous IOs
> in-flight. The bug symptom was unaligned command error which was
> observed using random write workload with libaio engine and block size
> not a divisor of zone size. To confirm the bug fix and to prevent future
> regression, add a test case which runs the workload.
> 
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> ---
>  t/zbd/test-zbd-support | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/t/zbd/test-zbd-support b/t/zbd/test-zbd-support
> index be129615..26aff373 100755
> --- a/t/zbd/test-zbd-support
> +++ b/t/zbd/test-zbd-support
> @@ -1201,6 +1201,24 @@ test56() {
>  		>> "${logfile}.${test_number}" 2>&1 || return $?
>  }
>  
> +# Test that repeated async write job does not cause zone reset during writes
> +# in-flight, when the block size is not a divisor of the zone size.
> +test57() {
> +	local bs off
> +
> +	require_zbd || return $SKIP_TESTCASE
> +
> +	bs=$((4096 * 7))
> +	off=$((first_sequential_zone_sector * 512))
> +
> +	run_fio --name=job --filename="${dev}" --rw=randwrite --bs="${bs}" \
> +		--offset="${off}" --size=$((4 * zone_size)) --iodepth=256 \
> +		"$(ioengine "libaio")" --time_based=1 --runtime=30s \
> +		--zonemode=zbd --direct=1 --zonesize="${zone_size}" \
> +		${job_var_opts[@]} \
> +		>> "${logfile}.${test_number}" 2>&1 || return $?
> +}
> +
>  SECONDS=0
>  tests=()
>  dynamic_analyzer=()
> 

Looks good.

Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>

-- 
Damien Le Moal
Western Digital Research