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

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

[Ankit Kumar]

v1 -> v2

 - Addressed the commit message review comments for both the patches

Ankit Kumar (2):
  HOWTO: 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(-)

-- 
2.30.0-rc0

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

[Ankit Kumar]

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

diff --git a/HOWTO b/HOWTO
index 59c7f1ff..d4e620de 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
-- 
2.30.0-rc0

[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.

It reverts part of the commit 6463db6c1
('fio: fix interaction between offset/size limited threads and
"max_open_zones"')
---
 zbd.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/zbd.c b/zbd.c
index 04c68dea..43f12b45 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;
 }
 
 /*
-- 
2.30.0-rc0

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

[Shinichiro Kawasaki]

On Aug 04, 2021 / 14:54, Ankit Kumar wrote:
> v1 -> v2
> 
>  - Addressed the commit message review comments for both the patches
> 
> Ankit Kumar (2):
>   HOWTO: Add missing documentation for job_max_open_zones
>   zbd: Improve random zone index generation logic

Ankit, when I commented the v1 patches, I overlooked that your commit messages
do not include your Signed-off-by tags. Many of the fio commits have the tags.
To clarify who contributed, I suggest to add it to the two patches.

-- 
Best Regards,
Shin'ichiro Kawasaki

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

[Jens Axboe]

On 8/4/21 5:07 AM, Shinichiro Kawasaki wrote:
> On Aug 04, 2021 / 14:54, Ankit Kumar wrote:
>> v1 -> v2
>>
>>  - Addressed the commit message review comments for both the patches
>>
>> Ankit Kumar (2):
>>   HOWTO: Add missing documentation for job_max_open_zones
>>   zbd: Improve random zone index generation logic
> 
> Ankit, when I commented the v1 patches, I overlooked that your commit messages
> do not include your Signed-off-by tags. Many of the fio commits have the tags.
> To clarify who contributed, I suggest to add it to the two patches.

Indeed, please always add those.

-- 
Jens Axboe

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

[Ankit Kumar]

v2 -> v2

 - Added missing signoff tags with the patches

Ankit Kumar (2):
  HOWTO: 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(-)

-- 
2.30.0-rc0

[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.

It reverts part of the commit 6463db6c1
('fio: fix interaction between offset/size limited threads and
"max_open_zones"')

Signed-off-by: Ankit Kumar <ankit.kumar@samsung.com>
---
 zbd.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/zbd.c b/zbd.c
index 04c68dea..43f12b45 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;
 }
 
 /*
-- 
2.30.0-rc0

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

[Shinichiro Kawasaki]

On Aug 04, 2021 / 16:53, 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.
> 
> It reverts part of the commit 6463db6c1
> ('fio: fix interaction between offset/size limited threads and
> "max_open_zones"')
> 
> Signed-off-by: Ankit Kumar <ankit.kumar@samsung.com>

Looks good to me. Also, I confirmed that this patch improves the randomness of
open zone selection, and it preserves the intent of the commit 6463db6c1.

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

-- 
Best Regards,
Shin'ichiro Kawasaki

[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 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