[PATCH 0/8] zbd: Fix initial zone write pointer of regular block devices

[PATCH 0/8] zbd: Fix initial zone write pointer of regular block devices

[Shin'ichiro Kawasaki]

With zonemode=zbd and regular block devices, fio initializes zone status as
empty. However, write pointers are set not at zone start but at zone end. This
inconsistency between write pointer position and zone status caused different
behavior of regular block devices from zoned block devices. This patch series
fixes the inconsistency and the behavior gap.

The 1st patch fixes the zone status initialization code by setting write
pointers at zone start. Following patches fixes t/zbd test script to handle
regular block devices and zoned block devices in same manner.

Shin'ichiro Kawasaki (8):
  zbd: Fix initial zone write pointer of regular block devices
  t/zbd: Fix pass condition of test case #3
  t/zbd: Add write_and_run_one_fio_job() helper function
  t/zbd: Combine write and read fio commands for test case #6
  t/zbd: Combine write and read fio commands for test case #15
  t/zbd: Combine write and read fio commands for test case #16
  t/zbd: Remove write before random read/write from test case #17
  t/zbd: Enable regular block devices for test case #47

 t/zbd/test-zbd-support | 78 +++++++++++++++++++++++-------------------
 zbd.c                  |  2 +-
 2 files changed, 44 insertions(+), 36 deletions(-)

-- 
2.26.2

[PATCH 1/8] zbd: Fix initial zone write pointer of regular block devices

[Shin'ichiro Kawasaki]

With zonemode=zbd and regular block devices, fio initializes zone status
as empty. However, write pointers are set not at zone start but at zone
end. This inconsistency between write pointer position and zone status
caused different behavior from zoned block devices. Fix the inconsistency
by setting initial write pointer at zone start.

Of note is that every fio run with zonemode=zbd and regular block devices
starts with empty zones. Then read workloads just ends without any data
to read. To run read workload, specify read_beyond_wp option. Or specify
two jobs to a single fio command: write job to prepare data for read and
read job which waits for the write job completion.

Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
 zbd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/zbd.c b/zbd.c
index 8cf8f812..cf2cded9 100644
--- a/zbd.c
+++ b/zbd.c
@@ -381,7 +381,7 @@ static int init_zone_info(struct thread_data *td, struct fio_file *f)
 		mutex_init_pshared_with_type(&p->mutex,
 					     PTHREAD_MUTEX_RECURSIVE);
 		p->start = i * zone_size;
-		p->wp = p->start + zone_size;
+		p->wp = p->start;
 		p->type = ZBD_ZONE_TYPE_SWR;
 		p->cond = ZBD_ZONE_COND_EMPTY;
 	}
-- 
2.26.2

[PATCH 2/8] t/zbd: Fix pass condition of test case #3

[Shin'ichiro Kawasaki]

The test case #3 checks that the read workload for empty zones ends
without reading any data. When zonemode=zbd, it is expected for both
regular block devices and zoned block devices. However, the test case
allowed for regular block devices to read data from empty zones. Remove
this special pass condition for regular block devices.

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

diff --git a/t/zbd/test-zbd-support b/t/zbd/test-zbd-support
index 4001be3b..1178afd4 100755
--- a/t/zbd/test-zbd-support
+++ b/t/zbd/test-zbd-support
@@ -170,13 +170,7 @@ test3() {
 	opts+=("--zonesize=${zone_size}")
     fi
     run_fio "${opts[@]}" >> "${logfile}.${test_number}" 2>&1 || return $?
-    grep -q 'READ:' "${logfile}.${test_number}"
-    rc=$?
-    if [ -n "$is_zbd" ]; then
-	[ $rc != 0 ]
-    else
-	[ $rc = 0 ]
-    fi
+    ! grep -q 'READ:' "${logfile}.${test_number}"
 }
 
 # Run fio with --read_beyond_wp=1 against an empty zone.
-- 
2.26.2

[PATCH 3/8] t/zbd: Add write_and_run_one_fio_job() helper function

[Shin'ichiro Kawasaki]

To test read workload behavior with zonemode=zbd and zoned block devices
with all zones empty, it is required to write data to the device before
the read workload. To avoid repeated codes for such test preparation,
introduce the helper function write_and_run_one_file_job(). It runs the
write job with specified offset and size to prepare the read data, then
run the specified job for read test.

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

diff --git a/t/zbd/test-zbd-support b/t/zbd/test-zbd-support
index 1178afd4..55363357 100755
--- a/t/zbd/test-zbd-support
+++ b/t/zbd/test-zbd-support
@@ -109,6 +109,20 @@ run_one_fio_job() {
 	    --thread=1 --direct=1
 }
 
+write_and_run_one_fio_job() {
+    local r
+    local write_offset="${1}"
+    local write_size="${2}"
+
+    shift 2
+    r=$(((RANDOM << 16) | RANDOM))
+    run_fio --filename="$dev" --randseed="$r"  --name="write_job" --rw=write \
+	    "$(ioengine "psync")" --bs="${logical_block_size}" \
+	    --zonemode=zbd --zonesize="${zone_size}" --thread=1 --direct=1 \
+	    --offset="${write_offset}" --size="${write_size}" \
+	    --name="$dev" --wait_for="write_job" "$@" --thread=1 --direct=1
+}
+
 # Run fio on the first four sequential zones of the disk.
 run_fio_on_seq() {
     local opts=()
-- 
2.26.2

[PATCH 4/8] t/zbd: Combine write and read fio commands for test case #6

[Shin'ichiro Kawasaki]

The test case #6 checks result of the read job which reads data written
by the write job of test case #5. It does not work if test case #6 is
executed alone with the -t option and the target device has all zones
empty.

Specify both the write job and the read job to a single fio command
using write_and_run_one_fio_job() helper function instead of
run_fio_on_seq() helper function.

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

diff --git a/t/zbd/test-zbd-support b/t/zbd/test-zbd-support
index 55363357..eccb5178 100755
--- a/t/zbd/test-zbd-support
+++ b/t/zbd/test-zbd-support
@@ -215,14 +215,18 @@ test5() {
     check_read $size || return $?
 }
 
-# Sequential read from sequential zones. Must be run after test5.
+# Sequential read from sequential zones.
 test6() {
     local size
 
     size=$((4 * zone_size))
-    run_fio_on_seq "$(ioengine "psync")" --iodepth=1 --rw=read	\
-		   --bs="$(max $((zone_size / 64)) "$logical_block_size")"\
-		   >>"${logfile}.${test_number}" 2>&1 || return $?
+    write_and_run_one_fio_job \
+	    $((first_sequential_zone_sector * 512)) "${size}" \
+	    --offset=$((first_sequential_zone_sector * 512)) \
+	    --size="${size}" --zonemode=zbd --zonesize="${zone_size}" \
+	    "$(ioengine "psync")" --iodepth=1 --rw=read \
+	    --bs="$(max $((zone_size / 64)) "$logical_block_size")" \
+	    >>"${logfile}.${test_number}" 2>&1 || return $?
     check_read $size || return $?
 }
 
-- 
2.26.2

[PATCH 5/8] t/zbd: Combine write and read fio commands for test case #15

[Shin'ichiro Kawasaki]

The test case #15 called fio twice. Once with write job to prepare data
to read. The other with read job for testing. But the first run has no
effect on the second run when the zone status is reinitialized.

Specify both the write job and the read job to a single fio command using
write_and_run_one_fio_job() helper function instead of run_one_fio_job().

Also apply same test pass condition for both regular block devices and
zoned block devices.

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

diff --git a/t/zbd/test-zbd-support b/t/zbd/test-zbd-support
index eccb5178..cd8492ff 100755
--- a/t/zbd/test-zbd-support
+++ b/t/zbd/test-zbd-support
@@ -349,30 +349,24 @@ test14() {
 # Sequential read on a mix of empty and full zones.
 test15() {
     local i off size
+    local w_off w_size
 
     for ((i=0;i<4;i++)); do
 	[ -n "$is_zbd" ] &&
 	    reset_zone "$dev" $((first_sequential_zone_sector +
 				 i*sectors_per_zone))
     done
-    off=$(((first_sequential_zone_sector + 2 * sectors_per_zone) * 512))
-    size=$((2 * zone_size))
-    run_one_fio_job "$(ioengine "psync")" --rw=write --bs=$((zone_size / 16))\
-		    --zonemode=zbd --zonesize="${zone_size}" --offset=$off \
-		    --size=$size >>"${logfile}.${test_number}" 2>&1 ||
-	return $?
-    check_written $size || return $?
+    w_off=$(((first_sequential_zone_sector + 2 * sectors_per_zone) * 512))
+    w_size=$((2 * zone_size))
     off=$((first_sequential_zone_sector * 512))
     size=$((4 * zone_size))
-    run_one_fio_job "$(ioengine "psync")" --rw=read --bs=$((zone_size / 16)) \
+    write_and_run_one_fio_job "${w_off}" "${w_size}" \
+		    "$(ioengine "psync")" --rw=read --bs=$((zone_size / 16)) \
 		    --zonemode=zbd --zonesize="${zone_size}" --offset=$off \
 		    --size=$((size)) >>"${logfile}.${test_number}" 2>&1 ||
 	return $?
-    if [ -n "$is_zbd" ]; then
-	check_read $((size / 2))
-    else
-	check_read $size
-    fi
+    check_written $((w_size)) || return $?
+    check_read $((size / 2))
 }
 
 # Random read on a mix of empty and full zones. Must be run after test15.
-- 
2.26.2

[PATCH 6/8] t/zbd: Combine write and read fio commands for test case #16

[Shin'ichiro Kawasaki]

The test case #16 checks result of read job which runs after write job of
test case #15. It does not work if test case #16 is executed alone with
the -t option and the target device has all zones empty.

Specify both the write job and the read job to a single fio command using
write_and_run_one_fio_job() helper function instead of run_one_fio_job().

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

diff --git a/t/zbd/test-zbd-support b/t/zbd/test-zbd-support
index cd8492ff..a3ecec1f 100755
--- a/t/zbd/test-zbd-support
+++ b/t/zbd/test-zbd-support
@@ -369,15 +369,25 @@ test15() {
     check_read $((size / 2))
 }
 
-# Random read on a mix of empty and full zones. Must be run after test15.
+# Random read on a mix of empty and full zones.
 test16() {
     local off size
+    local i w_off w_size
 
+    for ((i=0;i<4;i++)); do
+	[ -n "$is_zbd" ] &&
+	    reset_zone "$dev" $((first_sequential_zone_sector +
+				 i*sectors_per_zone))
+    done
+    w_off=$(((first_sequential_zone_sector + 2 * sectors_per_zone) * 512))
+    w_size=$((2 * zone_size))
     off=$((first_sequential_zone_sector * 512))
     size=$((4 * zone_size))
-    run_one_fio_job "$(ioengine "libaio")" --iodepth=64 --rw=randread --bs=16K \
+    write_and_run_one_fio_job "${w_off}" "${w_size}" \
+		    "$(ioengine "libaio")" --iodepth=64 --rw=randread --bs=16K \
 		    --zonemode=zbd --zonesize="${zone_size}" --offset=$off \
 		    --size=$size >>"${logfile}.${test_number}" 2>&1 || return $?
+    check_written $w_size || return $?
     check_read $size || return $?
 }
 
-- 
2.26.2

[PATCH 7/8] t/zbd: Remove write before random read/write from test case #17

[Shin'ichiro Kawasaki]

The test case #17 writes data to the last zone before random read/write.
This avoided read failure when read is chosen for the first operation of
the random read and write. Such failure case was prevented by the commit
fb0259fb276a ("zbd: Ensure first I/O is write for random read/write to
sequential zones"). Then the write is no longer required. Remove it.

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

diff --git a/t/zbd/test-zbd-support b/t/zbd/test-zbd-support
index a3ecec1f..8934e32b 100755
--- a/t/zbd/test-zbd-support
+++ b/t/zbd/test-zbd-support
@@ -397,15 +397,9 @@ test17() {
 
     off=$(((disk_size / zone_size - 1) * zone_size))
     size=$((disk_size - off))
-    # Overwrite the last zone to avoid that reading from that zone fails.
     if [ -n "$is_zbd" ]; then
 	reset_zone "$dev" $((off / 512)) || return $?
     fi
-    run_one_fio_job "$(ioengine "psync")" --rw=write --offset="$off"	\
-		    --zonemode=zbd --zonesize="${zone_size}"		\
-		    --bs="$zone_size" --size="$zone_size"		\
-		    >>"${logfile}.${test_number}" 2>&1 || return $?
-    check_written "$zone_size" || return $?
     run_one_fio_job "$(ioengine "libaio")" --iodepth=8 --rw=randrw --bs=4K \
 		    --zonemode=zbd --zonesize="${zone_size}"		\
 		    --offset=$off --loops=2 --norandommap=1\
-- 
2.26.2

[PATCH 8/8] t/zbd: Enable regular block devices for test case #47

[Shin'ichiro Kawasaki]

The test case #47 was not able to run for regular block devices since it
missed zonesize option required for regular block devices.

Enable the test case for regular block devices by calling the
run_fio_on_seq() helper function which adds the zonesize option.

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

diff --git a/t/zbd/test-zbd-support b/t/zbd/test-zbd-support
index 8934e32b..80dc3f30 100755
--- a/t/zbd/test-zbd-support
+++ b/t/zbd/test-zbd-support
@@ -773,10 +773,8 @@ test46() {
 test47() {
     local bs
 
-    [ -z "$is_zbd" ] && return 0
     bs=$((logical_block_size))
-    run_one_fio_job "$(ioengine "psync")" --rw=write --bs=$bs \
-		    --zonemode=zbd --zoneskip=1		 \
+    run_fio_on_seq "$(ioengine "psync")" --rw=write --bs=$bs --zoneskip=1 \
 		    >> "${logfile}.${test_number}" 2>&1 && return 1
     grep -q 'zoneskip 1 is not a multiple of the device zone size' "${logfile}.${test_number}"
 }
-- 
2.26.2

Re: [PATCH 0/8] zbd: Fix initial zone write pointer of regular block devices

[Damien Le Moal]

On 2020/06/26 18:43, Shin'ichiro Kawasaki wrote:
> With zonemode=zbd and regular block devices, fio initializes zone status as
> empty. However, write pointers are set not at zone start but at zone end. This
> inconsistency between write pointer position and zone status caused different
> behavior of regular block devices from zoned block devices. This patch series
> fixes the inconsistency and the behavior gap.
> 
> The 1st patch fixes the zone status initialization code by setting write
> pointers at zone start. Following patches fixes t/zbd test script to handle
> regular block devices and zoned block devices in same manner.
> 
> Shin'ichiro Kawasaki (8):
>   zbd: Fix initial zone write pointer of regular block devices
>   t/zbd: Fix pass condition of test case #3
>   t/zbd: Add write_and_run_one_fio_job() helper function
>   t/zbd: Combine write and read fio commands for test case #6
>   t/zbd: Combine write and read fio commands for test case #15
>   t/zbd: Combine write and read fio commands for test case #16
>   t/zbd: Remove write before random read/write from test case #17
>   t/zbd: Enable regular block devices for test case #47
> 
>  t/zbd/test-zbd-support | 78 +++++++++++++++++++++++-------------------
>  zbd.c                  |  2 +-
>  2 files changed, 44 insertions(+), 36 deletions(-)
> 

Jens,

I reviewed this series internally with Shin'ichiro before he posted.
So:

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


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 0/8] zbd: Fix initial zone write pointer of regular block devices

[Shinichiro Kawasaki]

On Jun 30, 2020 / 04:53, Damien Le Moal wrote:
> On 2020/06/26 18:43, Shin'ichiro Kawasaki wrote:
> > With zonemode=zbd and regular block devices, fio initializes zone status as
> > empty. However, write pointers are set not at zone start but at zone end. This
> > inconsistency between write pointer position and zone status caused different
> > behavior of regular block devices from zoned block devices. This patch series
> > fixes the inconsistency and the behavior gap.
> > 
> > The 1st patch fixes the zone status initialization code by setting write
> > pointers at zone start. Following patches fixes t/zbd test script to handle
> > regular block devices and zoned block devices in same manner.
> > 
> > Shin'ichiro Kawasaki (8):
> >   zbd: Fix initial zone write pointer of regular block devices
> >   t/zbd: Fix pass condition of test case #3
> >   t/zbd: Add write_and_run_one_fio_job() helper function
> >   t/zbd: Combine write and read fio commands for test case #6
> >   t/zbd: Combine write and read fio commands for test case #15
> >   t/zbd: Combine write and read fio commands for test case #16
> >   t/zbd: Remove write before random read/write from test case #17
> >   t/zbd: Enable regular block devices for test case #47
> > 
> >  t/zbd/test-zbd-support | 78 +++++++++++++++++++++++-------------------
> >  zbd.c                  |  2 +-
> >  2 files changed, 44 insertions(+), 36 deletions(-)
> > 
> 
> Jens,
> 
> I reviewed this series internally with Shin'ichiro before he posted.
> So:
> 
> Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>

Hi Jens, could you consider to merge this series to the upstream?
It will make zonemode=zbd implementation less confusing.

-- 
Best Regards,
Shin'ichiro Kawasaki