[PATCH 0/5] zbd: Fix failures unique to specific zoned devices

[PATCH 0/5] zbd: Fix failures unique to specific zoned devices

[Shin'ichiro Kawasaki]

Recently test case failures of t/zbd/test-zoned-support were observed, which
were unique to specific zoned block devices. Several test cases fail when the
device has large zone size in GB order. Some test cases fail when zone capacity
is unaligned to block sizes. Some other test cases fail when the zoned block
device has strict open zone resource limitations.

This patch series addresses the failures. The 1st patch is a preparation for the
next patch. The 2nd and the 3rd patches fix the failures observed due to large
zone size. The 4th patch fixes the failures observed due to zone capacity
unaligned to block size. The last patch adds an option to the test script to
support strict zone resource limitations.

Shin'ichiro Kawasaki (5):
  zbd: Remove cast to unsigned long long for printf
  zbd: Fix type of local variable min_bs
  t/zbd: Do not use too large block size in test case #4
  t/zbd: Align block size to zone capacity
  t/zbd: Add -w option to ensure no open zone before write tests

 t/zbd/functions        |  26 ++++++++++
 t/zbd/test-zbd-support |  40 +++++++++------
 zbd.c                  | 107 +++++++++++++++++++----------------------
 3 files changed, 100 insertions(+), 73 deletions(-)

-- 
2.31.1

[PATCH 1/5] zbd: Remove cast to unsigned long long for printf

[Shin'ichiro Kawasaki]

Many of the variables in zbd.c have type uint64_t. They are casted to
unsigned long long and printed with printf %llu format to handle
uint64_t types difference among architectures. This requires many
lengthy casts to unsigned long long.

To simplify the code, remove the casts to unsigned long long. Some of
the casts are simply unnecessary. To remove other casts, replace the
printf format %llu with PRIu64 so that uint64_t type difference among
architectures is handled accordingly.

Fio build pass of this change was confirmed with 32bit ARM cross
compiler and 64bit x86 compiler.

Suggested-by: Niklas Cassel <niklas.cassel@wdc.com>
Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
 zbd.c | 91 +++++++++++++++++++++++++++--------------------------------
 1 file changed, 42 insertions(+), 49 deletions(-)

diff --git a/zbd.c b/zbd.c
index c0b0b81c..f175d490 100644
--- a/zbd.c
+++ b/zbd.c
@@ -83,12 +83,12 @@ int zbd_report_zones(struct thread_data *td, struct fio_file *f,
 		ret = blkzoned_report_zones(td, f, offset, zones, nr_zones);
 	if (ret < 0) {
 		td_verror(td, errno, "report zones failed");
-		log_err("%s: report zones from sector %llu failed (%d).\n",
-			f->file_name, (unsigned long long)offset >> 9, errno);
+		log_err("%s: report zones from sector %"PRIu64" failed (%d).\n",
+			f->file_name, offset >> 9, errno);
 	} else if (ret == 0) {
 		td_verror(td, errno, "Empty zone report");
-		log_err("%s: report zones from sector %llu is empty.\n",
-			f->file_name, (unsigned long long)offset >> 9);
+		log_err("%s: report zones from sector %"PRIu64" is empty.\n",
+			f->file_name, offset >> 9);
 		ret = -EIO;
 	}
 
@@ -116,9 +116,8 @@ int zbd_reset_wp(struct thread_data *td, struct fio_file *f,
 		ret = blkzoned_reset_wp(td, f, offset, length);
 	if (ret < 0) {
 		td_verror(td, errno, "resetting wp failed");
-		log_err("%s: resetting wp for %llu sectors at sector %llu failed (%d).\n",
-			f->file_name, (unsigned long long)length >> 9,
-			(unsigned long long)offset >> 9, errno);
+		log_err("%s: resetting wp for %"PRIu64" sectors at sector %"PRIu64" failed (%d).\n",
+			f->file_name, length >> 9, offset >> 9, errno);
 	}
 
 	return ret;
@@ -318,16 +317,16 @@ static bool zbd_verify_sizes(void)
 					return false;
 				}
 			} else if (td->o.zone_size != f->zbd_info->zone_size) {
-				log_err("%s: job parameter zonesize %llu does not match disk zone size %llu.\n",
-					f->file_name, (unsigned long long) td->o.zone_size,
-					(unsigned long long) f->zbd_info->zone_size);
+				log_err("%s: job parameter zonesize %llu does not match disk zone size %"PRIu64".\n",
+					f->file_name, td->o.zone_size,
+					f->zbd_info->zone_size);
 				return false;
 			}
 
 			if (td->o.zone_skip % td->o.zone_size) {
 				log_err("%s: zoneskip %llu is not a multiple of the device zone size %llu.\n",
-					f->file_name, (unsigned long long) td->o.zone_skip,
-					(unsigned long long) td->o.zone_size);
+					f->file_name, td->o.zone_skip,
+					td->o.zone_size);
 				return false;
 			}
 
@@ -341,9 +340,9 @@ static bool zbd_verify_sizes(void)
 						 f->file_name);
 					return false;
 				}
-				log_info("%s: rounded up offset from %llu to %llu\n",
-					 f->file_name, (unsigned long long) f->file_offset,
-					 (unsigned long long) new_offset);
+				log_info("%s: rounded up offset from %"PRIu64" to %"PRIu64"\n",
+					 f->file_name, f->file_offset,
+					 new_offset);
 				f->io_size -= (new_offset - f->file_offset);
 				f->file_offset = new_offset;
 			}
@@ -357,9 +356,9 @@ static bool zbd_verify_sizes(void)
 						 f->file_name);
 					return false;
 				}
-				log_info("%s: rounded down io_size from %llu to %llu\n",
-					 f->file_name, (unsigned long long) f->io_size,
-					 (unsigned long long) new_end - f->file_offset);
+				log_info("%s: rounded down io_size from %"PRIu64" to %"PRIu64"\n",
+					 f->file_name, f->io_size,
+					 new_end - f->file_offset);
 				f->io_size = new_end - f->file_offset;
 			}
 		}
@@ -388,17 +387,17 @@ static bool zbd_verify_bs(void)
 				continue;
 			zone_size = f->zbd_info->zone_size;
 			if (td_trim(td) && td->o.bs[DDIR_TRIM] != zone_size) {
-				log_info("%s: trim block size %llu is not the zone size %llu\n",
+				log_info("%s: trim block size %llu is not the zone size %"PRIu64"\n",
 					 f->file_name, td->o.bs[DDIR_TRIM],
-					 (unsigned long long)zone_size);
+					 zone_size);
 				return false;
 			}
 			for (k = 0; k < FIO_ARRAY_SIZE(td->o.bs); k++) {
 				if (td->o.verify != VERIFY_NONE &&
 				    zone_size % td->o.bs[k] != 0) {
-					log_info("%s: block size %llu is not a divisor of the zone size %llu\n",
+					log_info("%s: block size %llu is not a divisor of the zone size %"PRIu64"\n",
 						 f->file_name, td->o.bs[k],
-						 (unsigned long long)zone_size);
+						 zone_size);
 					return false;
 				}
 			}
@@ -448,8 +447,7 @@ static int init_zone_info(struct thread_data *td, struct fio_file *f)
 
 	if (zone_capacity > zone_size) {
 		log_err("%s: job parameter zonecapacity %llu is larger than zone size %llu\n",
-			f->file_name, (unsigned long long) td->o.zone_capacity,
-			(unsigned long long) td->o.zone_size);
+			f->file_name, td->o.zone_capacity, td->o.zone_size);
 		return 1;
 	}
 
@@ -525,15 +523,14 @@ static int parse_zone_info(struct thread_data *td, struct fio_file *f)
 	if (td->o.zone_size == 0) {
 		td->o.zone_size = zone_size;
 	} else if (td->o.zone_size != zone_size) {
-		log_err("fio: %s job parameter zonesize %llu does not match disk zone size %llu.\n",
-			f->file_name, (unsigned long long) td->o.zone_size,
-			(unsigned long long) zone_size);
+		log_err("fio: %s job parameter zonesize %llu does not match disk zone size %"PRIu64".\n",
+			f->file_name, td->o.zone_size, zone_size);
 		ret = -EINVAL;
 		goto out;
 	}
 
-	dprint(FD_ZBD, "Device %s has %d zones of size %llu KB\n", f->file_name,
-	       nr_zones, (unsigned long long) zone_size / 1024);
+	dprint(FD_ZBD, "Device %s has %d zones of size %"PRIu64" KB\n", f->file_name,
+	       nr_zones, zone_size / 1024);
 
 	zbd_info = scalloc(1, sizeof(*zbd_info) +
 			   (nr_zones + 1) * sizeof(zbd_info->zone_info[0]));
@@ -587,9 +584,8 @@ static int parse_zone_info(struct thread_data *td, struct fio_file *f)
 					   ZBD_REPORT_MAX_ZONES));
 		if (nrz < 0) {
 			ret = nrz;
-			log_info("fio: report zones (offset %llu) failed for %s (%d).\n",
-			 	 (unsigned long long)offset,
-				 f->file_name, -ret);
+			log_info("fio: report zones (offset %"PRIu64") failed for %s (%d).\n",
+				 offset, f->file_name, -ret);
 			goto out;
 		}
 	}
@@ -1440,8 +1436,8 @@ static struct fio_zone_info *zbd_replay_write_order(struct thread_data *td,
 	}
 
 	if (z->verify_block * min_bs >= z->capacity) {
-		log_err("%s: %d * %d >= %llu\n", f->file_name, z->verify_block,
-			min_bs, (unsigned long long)z->capacity);
+		log_err("%s: %d * %d >= %"PRIu64"\n", f->file_name, z->verify_block,
+			min_bs, z->capacity);
 		/*
 		 * If the assertion below fails during a test run, adding
 		 * "--experimental_verify=1" to the command line may help.
@@ -1450,8 +1446,8 @@ static struct fio_zone_info *zbd_replay_write_order(struct thread_data *td,
 	}
 	io_u->offset = z->start + z->verify_block * min_bs;
 	if (io_u->offset + io_u->buflen >= zbd_zone_capacity_end(z)) {
-		log_err("%s: %llu + %llu >= %llu\n", f->file_name, io_u->offset,
-			io_u->buflen, (unsigned long long) zbd_zone_capacity_end(z));
+		log_err("%s: %llu + %llu >= %"PRIu64"\n", f->file_name, io_u->offset,
+			io_u->buflen, zbd_zone_capacity_end(z));
 		assert(false);
 	}
 	z->verify_block += io_u->buflen / min_bs;
@@ -1672,10 +1668,9 @@ void setup_zbd_zone_mode(struct thread_data *td, struct io_u *io_u)
 	    f->last_pos[ddir] >= zbd_zone_capacity_end(z)) {
 		dprint(FD_ZBD,
 		       "%s: Jump from zone capacity limit to zone end:"
-		       " (%llu -> %llu) for zone %u (%llu)\n",
-		       f->file_name, (unsigned long long) f->last_pos[ddir],
-		       (unsigned long long) zbd_zone_end(z), zone_idx,
-		       (unsigned long long) z->capacity);
+		       " (%"PRIu64" -> %"PRIu64") for zone %u (%"PRIu64")\n",
+		       f->file_name, f->last_pos[ddir],
+		       zbd_zone_end(z), zone_idx, z->capacity);
 		td->io_skip_bytes += zbd_zone_end(z) - f->last_pos[ddir];
 		f->last_pos[ddir] = zbd_zone_end(z);
 	}
@@ -1785,9 +1780,9 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 
 		if (io_u->offset + min_bs > (zb + 1)->start) {
 			dprint(FD_IO,
-			       "%s: off=%llu + min_bs=%u > next zone %llu\n",
+			       "%s: off=%llu + min_bs=%u > next zone %"PRIu64"\n",
 			       f->file_name, io_u->offset,
-			       min_bs, (unsigned long long) (zb + 1)->start);
+			       min_bs, (zb + 1)->start);
 			io_u->offset = zb->start + (zb + 1)->start - io_u->offset;
 			new_len = min(io_u->buflen, (zb + 1)->start - io_u->offset);
 		} else {
@@ -1878,9 +1873,8 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 		if (io_u->buflen > zbdi->zone_size) {
 			td_verror(td, EINVAL, "I/O buflen exceeds zone size");
 			dprint(FD_IO,
-			       "%s: I/O buflen %llu exceeds zone size %llu\n",
-			       f->file_name, io_u->buflen,
-			       (unsigned long long) zbdi->zone_size);
+			       "%s: I/O buflen %llu exceeds zone size %"PRIu64"\n",
+			       f->file_name, io_u->buflen, zbdi->zone_size);
 			goto eof;
 		}
 		if (!zbd_open_zone(td, f, zone_idx_b)) {
@@ -1917,9 +1911,8 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 
 			if (zb->capacity < min_bs) {
 				td_verror(td, EINVAL, "ZCAP is less min_bs");
-				log_err("zone capacity %llu smaller than minimum block size %d\n",
-					(unsigned long long)zb->capacity,
-					min_bs);
+				log_err("zone capacity %"PRIu64" smaller than minimum block size %d\n",
+					zb->capacity, min_bs);
 				goto eof;
 			}
 		}
@@ -2006,7 +1999,7 @@ char *zbd_write_status(const struct thread_stat *ts)
 {
 	char *res;
 
-	if (asprintf(&res, "; %llu zone resets", (unsigned long long) ts->nr_zone_resets) < 0)
+	if (asprintf(&res, "; %"PRIu64" zone resets", ts->nr_zone_resets) < 0)
 		return NULL;
 	return res;
 }
-- 
2.31.1

[PATCH 2/5] zbd: Fix type of local variable min_bs

[Shin'ichiro Kawasaki]

In zbd.c, thread option min_bs[] is referred and stored in the local
variable min_bs. Elements of min_bs[] have type unsigned long long, but
the local variable min_bs has type uint32_t. When an element of min_bs[]
has value larger than UINT32_MAX, it overflows on assignment to min_bs.

To avoid the overflow, fix type of the local variable min_bs from
uint32_t to uint64_t. Use uint64_t rather than unsigned long long to be
more specific about data size and consistency in zbd.c. The variable is
passed to the helper function zbd_find_zone(), then fix the type of the
argument of the function also.

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

diff --git a/zbd.c b/zbd.c
index f175d490..c18998c4 100644
--- a/zbd.c
+++ b/zbd.c
@@ -968,7 +968,7 @@ static int zbd_reset_zones(struct thread_data *td, struct fio_file *f,
 			   struct fio_zone_info *const ze)
 {
 	struct fio_zone_info *z;
-	const uint32_t min_bs = td->o.min_bs[DDIR_WRITE];
+	const uint64_t min_bs = td->o.min_bs[DDIR_WRITE];
 	int res = 0;
 
 	assert(min_bs);
@@ -1141,7 +1141,7 @@ static bool is_zone_open(const struct thread_data *td, const struct fio_file *f,
 static bool zbd_open_zone(struct thread_data *td, const struct fio_file *f,
 			  uint32_t zone_idx)
 {
-	const uint32_t min_bs = td->o.min_bs[DDIR_WRITE];
+	const uint64_t min_bs = td->o.min_bs[DDIR_WRITE];
 	struct zoned_block_device_info *zbdi = f->zbd_info;
 	struct fio_zone_info *z = get_zone(f, zone_idx);
 	bool res = true;
@@ -1224,7 +1224,7 @@ static bool any_io_in_flight(void)
 static struct fio_zone_info *zbd_convert_to_open_zone(struct thread_data *td,
 						      struct io_u *io_u)
 {
-	const uint32_t min_bs = td->o.min_bs[io_u->ddir];
+	const uint64_t min_bs = td->o.min_bs[io_u->ddir];
 	struct fio_file *f = io_u->file;
 	struct zoned_block_device_info *zbdi = f->zbd_info;
 	struct fio_zone_info *z;
@@ -1427,7 +1427,7 @@ static struct fio_zone_info *zbd_replay_write_order(struct thread_data *td,
 						    struct fio_zone_info *z)
 {
 	const struct fio_file *f = io_u->file;
-	const uint32_t min_bs = td->o.min_bs[DDIR_WRITE];
+	const uint64_t min_bs = td->o.min_bs[DDIR_WRITE];
 
 	if (!zbd_open_zone(td, f, zbd_zone_nr(f, z))) {
 		zone_unlock(z);
@@ -1436,7 +1436,7 @@ static struct fio_zone_info *zbd_replay_write_order(struct thread_data *td,
 	}
 
 	if (z->verify_block * min_bs >= z->capacity) {
-		log_err("%s: %d * %d >= %"PRIu64"\n", f->file_name, z->verify_block,
+		log_err("%s: %d * %"PRIu64" >= %"PRIu64"\n", f->file_name, z->verify_block,
 			min_bs, z->capacity);
 		/*
 		 * If the assertion below fails during a test run, adding
@@ -1463,7 +1463,7 @@ static struct fio_zone_info *zbd_replay_write_order(struct thread_data *td,
  * pointer, hold the mutex for the zone.
  */
 static struct fio_zone_info *
-zbd_find_zone(struct thread_data *td, struct io_u *io_u, uint32_t min_bytes,
+zbd_find_zone(struct thread_data *td, struct io_u *io_u, uint64_t min_bytes,
 	      struct fio_zone_info *zb, struct fio_zone_info *zl)
 {
 	struct fio_file *f = io_u->file;
@@ -1495,7 +1495,7 @@ zbd_find_zone(struct thread_data *td, struct io_u *io_u, uint32_t min_bytes,
 				zone_unlock(z2);
 		}
 	}
-	dprint(FD_ZBD, "%s: no zone has %d bytes of readable data\n",
+	dprint(FD_ZBD, "%s: no zone has %"PRIu64" bytes of readable data\n",
 	       f->file_name, min_bytes);
 	return NULL;
 }
@@ -1754,7 +1754,7 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 	uint32_t zone_idx_b;
 	struct fio_zone_info *zb, *zl, *orig_zb;
 	uint32_t orig_len = io_u->buflen;
-	uint32_t min_bs = td->o.min_bs[io_u->ddir];
+	uint64_t min_bs = td->o.min_bs[io_u->ddir];
 	uint64_t new_len;
 	int64_t range;
 
@@ -1780,7 +1780,7 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 
 		if (io_u->offset + min_bs > (zb + 1)->start) {
 			dprint(FD_IO,
-			       "%s: off=%llu + min_bs=%u > next zone %"PRIu64"\n",
+			       "%s: off=%llu + min_bs=%"PRIu64" > next zone %"PRIu64"\n",
 			       f->file_name, io_u->offset,
 			       min_bs, (zb + 1)->start);
 			io_u->offset = zb->start + (zb + 1)->start - io_u->offset;
@@ -1911,7 +1911,7 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 
 			if (zb->capacity < min_bs) {
 				td_verror(td, EINVAL, "ZCAP is less min_bs");
-				log_err("zone capacity %"PRIu64" smaller than minimum block size %d\n",
+				log_err("zone capacity %"PRIu64" smaller than minimum block size %"PRIu64"\n",
 					zb->capacity, min_bs);
 				goto eof;
 			}
@@ -1942,7 +1942,7 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 			goto accept;
 		}
 		td_verror(td, EIO, "zone remainder too small");
-		log_err("zone remainder %lld smaller than min block size %d\n",
+		log_err("zone remainder %lld smaller than min block size %"PRIu64"\n",
 			(zbd_zone_capacity_end(zb) - io_u->offset), min_bs);
 		goto eof;
 	case DDIR_TRIM:
-- 
2.31.1

[PATCH 3/5] t/zbd: Do not use too large block size in test case #4

[Shin'ichiro Kawasaki]

The test case #4 specifies zone size as block size to read a zone. For
some devices, zone size is very large in GB order, then single pread64
system call can not complete the request. This makes the test case fail.

To avoid the failure, keep the block size adequate. If zone size is too
large, use logical_block_size * 256 as the block size.

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

diff --git a/t/zbd/test-zbd-support b/t/zbd/test-zbd-support
index 5103c406..f9dc9001 100755
--- a/t/zbd/test-zbd-support
+++ b/t/zbd/test-zbd-support
@@ -310,7 +310,8 @@ test4() {
     off=$((first_sequential_zone_sector * 512 + 129 * zone_size))
     size=$((zone_size))
     [ -n "$is_zbd" ] && reset_zone "$dev" $((off / 512))
-    opts+=("--name=$dev" "--filename=$dev" "--offset=$off" "--bs=$size")
+    opts+=("--name=$dev" "--filename=$dev" "--offset=$off")
+    opts+=(--bs="$(min $((logical_block_size * 256)) $size)")
     opts+=("--size=$size" "--thread=1" "--read_beyond_wp=1")
     opts+=("$(ioengine "psync")" "--rw=read" "--direct=1" "--disable_lat=1")
     opts+=("--zonemode=zbd" "--zonesize=${zone_size}")
-- 
2.31.1

[PATCH 4/5] t/zbd: Align block size to zone capacity

[Shin'ichiro Kawasaki]

The test cases #5, #6, #15 and #37 writes data and read it back (or
write with verify option for read back). When test target zones have
zone capacity unaligned to the block size, read request can not be made
to all of the written data, and the test cases fail.

To avoid the failures, check zone capacity of zones and get block size
which can align to the zone capacities. Then use the block size for the
test cases.

Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
 t/zbd/functions        | 26 ++++++++++++++++++++++++++
 t/zbd/test-zbd-support | 21 ++++++++++++---------
 2 files changed, 38 insertions(+), 9 deletions(-)

diff --git a/t/zbd/functions b/t/zbd/functions
index 08a2c629..e4e248b9 100644
--- a/t/zbd/functions
+++ b/t/zbd/functions
@@ -64,6 +64,32 @@ check_blkzone() {
 	fi
 }
 
+# Check zone capacity of each zone and report block size aligned to the zone
+# capacities. If zone capacity is same as zone size for zones, report zone size.
+zone_cap_bs() {
+	local dev="${1}"
+	local zone_size="${2}"
+	local sed_str='s/.*len \([0-9A-Za-z]*\), cap \([0-9A-Za-z]*\).*/\1 \2/p'
+	local cap bs="$zone_size"
+
+	# When blkzone is not available or blkzone does not report capacity,
+	# assume that zone capacity is same as zone size for all zones.
+	if [ -z "${blkzone}" ] || ! blkzone_reports_capacity "${dev}"; then
+		echo "$zone_size"
+		return
+	fi
+
+	while read -r -a line; do
+		((line[0] == line[1])) && continue
+		cap=$((line[1] * 512))
+		while ((bs > 512 && cap % bs)); do
+			bs=$((bs / 2))
+		done
+	done < <(blkzone report "${dev}" | sed -n "${sed_str}")
+
+	echo "$bs"
+}
+
 # Reports the starting sector and length of the first sequential zone of device
 # $1.
 first_sequential_zone() {
diff --git a/t/zbd/test-zbd-support b/t/zbd/test-zbd-support
index f9dc9001..4ee46de3 100755
--- a/t/zbd/test-zbd-support
+++ b/t/zbd/test-zbd-support
@@ -321,15 +321,15 @@ test4() {
 
 # Sequential write to sequential zones.
 test5() {
-    local size off capacity
+    local size off capacity bs
 
     prep_write
     off=$((first_sequential_zone_sector * 512))
     capacity=$(total_zone_capacity 4 $off $dev)
     size=$((4 * zone_size))
+    bs=$(min "$(max $((zone_size / 64)) "$logical_block_size")" "$zone_cap_bs")
     run_fio_on_seq "$(ioengine "psync")" --iodepth=1 --rw=write	\
-		   --bs="$(max $((zone_size / 64)) "$logical_block_size")"\
-		   --do_verify=1 --verify=md5				\
+		   --bs="$bs" --do_verify=1 --verify=md5 \
 		   >>"${logfile}.${test_number}" 2>&1 || return $?
     check_written $capacity || return $?
     check_read $capacity || return $?
@@ -337,18 +337,18 @@ test5() {
 
 # Sequential read from sequential zones.
 test6() {
-    local size off capacity
+    local size off capacity bs
 
     prep_write
     off=$((first_sequential_zone_sector * 512))
     capacity=$(total_zone_capacity 4 $off $dev)
     size=$((4 * zone_size))
+    bs=$(min "$(max $((zone_size / 64)) "$logical_block_size")" "$zone_cap_bs")
     write_and_run_one_fio_job \
 	    $((first_sequential_zone_sector * 512)) "${size}" \
 	    --offset="${off}" \
 	    --size="${size}" --zonemode=zbd --zonesize="${zone_size}" \
-	    "$(ioengine "psync")" --iodepth=1 --rw=read \
-	    --bs="$(max $((zone_size / 64)) "$logical_block_size")" \
+	    "$(ioengine "psync")" --iodepth=1 --rw=read --bs="$bs" \
 	    >>"${logfile}.${test_number}" 2>&1 || return $?
     check_read $capacity || return $?
 }
@@ -486,7 +486,7 @@ test14() {
 
 # Sequential read on a mix of empty and full zones.
 test15() {
-    local i off size
+    local i off size bs
     local w_off w_size w_capacity
 
     for ((i=0;i<4;i++)); do
@@ -500,8 +500,9 @@ test15() {
     w_capacity=$(total_zone_capacity 2 $w_off $dev)
     off=$((first_sequential_zone_sector * 512))
     size=$((4 * zone_size))
+    bs=$(min $((zone_size / 16)) "$zone_cap_bs")
     write_and_run_one_fio_job "${w_off}" "${w_size}" \
-		    "$(ioengine "psync")" --rw=read --bs=$((zone_size / 16)) \
+		    "$(ioengine "psync")" --rw=read --bs="$bs" \
 		    --zonemode=zbd --zonesize="${zone_size}" --offset=$off \
 		    --size=$((size)) >>"${logfile}.${test_number}" 2>&1 ||
 	return $?
@@ -853,7 +854,7 @@ test37() {
 	off=$(((first_sequential_zone_sector - 1) * 512))
     fi
     size=$((zone_size + 2 * 512))
-    bs=$((zone_size / 4))
+    bs=$(min $((zone_size / 4)) "$zone_cap_bs")
     run_one_fio_job --offset=$off --size=$size "$(ioengine "psync")"	\
 		    --iodepth=1 --rw=write --do_verify=1 --verify=md5	\
 		    --bs=$bs --zonemode=zbd --zonesize="${zone_size}"	\
@@ -1378,6 +1379,8 @@ fi
 echo -n "First sequential zone starts at sector $first_sequential_zone_sector;"
 echo " zone size: $((zone_size >> 20)) MB"
 
+zone_cap_bs=$(zone_cap_bs "$dev" "$zone_size")
+
 if [ "${#tests[@]}" = 0 ]; then
     readarray -t tests < <(declare -F | grep "test[0-9]*" | \
 				   tr -c -d "[:digit:]\n" | sort -n)
-- 
2.31.1

[PATCH 5/5] t/zbd: Add -w option to ensure no open zone before write tests

[Shin'ichiro Kawasaki]

The commit b34eb155e4a6 ("t/zbd: Reset all zones before test when max
open zones is specified") introduced -o max_open_zones option to the
script t/zbd/test-zbd-support. It passes max_open_zones value to fio and
resets all zones of the test target device before each test case run
with write operation. This zone reset by the script ensures that no zone
out of the IO range is in open status and the write operation do not
exceed the max_open_zones limit.

On the other hand, since commit d2f442bc0bd5 ("ioengines: add
get_max_open_zones zoned block device operation"), fio automatically
fetches the max_open_zones value. So it is no longer required to pass
the max_open_zones value from the script to fio. To simplify the script
usage, introduce -w option which does not require max_open_zones value.
This option just resets zones before test cases with write operation.

Of note is that fio itself resets the zones exceeding max_open_zones
limit since the commit 954217b90191 ("zbd: Initialize open zones list
referring zone status at fio start"), but it just resets zones within
the fio IO range. Still zone reset by the test script is required for
zones out of IO range. Zone reset out of IO range by fio is not
implemented since it may cause unexpected data erasure.

Suggested-by: Niklas Cassel <niklas.cassel@wdc.com>
Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
 t/zbd/test-zbd-support | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/t/zbd/test-zbd-support b/t/zbd/test-zbd-support
index 4ee46de3..7e2fff00 100755
--- a/t/zbd/test-zbd-support
+++ b/t/zbd/test-zbd-support
@@ -12,6 +12,7 @@ usage() {
 	echo -e "\t-v Run fio with valgrind --read-var-info option"
 	echo -e "\t-l Test with libzbc ioengine"
 	echo -e "\t-r Reset all zones before test start"
+	echo -e "\t-w Reset all zones before executing each write test case"
 	echo -e "\t-o <max_open_zones> Run fio with max_open_zones limit"
 	echo -e "\t-t <test #> Run only a single test case with specified number"
 	echo -e "\t-q Quit the test run after any failed test"
@@ -182,13 +183,14 @@ run_fio_on_seq() {
     run_one_fio_job "${opts[@]}" "$@"
 }
 
-# Prepare for write test by resetting zones. When max_open_zones option is
-# specified, reset all zones of the test target to ensure that zones out of the
-# test target range do not have open zones. This allows the write test to the
-# target range to be able to open zones up to max_open_zones.
+# Prepare for write test by resetting zones. When reset_before_write or
+# max_open_zones option is specified, reset all zones of the test target to
+# ensure that zones out of the test target range do not have open zones. This
+# allows the write test to the target range to be able to open zones up to
+# max_open_zones limit specified as the option or obtained from sysfs.
 prep_write() {
-	[[ -n "${max_open_zones_opt}" && -n "${is_zbd}" ]] &&
-		reset_zone "${dev}" -1
+	[[ -n "${reset_before_write}" || -n "${max_open_zones_opt}" ]] &&
+		[[ -n "${is_zbd}" ]] && reset_zone "${dev}" -1
 }
 
 SKIP_TESTCASE=255
@@ -1247,6 +1249,7 @@ SECONDS=0
 tests=()
 dynamic_analyzer=()
 reset_all_zones=
+reset_before_write=
 use_libzbc=
 zbd_debug=
 max_open_zones_opt=
@@ -1261,6 +1264,7 @@ while [ "${1#-}" != "$1" ]; do
 	shift;;
     -l) use_libzbc=1; shift;;
     -r) reset_all_zones=1; shift;;
+    -w) reset_before_write=1; shift;;
     -t) tests+=("$2"); shift; shift;;
     -o) max_open_zones_opt="${2}"; shift; shift;;
     -v) dynamic_analyzer=(valgrind "--read-var-info=yes");
-- 
2.31.1

Re: [PATCH 1/5] zbd: Remove cast to unsigned long long for printf

[Niklas Cassel]

On Wed, Oct 13, 2021 at 03:08:59PM +0900, Shin'ichiro Kawasaki wrote:
> Many of the variables in zbd.c have type uint64_t. They are casted to
> unsigned long long and printed with printf %llu format to handle
> uint64_t types difference among architectures. This requires many
> lengthy casts to unsigned long long.
> 
> To simplify the code, remove the casts to unsigned long long. Some of
> the casts are simply unnecessary. To remove other casts, replace the
> printf format %llu with PRIu64 so that uint64_t type difference among
> architectures is handled accordingly.
> 
> Fio build pass of this change was confirmed with 32bit ARM cross
> compiler and 64bit x86 compiler.
> 
> Suggested-by: Niklas Cassel <niklas.cassel@wdc.com>
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> ---

Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com>

Re: [PATCH 2/5] zbd: Fix type of local variable min_bs

[Niklas Cassel]

On Wed, Oct 13, 2021 at 03:09:00PM +0900, Shin'ichiro Kawasaki wrote:
> In zbd.c, thread option min_bs[] is referred and stored in the local
> variable min_bs. Elements of min_bs[] have type unsigned long long, but
> the local variable min_bs has type uint32_t. When an element of min_bs[]
> has value larger than UINT32_MAX, it overflows on assignment to min_bs.
> 
> To avoid the overflow, fix type of the local variable min_bs from
> uint32_t to uint64_t. Use uint64_t rather than unsigned long long to be
> more specific about data size and consistency in zbd.c. The variable is
> passed to the helper function zbd_find_zone(), then fix the type of the
> argument of the function also.
> 
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> ---

Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com>

Re: [PATCH 3/5] t/zbd: Do not use too large block size in test case #4

[Niklas Cassel]

On Wed, Oct 13, 2021 at 03:09:01PM +0900, Shin'ichiro Kawasaki wrote:
> The test case #4 specifies zone size as block size to read a zone. For
> some devices, zone size is very large in GB order, then single pread64
> system call can not complete the request. This makes the test case fail.
> 
> To avoid the failure, keep the block size adequate. If zone size is too
> large, use logical_block_size * 256 as the block size.
> 
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> ---

Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com>

Re: [PATCH 4/5] t/zbd: Align block size to zone capacity

[Niklas Cassel]

On Wed, Oct 13, 2021 at 03:09:02PM +0900, Shin'ichiro Kawasaki wrote:
> The test cases #5, #6, #15 and #37 writes data and read it back (or
> write with verify option for read back). When test target zones have
> zone capacity unaligned to the block size, read request can not be made
> to all of the written data, and the test cases fail.
> 
> To avoid the failures, check zone capacity of zones and get block size
> which can align to the zone capacities. Then use the block size for the
> test cases.
> 
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> ---

Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com>

Re: [PATCH 5/5] t/zbd: Add -w option to ensure no open zone before write tests

[Niklas Cassel]

On Wed, Oct 13, 2021 at 03:09:03PM +0900, Shin'ichiro Kawasaki wrote:
> The commit b34eb155e4a6 ("t/zbd: Reset all zones before test when max
> open zones is specified") introduced -o max_open_zones option to the
> script t/zbd/test-zbd-support. It passes max_open_zones value to fio and
> resets all zones of the test target device before each test case run
> with write operation. This zone reset by the script ensures that no zone
> out of the IO range is in open status and the write operation do not
> exceed the max_open_zones limit.
> 
> On the other hand, since commit d2f442bc0bd5 ("ioengines: add
> get_max_open_zones zoned block device operation"), fio automatically
> fetches the max_open_zones value. So it is no longer required to pass
> the max_open_zones value from the script to fio. To simplify the script
> usage, introduce -w option which does not require max_open_zones value.
> This option just resets zones before test cases with write operation.
> 
> Of note is that fio itself resets the zones exceeding max_open_zones
> limit since the commit 954217b90191 ("zbd: Initialize open zones list
> referring zone status at fio start"), but it just resets zones within
> the fio IO range. Still zone reset by the test script is required for
> zones out of IO range. Zone reset out of IO range by fio is not
> implemented since it may cause unexpected data erasure.
> 
> Suggested-by: Niklas Cassel <niklas.cassel@wdc.com>
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> ---

Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com>

Re: [PATCH 0/5] zbd: Fix failures unique to specific zoned devices

[Jens Axboe]

On Wed, 13 Oct 2021 15:08:58 +0900, Shin'ichiro Kawasaki wrote:
> Recently test case failures of t/zbd/test-zoned-support were observed, which
> were unique to specific zoned block devices. Several test cases fail when the
> device has large zone size in GB order. Some test cases fail when zone capacity
> is unaligned to block sizes. Some other test cases fail when the zoned block
> device has strict open zone resource limitations.
> 
> This patch series addresses the failures. The 1st patch is a preparation for the
> next patch. The 2nd and the 3rd patches fix the failures observed due to large
> zone size. The 4th patch fixes the failures observed due to zone capacity
> unaligned to block size. The last patch adds an option to the test script to
> support strict zone resource limitations.
> 
> [...]

Applied, thanks!

[1/5] zbd: Remove cast to unsigned long long for printf
      commit: ee5e3436012aad0d2a06c7638dad39f01ee44c1e
[2/5] zbd: Fix type of local variable min_bs
      commit: 07fc3f57d106e58e0bcfff1a46ec8b4266770388
[3/5] t/zbd: Do not use too large block size in test case #4
      commit: 1f5dd7fa0f2425b5c21ce2fbe2944a57df462632
[4/5] t/zbd: Align block size to zone capacity
      commit: 1ae82d673cf5e704b93da56655f6189f43a1592e
[5/5] t/zbd: Add -w option to ensure no open zone before write tests
      commit: ccf9d1efdae8a440fff750037117428f27e40b43

Best regards,
-- 
Jens Axboe