[PATCH 0/2] ZBD fix with random zone index logic and missing documentation.

[PATCH 0/2] ZBD fix with random zone index logic and missing documentation.

[Ankit Kumar]

ZBD fix with random zone index logic and missing documentation. 

Ankit Kumar (2):
  Add missing documentation for job_max_open_zones.
  zbd: Improve random zone index generation logic.

 HOWTO | 5 +++++
 zbd.c | 5 +++--
 2 files changed, 8 insertions(+), 2 deletions(-)

-- 
1.8.3.1

[PATCH 1/2] Add missing documentation for job_max_open_zones.

[Ankit Kumar]

---
 HOWTO | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/HOWTO b/HOWTO
index 59c7f1f..d4e620d 100644
--- a/HOWTO
+++ b/HOWTO
@@ -1055,6 +1055,11 @@ Target file/device
 	number of open zones is defined as the number of zones to which write
 	commands are issued.
 
+.. option:: job_max_open_zones=int
+
+	Limit on the number of simultaneously opened zones per single
+	thread/process.
+
 .. option:: zone_reset_threshold=float
 
 	A number between zero and one that indicates the ratio of logical
-- 
1.8.3.1

Re: [PATCH 1/2] Add missing documentation for job_max_open_zones.

[Shinichiro Kawasaki]

Hello Ankit,

On Aug 03, 2021 / 15:58, Ankit Kumar wrote:
> ---
>  HOWTO | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/HOWTO b/HOWTO
> index 59c7f1f..d4e620d 100644
> --- a/HOWTO
> +++ b/HOWTO
> @@ -1055,6 +1055,11 @@ Target file/device
>  	number of open zones is defined as the number of zones to which write
>  	commands are issued.
>  
> +.. option:: job_max_open_zones=int
> +
> +	Limit on the number of simultaneously opened zones per single
> +	thread/process.
> +

This change looks good to me, but I think the commit message title can be
improved. "HOWTO: " prefix will help to identify the change target file. Also,
I suggest to remove the period in the title. As far as I know, all fio commits
do not have period in their commit message titles.

>  .. option:: zone_reset_threshold=float
>  
>  	A number between zero and one that indicates the ratio of logical
> -- 
> 1.8.3.1
> 

-- 
Best Regards,
Shin'ichiro Kawasaki

[PATCH 2/2] zbd: Improve random zone index generation logic.

[Ankit Kumar]

Existing random zone index generation logic is dependent on the file size.
For smaller I/O sizes the random zone index always return a particular
section of open zones. As this index is used to return one of the open zones,
it was observed that after one of the max_open_zones / job_max_open_zones limit
is reached all further I/O's are redirected to only a few open zones till they
are full.

This patch modifies the random zone index genration logic so that it is uniform
across all the open zones.
---
 zbd.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/zbd.c b/zbd.c
index 04c68de..43f12b4 100644
--- a/zbd.c
+++ b/zbd.c
@@ -1184,11 +1184,12 @@ out:
 	return res;
 }
 
-/* Anything goes as long as it is not a constant. */
+/* Return random zone index for one of the open zones. */
 static uint32_t pick_random_zone_idx(const struct fio_file *f,
 				     const struct io_u *io_u)
 {
-	return io_u->offset * f->zbd_info->num_open_zones / f->real_file_size;
+	return (io_u->offset - f->file_offset) * f->zbd_info->num_open_zones /
+		f->io_size;
 }
 
 /*
-- 
1.8.3.1

Re: [PATCH 2/2] zbd: Improve random zone index generation logic.

[Shinichiro Kawasaki]

On Aug 03, 2021 / 15:58, Ankit Kumar wrote:
> Existing random zone index generation logic is dependent on the file size.
> For smaller I/O sizes the random zone index always return a particular
> section of open zones. As this index is used to return one of the open zones,
> it was observed that after one of the max_open_zones / job_max_open_zones limit
> is reached all further I/O's are redirected to only a few open zones till they
> are full.
> 
> This patch modifies the random zone index genration logic so that it is uniform
> across all the open zones.
> ---
>  zbd.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/zbd.c b/zbd.c
> index 04c68de..43f12b4 100644
> --- a/zbd.c
> +++ b/zbd.c
> @@ -1184,11 +1184,12 @@ out:
>  	return res;
>  }
>  
> -/* Anything goes as long as it is not a constant. */
> +/* Return random zone index for one of the open zones. */
>  static uint32_t pick_random_zone_idx(const struct fio_file *f,
>  				     const struct io_u *io_u)
>  {
> -	return io_u->offset * f->zbd_info->num_open_zones / f->real_file_size;
> +	return (io_u->offset - f->file_offset) * f->zbd_info->num_open_zones /
> +		f->io_size;
>  }

One nit I suggest is to remove the period in the commit message title. Other
than that, I confirmed the change is good. I confirmed that it improves the
randomness of the open zone choice. It reverts the part of the commit 6463db6c1
('fio: fix interaction between offset/size limited threads and
"max_open_zones"'), but I confirmed that still the intent of the commit
6463db6c1 is kept after applying this change.

Tested-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>

-- 
Best Regards,
Shin'ichiro Kawasaki