Re: [PATCH 3/3] zbd: Ensure first I/O is write for random read/write to sequential zones

Re: [PATCH 3/3] zbd: Ensure first I/O is write for random read/write to sequential zones

[Alexey Dobriyan]

> zbd_adjust_block


> +	if (td_rw(td) && !f->zbd_info->sectors_with_data
> +	    && !td->o.read_beyond_wp)
> +		io_u->ddir = DDIR_WRITE;

Oh, you've noticed this too. :-)

This will issue the first write with wrong length if bs[DDIR_READ] != bs[DDIR_WRITE].
Adjusting in zbd_adjust_block() is too late.

Re: [PATCH 3/3] zbd: Ensure first I/O is write for random read/write to sequential zones

[Shinichiro Kawasaki]

On Apr 14, 2020 / 00:21, Alexey Dobriyan wrote:
> > zbd_adjust_block
> 
> 
> > +	if (td_rw(td) && !f->zbd_info->sectors_with_data
> > +	    && !td->o.read_beyond_wp)
> > +		io_u->ddir = DDIR_WRITE;
> 
> Oh, you've noticed this too. :-)
> 
> This will issue the first write with wrong length if bs[DDIR_READ] != bs[DDIR_WRITE].

Hi Alexey, thanks for pointing out that.

> Adjusting in zbd_adjust_block() is too late.

Hmm, you are right. To align the new ddir with buflen and offset, need to call
get_next_offset() and get_next_buflen() again from zbd_adjust_block(). Those
functions are now static to io_u.c and will have duplicate code...

It is not ideal to add more zoned block device related code to io_u.c, but now I
think it is the simplest to check conditions and enforce ddir=DDIR_WRITE in
set_rw_ddir() of io_u.c. It modifies ddir before get_next_offset() and
get_next_buflen() calls then makes io_u fields consistent. Will create a patch
to ask opinions on this list.

-- 
Best Regards,
Shin'ichiro Kawasaki

[PATCH 0/3] zbd: Fix issues found with test script

[Shin'ichiro Kawasaki]

This series addresses issues found with t/zbd/test-zbd-support script runs with
various conditions. The three patches fix issues about maximum open zone check,
reset all zones and random read/write on empty zones, respectively.

Please consider this series for the fio upstream.

Shin'ichiro Kawasaki (3):
  t/zbd: Fix a bug in max_open_zones()
  t/zbd: Fix a bug in reset_zone() for all zones reset
  zbd: Ensure first I/O is write for random read/write to sequential
    zones

 t/zbd/functions |  6 +++---
 zbd.c           | 10 ++++++++++
 2 files changed, 13 insertions(+), 3 deletions(-)

-- 
2.25.1

[PATCH 3/3] zbd: Ensure first I/O is write for random read/write to sequential zones

[Shin'ichiro Kawasaki]

In case read is chosen for the first random I/O for sequential write
required zones, fio stops because no data can be read from the zones with
empty status. Enforce to write at the first I/O to make sure data to read
exists for the following read operations.

The unexpected fio stop symptom was observed with test case #30 of
t/zbd/test-zbd-support. When the test case was run repeatedly resetting
all zones with -r option, it often passes with too short run time.

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

diff --git a/zbd.c b/zbd.c
index 0b0d4f40..abfa802c 100644
--- a/zbd.c
+++ b/zbd.c
@@ -1348,6 +1348,16 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 	if (!zbd_zone_swr(zb))
 		return io_u_accept;
 
+	/*
+	 * In case read direction is chosen for the first random I/O, fio with
+	 * zonemode=zbd stops because no data can be read from zoned block
+	 * devices with all empty zones. Overwrite the first I/O direction as
+	 * write to make sure data to read exists.
+	 */
+	if (td_rw(td) && !f->zbd_info->sectors_with_data
+	    && !td->o.read_beyond_wp)
+		io_u->ddir = DDIR_WRITE;
+
 	/*
 	 * Accept the I/O offset for reads if reading beyond the write pointer
 	 * is enabled.
-- 
2.25.1

Re: [PATCH 3/3] zbd: Ensure first I/O is write for random read/write to sequential zones

[Damien Le Moal]

On 2020/04/13 17:33, Shin'ichiro Kawasaki wrote:
> In case read is chosen for the first random I/O for sequential write
> required zones, fio stops because no data can be read from the zones with
> empty status. Enforce to write at the first I/O to make sure data to read
> exists for the following read operations.
> 
> The unexpected fio stop symptom was observed with test case #30 of
> t/zbd/test-zbd-support. When the test case was run repeatedly resetting
> all zones with -r option, it often passes with too short run time.
> 
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> ---
>  zbd.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/zbd.c b/zbd.c
> index 0b0d4f40..abfa802c 100644
> --- a/zbd.c
> +++ b/zbd.c
> @@ -1348,6 +1348,16 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
>  	if (!zbd_zone_swr(zb))
>  		return io_u_accept;
>  
> +	/*
> +	 * In case read direction is chosen for the first random I/O, fio with
> +	 * zonemode=zbd stops because no data can be read from zoned block
> +	 * devices with all empty zones. Overwrite the first I/O direction as
> +	 * write to make sure data to read exists.
> +	 */
> +	if (td_rw(td) && !f->zbd_info->sectors_with_data
> +	    && !td->o.read_beyond_wp)
> +		io_u->ddir = DDIR_WRITE;
> +
>  	/*
>  	 * Accept the I/O offset for reads if reading beyond the write pointer
>  	 * is enabled.
> 

Looks OK to me.

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


-- 
Damien Le Moal
Western Digital Research