[PATCH v2 1/2] zbd: add missing client/server support for option max_open_zones

[PATCH v2 1/2] zbd: add missing client/server support for option max_open_zones

[Niklas Cassel]

From: Niklas Cassel <niklas.cassel@wdc.com>

Ensure that we convert the max_open_zones option for client/server.

Use __cpu_to_le32()/__le32_to_cpu() rather than
cpu_to_le32()/le32_to_cpu(), since max_open_zones is defined
as int rather than unsigned int in thread_options.h.

Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
---
Changes since V1:
- New patch.

 cconv.c          | 2 ++
 server.h         | 2 +-
 thread_options.h | 1 +
 3 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/cconv.c b/cconv.c
index aa06e3ea..d4dfb81b 100644
--- a/cconv.c
+++ b/cconv.c
@@ -231,6 +231,7 @@ void convert_thread_options_to_cpu(struct thread_options *o,
 	o->zone_capacity = le64_to_cpu(top->zone_capacity);
 	o->zone_skip = le64_to_cpu(top->zone_skip);
 	o->zone_mode = le32_to_cpu(top->zone_mode);
+	o->max_open_zones = __le32_to_cpu(top->max_open_zones);
 	o->lockmem = le64_to_cpu(top->lockmem);
 	o->offset_increment_percent = le32_to_cpu(top->offset_increment_percent);
 	o->offset_increment = le64_to_cpu(top->offset_increment);
@@ -573,6 +574,7 @@ void convert_thread_options_to_net(struct thread_options_pack *top,
 	top->zone_capacity = __cpu_to_le64(o->zone_capacity);
 	top->zone_skip = __cpu_to_le64(o->zone_skip);
 	top->zone_mode = __cpu_to_le32(o->zone_mode);
+	top->max_open_zones = __cpu_to_le32(o->max_open_zones);
 	top->lockmem = __cpu_to_le64(o->lockmem);
 	top->ddir_seq_add = __cpu_to_le64(o->ddir_seq_add);
 	top->file_size_low = __cpu_to_le64(o->file_size_low);
diff --git a/server.h b/server.h
index b45b319b..8cf3a60b 100644
--- a/server.h
+++ b/server.h
@@ -48,7 +48,7 @@ struct fio_net_cmd_reply {
 };
 
 enum {
-	FIO_SERVER_VER			= 89,
+	FIO_SERVER_VER			= 90,
 
 	FIO_SERVER_MAX_FRAGMENT_PDU	= 1024,
 	FIO_SERVER_MAX_CMD_MB		= 2048,
diff --git a/thread_options.h b/thread_options.h
index 5ecc72d7..4d48e462 100644
--- a/thread_options.h
+++ b/thread_options.h
@@ -656,6 +656,7 @@ struct thread_options_pack {
 	uint32_t allow_mounted_write;
 
 	uint32_t zone_mode;
+	int32_t max_open_zones;
 } __attribute__((packed));
 
 extern void convert_thread_options_to_cpu(struct thread_options *o, struct thread_options_pack *top);
-- 
2.25.1

[PATCH v2 2/2] zbd: add a new --ignore_zone_limits option

[Niklas Cassel]

From: Niklas Cassel <niklas.cassel@wdc.com>

In commit d2f442bc0bd5 ("ioengines: add get_max_open_zones zoned block
device operation") we added a check that verifies that the specified
--max_open_zones value is lower than the max value reported by the device.

For ZNS devices there is a max open zones and a max active zones limit.
For ZAC/ZBC devices there is only a max open zones limit.

On ZAC/ZBC, there is thus no limit on the amount of zones that can be
in zone state closed.
When doing a write to an empty or closed zone, a ZAC/ZBC drive will
close an arbitrary implicit open zone in order in order to handle the
write.

The ZNS specification has no requirement on closing a zone in order to
handle a write to an empty or closed zone. The drive is free to return
an error.

Even on ZAC/ZBC, you do not want to exceed the max open zones limit,
since it will lead to additional implicit close zone and implicit open
zone operations, which will degrade performance.
However, it seems that this is sometimes done on purpose, in order to
measure the overhead of these additional operations. Therefore, add
an option that allows the user to ignore the reported device limits.

Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
---
Changes since V1:
- Added option to thread_options_pack and fixed missing cconv support.
- Bumped server version.
- Changed from bool to unsigned int in thread_options.h while still
  being defined as bool in options.c. This matches how it's done for
  other bool options, e.g. do_verify.

 cconv.c          |  2 ++
 fio.1            |  5 +++++
 options.c        | 10 ++++++++++
 server.h         |  2 +-
 thread_options.h |  2 ++
 zbd.c            |  2 +-
 6 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/cconv.c b/cconv.c
index d4dfb81b..74c24106 100644
--- a/cconv.c
+++ b/cconv.c
@@ -232,6 +232,7 @@ void convert_thread_options_to_cpu(struct thread_options *o,
 	o->zone_skip = le64_to_cpu(top->zone_skip);
 	o->zone_mode = le32_to_cpu(top->zone_mode);
 	o->max_open_zones = __le32_to_cpu(top->max_open_zones);
+	o->ignore_zone_limits = le32_to_cpu(top->ignore_zone_limits);
 	o->lockmem = le64_to_cpu(top->lockmem);
 	o->offset_increment_percent = le32_to_cpu(top->offset_increment_percent);
 	o->offset_increment = le64_to_cpu(top->offset_increment);
@@ -575,6 +576,7 @@ void convert_thread_options_to_net(struct thread_options_pack *top,
 	top->zone_skip = __cpu_to_le64(o->zone_skip);
 	top->zone_mode = __cpu_to_le32(o->zone_mode);
 	top->max_open_zones = __cpu_to_le32(o->max_open_zones);
+	top->ignore_zone_limits = cpu_to_le32(o->ignore_zone_limits);
 	top->lockmem = __cpu_to_le64(o->lockmem);
 	top->ddir_seq_add = __cpu_to_le64(o->ddir_seq_add);
 	top->file_size_low = __cpu_to_le64(o->file_size_low);
diff --git a/fio.1 b/fio.1
index ab08cb01..3519797a 100644
--- a/fio.1
+++ b/fio.1
@@ -835,6 +835,11 @@ threads/processes.
 .BI job_max_open_zones \fR=\fPint
 Limit on the number of simultaneously opened zones per single thread/process.
 .TP
+.BI ignore_zone_limits \fR=\fPbool
+If this isn't set, fio will query the max open zones limit from the zoned block
+device, and exit if the specified \fBmax_open_zones\fR value is larger than the
+max value reported by the device. Default: false.
+.TP
 .BI zone_reset_threshold \fR=\fPfloat
 A number between zero and one that indicates the ratio of logical blocks with
 data to the total number of logical blocks in the test above which zones
diff --git a/options.c b/options.c
index b82a10aa..a8986d11 100644
--- a/options.c
+++ b/options.c
@@ -3492,6 +3492,16 @@ struct fio_option fio_options[FIO_MAX_OPTS] = {
 		.category = FIO_OPT_C_IO,
 		.group	= FIO_OPT_G_INVALID,
 	},
+	{
+		.name	= "ignore_zone_limits",
+		.lname	= "Ignore zone resource limits",
+		.type	= FIO_OPT_BOOL,
+		.off1	= offsetof(struct thread_options, ignore_zone_limits),
+		.def	= "0",
+		.help	= "Ignore the zone resource limits (max open/active zones) reported by the device",
+		.category = FIO_OPT_C_IO,
+		.group	= FIO_OPT_G_INVALID,
+	},
 	{
 		.name	= "zone_reset_threshold",
 		.lname	= "Zone reset threshold",
diff --git a/server.h b/server.h
index 8cf3a60b..c128df28 100644
--- a/server.h
+++ b/server.h
@@ -48,7 +48,7 @@ struct fio_net_cmd_reply {
 };
 
 enum {
-	FIO_SERVER_VER			= 90,
+	FIO_SERVER_VER			= 91,
 
 	FIO_SERVER_MAX_FRAGMENT_PDU	= 1024,
 	FIO_SERVER_MAX_CMD_MB		= 2048,
diff --git a/thread_options.h b/thread_options.h
index 4d48e462..05c2d138 100644
--- a/thread_options.h
+++ b/thread_options.h
@@ -355,6 +355,7 @@ struct thread_options {
 	unsigned int read_beyond_wp;
 	int max_open_zones;
 	unsigned int job_max_open_zones;
+	unsigned int ignore_zone_limits;
 	fio_fp64_t zrt;
 	fio_fp64_t zrf;
 };
@@ -657,6 +658,7 @@ struct thread_options_pack {
 
 	uint32_t zone_mode;
 	int32_t max_open_zones;
+	uint32_t ignore_zone_limits;
 } __attribute__((packed));
 
 extern void convert_thread_options_to_cpu(struct thread_options *o, struct thread_options_pack *top);
diff --git a/zbd.c b/zbd.c
index 68cd58e1..5d9e331a 100644
--- a/zbd.c
+++ b/zbd.c
@@ -588,7 +588,7 @@ static int zbd_set_max_open_zones(struct thread_data *td, struct fio_file *f)
 	unsigned int max_open_zones;
 	int ret;
 
-	if (zbd->model != ZBD_HOST_MANAGED) {
+	if (zbd->model != ZBD_HOST_MANAGED || td->o.ignore_zone_limits) {
 		/* Only host-managed devices have a max open limit */
 		zbd->max_open_zones = td->o.max_open_zones;
 		goto out;
-- 
2.25.1

Re: [PATCH v2 1/2] zbd: add missing client/server support for option max_open_zones

[Damien Le Moal]

On 2021/05/24 23:41, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@wdc.com>
> 
> Ensure that we convert the max_open_zones option for client/server.
> 
> Use __cpu_to_le32()/__le32_to_cpu() rather than
> cpu_to_le32()/le32_to_cpu(), since max_open_zones is defined
> as int rather than unsigned int in thread_options.h.
> 
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> ---
> Changes since V1:
> - New patch.
> 
>  cconv.c          | 2 ++
>  server.h         | 2 +-
>  thread_options.h | 1 +
>  3 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/cconv.c b/cconv.c
> index aa06e3ea..d4dfb81b 100644
> --- a/cconv.c
> +++ b/cconv.c
> @@ -231,6 +231,7 @@ void convert_thread_options_to_cpu(struct thread_options *o,
>  	o->zone_capacity = le64_to_cpu(top->zone_capacity);
>  	o->zone_skip = le64_to_cpu(top->zone_skip);
>  	o->zone_mode = le32_to_cpu(top->zone_mode);
> +	o->max_open_zones = __le32_to_cpu(top->max_open_zones);
>  	o->lockmem = le64_to_cpu(top->lockmem);
>  	o->offset_increment_percent = le32_to_cpu(top->offset_increment_percent);
>  	o->offset_increment = le64_to_cpu(top->offset_increment);
> @@ -573,6 +574,7 @@ void convert_thread_options_to_net(struct thread_options_pack *top,
>  	top->zone_capacity = __cpu_to_le64(o->zone_capacity);
>  	top->zone_skip = __cpu_to_le64(o->zone_skip);
>  	top->zone_mode = __cpu_to_le32(o->zone_mode);
> +	top->max_open_zones = __cpu_to_le32(o->max_open_zones);
>  	top->lockmem = __cpu_to_le64(o->lockmem);
>  	top->ddir_seq_add = __cpu_to_le64(o->ddir_seq_add);
>  	top->file_size_low = __cpu_to_le64(o->file_size_low);
> diff --git a/server.h b/server.h
> index b45b319b..8cf3a60b 100644
> --- a/server.h
> +++ b/server.h
> @@ -48,7 +48,7 @@ struct fio_net_cmd_reply {
>  };
>  
>  enum {
> -	FIO_SERVER_VER			= 89,
> +	FIO_SERVER_VER			= 90,
>  
>  	FIO_SERVER_MAX_FRAGMENT_PDU	= 1024,
>  	FIO_SERVER_MAX_CMD_MB		= 2048,
> diff --git a/thread_options.h b/thread_options.h
> index 5ecc72d7..4d48e462 100644
> --- a/thread_options.h
> +++ b/thread_options.h
> @@ -656,6 +656,7 @@ struct thread_options_pack {
>  	uint32_t allow_mounted_write;
>  
>  	uint32_t zone_mode;
> +	int32_t max_open_zones;
>  } __attribute__((packed));
>  
>  extern void convert_thread_options_to_cpu(struct thread_options *o, struct thread_options_pack *top);
> 

Looks good.

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


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH v2 2/2] zbd: add a new --ignore_zone_limits option

[Damien Le Moal]

On 2021/05/24 23:41, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@wdc.com>
> 
> In commit d2f442bc0bd5 ("ioengines: add get_max_open_zones zoned block
> device operation") we added a check that verifies that the specified
> --max_open_zones value is lower than the max value reported by the device.
> 
> For ZNS devices there is a max open zones and a max active zones limit.
> For ZAC/ZBC devices there is only a max open zones limit.
> 
> On ZAC/ZBC, there is thus no limit on the amount of zones that can be
> in zone state closed.
> When doing a write to an empty or closed zone, a ZAC/ZBC drive will
> close an arbitrary implicit open zone in order in order to handle the

one "in order" too much :)

> write.
> 
> The ZNS specification has no requirement on closing a zone in order to
> handle a write to an empty or closed zone. The drive is free to return
> an error.
> 
> Even on ZAC/ZBC, you do not want to exceed the max open zones limit,
> since it will lead to additional implicit close zone and implicit open
> zone operations, which will degrade performance.

s/will/may

> However, it seems that this is sometimes done on purpose, in order to
> measure the overhead of these additional operations. Therefore, add
> an option that allows the user to ignore the reported device limits.
> 
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> ---
> Changes since V1:
> - Added option to thread_options_pack and fixed missing cconv support.
> - Bumped server version.
> - Changed from bool to unsigned int in thread_options.h while still
>   being defined as bool in options.c. This matches how it's done for
>   other bool options, e.g. do_verify.
> 
>  cconv.c          |  2 ++
>  fio.1            |  5 +++++
>  options.c        | 10 ++++++++++
>  server.h         |  2 +-
>  thread_options.h |  2 ++
>  zbd.c            |  2 +-
>  6 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/cconv.c b/cconv.c
> index d4dfb81b..74c24106 100644
> --- a/cconv.c
> +++ b/cconv.c
> @@ -232,6 +232,7 @@ void convert_thread_options_to_cpu(struct thread_options *o,
>  	o->zone_skip = le64_to_cpu(top->zone_skip);
>  	o->zone_mode = le32_to_cpu(top->zone_mode);
>  	o->max_open_zones = __le32_to_cpu(top->max_open_zones);
> +	o->ignore_zone_limits = le32_to_cpu(top->ignore_zone_limits);
>  	o->lockmem = le64_to_cpu(top->lockmem);
>  	o->offset_increment_percent = le32_to_cpu(top->offset_increment_percent);
>  	o->offset_increment = le64_to_cpu(top->offset_increment);
> @@ -575,6 +576,7 @@ void convert_thread_options_to_net(struct thread_options_pack *top,
>  	top->zone_skip = __cpu_to_le64(o->zone_skip);
>  	top->zone_mode = __cpu_to_le32(o->zone_mode);
>  	top->max_open_zones = __cpu_to_le32(o->max_open_zones);
> +	top->ignore_zone_limits = cpu_to_le32(o->ignore_zone_limits);
>  	top->lockmem = __cpu_to_le64(o->lockmem);
>  	top->ddir_seq_add = __cpu_to_le64(o->ddir_seq_add);
>  	top->file_size_low = __cpu_to_le64(o->file_size_low);
> diff --git a/fio.1 b/fio.1
> index ab08cb01..3519797a 100644
> --- a/fio.1
> +++ b/fio.1
> @@ -835,6 +835,11 @@ threads/processes.
>  .BI job_max_open_zones \fR=\fPint
>  Limit on the number of simultaneously opened zones per single thread/process.
>  .TP
> +.BI ignore_zone_limits \fR=\fPbool
> +If this isn't set, fio will query the max open zones limit from the zoned block
> +device, and exit if the specified \fBmax_open_zones\fR value is larger than the
> +max value reported by the device. Default: false.

s/max/maximum or s/max value/limit

> +.TP
>  .BI zone_reset_threshold \fR=\fPfloat
>  A number between zero and one that indicates the ratio of logical blocks with
>  data to the total number of logical blocks in the test above which zones
> diff --git a/options.c b/options.c
> index b82a10aa..a8986d11 100644
> --- a/options.c
> +++ b/options.c
> @@ -3492,6 +3492,16 @@ struct fio_option fio_options[FIO_MAX_OPTS] = {
>  		.category = FIO_OPT_C_IO,
>  		.group	= FIO_OPT_G_INVALID,
>  	},
> +	{
> +		.name	= "ignore_zone_limits",
> +		.lname	= "Ignore zone resource limits",
> +		.type	= FIO_OPT_BOOL,
> +		.off1	= offsetof(struct thread_options, ignore_zone_limits),
> +		.def	= "0",
> +		.help	= "Ignore the zone resource limits (max open/active zones) reported by the device",
> +		.category = FIO_OPT_C_IO,
> +		.group	= FIO_OPT_G_INVALID,
> +	},
>  	{
>  		.name	= "zone_reset_threshold",
>  		.lname	= "Zone reset threshold",
> diff --git a/server.h b/server.h
> index 8cf3a60b..c128df28 100644
> --- a/server.h
> +++ b/server.h
> @@ -48,7 +48,7 @@ struct fio_net_cmd_reply {
>  };
>  
>  enum {
> -	FIO_SERVER_VER			= 90,
> +	FIO_SERVER_VER			= 91,
>  
>  	FIO_SERVER_MAX_FRAGMENT_PDU	= 1024,
>  	FIO_SERVER_MAX_CMD_MB		= 2048,
> diff --git a/thread_options.h b/thread_options.h
> index 4d48e462..05c2d138 100644
> --- a/thread_options.h
> +++ b/thread_options.h
> @@ -355,6 +355,7 @@ struct thread_options {
>  	unsigned int read_beyond_wp;
>  	int max_open_zones;
>  	unsigned int job_max_open_zones;
> +	unsigned int ignore_zone_limits;
>  	fio_fp64_t zrt;
>  	fio_fp64_t zrf;
>  };
> @@ -657,6 +658,7 @@ struct thread_options_pack {
>  
>  	uint32_t zone_mode;
>  	int32_t max_open_zones;
> +	uint32_t ignore_zone_limits;
>  } __attribute__((packed));
>  
>  extern void convert_thread_options_to_cpu(struct thread_options *o, struct thread_options_pack *top);
> diff --git a/zbd.c b/zbd.c
> index 68cd58e1..5d9e331a 100644
> --- a/zbd.c
> +++ b/zbd.c
> @@ -588,7 +588,7 @@ static int zbd_set_max_open_zones(struct thread_data *td, struct fio_file *f)
>  	unsigned int max_open_zones;
>  	int ret;
>  
> -	if (zbd->model != ZBD_HOST_MANAGED) {
> +	if (zbd->model != ZBD_HOST_MANAGED || td->o.ignore_zone_limits) {
>  		/* Only host-managed devices have a max open limit */
>  		zbd->max_open_zones = td->o.max_open_zones;
>  		goto out;
> 

With the nits above corrected,

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

-- 
Damien Le Moal
Western Digital Research