[PATCH v3 00/38] ZBD fixes and improvements

[PATCH v3 00/38] ZBD fixes and improvements

[Dmitry Fomichev]

v2 -> v3:

 - fix two libzbc-related bugs in test scripts (found by Shinichiro).
 - modify libzbc ioengine code to allow fio to operate successfully
   against null_blk devices while using this ioengine.
 - add -l option to run-tests-against-nullb script to allow running
   the tests while using libzbc ioengine.
 - add -n option to run-tests-against-nullb to allow running the
   specified tests multiple times. This option is the most useful
   in combination with the existing -s and -t options.
 - fix sporadic failed assertion in test #51 that is easily triggered
   by running run-tests-against-nullb script with -l and -n options.
 - make minor coding style improvements in run-tests-against-nullb.

v1 -> v2:

 - replace both run-tests-against-conventional-nullb and 
   run-tests-against-conventional-nullb with a single script,
   run-tests-against-nullb, that runs test-zbd-support unit tests
   over a variety on different zoned configurations.
 - add five new test cases to test-zbd-support to cover the code
   changes made in zbd.c as a part of this series.
 - mark all test cases in test-zbd-support script that are not
   applicable for the device configuration as SKIP instead of
   reporting them as passed.
 - properly handle writes to conventional zones that cross over
   to sequential zones.
 - make additional improvements in zone locking parts of zbd.c.
 - implement miscellaneous test script enhancements.

This patch series contains bug fixes and refactoring changes
related to support for Zoned Block Devices (ZBD) in fio.
The highlights:

 - fix several errors related to running workloads that span
   a mix of conventional zones and write pointer zones.
 - improve counting of sectors with data (SWD).
 - remove dependencies on particular zone types in the code.
 - add code to gracefully handle offline zones.

Aravind Ramesh (1):
  zbd: initialize sectors with data at start time

Dmitry Fomichev (25):
  zbd: return ENOMEM if zone buffer allocation fails
  zbd: use zbd_zone_nr() more actively in the code
  zbd: add get_zone() helper function
  zbd: introduce zone_unlock()
  zbd: engines/libzbc: don't fail on assert for offline zones
  zbd: remove dependency on zone type during i/o
  zbd: skip offline zones in zbd_convert_to_open_zone()
  zbd: avoid zone buffer overrun
  zbd: don't unlock zone mutex after verify replay
  zbd: use zone_lock() in zbd_process_swd()
  zbd: don't log "zone nnnn is not open" message
  zbd: handle conventional start zone in zbd_convert_to_open_zone()
  zbd: improve replay range validation
  engines/libzbc: enable block backend
  zbd: avoid failing assertion in zbd_convert_to_open_zone()
  zbd: set thread errors in zbd_adjust_block()
  t/zbd: check for error in test #2
  t/zbd: add run-tests-against-nullb script
  t/zbd: add an option to bail on a failed test
  t/zbd: prevent test #31 from looping
  t/zbd: add checks for offline zone condition
  t/zbd: add test #54 to exercise ZBD verification
  t/zbd: show elapsed time in test-zbd-support
  t/zbd: increase timeout in test #48
  t/zbd: avoid looping on invalid command line options

Shin'ichiro Kawasaki (12):
  zbd: do not lock conventional zones on I/O adjustment
  zbd: do not set zbd handlers for conventional zones
  zbd: count sectors with data for write pointer zones
  zbd: initialize min_zone and max_zone for all zone types
  zbd: disable crossing from conventional to sequential zones
  t/zbd: add -t option to run-tests-against-nullb
  t/zbd: skip tests when test prerequisites are not met
  t/zbd: skip tests that need too many sequential zones
  t/zbd: test that conventional zones are not locked during random i/o
  t/zbd: test that zone_reset_threshold calculation is correct
  t/zbd: test random I/O direction in all-conventional case
  t/zbd: fix wrong units in test case #37

 Makefile                              |   5 +-
 engines/libzbc.c                      |   5 +-
 oslib/linux-blkzoned.c                |   2 +-
 t/run-fio-tests.py                    |   8 +-
 t/zbd/functions                       |  56 +++-
 t/zbd/run-tests-against-nullb         | 354 +++++++++++++++++++++++++
 t/zbd/run-tests-against-regular-nullb |  27 --
 t/zbd/run-tests-against-zoned-nullb   |  53 ----
 t/zbd/test-zbd-support                | 301 +++++++++++++++++++---
 zbd.c                                 | 357 ++++++++++++++++----------
 zbd.h                                 |   5 +
 11 files changed, 912 insertions(+), 261 deletions(-)
 create mode 100755 t/zbd/run-tests-against-nullb
 delete mode 100755 t/zbd/run-tests-against-regular-nullb
 delete mode 100755 t/zbd/run-tests-against-zoned-nullb

-- 
2.28.0

[PATCH v3 01/38] zbd: return ENOMEM if zone buffer allocation fails

[Dmitry Fomichev]

parse_zone_info() function tries to allocate a buffer of
ZBD_REPORT_MAX_ZONES zone descriptors and exits if this allocation
fails. The problem is that it returns 0 error code in this case and
the caller may interpret this as the success.

Just return ENOMEM if we can't allocate that buffer.

Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
 zbd.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/zbd.c b/zbd.c
index 9327816a..ad1ad1a9 100644
--- a/zbd.c
+++ b/zbd.c
@@ -443,7 +443,7 @@ static int parse_zone_info(struct thread_data *td, struct fio_file *f)
 	struct fio_zone_info *p;
 	uint64_t zone_size, offset;
 	struct zoned_block_device_info *zbd_info = NULL;
-	int i, j, ret = 0;
+	int i, j, ret = -ENOMEM;
 
 	zones = calloc(ZBD_REPORT_MAX_ZONES, sizeof(struct zbd_zone));
 	if (!zones)
@@ -475,7 +475,6 @@ static int parse_zone_info(struct thread_data *td, struct fio_file *f)
 
 	zbd_info = scalloc(1, sizeof(*zbd_info) +
 			   (nr_zones + 1) * sizeof(zbd_info->zone_info[0]));
-	ret = -ENOMEM;
 	if (!zbd_info)
 		goto out;
 	mutex_init_pshared(&zbd_info->mutex);
-- 
2.28.0

Re: [PATCH v3 01/38] zbd: return ENOMEM if zone buffer allocation fails

[Shinichiro Kawasaki]

On Jan 07, 2021 / 06:57, Dmitry Fomichev wrote:
> parse_zone_info() function tries to allocate a buffer of
> ZBD_REPORT_MAX_ZONES zone descriptors and exits if this allocation
> fails. The problem is that it returns 0 error code in this case and
> the caller may interpret this as the success.
> 
> Just return ENOMEM if we can't allocate that buffer.
> 
> Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>

Looks good to me.

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

-- 
Best Regards,
Shin'ichiro Kawasaki

[PATCH v3 02/38] zbd: use zbd_zone_nr() more actively in the code

[Dmitry Fomichev]

The function zbd_zone_nr() is always called with the first argument
being f->zbd_info. If "f" is made the first argument instead, calls
of this function become more compact end easier to read.

Besides this change, convert several places in the code where the same
zone number calculation is open coded to zbd_zone_nr() calls.
This is a refactoring patch, no change in functionality.

Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
 zbd.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/zbd.c b/zbd.c
index ad1ad1a9..841427e7 100644
--- a/zbd.c
+++ b/zbd.c
@@ -694,10 +694,10 @@ int zbd_setup_files(struct thread_data *td)
 	return 0;
 }
 
-static unsigned int zbd_zone_nr(struct zoned_block_device_info *zbd_info,
-				struct fio_zone_info *zone)
+static inline unsigned int zbd_zone_nr(const struct fio_file *f,
+				       struct fio_zone_info *zone)
 {
-	return zone - zbd_info->zone_info;
+	return zone - f->zbd_info->zone_info;
 }
 
 /**
@@ -723,7 +723,7 @@ static int zbd_reset_zone(struct thread_data *td, struct fio_file *f,
 	assert(is_valid_offset(f, offset + length - 1));
 
 	dprint(FD_ZBD, "%s: resetting wp of zone %u.\n", f->file_name,
-		zbd_zone_nr(f->zbd_info, z));
+		zbd_zone_nr(f, z));
 	switch (f->zbd_info->model) {
 	case ZBD_HOST_AWARE:
 	case ZBD_HOST_MANAGED:
@@ -793,9 +793,9 @@ static int zbd_reset_zones(struct thread_data *td, struct fio_file *f,
 	assert(min_bs);
 
 	dprint(FD_ZBD, "%s: examining zones %u .. %u\n", f->file_name,
-		zbd_zone_nr(f->zbd_info, zb), zbd_zone_nr(f->zbd_info, ze));
+		zbd_zone_nr(f, zb), zbd_zone_nr(f, ze));
 	for (z = zb; z < ze; z++) {
-		uint32_t nz = z - f->zbd_info->zone_info;
+		uint32_t nz = zbd_zone_nr(f, z);
 
 		if (!zbd_zone_swr(z))
 			continue;
@@ -811,8 +811,7 @@ static int zbd_reset_zones(struct thread_data *td, struct fio_file *f,
 		}
 		if (reset_wp) {
 			dprint(FD_ZBD, "%s: resetting zone %u\n",
-			       f->file_name,
-			       zbd_zone_nr(f->zbd_info, z));
+			       f->file_name, zbd_zone_nr(f, z));
 			if (zbd_reset_zone(td, f, z) < 0)
 				res = 1;
 		}
@@ -1197,7 +1196,7 @@ static struct fio_zone_info *zbd_replay_write_order(struct thread_data *td,
 	const struct fio_file *f = io_u->file;
 	const uint32_t min_bs = td->o.min_bs[DDIR_WRITE];
 
-	if (!zbd_open_zone(td, f, z - f->zbd_info->zone_info)) {
+	if (!zbd_open_zone(td, f, zbd_zone_nr(f, z))) {
 		pthread_mutex_unlock(&z->mutex);
 		z = zbd_convert_to_open_zone(td, io_u);
 		assert(z);
@@ -1271,7 +1270,7 @@ static void zbd_end_zone_io(struct thread_data *td, const struct io_u *io_u,
 	if (io_u->ddir == DDIR_WRITE &&
 	    io_u->offset + io_u->buflen >= zbd_zone_capacity_end(z)) {
 		pthread_mutex_lock(&f->zbd_info->mutex);
-		zbd_close_zone(td, f, z - f->zbd_info->zone_info);
+		zbd_close_zone(td, f, zbd_zone_nr(f, z));
 		pthread_mutex_unlock(&f->zbd_info->mutex);
 	}
 }
@@ -1430,8 +1429,7 @@ void setup_zbd_zone_mode(struct thread_data *td, struct io_u *io_u)
 		       "%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),
-		       zbd_zone_nr(f->zbd_info, z),
+		       (unsigned long long) zbd_zone_end(z), zone_idx,
 		       (unsigned long long) z->capacity);
 		td->io_skip_bytes += zbd_zone_end(z) - f->last_pos[ddir];
 		f->last_pos[ddir] = zbd_zone_end(z);
@@ -1612,7 +1610,7 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 			zb = zbd_convert_to_open_zone(td, io_u);
 			if (!zb)
 				goto eof;
-			zone_idx_b = zb - f->zbd_info->zone_info;
+			zone_idx_b = zbd_zone_nr(f, zb);
 		}
 		/* Check whether the zone reset threshold has been exceeded */
 		if (td->o.zrf.u.f) {
-- 
2.28.0

Re: [PATCH v3 02/38] zbd: use zbd_zone_nr() more actively in the code

[Shinichiro Kawasaki]

On Jan 07, 2021 / 06:57, Dmitry Fomichev wrote:
> The function zbd_zone_nr() is always called with the first argument
> being f->zbd_info. If "f" is made the first argument instead, calls
> of this function become more compact end easier to read.
> 
> Besides this change, convert several places in the code where the same
> zone number calculation is open coded to zbd_zone_nr() calls.
> This is a refactoring patch, no change in functionality.
> 
> Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>

Looks good to me.

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

-- 
Best Regards,
Shin'ichiro Kawasaki

[PATCH v3 03/38] zbd: add get_zone() helper function

[Dmitry Fomichev]

The following pattern is used very widely in zbd.c -

zone = &f->zbd_info->zone_info[zone_idx] .

For the sake of code clarity, wrap this construct into an inline
helper. No change in functionality.

Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
 zbd.c | 43 ++++++++++++++++++++++++-------------------
 1 file changed, 24 insertions(+), 19 deletions(-)

diff --git a/zbd.c b/zbd.c
index 841427e7..f6dabdb4 100644
--- a/zbd.c
+++ b/zbd.c
@@ -204,6 +204,12 @@ static bool is_valid_offset(const struct fio_file *f, uint64_t offset)
 	return (uint64_t)(offset - f->file_offset) < f->io_size;
 }
 
+static inline struct fio_zone_info *get_zone(const struct fio_file *f,
+					     unsigned int zone_nr)
+{
+	return &f->zbd_info->zone_info[zone_nr];
+}
+
 /* Verify whether direct I/O is used for all host-managed zoned drives. */
 static bool zbd_using_direct_io(void)
 {
@@ -235,7 +241,7 @@ static bool zbd_is_seq_job(struct fio_file *f)
 	zone_idx_b = zbd_zone_idx(f, f->file_offset);
 	zone_idx_e = zbd_zone_idx(f, f->file_offset + f->io_size - 1);
 	for (zone_idx = zone_idx_b; zone_idx <= zone_idx_e; zone_idx++)
-		if (zbd_zone_swr(&f->zbd_info->zone_info[zone_idx]))
+		if (zbd_zone_swr(get_zone(f, zone_idx)))
 			return true;
 
 	return false;
@@ -286,7 +292,7 @@ static bool zbd_verify_sizes(void)
 			}
 
 			zone_idx = zbd_zone_idx(f, f->file_offset);
-			z = &f->zbd_info->zone_info[zone_idx];
+			z = get_zone(f, zone_idx);
 			if ((f->file_offset != z->start) &&
 			    (td->o.td_ddir != TD_DDIR_READ)) {
 				new_offset = zbd_zone_end(z);
@@ -302,7 +308,7 @@ static bool zbd_verify_sizes(void)
 				f->file_offset = new_offset;
 			}
 			zone_idx = zbd_zone_idx(f, f->file_offset + f->io_size);
-			z = &f->zbd_info->zone_info[zone_idx];
+			z = get_zone(f, zone_idx);
 			new_end = z->start;
 			if ((td->o.td_ddir != TD_DDIR_READ) &&
 			    (f->file_offset + f->io_size != new_end)) {
@@ -769,7 +775,7 @@ static void zbd_close_zone(struct thread_data *td, const struct fio_file *f,
 		sizeof(f->zbd_info->open_zones[0]));
 	f->zbd_info->num_open_zones--;
 	td->num_open_zones--;
-	f->zbd_info->zone_info[zone_idx].open = 0;
+	get_zone(f, zone_idx)->open = 0;
 }
 
 /*
@@ -869,8 +875,8 @@ static uint64_t zbd_process_swd(const struct fio_file *f, enum swd_action a)
 	struct fio_zone_info *zb, *ze, *z;
 	uint64_t swd = 0;
 
-	zb = &f->zbd_info->zone_info[f->min_zone];
-	ze = &f->zbd_info->zone_info[f->max_zone];
+	zb = get_zone(f, f->min_zone);
+	ze = get_zone(f, f->max_zone);
 	for (z = zb; z < ze; z++) {
 		pthread_mutex_lock(&z->mutex);
 		swd += z->wp - z->start;
@@ -925,8 +931,8 @@ void zbd_file_reset(struct thread_data *td, struct fio_file *f)
 	if (!f->zbd_info || !td_write(td))
 		return;
 
-	zb = &f->zbd_info->zone_info[f->min_zone];
-	ze = &f->zbd_info->zone_info[f->max_zone];
+	zb = get_zone(f, f->min_zone);
+	ze = get_zone(f, f->max_zone);
 	zbd_init_swd(f);
 	/*
 	 * If data verification is enabled reset the affected zones before
@@ -966,7 +972,7 @@ 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];
-	struct fio_zone_info *z = &f->zbd_info->zone_info[zone_idx];
+	struct fio_zone_info *z = get_zone(f, zone_idx);
 	bool res = true;
 
 	if (z->cond == ZBD_ZONE_COND_OFFLINE)
@@ -1059,7 +1065,7 @@ static struct fio_zone_info *zbd_convert_to_open_zone(struct thread_data *td,
 	for (;;) {
 		uint32_t tmp_idx;
 
-		z = &f->zbd_info->zone_info[zone_idx];
+		z = get_zone(f, zone_idx);
 
 		zone_lock(td, f, z);
 		pthread_mutex_lock(&f->zbd_info->mutex);
@@ -1147,7 +1153,7 @@ open_other_zone:
 		if (!is_valid_offset(f, z->start)) {
 			/* Wrap-around. */
 			zone_idx = f->min_zone;
-			z = &f->zbd_info->zone_info[zone_idx];
+			z = get_zone(f, zone_idx);
 		}
 		assert(is_valid_offset(f, z->start));
 		zone_lock(td, f, z);
@@ -1168,7 +1174,7 @@ open_other_zone:
 		pthread_mutex_unlock(&f->zbd_info->mutex);
 		pthread_mutex_unlock(&z->mutex);
 
-		z = &f->zbd_info->zone_info[zone_idx];
+		z = get_zone(f, zone_idx);
 
 		zone_lock(td, f, z);
 		if (z->wp + min_bs <= zbd_zone_capacity_end(z))
@@ -1224,8 +1230,7 @@ zbd_find_zone(struct thread_data *td, struct io_u *io_u,
 	const uint32_t min_bs = td->o.min_bs[io_u->ddir];
 	struct fio_file *f = io_u->file;
 	struct fio_zone_info *z1, *z2;
-	const struct fio_zone_info *const zf =
-		&f->zbd_info->zone_info[f->min_zone];
+	const struct fio_zone_info *const zf = get_zone(f, f->min_zone);
 
 	/*
 	 * Skip to the next non-empty zone in case of sequential I/O and to
@@ -1298,7 +1303,7 @@ static void zbd_queue_io(struct thread_data *td, struct io_u *io_u, int q,
 
 	zone_idx = zbd_zone_idx(f, io_u->offset);
 	assert(zone_idx < zbd_info->nr_zones);
-	z = &zbd_info->zone_info[zone_idx];
+	z = get_zone(f, zone_idx);
 
 	if (!zbd_zone_swr(z))
 		return;
@@ -1359,7 +1364,7 @@ static void zbd_put_io(struct thread_data *td, const struct io_u *io_u)
 
 	zone_idx = zbd_zone_idx(f, io_u->offset);
 	assert(zone_idx < zbd_info->nr_zones);
-	z = &zbd_info->zone_info[zone_idx];
+	z = get_zone(f, zone_idx);
 
 	if (!zbd_zone_swr(z))
 		return;
@@ -1415,7 +1420,7 @@ void setup_zbd_zone_mode(struct thread_data *td, struct io_u *io_u)
 	assert(td->o.zone_size);
 
 	zone_idx = zbd_zone_idx(f, f->last_pos[ddir]);
-	z = &f->zbd_info->zone_info[zone_idx];
+	z = get_zone(f, zone_idx);
 
 	/*
 	 * When the zone capacity is smaller than the zone size and the I/O is
@@ -1523,7 +1528,7 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 	assert(is_valid_offset(f, io_u->offset));
 	assert(io_u->buflen);
 	zone_idx_b = zbd_zone_idx(f, io_u->offset);
-	zb = &f->zbd_info->zone_info[zone_idx_b];
+	zb = get_zone(f, zone_idx_b);
 	orig_zb = zb;
 
 	/* Accept the I/O offset for conventional zones. */
@@ -1559,7 +1564,7 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 		if (range < min_bs ||
 		    ((!td_random(td)) && (io_u->offset + min_bs > zb->wp))) {
 			pthread_mutex_unlock(&zb->mutex);
-			zl = &f->zbd_info->zone_info[f->max_zone];
+			zl = get_zone(f, f->max_zone);
 			zb = zbd_find_zone(td, io_u, zb, zl);
 			if (!zb) {
 				dprint(FD_ZBD,
-- 
2.28.0

Re: [PATCH v3 03/38] zbd: add get_zone() helper function

[Shinichiro Kawasaki]

On Jan 07, 2021 / 06:57, Dmitry Fomichev wrote:
> The following pattern is used very widely in zbd.c -
> 
> zone = &f->zbd_info->zone_info[zone_idx] .
> 
> For the sake of code clarity, wrap this construct into an inline
> helper. No change in functionality.
> 
> Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>

Looks good to me.

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

-- 
Best Regards,
Shin'ichiro Kawasaki

[PATCH v3 04/38] zbd: introduce zone_unlock()

[Dmitry Fomichev]

ZBD code already defines a helper function to lock a device zone,
zone_lock(). There is no zone_unlock() function though.

Wrap zone mutex unlock to zone_unlock() helper along with an assert
to make sure that the unlock is successful, i.e. that the function
is being called with the pointer to a locked zone.

Suggested-by: Niklas Cassel <niklas.cassel@wdc.com>
Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
 zbd.c | 42 ++++++++++++++++++++++++------------------
 1 file changed, 24 insertions(+), 18 deletions(-)

diff --git a/zbd.c b/zbd.c
index f6dabdb4..0bda4c03 100644
--- a/zbd.c
+++ b/zbd.c
@@ -199,6 +199,14 @@ static void zone_lock(struct thread_data *td, struct fio_file *f, struct fio_zon
 	}
 }
 
+static inline void zone_unlock(struct fio_zone_info *z)
+{
+	int ret;
+
+	ret = pthread_mutex_unlock(&z->mutex);
+	assert(!ret);
+}
+
 static bool is_valid_offset(const struct fio_file *f, uint64_t offset)
 {
 	return (uint64_t)(offset - f->file_offset) < f->io_size;
@@ -821,7 +829,7 @@ static int zbd_reset_zones(struct thread_data *td, struct fio_file *f,
 			if (zbd_reset_zone(td, f, z) < 0)
 				res = 1;
 		}
-		pthread_mutex_unlock(&z->mutex);
+		zone_unlock(z);
 	}
 
 	return res;
@@ -892,7 +900,7 @@ static uint64_t zbd_process_swd(const struct fio_file *f, enum swd_action a)
 	}
 	pthread_mutex_unlock(&f->zbd_info->mutex);
 	for (z = zb; z < ze; z++)
-		pthread_mutex_unlock(&z->mutex);
+		zone_unlock(z);
 
 	return swd;
 }
@@ -1102,7 +1110,7 @@ static struct fio_zone_info *zbd_convert_to_open_zone(struct thread_data *td,
 		dprint(FD_ZBD, "%s(%s): no candidate zone\n",
 			__func__, f->file_name);
 		pthread_mutex_unlock(&f->zbd_info->mutex);
-		pthread_mutex_unlock(&z->mutex);
+		zone_unlock(z);
 		return NULL;
 
 found_candidate_zone:
@@ -1111,7 +1119,7 @@ found_candidate_zone:
 			break;
 		zone_idx = new_zone_idx;
 		pthread_mutex_unlock(&f->zbd_info->mutex);
-		pthread_mutex_unlock(&z->mutex);
+		zone_unlock(z);
 	}
 
 	/* Both z->mutex and f->zbd_info->mutex are held. */
@@ -1148,7 +1156,7 @@ open_other_zone:
 	/* Zone 'z' is full, so try to open a new zone. */
 	for (i = f->io_size / f->zbd_info->zone_size; i > 0; i--) {
 		zone_idx++;
-		pthread_mutex_unlock(&z->mutex);
+		zone_unlock(z);
 		z++;
 		if (!is_valid_offset(f, z->start)) {
 			/* Wrap-around. */
@@ -1172,7 +1180,7 @@ open_other_zone:
 		if (zone_idx < f->min_zone || zone_idx >= f->max_zone)
 			continue;
 		pthread_mutex_unlock(&f->zbd_info->mutex);
-		pthread_mutex_unlock(&z->mutex);
+		zone_unlock(z);
 
 		z = get_zone(f, zone_idx);
 
@@ -1182,7 +1190,7 @@ open_other_zone:
 		pthread_mutex_lock(&f->zbd_info->mutex);
 	}
 	pthread_mutex_unlock(&f->zbd_info->mutex);
-	pthread_mutex_unlock(&z->mutex);
+	zone_unlock(z);
 	dprint(FD_ZBD, "%s(%s): did not open another zone\n", __func__,
 	       f->file_name);
 	return NULL;
@@ -1203,7 +1211,7 @@ static struct fio_zone_info *zbd_replay_write_order(struct thread_data *td,
 	const uint32_t min_bs = td->o.min_bs[DDIR_WRITE];
 
 	if (!zbd_open_zone(td, f, zbd_zone_nr(f, z))) {
-		pthread_mutex_unlock(&z->mutex);
+		zone_unlock(z);
 		z = zbd_convert_to_open_zone(td, io_u);
 		assert(z);
 	}
@@ -1241,7 +1249,7 @@ zbd_find_zone(struct thread_data *td, struct io_u *io_u,
 			zone_lock(td, f, z1);
 			if (z1->start + min_bs <= z1->wp)
 				return z1;
-			pthread_mutex_unlock(&z1->mutex);
+			zone_unlock(z1);
 		} else if (!td_random(td)) {
 			break;
 		}
@@ -1250,7 +1258,7 @@ zbd_find_zone(struct thread_data *td, struct io_u *io_u,
 			zone_lock(td, f, z2);
 			if (z2->start + min_bs <= z2->wp)
 				return z2;
-			pthread_mutex_unlock(&z2->mutex);
+			zone_unlock(z2);
 		}
 	}
 	dprint(FD_ZBD, "%s: adjusting random read offset failed\n",
@@ -1342,7 +1350,7 @@ static void zbd_queue_io(struct thread_data *td, struct io_u *io_u, int q,
 unlock:
 	if (!success || q != FIO_Q_QUEUED) {
 		/* BUSY or COMPLETED: unlock the zone */
-		pthread_mutex_unlock(&z->mutex);
+		zone_unlock(z);
 		io_u->zbd_put_io = NULL;
 	}
 }
@@ -1357,7 +1365,6 @@ static void zbd_put_io(struct thread_data *td, const struct io_u *io_u)
 	struct zoned_block_device_info *zbd_info = f->zbd_info;
 	struct fio_zone_info *z;
 	uint32_t zone_idx;
-	int ret;
 
 	if (!zbd_info)
 		return;
@@ -1375,8 +1382,7 @@ static void zbd_put_io(struct thread_data *td, const struct io_u *io_u)
 
 	zbd_end_zone_io(td, io_u, z);
 
-	ret = pthread_mutex_unlock(&z->mutex);
-	assert(ret == 0);
+	zone_unlock(z);
 	zbd_check_swd(f);
 }
 
@@ -1551,7 +1557,7 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 	case DDIR_READ:
 		if (td->runstate == TD_VERIFYING && td_write(td)) {
 			zb = zbd_replay_write_order(td, io_u, zb);
-			pthread_mutex_unlock(&zb->mutex);
+			zone_unlock(zb);
 			goto accept;
 		}
 		/*
@@ -1563,7 +1569,7 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 			zb->wp - zb->start : 0;
 		if (range < min_bs ||
 		    ((!td_random(td)) && (io_u->offset + min_bs > zb->wp))) {
-			pthread_mutex_unlock(&zb->mutex);
+			zone_unlock(zb);
 			zl = get_zone(f, f->max_zone);
 			zb = zbd_find_zone(td, io_u, zb, zl);
 			if (!zb) {
@@ -1611,7 +1617,7 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 		if (io_u->buflen > f->zbd_info->zone_size)
 			goto eof;
 		if (!zbd_open_zone(td, f, zone_idx_b)) {
-			pthread_mutex_unlock(&zb->mutex);
+			zone_unlock(zb);
 			zb = zbd_convert_to_open_zone(td, io_u);
 			if (!zb)
 				goto eof;
@@ -1699,7 +1705,7 @@ accept:
 
 eof:
 	if (zb)
-		pthread_mutex_unlock(&zb->mutex);
+		zone_unlock(zb);
 	return io_u_eof;
 }
 
-- 
2.28.0

Re: [PATCH v3 04/38] zbd: introduce zone_unlock()

[Shinichiro Kawasaki]

On Jan 07, 2021 / 06:57, Dmitry Fomichev wrote:
> ZBD code already defines a helper function to lock a device zone,
> zone_lock(). There is no zone_unlock() function though.
> 
> Wrap zone mutex unlock to zone_unlock() helper along with an assert
> to make sure that the unlock is successful, i.e. that the function
> is being called with the pointer to a locked zone.
> 
> Suggested-by: Niklas Cassel <niklas.cassel@wdc.com>
> Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>

Looks good to me.

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

-- 
Best Regards,
Shin'ichiro Kawasaki

[PATCH v3 05/38] zbd: engines/libzbc: don't fail on assert for offline zones

[Dmitry Fomichev]

If fio is run against a zoned device that has any zones in OFFLINE
condition, the following assertion is raised -

fio: zbd.c:473: parse_zone_info: Assertion `z->wp <= z->start + zone_size' failed.

This happens because offline zones have no valid write pointer and
it is reported by libzbc and blkzoned as (uint64_t)(-1). To avoid
violating this assertion, set the write pointer in all offline zones
to point at the start of the zone.

Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
 engines/libzbc.c       | 2 +-
 oslib/linux-blkzoned.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/engines/libzbc.c b/engines/libzbc.c
index 4b900233..552aab65 100644
--- a/engines/libzbc.c
+++ b/engines/libzbc.c
@@ -283,7 +283,7 @@ static int libzbc_report_zones(struct thread_data *td, struct fio_file *f,
 		default:
 			/* Treat all these conditions as offline (don't use!) */
 			zbdz->cond = ZBD_ZONE_COND_OFFLINE;
-			break;
+			zbdz->wp = zbdz->start;
 		}
 
 
diff --git a/oslib/linux-blkzoned.c b/oslib/linux-blkzoned.c
index 0a8a577a..f37c67fc 100644
--- a/oslib/linux-blkzoned.c
+++ b/oslib/linux-blkzoned.c
@@ -203,7 +203,7 @@ int blkzoned_report_zones(struct thread_data *td, struct fio_file *f,
 		default:
 			/* Treat all these conditions as offline (don't use!) */
 			z->cond = ZBD_ZONE_COND_OFFLINE;
-			break;
+			z->wp = z->start;
 		}
 	}
 
-- 
2.28.0

Re: [PATCH v3 05/38] zbd: engines/libzbc: don't fail on assert for offline zones

[Shinichiro Kawasaki]

On Jan 07, 2021 / 06:57, Dmitry Fomichev wrote:
> If fio is run against a zoned device that has any zones in OFFLINE
> condition, the following assertion is raised -
> 
> fio: zbd.c:473: parse_zone_info: Assertion `z->wp <= z->start + zone_size' failed.
> 
> This happens because offline zones have no valid write pointer and
> it is reported by libzbc and blkzoned as (uint64_t)(-1). To avoid
> violating this assertion, set the write pointer in all offline zones
> to point at the start of the zone.
> 
> Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>

Looks good to me.

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

-- 
Best Regards,
Shin'ichiro Kawasaki

[PATCH v3 06/38] zbd: remove dependency on zone type during i/o

[Dmitry Fomichev]

Two different type of zones have a write pointer: Sequential Write
Required (SWR) and Sequential Write Preferred (SWP). Introduce the
zone flag "has_wp" in struct zbd_zone_info and set it to 1 for these
zone types upon initialization, thus avoiding the necessity to check
multiple zone types in core zbd code. This flag replaces zbd_zone_swr()
function and lays the groundwork for supporting additional write
pointer zone types in the future.

The overall functionality stays the same after this commit.

Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
 zbd.c | 31 ++++++++++++++++---------------
 zbd.h |  2 ++
 2 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/zbd.c b/zbd.c
index 0bda4c03..776d9b2c 100644
--- a/zbd.c
+++ b/zbd.c
@@ -131,15 +131,6 @@ static uint32_t zbd_zone_idx(const struct fio_file *f, uint64_t offset)
 	return min(zone_idx, f->zbd_info->nr_zones);
 }
 
-/**
- * zbd_zone_swr - Test whether a zone requires sequential writes
- * @z: zone info pointer.
- */
-static inline bool zbd_zone_swr(struct fio_zone_info *z)
-{
-	return z->type == ZBD_ZONE_TYPE_SWR;
-}
-
 /**
  * zbd_zone_end - Return zone end location
  * @z: zone info pointer.
@@ -171,7 +162,7 @@ static bool zbd_zone_full(const struct fio_file *f, struct fio_zone_info *z,
 {
 	assert((required & 511) == 0);
 
-	return zbd_zone_swr(z) &&
+	return z->has_wp &&
 		z->wp + required > zbd_zone_capacity_end(z);
 }
 
@@ -249,7 +240,7 @@ static bool zbd_is_seq_job(struct fio_file *f)
 	zone_idx_b = zbd_zone_idx(f, f->file_offset);
 	zone_idx_e = zbd_zone_idx(f, f->file_offset + f->io_size - 1);
 	for (zone_idx = zone_idx_b; zone_idx <= zone_idx_e; zone_idx++)
-		if (zbd_zone_swr(get_zone(f, zone_idx)))
+		if (get_zone(f, zone_idx)->has_wp)
 			return true;
 
 	return false;
@@ -429,6 +420,7 @@ static int init_zone_info(struct thread_data *td, struct fio_file *f)
 		p->type = ZBD_ZONE_TYPE_SWR;
 		p->cond = ZBD_ZONE_COND_EMPTY;
 		p->capacity = zone_capacity;
+		p->has_wp = 1;
 	}
 	/* a sentinel */
 	p->start = nr_zones * zone_size;
@@ -512,8 +504,17 @@ static int parse_zone_info(struct thread_data *td, struct fio_file *f)
 				p->wp = z->wp;
 				break;
 			}
+
+			switch (z->type) {
+			case ZBD_ZONE_TYPE_SWR:
+				p->has_wp = 1;
+				break;
+			default:
+				p->has_wp = 0;
+			}
 			p->type = z->type;
 			p->cond = z->cond;
+
 			if (j > 0 && p->start != p[-1].start + zone_size) {
 				log_info("%s: invalid zone data\n",
 					 f->file_name);
@@ -811,7 +812,7 @@ static int zbd_reset_zones(struct thread_data *td, struct fio_file *f,
 	for (z = zb; z < ze; z++) {
 		uint32_t nz = zbd_zone_nr(f, z);
 
-		if (!zbd_zone_swr(z))
+		if (!z->has_wp)
 			continue;
 		zone_lock(td, f, z);
 		if (all_zones) {
@@ -1313,7 +1314,7 @@ static void zbd_queue_io(struct thread_data *td, struct io_u *io_u, int q,
 	assert(zone_idx < zbd_info->nr_zones);
 	z = get_zone(f, zone_idx);
 
-	if (!zbd_zone_swr(z))
+	if (!z->has_wp)
 		return;
 
 	if (!success)
@@ -1373,7 +1374,7 @@ static void zbd_put_io(struct thread_data *td, const struct io_u *io_u)
 	assert(zone_idx < zbd_info->nr_zones);
 	z = get_zone(f, zone_idx);
 
-	if (!zbd_zone_swr(z))
+	if (!z->has_wp)
 		return;
 
 	dprint(FD_ZBD,
@@ -1538,7 +1539,7 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 	orig_zb = zb;
 
 	/* Accept the I/O offset for conventional zones. */
-	if (!zbd_zone_swr(zb))
+	if (!zb->has_wp)
 		return io_u_accept;
 
 	/*
diff --git a/zbd.h b/zbd.h
index bff55f99..059a9f9e 100644
--- a/zbd.h
+++ b/zbd.h
@@ -28,6 +28,7 @@ enum io_u_action {
  * @mutex: protects the modifiable members in this structure
  * @type: zone type (BLK_ZONE_TYPE_*)
  * @cond: zone state (BLK_ZONE_COND_*)
+ * @has_wp: whether or not this zone can have a valid write pointer
  * @open: whether or not this zone is currently open. Only relevant if
  *		max_open_zones > 0.
  * @reset_zone: whether or not this zone should be reset before writing to it
@@ -40,6 +41,7 @@ struct fio_zone_info {
 	uint32_t		verify_block;
 	enum zbd_zone_type	type:2;
 	enum zbd_zone_cond	cond:4;
+	unsigned int		has_wp:1;
 	unsigned int		open:1;
 	unsigned int		reset_zone:1;
 };
-- 
2.28.0

Re: [PATCH v3 06/38] zbd: remove dependency on zone type during i/o

[Shinichiro Kawasaki]

On Jan 07, 2021 / 06:57, Dmitry Fomichev wrote:
> Two different type of zones have a write pointer: Sequential Write
> Required (SWR) and Sequential Write Preferred (SWP). Introduce the
> zone flag "has_wp" in struct zbd_zone_info and set it to 1 for these
> zone types upon initialization, thus avoiding the necessity to check
> multiple zone types in core zbd code. This flag replaces zbd_zone_swr()
> function and lays the groundwork for supporting additional write
> pointer zone types in the future.
> 
> The overall functionality stays the same after this commit.
> 
> Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>

Looks good to me.

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

-- 
Best Regards,
Shin'ichiro Kawasaki

[PATCH v3 07/38] zbd: skip offline zones in zbd_convert_to_open_zone()

[Dmitry Fomichev]

Since all I/Os to an offline zone will fail, add a check in
zbd_convert_to_open_zone() to ignore zones that have this condition.

Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
 zbd.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/zbd.c b/zbd.c
index 776d9b2c..69befd5a 100644
--- a/zbd.c
+++ b/zbd.c
@@ -1078,7 +1078,8 @@ static struct fio_zone_info *zbd_convert_to_open_zone(struct thread_data *td,
 
 		zone_lock(td, f, z);
 		pthread_mutex_lock(&f->zbd_info->mutex);
-		if (td->o.max_open_zones == 0 && td->o.job_max_open_zones == 0)
+		if (z->cond != ZBD_ZONE_COND_OFFLINE &&
+		    td->o.max_open_zones == 0 && td->o.job_max_open_zones == 0)
 			goto examine_zone;
 		if (f->zbd_info->num_open_zones == 0) {
 			dprint(FD_ZBD, "%s(%s): no zones are open\n",
@@ -1200,6 +1201,7 @@ out:
 	dprint(FD_ZBD, "%s(%s): returning zone %d\n", __func__, f->file_name,
 	       zone_idx);
 	io_u->offset = z->start;
+	assert(z->cond != ZBD_ZONE_COND_OFFLINE);
 	return z;
 }
 
-- 
2.28.0

Re: [PATCH v3 07/38] zbd: skip offline zones in zbd_convert_to_open_zone()

[Shinichiro Kawasaki]

On Jan 07, 2021 / 06:57, Dmitry Fomichev wrote:
> Since all I/Os to an offline zone will fail, add a check in
> zbd_convert_to_open_zone() to ignore zones that have this condition.
> 
> Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>

Looks good to me.

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

-- 
Best Regards,
Shin'ichiro Kawasaki

[PATCH v3 08/38] zbd: avoid zone buffer overrun

[Dmitry Fomichev]

If the total number of zones on a drive is calculated to a value that
is less than the number of zones it can actually report, zone info
buffer can be overrun. This may happen not only due to drive firmware
problems, but also because of underlying software incorrectly
reporting zoned device capacity.

Fix this by more carefully setting zone report size.

Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
 zbd.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/zbd.c b/zbd.c
index 69befd5a..db0d650a 100644
--- a/zbd.c
+++ b/zbd.c
@@ -526,8 +526,9 @@ static int parse_zone_info(struct thread_data *td, struct fio_file *f)
 		offset = z->start + z->len;
 		if (j >= nr_zones)
 			break;
-		nrz = zbd_report_zones(td, f, offset,
-					    zones, ZBD_REPORT_MAX_ZONES);
+		nrz = zbd_report_zones(td, f, offset, zones,
+				       min((uint32_t)(nr_zones - j),
+					   ZBD_REPORT_MAX_ZONES));
 		if (nrz < 0) {
 			ret = nrz;
 			log_info("fio: report zones (offset %llu) failed for %s (%d).\n",
-- 
2.28.0

Re: [PATCH v3 08/38] zbd: avoid zone buffer overrun

[Shinichiro Kawasaki]

On Jan 07, 2021 / 06:57, Dmitry Fomichev wrote:
> If the total number of zones on a drive is calculated to a value that
> is less than the number of zones it can actually report, zone info
> buffer can be overrun. This may happen not only due to drive firmware
> problems, but also because of underlying software incorrectly
> reporting zoned device capacity.
> 
> Fix this by more carefully setting zone report size.
> 
> Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>

Looks good to me.

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

-- 
Best Regards,
Shin'ichiro Kawasaki

[PATCH v3 09/38] zbd: don't unlock zone mutex after verify replay

[Dmitry Fomichev]

zbd_adjust_block() always returns with the zone locked if the i/o is
accepted. The corresponding unlock happens in zbd_put_io(). The
function description says -

 * Locking strategy: returns with z->mutex locked if and only if z refers
 * to a sequential zone and if io_u_accept is returned. z is the zone that
 * corresponds to io_u->offset at the end of this function.

Remove the recently added unlock after zbd_replay_write_order() call.
Add a Coverity annotation to mark the absence of unlock as intentional.

Fixes: b2726d53bb5d ("zbd: Add a missing pthread_mutex_unlock() call")
Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
 zbd.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/zbd.c b/zbd.c
index db0d650a..c1db215e 100644
--- a/zbd.c
+++ b/zbd.c
@@ -1561,7 +1561,12 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 	case DDIR_READ:
 		if (td->runstate == TD_VERIFYING && td_write(td)) {
 			zb = zbd_replay_write_order(td, io_u, zb);
-			zone_unlock(zb);
+			/*
+			 * Since we return with the zone lock still held,
+			 * add an annotation to let Coverity know that it
+			 * is intentional.
+			 */
+			/* coverity[missing_unlock] */
 			goto accept;
 		}
 		/*
-- 
2.28.0

Re: [PATCH v3 09/38] zbd: don't unlock zone mutex after verify replay

[Shinichiro Kawasaki]

On Jan 07, 2021 / 06:57, Dmitry Fomichev wrote:
> zbd_adjust_block() always returns with the zone locked if the i/o is
> accepted. The corresponding unlock happens in zbd_put_io(). The
> function description says -
> 
>  * Locking strategy: returns with z->mutex locked if and only if z refers
>  * to a sequential zone and if io_u_accept is returned. z is the zone that
>  * corresponds to io_u->offset at the end of this function.
> 
> Remove the recently added unlock after zbd_replay_write_order() call.
> Add a Coverity annotation to mark the absence of unlock as intentional.
> 
> Fixes: b2726d53bb5d ("zbd: Add a missing pthread_mutex_unlock() call")
> Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>

Looks good to me.

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

-- 
Best Regards,
Shin'ichiro Kawasaki

[PATCH v3 10/38] zbd: do not lock conventional zones on I/O adjustment

[Dmitry Fomichev]

From: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>

When a random workload runs against write pointer zones, I/Os are
adjusted to meet write pointer restrictions. During read, I/O offsets
are adjusted to point to zones with data to read and during write, I/O
offsets are adjusted to be at write pointers of open zones.

However, when a random workload runs in a range that contains both
write pointer zones and conventional zones, I/Os to write pointer
zones can potentially be adjusted to conventional zones. The functions
zbd_find_zone() and zbd_convert_to_open_zone() search for zones
regardless of their type, and therefore they may return conventional
zones. These functions lock the found zone to guard its open status
and write pointer position, but this lock is meaningless for
conventional zones. This unwanted lock of conventional zones has been
observed to cause a deadlock.

Furthermore, zbd_convert_to_open_zone() may add the found conventional
zone to the array of open zones. However, conventional zones should
never be added to the array of open zones as conventional zones never
take the "implicit open" condition and not counted as part of the
device open zone management.

To avoid the deadlock, modify zbd_find_zone() not to lock zone when it
checks conventional zone without write pointer. To avoid the deadlock
and the conventional zone open, modify zbd_convert_to_open_zone() to
ignore conventional zones.

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

diff --git a/zbd.c b/zbd.c
index c1db215e..84ac6b6f 100644
--- a/zbd.c
+++ b/zbd.c
@@ -1033,7 +1033,8 @@ static uint32_t pick_random_zone_idx(const struct fio_file *f,
 /*
  * Modify the offset of an I/O unit that does not refer to an open zone such
  * that it refers to an open zone. Close an open zone and open a new zone if
- * necessary. This algorithm can only work correctly if all write pointers are
+ * necessary. The open zone is searched across sequential zones.
+ * This algorithm can only work correctly if all write pointers are
  * a multiple of the fio block size. The caller must neither hold z->mutex
  * nor f->zbd_info->mutex. Returns with z->mutex held upon success.
  */
@@ -1159,7 +1160,8 @@ open_other_zone:
 	/* Zone 'z' is full, so try to open a new zone. */
 	for (i = f->io_size / f->zbd_info->zone_size; i > 0; i--) {
 		zone_idx++;
-		zone_unlock(z);
+		if (z->has_wp)
+			zone_unlock(z);
 		z++;
 		if (!is_valid_offset(f, z->start)) {
 			/* Wrap-around. */
@@ -1167,6 +1169,8 @@ open_other_zone:
 			z = get_zone(f, zone_idx);
 		}
 		assert(is_valid_offset(f, z->start));
+		if (!z->has_wp)
+			continue;
 		zone_lock(td, f, z);
 		if (z->open)
 			continue;
@@ -1202,6 +1206,7 @@ out:
 	dprint(FD_ZBD, "%s(%s): returning zone %d\n", __func__, f->file_name,
 	       zone_idx);
 	io_u->offset = z->start;
+	assert(z->has_wp);
 	assert(z->cond != ZBD_ZONE_COND_OFFLINE);
 	return z;
 }
@@ -1228,12 +1233,12 @@ static struct fio_zone_info *zbd_replay_write_order(struct thread_data *td,
 }
 
 /*
- * Find another zone for which @io_u fits below the write pointer. Start
- * searching in zones @zb + 1 .. @zl and continue searching in zones
- * @zf .. @zb - 1.
+ * Find another zone for which @io_u fits in the readable data in the zone.
+ * Search in zones @zb + 1 .. @zl. For random workload, also search in zones
+ * @zb - 1 .. @zf.
  *
- * Either returns NULL or returns a zone pointer and holds the mutex for that
- * zone.
+ * Either returns NULL or returns a zone pointer. When the zone has write
+ * pointer, hold the mutex for the zone.
  */
 static struct fio_zone_info *
 zbd_find_zone(struct thread_data *td, struct io_u *io_u,
@@ -1250,19 +1255,23 @@ zbd_find_zone(struct thread_data *td, struct io_u *io_u,
 	 */
 	for (z1 = zb + 1, z2 = zb - 1; z1 < zl || z2 >= zf; z1++, z2--) {
 		if (z1 < zl && z1->cond != ZBD_ZONE_COND_OFFLINE) {
-			zone_lock(td, f, z1);
+			if (z1->has_wp)
+				zone_lock(td, f, z1);
 			if (z1->start + min_bs <= z1->wp)
 				return z1;
-			zone_unlock(z1);
+			if (z1->has_wp)
+				zone_unlock(z1);
 		} else if (!td_random(td)) {
 			break;
 		}
 		if (td_random(td) && z2 >= zf &&
 		    z2->cond != ZBD_ZONE_COND_OFFLINE) {
-			zone_lock(td, f, z2);
+			if (z2->has_wp)
+				zone_lock(td, f, z2);
 			if (z2->start + min_bs <= z2->wp)
 				return z2;
-			zone_unlock(z2);
+			if (z2->has_wp)
+				zone_unlock(z2);
 		}
 	}
 	dprint(FD_ZBD, "%s: adjusting random read offset failed\n",
-- 
2.28.0

[PATCH v3 11/38] zbd: do not set zbd handlers for conventional zones

[Dmitry Fomichev]

From: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>

When zbd_adjust_block() modifies io_u to satisfy write pointer
restrictions, it may change the zone for the io_u. The function sets
pointers to zbd_queue_io() and zbd_put_io() handlers to io_u to
further process write pointer zones. However, when the I/O is
redirected to a conventional zone, these handlers should not
be set in io_u.

Skip setting the handlers when this function returns a conventional
zone. When zbd_adjust_block() can not find a zone to fit the I/O,
the existing code unlocks the zone pointer 'zb' used in the function.
This unlock should not be performed if 'zb' points to a conventional
zone upon return, skip it in this case.

These changes make the assert for 'zb' pointer near 'accept' label in
zbd_adjust_block() unnecessary. Replace it with assert for zb->has_wp,
since the zone at the step shall have write pointer.

Since zone locking functions (zone_lock(), zbd_queue_io() and
zbd_put_io()) are supposed to be called only for write pointer zones,
add assertions to zone_lock() and zone_unlock() to make sure this is
the case. This allows us to convert a few existing conditional checks
to assertions to make zone type validation more strict.

Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
 zbd.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/zbd.c b/zbd.c
index 84ac6b6f..f7c29250 100644
--- a/zbd.c
+++ b/zbd.c
@@ -174,6 +174,8 @@ static void zone_lock(struct thread_data *td, struct fio_file *f, struct fio_zon
 	/* A thread should never lock zones outside its working area. */
 	assert(f->min_zone <= nz && nz < f->max_zone);
 
+	assert(z->has_wp);
+
 	/*
 	 * Lock the io_u target zone. The zone will be unlocked if io_u offset
 	 * is changed or when io_u completes and zbd_put_io() executed.
@@ -194,6 +196,7 @@ static inline void zone_unlock(struct fio_zone_info *z)
 {
 	int ret;
 
+	assert(z->has_wp);
 	ret = pthread_mutex_unlock(&z->mutex);
 	assert(!ret);
 }
@@ -1326,8 +1329,7 @@ static void zbd_queue_io(struct thread_data *td, struct io_u *io_u, int q,
 	assert(zone_idx < zbd_info->nr_zones);
 	z = get_zone(f, zone_idx);
 
-	if (!z->has_wp)
-		return;
+	assert(z->has_wp);
 
 	if (!success)
 		goto unlock;
@@ -1386,8 +1388,7 @@ static void zbd_put_io(struct thread_data *td, const struct io_u *io_u)
 	assert(zone_idx < zbd_info->nr_zones);
 	z = get_zone(f, zone_idx);
 
-	if (!z->has_wp)
-		return;
+	assert(z->has_wp);
 
 	dprint(FD_ZBD,
 	       "%s: terminate I/O (%lld, %llu) for zone %u\n",
@@ -1617,6 +1618,12 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 			io_u->offset = zb->start +
 				((io_u->offset - orig_zb->start) %
 				 (range - io_u->buflen)) / min_bs * min_bs;
+		/*
+		 * When zbd_find_zone() returns a conventional zone,
+		 * we can simply accept the new i/o offset here.
+		 */
+		if (!zb->has_wp)
+			return io_u_accept;
 		/*
 		 * Make sure the I/O does not cross over the zone wp position.
 		 */
@@ -1713,7 +1720,7 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 	assert(false);
 
 accept:
-	assert(zb);
+	assert(zb->has_wp);
 	assert(zb->cond != ZBD_ZONE_COND_OFFLINE);
 	assert(!io_u->zbd_queue_io);
 	assert(!io_u->zbd_put_io);
@@ -1722,7 +1729,7 @@ accept:
 	return io_u_accept;
 
 eof:
-	if (zb)
+	if (zb && zb->has_wp)
 		zone_unlock(zb);
 	return io_u_eof;
 }
-- 
2.28.0

[PATCH v3 12/38] zbd: count sectors with data for write pointer zones

[Dmitry Fomichev]

From: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>

ZBD fio code tracks 'sectors with data' for two different purposes.
The first one is to process zone_reset_threshold. When the ratio of
sectors with data in zones with write pointer goes beyond the specified
number, zone reset is triggered. The second purpose is to control the
direction of the first I/O of random mixed read/write workloads. If all
write pointer zones in the I/O range are reset at the beginning of such
a workload, fio has no data to read and will immediately end the run of
the test section. To avoid this, fio checks 'sectors with data' and if
it is zero (i.e. it is the very first I/O), it modifies the direction
of that I/O from read to write.

Currently, when the workload range includes both conventional and
sequential zones, all sectors in conventional zones are counted as
'sectors with data' along with sectors in sequential zones.
This leads to incorrect handling  of 'zone_reset_threshold' option -
zone reset timing of sequential zones is affected by the amount of
data read from or written to conventional zones. To avoid this,
conventional zones should be excluded from 'sectors with data'
calculation.

On the other hand, if the sectors of conventional zones were excluded
from the sectors with data, it could result in the wrong initial I/O
direction for random workloads. When the zones in I/O region are all
conventional, 'sectors with data' would always be zero. Because of
this, read operations are always changed to writes and reads are never
performed.

To avoid this contradiction, introduce another counter,
'wp_sector_with_data'. It works similar to the existing
'sectors_with_data', but it counts data sectors only in write pointer
zones. Use this newly introduced count for zone_reset_threshold checks
and keep on using the original count for the initial random I/O
direction determination.

When counting sectors with data, lock only write pointer zones, no need
to lock conventional zones.

Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
 zbd.c | 25 ++++++++++++++++++-------
 zbd.h |  3 +++
 2 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/zbd.c b/zbd.c
index f7c29250..757e72d5 100644
--- a/zbd.c
+++ b/zbd.c
@@ -734,9 +734,10 @@ static int zbd_reset_zone(struct thread_data *td, struct fio_file *f,
 {
 	uint64_t offset = z->start;
 	uint64_t length = (z+1)->start - offset;
+	uint64_t data_in_zone = z->wp - z->start;
 	int ret = 0;
 
-	if (z->wp == z->start)
+	if (!data_in_zone)
 		return 0;
 
 	assert(is_valid_offset(f, offset + length - 1));
@@ -755,7 +756,8 @@ static int zbd_reset_zone(struct thread_data *td, struct fio_file *f,
 	}
 
 	pthread_mutex_lock(&f->zbd_info->mutex);
-	f->zbd_info->sectors_with_data -= z->wp - z->start;
+	f->zbd_info->sectors_with_data -= data_in_zone;
+	f->zbd_info->wp_sectors_with_data -= data_in_zone;
 	pthread_mutex_unlock(&f->zbd_info->mutex);
 	z->wp = z->start;
 	z->verify_block = 0;
@@ -887,25 +889,32 @@ static uint64_t zbd_process_swd(const struct fio_file *f, enum swd_action a)
 {
 	struct fio_zone_info *zb, *ze, *z;
 	uint64_t swd = 0;
+	uint64_t wp_swd = 0;
 
 	zb = get_zone(f, f->min_zone);
 	ze = get_zone(f, f->max_zone);
 	for (z = zb; z < ze; z++) {
-		pthread_mutex_lock(&z->mutex);
+		if (z->has_wp) {
+			pthread_mutex_lock(&z->mutex);
+			wp_swd += z->wp - z->start;
+		}
 		swd += z->wp - z->start;
 	}
 	pthread_mutex_lock(&f->zbd_info->mutex);
 	switch (a) {
 	case CHECK_SWD:
 		assert(f->zbd_info->sectors_with_data == swd);
+		assert(f->zbd_info->wp_sectors_with_data == wp_swd);
 		break;
 	case SET_SWD:
 		f->zbd_info->sectors_with_data = swd;
+		f->zbd_info->wp_sectors_with_data = wp_swd;
 		break;
 	}
 	pthread_mutex_unlock(&f->zbd_info->mutex);
 	for (z = zb; z < ze; z++)
-		zone_unlock(z);
+		if (z->has_wp)
+			zone_unlock(z);
 
 	return swd;
 }
@@ -916,7 +925,7 @@ static uint64_t zbd_process_swd(const struct fio_file *f, enum swd_action a)
  */
 static const bool enable_check_swd = false;
 
-/* Check whether the value of zbd_info.sectors_with_data is correct. */
+/* Check whether the values of zbd_info.*sectors_with_data are correct. */
 static void zbd_check_swd(const struct fio_file *f)
 {
 	if (!enable_check_swd)
@@ -1347,8 +1356,10 @@ static void zbd_queue_io(struct thread_data *td, struct io_u *io_u, int q,
 		 * z->wp > zone_end means that one or more I/O errors
 		 * have occurred.
 		 */
-		if (z->wp <= zone_end)
+		if (z->wp <= zone_end) {
 			zbd_info->sectors_with_data += zone_end - z->wp;
+			zbd_info->wp_sectors_with_data += zone_end - z->wp;
+		}
 		pthread_mutex_unlock(&zbd_info->mutex);
 		z->wp = zone_end;
 		break;
@@ -1650,7 +1661,7 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 		}
 		/* Check whether the zone reset threshold has been exceeded */
 		if (td->o.zrf.u.f) {
-			if (f->zbd_info->sectors_with_data >=
+			if (f->zbd_info->wp_sectors_with_data >=
 			    f->io_size * td->o.zrt.u.f &&
 			    zbd_dec_and_reset_write_cnt(td, f)) {
 				zb->reset_zone = 1;
diff --git a/zbd.h b/zbd.h
index 059a9f9e..cc3ab624 100644
--- a/zbd.h
+++ b/zbd.h
@@ -55,6 +55,8 @@ struct fio_zone_info {
  *		num_open_zones).
  * @zone_size: size of a single zone in bytes.
  * @sectors_with_data: total size of data in all zones in units of 512 bytes
+ * @wp_sectors_with_data: total size of data in zones with write pointers in
+ *                        units of 512 bytes
  * @zone_size_log2: log2 of the zone size in bytes if it is a power of 2 or 0
  *		if the zone size is not a power of 2.
  * @nr_zones: number of zones
@@ -75,6 +77,7 @@ struct zoned_block_device_info {
 	pthread_mutex_t		mutex;
 	uint64_t		zone_size;
 	uint64_t		sectors_with_data;
+	uint64_t		wp_sectors_with_data;
 	uint32_t		zone_size_log2;
 	uint32_t		nr_zones;
 	uint32_t		refcount;
-- 
2.28.0

[PATCH v3 13/38] zbd: initialize min_zone and max_zone for all zone types

[Dmitry Fomichev]

From: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>

The function zbd_verify_sizes() checks if the given I/O range includes
write pointer zones. When all zones in the I/O range are conventional,
it skips checks for size options and leaves min_zone and max_zone in
struct fio_file with zero values. These uninitialized min_zone and
max_zone fields trigger unexpected behaviors such as unset
sectors_with_data.

Fix this by moving min_zone and max_zone set up from zbd_verify_sizes()
to zbd_setup_files(). This allows for setting up the values regardless
of zone types in I/O range.

Bypass the assertion to ensure that max_zone is larger than min_zone if
all zones in the I/O range are conventional. In this case, io_size
can be smaller than zone size and, consequently, min_zone may become
the same as max_zone.

Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
 zbd.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/zbd.c b/zbd.c
index 757e72d5..563f941b 100644
--- a/zbd.c
+++ b/zbd.c
@@ -324,10 +324,6 @@ static bool zbd_verify_sizes(void)
 					 (unsigned long long) new_end - f->file_offset);
 				f->io_size = new_end - f->file_offset;
 			}
-
-			f->min_zone = zbd_zone_idx(f, f->file_offset);
-			f->max_zone = zbd_zone_idx(f, f->file_offset + f->io_size);
-			assert(f->min_zone < f->max_zone);
 		}
 	}
 
@@ -680,6 +676,18 @@ int zbd_setup_files(struct thread_data *td)
 		if (!zbd)
 			continue;
 
+		f->min_zone = zbd_zone_idx(f, f->file_offset);
+		f->max_zone = zbd_zone_idx(f, f->file_offset + f->io_size);
+
+		/*
+		 * When all zones in the I/O range are conventional, io_size
+		 * can be smaller than zone size, making min_zone the same
+		 * as max_zone. This is why the assert below needs to be made
+		 * conditional.
+		 */
+		if (zbd_is_seq_job(f))
+			assert(f->min_zone < f->max_zone);
+
 		zbd->max_open_zones = zbd->max_open_zones ?: ZBD_MAX_OPEN_ZONES;
 
 		if (td->o.max_open_zones > 0 &&
-- 
2.28.0

[PATCH v3 14/38] zbd: initialize sectors with data at start time

[Dmitry Fomichev]

From: Aravind Ramesh <aravind.ramesh@wdc.com>

Based on the flag enable_check_swd, which is false by default, fio
does not initialize the swd value at startup, initializing the swd
value to be zero, even if some zones have sectors with data. This can
result in fio reflecting less than actual swd after a few writes are
completed. In workloads where verify is enabled, fio resets all the
zones and while resetting, it decrements the swd counter with the
actual number of swds in that zone(swd-count - swd-in-zone),
since swd-count is initialized to 0, it results in overflow of the
variable causing unpredictable issues.

So, initialize the swd to the correct value.

Fixes: 409a4f291e7f ("zbd: avoid initializing swd when unnecessary")
Signed-off-by: Aravind Ramesh <aravind.ramesh@wdc.com>
Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
 zbd.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/zbd.c b/zbd.c
index 563f941b..45d045e9 100644
--- a/zbd.c
+++ b/zbd.c
@@ -946,9 +946,6 @@ static void zbd_init_swd(struct fio_file *f)
 {
 	uint64_t swd;
 
-	if (!enable_check_swd)
-		return;
-
 	swd = zbd_process_swd(f, SET_SWD);
 	dprint(FD_ZBD, "%s(%s): swd = %" PRIu64 "\n", __func__, f->file_name,
 	       swd);
-- 
2.28.0

Re: [PATCH v3 14/38] zbd: initialize sectors with data at start time

[Shinichiro Kawasaki]

On Jan 07, 2021 / 06:57, Dmitry Fomichev wrote:
> From: Aravind Ramesh <aravind.ramesh@wdc.com>
> 
> Based on the flag enable_check_swd, which is false by default, fio
> does not initialize the swd value at startup, initializing the swd
> value to be zero, even if some zones have sectors with data. This can
> result in fio reflecting less than actual swd after a few writes are
> completed. In workloads where verify is enabled, fio resets all the
> zones and while resetting, it decrements the swd counter with the
> actual number of swds in that zone(swd-count - swd-in-zone),
> since swd-count is initialized to 0, it results in overflow of the
> variable causing unpredictable issues.
> 
> So, initialize the swd to the correct value.
> 
> Fixes: 409a4f291e7f ("zbd: avoid initializing swd when unnecessary")
> Signed-off-by: Aravind Ramesh <aravind.ramesh@wdc.com>
> Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>

Looks good to me.

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

-- 
Best Regards,
Shin'ichiro Kawasaki

[PATCH v3 15/38] zbd: use zone_lock() in zbd_process_swd()

[Dmitry Fomichev]

Most of ZBD code in fio uses zone_lock() to lock write pointer zones.
This wrapper, besides doing the obvious pthread mutex lock, quiesce
the outstanding i/o when running via asynchronous ioengines. This is
necessary to avoid deadlocks. The function zbd_process_swd(), however,
still uses the naked pthread mutex to lock zones and this leads to a
deadlock when running ZBD test #48 against regular nullb devices.

The fix added in the same patch series that introduced test #48 was to
NOT initialize SWD at all, but this solution is found to create
problems with verify. As the proper fix, modify zbd_process_swd()
to use zone_lock(). This makes the test #48 pass even when SWD counter
is initialized.

Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
 zbd.c | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/zbd.c b/zbd.c
index 45d045e9..c27efd54 100644
--- a/zbd.c
+++ b/zbd.c
@@ -166,7 +166,8 @@ static bool zbd_zone_full(const struct fio_file *f, struct fio_zone_info *z,
 		z->wp + required > zbd_zone_capacity_end(z);
 }
 
-static void zone_lock(struct thread_data *td, struct fio_file *f, struct fio_zone_info *z)
+static void zone_lock(struct thread_data *td, const struct fio_file *f,
+		      struct fio_zone_info *z)
 {
 	struct zoned_block_device_info *zbd = f->zbd_info;
 	uint32_t nz = z - zbd->zone_info;
@@ -893,7 +894,8 @@ enum swd_action {
 };
 
 /* Calculate the number of sectors with data (swd) and perform action 'a' */
-static uint64_t zbd_process_swd(const struct fio_file *f, enum swd_action a)
+static uint64_t zbd_process_swd(struct thread_data *td,
+				const struct fio_file *f, enum swd_action a)
 {
 	struct fio_zone_info *zb, *ze, *z;
 	uint64_t swd = 0;
@@ -903,7 +905,7 @@ static uint64_t zbd_process_swd(const struct fio_file *f, enum swd_action a)
 	ze = get_zone(f, f->max_zone);
 	for (z = zb; z < ze; z++) {
 		if (z->has_wp) {
-			pthread_mutex_lock(&z->mutex);
+			zone_lock(td, f, z);
 			wp_swd += z->wp - z->start;
 		}
 		swd += z->wp - z->start;
@@ -934,33 +936,27 @@ static uint64_t zbd_process_swd(const struct fio_file *f, enum swd_action a)
 static const bool enable_check_swd = false;
 
 /* Check whether the values of zbd_info.*sectors_with_data are correct. */
-static void zbd_check_swd(const struct fio_file *f)
+static void zbd_check_swd(struct thread_data *td, const struct fio_file *f)
 {
 	if (!enable_check_swd)
 		return;
 
-	zbd_process_swd(f, CHECK_SWD);
-}
-
-static void zbd_init_swd(struct fio_file *f)
-{
-	uint64_t swd;
-
-	swd = zbd_process_swd(f, SET_SWD);
-	dprint(FD_ZBD, "%s(%s): swd = %" PRIu64 "\n", __func__, f->file_name,
-	       swd);
+	zbd_process_swd(td, f, CHECK_SWD);
 }
 
 void zbd_file_reset(struct thread_data *td, struct fio_file *f)
 {
 	struct fio_zone_info *zb, *ze;
+	uint64_t swd;
 
 	if (!f->zbd_info || !td_write(td))
 		return;
 
 	zb = get_zone(f, f->min_zone);
 	ze = get_zone(f, f->max_zone);
-	zbd_init_swd(f);
+	swd = zbd_process_swd(td, f, SET_SWD);
+	dprint(FD_ZBD, "%s(%s): swd = %" PRIu64 "\n", __func__, f->file_name,
+	       swd);
 	/*
 	 * If data verification is enabled reset the affected zones before
 	 * writing any data to avoid that a zone reset has to be issued while
@@ -1413,7 +1409,7 @@ static void zbd_put_io(struct thread_data *td, const struct io_u *io_u)
 	zbd_end_zone_io(td, io_u, z);
 
 	zone_unlock(z);
-	zbd_check_swd(f);
+	zbd_check_swd(td, f);
 }
 
 /*
@@ -1579,7 +1575,7 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 	    io_u->ddir == DDIR_READ && td->o.read_beyond_wp)
 		return io_u_accept;
 
-	zbd_check_swd(f);
+	zbd_check_swd(td, f);
 
 	zone_lock(td, f, zb);
 
-- 
2.28.0

Re: [PATCH v3 15/38] zbd: use zone_lock() in zbd_process_swd()

[Shinichiro Kawasaki]

On Jan 07, 2021 / 06:57, Dmitry Fomichev wrote:
> Most of ZBD code in fio uses zone_lock() to lock write pointer zones.
> This wrapper, besides doing the obvious pthread mutex lock, quiesce
> the outstanding i/o when running via asynchronous ioengines. This is
> necessary to avoid deadlocks. The function zbd_process_swd(), however,
> still uses the naked pthread mutex to lock zones and this leads to a
> deadlock when running ZBD test #48 against regular nullb devices.
> 
> The fix added in the same patch series that introduced test #48 was to
> NOT initialize SWD at all, but this solution is found to create
> problems with verify. As the proper fix, modify zbd_process_swd()
> to use zone_lock(). This makes the test #48 pass even when SWD counter
> is initialized.
> 
> Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>

Looks good to me.

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

-- 
Best Regards,
Shin'ichiro Kawasaki

[PATCH v3 16/38] zbd: disable crossing from conventional to sequential zones

[Dmitry Fomichev]

From: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>

Write I/Os to conventional zones may have the range that spans across
zone boundaries. Such writes may cause I/O errors when its next zone
is a sequential zone.

To avoid such I/O errors, check for the cross over from a conventional
to a sequential zone. When the write crosses the boundary, shrink the
I/O length to fit within the first zone. If the offset is too close to
the end of the zone, wrap it around to the beginning of the same zone.

Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
 zbd.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/zbd.c b/zbd.c
index c27efd54..8fb9c8cf 100644
--- a/zbd.c
+++ b/zbd.c
@@ -1563,9 +1563,31 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 	zb = get_zone(f, zone_idx_b);
 	orig_zb = zb;
 
-	/* Accept the I/O offset for conventional zones. */
-	if (!zb->has_wp)
+	if (!zb->has_wp) {
+		/* Accept non-write I/Os for conventional zones. */
+		if (io_u->ddir != DDIR_WRITE)
+			return io_u_accept;
+		/*
+		 * Make sure that writes to conventional zones
+		 * don't cross over to any sequential zones.
+		 */
+		if (!(zb + 1)->has_wp ||
+		    io_u->offset + io_u->buflen <= (zb + 1)->start)
+			return io_u_accept;
+
+		if (io_u->offset + min_bs > (zb + 1)->start) {
+			dprint(FD_IO,
+			       "%s: off=%llu + min_bs=%u > next zone %lu\n",
+			       f->file_name, io_u->offset, 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 {
+			new_len = (zb + 1)->start - io_u->offset;
+		}
+		io_u->buflen = new_len / min_bs * min_bs;
 		return io_u_accept;
+	}
 
 	/*
 	 * Accept the I/O offset for reads if reading beyond the write pointer
-- 
2.28.0

[PATCH v3 17/38] zbd: don't log "zone nnnn is not open" message

[Dmitry Fomichev]

This log message has been added recently (it could have been my idea
to add it during internal review) and it turns out that the message
tends to flood the log when any decent workload is run with
--zonemode=zbd. Remove logging of this debug message.

Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
 zbd.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/zbd.c b/zbd.c
index 8fb9c8cf..249835c2 100644
--- a/zbd.c
+++ b/zbd.c
@@ -786,11 +786,8 @@ static void zbd_close_zone(struct thread_data *td, const struct fio_file *f,
 		if (f->zbd_info->open_zones[open_zone_idx] == zone_idx)
 			break;
 	}
-	if (open_zone_idx == f->zbd_info->num_open_zones) {
-		dprint(FD_ZBD, "%s: zone %d is not open\n",
-		       f->file_name, zone_idx);
+	if (open_zone_idx == f->zbd_info->num_open_zones)
 		return;
-	}
 
 	dprint(FD_ZBD, "%s: closing zone %d\n", f->file_name, zone_idx);
 	memmove(f->zbd_info->open_zones + open_zone_idx,
-- 
2.28.0

Re: [PATCH v3 17/38] zbd: don't log "zone nnnn is not open" message

[Shinichiro Kawasaki]

On Jan 07, 2021 / 06:57, Dmitry Fomichev wrote:
> This log message has been added recently (it could have been my idea
> to add it during internal review) and it turns out that the message
> tends to flood the log when any decent workload is run with
> --zonemode=zbd. Remove logging of this debug message.
> 
> Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>

Looks good to me.

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

-- 
Best Regards,
Shin'ichiro Kawasaki

[PATCH v3 18/38] zbd: handle conventional start zone in zbd_convert_to_open_zone()

[Dmitry Fomichev]

At the beginning of zbd_convert_to_open_zone() function, a zone
is picked in semi-random manner to become a candidate zone for
redirecting the incoming write. In some circumstances, such as
unlimited MaxOpen or i/o range that spans the boundary between
conventional and sequential zones, a conventional zone may be
selected.

This may create problems in the subsequent for (;;) loop in the
same function. Failed assertions were observed during the execution
of newly introduced test #51 that showed that the code in that loop
was trying to lock and unlock conventional zones.

Check if the zone which has been initially picked is conventional.
If yes, force the zone selection to be re-tried until a sequential
zone is selected for further processing.

Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
 zbd.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/zbd.c b/zbd.c
index 249835c2..df61dc37 100644
--- a/zbd.c
+++ b/zbd.c
@@ -1087,16 +1087,18 @@ static struct fio_zone_info *zbd_convert_to_open_zone(struct thread_data *td,
 		uint32_t tmp_idx;
 
 		z = get_zone(f, zone_idx);
-
-		zone_lock(td, f, z);
+		if (z->has_wp)
+			zone_lock(td, f, z);
 		pthread_mutex_lock(&f->zbd_info->mutex);
-		if (z->cond != ZBD_ZONE_COND_OFFLINE &&
-		    td->o.max_open_zones == 0 && td->o.job_max_open_zones == 0)
-			goto examine_zone;
-		if (f->zbd_info->num_open_zones == 0) {
-			dprint(FD_ZBD, "%s(%s): no zones are open\n",
-			       __func__, f->file_name);
-			goto open_other_zone;
+		if (z->has_wp) {
+			if (z->cond != ZBD_ZONE_COND_OFFLINE &&
+			    td->o.max_open_zones == 0 && td->o.job_max_open_zones == 0)
+				goto examine_zone;
+			if (f->zbd_info->num_open_zones == 0) {
+				dprint(FD_ZBD, "%s(%s): no zones are open\n",
+				       __func__, f->file_name);
+				goto open_other_zone;
+			}
 		}
 
 		/*
@@ -1124,7 +1126,8 @@ static struct fio_zone_info *zbd_convert_to_open_zone(struct thread_data *td,
 		dprint(FD_ZBD, "%s(%s): no candidate zone\n",
 			__func__, f->file_name);
 		pthread_mutex_unlock(&f->zbd_info->mutex);
-		zone_unlock(z);
+		if (z->has_wp)
+			zone_unlock(z);
 		return NULL;
 
 found_candidate_zone:
@@ -1133,7 +1136,8 @@ found_candidate_zone:
 			break;
 		zone_idx = new_zone_idx;
 		pthread_mutex_unlock(&f->zbd_info->mutex);
-		zone_unlock(z);
+		if (z->has_wp)
+			zone_unlock(z);
 	}
 
 	/* Both z->mutex and f->zbd_info->mutex are held. */
-- 
2.28.0

Re: [PATCH v3 18/38] zbd: handle conventional start zone in zbd_convert_to_open_zone()

[Shinichiro Kawasaki]

On Jan 07, 2021 / 06:57, Dmitry Fomichev wrote:
> At the beginning of zbd_convert_to_open_zone() function, a zone
> is picked in semi-random manner to become a candidate zone for
> redirecting the incoming write. In some circumstances, such as
> unlimited MaxOpen or i/o range that spans the boundary between
> conventional and sequential zones, a conventional zone may be
> selected.
> 
> This may create problems in the subsequent for (;;) loop in the
> same function. Failed assertions were observed during the execution
> of newly introduced test #51 that showed that the code in that loop
> was trying to lock and unlock conventional zones.
> 
> Check if the zone which has been initially picked is conventional.
> If yes, force the zone selection to be re-tried until a sequential
> zone is selected for further processing.
> 
> Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>

Looks good to me.

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

-- 
Best Regards,
Shin'ichiro Kawasaki

[PATCH v3 19/38] zbd: improve replay range validation

[Dmitry Fomichev]

Make sure that verification read always reads valid data.

Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
 zbd.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/zbd.c b/zbd.c
index df61dc37..2656f3ea 100644
--- a/zbd.c
+++ b/zbd.c
@@ -1239,10 +1239,23 @@ static struct fio_zone_info *zbd_replay_write_order(struct thread_data *td,
 		assert(z);
 	}
 
-	if (z->verify_block * min_bs >= z->capacity)
+	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);
-	io_u->offset = z->start + z->verify_block++ * min_bs;
+		/*
+		 * If the assertion below fails during a test run, adding
+		 * "--experimental_verify=1" to the command line may help.
+		 */
+		assert(false);
+	}
+	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 >= %lu\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;
+
 	return z;
 }
 
-- 
2.28.0

Re: [PATCH v3 19/38] zbd: improve replay range validation

[Shinichiro Kawasaki]

On Jan 07, 2021 / 06:57, Dmitry Fomichev wrote:
> Make sure that verification read always reads valid data.
> 
> Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>

The code looks good to me. I felt some more background explanation in commit
message might add some more help, but anyway,

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

-- 
Best Regards,
Shin'ichiro Kawasaki

[PATCH v3 20/38] engines/libzbc: enable block backend

[Dmitry Fomichev]

When opening a device, the current version of libzbc ioengine instructs
libzbc to only try SCSI and ATA backends for scanning the drive. This
prevents opening null_blk devices that fail to be accepted by the both
above mentioned backends and require the block backend to be enabled.

Set the appropriate flag to enable the block backend in zbc_open()
libzbc call.

Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
 engines/libzbc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/engines/libzbc.c b/engines/libzbc.c
index 552aab65..2aacf7bb 100644
--- a/engines/libzbc.c
+++ b/engines/libzbc.c
@@ -86,7 +86,8 @@ static int libzbc_open_dev(struct thread_data *td, struct fio_file *f,
 		return -ENOMEM;
 
 	ret = zbc_open(f->file_name,
-		       flags | ZBC_O_DRV_SCSI | ZBC_O_DRV_ATA, &ld->zdev);
+		       flags | ZBC_O_DRV_BLOCK | ZBC_O_DRV_SCSI | ZBC_O_DRV_ATA,
+		       &ld->zdev);
 	if (ret) {
 		log_err("%s: zbc_open() failed, err=%d\n",
 			f->file_name, ret);
-- 
2.28.0

Re: [PATCH v3 20/38] engines/libzbc: enable block backend

[Shinichiro Kawasaki]

On Jan 07, 2021 / 06:57, Dmitry Fomichev wrote:
> When opening a device, the current version of libzbc ioengine instructs
> libzbc to only try SCSI and ATA backends for scanning the drive. This
> prevents opening null_blk devices that fail to be accepted by the both
> above mentioned backends and require the block backend to be enabled.
> 
> Set the appropriate flag to enable the block backend in zbc_open()
> libzbc call.
> 
> Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>

Looks good to me.

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

-- 
Best Regards,
Shin'ichiro Kawasaki

[PATCH v3 21/38] zbd: avoid failing assertion in zbd_convert_to_open_zone()

[Dmitry Fomichev]

The test run against null_blk with the following command line -

t/zbd/run-tests-against-nullb -l -q -s 12 -t 51 -n 100

stops with a failure and the message below can be seen in the test log:

fio: zbd.c:1110: zbd_convert_to_open_zone: Assertion `open_zone_idx < f->zbd_info->num_open_zones' failed.

This assertion fails because pick_random_zone_idx() function returns
index 0 if no zones are currently open. In this case, open_zone_idx and
f->zbd_info->num_open_zones are both zero. Since this situation is
normal, simply modify the assert statement to avoid failing.

Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
 zbd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/zbd.c b/zbd.c
index 2656f3ea..f51db783 100644
--- a/zbd.c
+++ b/zbd.c
@@ -1107,7 +1107,8 @@ static struct fio_zone_info *zbd_convert_to_open_zone(struct thread_data *td,
 		 * Ignore zones which don't belong to thread's offset/size area.
 		 */
 		open_zone_idx = pick_random_zone_idx(f, io_u);
-		assert(open_zone_idx < f->zbd_info->num_open_zones);
+		assert(!open_zone_idx ||
+		       open_zone_idx < f->zbd_info->num_open_zones);
 		tmp_idx = open_zone_idx;
 		for (i = 0; i < f->zbd_info->num_open_zones; i++) {
 			uint32_t tmpz;
-- 
2.28.0

Re: [PATCH v3 21/38] zbd: avoid failing assertion in zbd_convert_to_open_zone()

[Shinichiro Kawasaki]

On Jan 07, 2021 / 06:57, Dmitry Fomichev wrote:
> The test run against null_blk with the following command line -
> 
> t/zbd/run-tests-against-nullb -l -q -s 12 -t 51 -n 100
> 
> stops with a failure and the message below can be seen in the test log:
> 
> fio: zbd.c:1110: zbd_convert_to_open_zone: Assertion `open_zone_idx < f->zbd_info->num_open_zones' failed.
> 
> This assertion fails because pick_random_zone_idx() function returns
> index 0 if no zones are currently open. In this case, open_zone_idx and
> f->zbd_info->num_open_zones are both zero. Since this situation is
> normal, simply modify the assert statement to avoid failing.
> 
> Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>

Looks good to me.

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

-- 
Best Regards,
Shin'ichiro Kawasaki

[PATCH v3 22/38] zbd: set thread errors in zbd_adjust_block()

[Dmitry Fomichev]

Several error conditions that are encountered during zone processing
in zbd_adjust_block() function cause it to return io_u_eof value.
This stops the i/o to the given file, but there is no error raised or
reported if this code is returned. For a few particular conditions,
just stopping the i/o is reasonable, but others are serious errors
that should be reported.

Add td_verror() calls to raise thread errors for a few abnormal
conditions during adjusting the i/o. The only test that needs to be
modified because of this changes is test #2.

Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
 zbd.c | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/zbd.c b/zbd.c
index f51db783..e8fa54ca 100644
--- a/zbd.c
+++ b/zbd.c
@@ -1688,13 +1688,22 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 		assert(io_u->offset + io_u->buflen <= zb->wp);
 		goto accept;
 	case DDIR_WRITE:
-		if (io_u->buflen > f->zbd_info->zone_size)
+		if (io_u->buflen > f->zbd_info->zone_size) {
+			td_verror(td, EINVAL, "I/O buflen exceeds zone size");
+			dprint(FD_IO,
+			       "%s: I/O buflen %llu exceeds zone size %lu\n",
+			       f->file_name, io_u->buflen,
+			       f->zbd_info->zone_size);
 			goto eof;
+		}
 		if (!zbd_open_zone(td, f, zone_idx_b)) {
 			zone_unlock(zb);
 			zb = zbd_convert_to_open_zone(td, io_u);
-			if (!zb)
+			if (!zb) {
+				dprint(FD_IO, "%s: can't convert to open zone",
+				       f->file_name);
 				goto eof;
+			}
 			zone_idx_b = zbd_zone_nr(f, zb);
 		}
 		/* Check whether the zone reset threshold has been exceeded */
@@ -1721,6 +1730,7 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 				goto eof;
 
 			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);
@@ -1731,8 +1741,9 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 		assert(!zbd_zone_full(f, zb, min_bs));
 		io_u->offset = zb->wp;
 		if (!is_valid_offset(f, io_u->offset)) {
-			dprint(FD_ZBD, "Dropped request with offset %llu\n",
-			       io_u->offset);
+			td_verror(td, EINVAL, "invalid WP value");
+			dprint(FD_ZBD, "%s: dropped request with offset %llu\n",
+			       f->file_name, io_u->offset);
 			goto eof;
 		}
 		/*
@@ -1751,9 +1762,9 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 			       orig_len, io_u->buflen);
 			goto accept;
 		}
-		log_err("Zone remainder %lld smaller than minimum block size %d\n",
-			(zbd_zone_capacity_end(zb) - io_u->offset),
-			min_bs);
+		td_verror(td, EIO, "zone remainder too small");
+		log_err("zone remainder %lld smaller than min block size %d\n",
+			(zbd_zone_capacity_end(zb) - io_u->offset), min_bs);
 		goto eof;
 	case DDIR_TRIM:
 		/* fall-through */
-- 
2.28.0

Re: [PATCH v3 22/38] zbd: set thread errors in zbd_adjust_block()

[Shinichiro Kawasaki]

On Jan 07, 2021 / 06:57, Dmitry Fomichev wrote:
> Several error conditions that are encountered during zone processing
> in zbd_adjust_block() function cause it to return io_u_eof value.
> This stops the i/o to the given file, but there is no error raised or
> reported if this code is returned. For a few particular conditions,
> just stopping the i/o is reasonable, but others are serious errors
> that should be reported.
> 
> Add td_verror() calls to raise thread errors for a few abnormal
> conditions during adjusting the i/o. The only test that needs to be
> modified because of this changes is test #2.
> 
> Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>

Looks good to me.

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

-- 
Best Regards,
Shin'ichiro Kawasaki

[PATCH v3 23/38] t/zbd: check for error in test #2

[Dmitry Fomichev]

With the preceding commit in place, fio gives an error if user attempts
to run write I/O size that is larger than the zone size. Grep for that
message instead of checking that no write has happened.

Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
 t/zbd/test-zbd-support | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/zbd/test-zbd-support b/t/zbd/test-zbd-support
index acde3b3a..652dddfc 100755
--- a/t/zbd/test-zbd-support
+++ b/t/zbd/test-zbd-support
@@ -221,8 +221,8 @@ test2() {
     if [ -z "$is_zbd" ]; then
 	opts+=("--zonesize=${zone_size}")
     fi
-    run_fio "${opts[@]}" >> "${logfile}.${test_number}" 2>&1 || return $?
-    ! grep -q 'WRITE:' "${logfile}.${test_number}"
+    run_fio "${opts[@]}" >> "${logfile}.${test_number}" 2>&1 && return 1
+    grep -q 'buflen exceeds zone size' "${logfile}.${test_number}"
 }
 
 # Run fio against an empty zone. This causes fio to report "No I/O performed".
-- 
2.28.0

Re: [PATCH v3 23/38] t/zbd: check for error in test #2

[Shinichiro Kawasaki]

On Jan 07, 2021 / 06:57, Dmitry Fomichev wrote:
> With the preceding commit in place, fio gives an error if user attempts
> to run write I/O size that is larger than the zone size. Grep for that
> message instead of checking that no write has happened.
> 
> Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>

Looks good to me.

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

-- 
Best Regards,
Shin'ichiro Kawasaki

[PATCH v3 24/38] t/zbd: add run-tests-against-nullb script

[Dmitry Fomichev]

This script combines the t/zbd/run-tests-against-zoned-nullb script
functionality with t/zbd/run-tests-against-regular-nullb and adds
more zoned device configurations to test. This considerably improves
ZBD test coverage.

The added script makes the two old scripts named above obsolete,
remove them. Modify t/run-fio-tests.py and Makefile to refer to the
new script instead of the old one. Since the full test now runs
significantly longer than the two old ones combined due to many more
zoned configurations, only execute a few individual sections as a
part of testing n "make fulltest" and run-fio-tests.py. One extra test
section with 10% conventional zones is executed from the Makefile.
The Python tests only exercise all-conventional and all-sequential
configurations, exactly as before.

The script returns a non-zero return code if at least one of the
executed sections had a failed test.

Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
 Makefile                              |   5 +-
 t/run-fio-tests.py                    |   8 +-
 t/zbd/run-tests-against-nullb         | 338 ++++++++++++++++++++++++++
 t/zbd/run-tests-against-regular-nullb |  27 --
 t/zbd/run-tests-against-zoned-nullb   |  53 ----
 5 files changed, 345 insertions(+), 86 deletions(-)
 create mode 100755 t/zbd/run-tests-against-nullb
 delete mode 100755 t/zbd/run-tests-against-regular-nullb
 delete mode 100755 t/zbd/run-tests-against-zoned-nullb

diff --git a/Makefile b/Makefile
index a838af9a..19bfb996 100644
--- a/Makefile
+++ b/Makefile
@@ -600,9 +600,10 @@ fulltest:
 	   make -j &&						 	\
 	   sudo make install)						\
 	fi &&					 			\
-	sudo t/zbd/run-tests-against-regular-nullb &&		 	\
+	sudo t/zbd/run-tests-against-nullb -s 1 &&		 	\
 	if [ -e /sys/module/null_blk/parameters/zoned ]; then		\
-		sudo t/zbd/run-tests-against-zoned-nullb;	 	\
+		sudo t/zbd/run-tests-against-nullb -s 2;	 	\
+		sudo t/zbd/run-tests-against-nullb -s 4;	 	\
 	fi
 
 install: $(PROGS) $(SCRIPTS) $(ENGS_OBJS) tools/plot/fio2gnuplot.1 FORCE
diff --git a/t/run-fio-tests.py b/t/run-fio-tests.py
index e5c2f17c..a59cdfe0 100755
--- a/t/run-fio-tests.py
+++ b/t/run-fio-tests.py
@@ -879,8 +879,8 @@ TEST_LIST = [
     {
         'test_id':          1007,
         'test_class':       FioExeTest,
-        'exe':              't/zbd/run-tests-against-regular-nullb',
-        'parameters':       None,
+        'exe':              't/zbd/run-tests-against-nullb',
+        'parameters':       ['-s', '1'],
         'success':          SUCCESS_DEFAULT,
         'requirements':     [Requirements.linux, Requirements.zbd,
                              Requirements.root],
@@ -888,8 +888,8 @@ TEST_LIST = [
     {
         'test_id':          1008,
         'test_class':       FioExeTest,
-        'exe':              't/zbd/run-tests-against-zoned-nullb',
-        'parameters':       None,
+        'exe':              't/zbd/run-tests-against-nullb',
+        'parameters':       ['-s', '2'],
         'success':          SUCCESS_DEFAULT,
         'requirements':     [Requirements.linux, Requirements.zbd,
                              Requirements.root, Requirements.zoned_nullb],
diff --git a/t/zbd/run-tests-against-nullb b/t/zbd/run-tests-against-nullb
new file mode 100755
index 00000000..8d1eb005
--- /dev/null
+++ b/t/zbd/run-tests-against-nullb
@@ -0,0 +1,338 @@
+#!/bin/bash
+#
+# Copyright (C) 2020 Western Digital Corporation or its affiliates.
+#
+# This file is released under the GPL.
+#
+# Run t/zbd/test-zbd-support script against a variety of conventional,
+# zoned and mixed zone configurations.
+#
+
+function usage()
+{
+	echo "This script runs the tests from t/zbd/test-zbd-support script"
+        echo "against a nullb device in a variety of conventional and zoned"
+	echo "configurations."
+	echo "Usage: ${0} [OPTIONS]"
+	echo "Options:"
+	echo -e "\t-h Show this message."
+	echo -e "\t-L List the device layouts for every section without running"
+	echo -e "\t   tests."
+	echo -e "\t-s <#section> Only run the section with the given number."
+	echo -e "\t-l Use libzbc ioengine to run the tests."
+	echo -e "\t-o <max_open_zones> Specify MaxOpen value, (${set_max_open} by default)."
+	echo -e "\t-n <#number of runs> Set the number of times to run the entire suite "
+	echo -e "\t   or an individual section/test."
+	echo -e "\t-r Remove the /dev/nullb0 device that may still exist after"
+	echo -e "\t   running this script."
+	exit 1
+}
+
+function cleanup_nullb()
+{
+	for d in /sys/kernel/config/nullb/*; do [ -d "$d" ] && rmdir "$d"; done
+	modprobe -r null_blk
+	modprobe null_blk nr_devices=0 || exit $?
+	for d in /sys/kernel/config/nullb/*; do
+		[ -d "$d" ] && rmdir "$d"
+	done
+	modprobe -r null_blk
+	[ -e /sys/module/null_blk ] && exit $?
+}
+
+function create_nullb()
+{
+	modprobe null_blk nr_devices=0 &&
+	cd /sys/kernel/config/nullb &&
+	mkdir nullb0 &&
+	cd nullb0 || return $?
+}
+
+function configure_nullb()
+{
+	echo 0 > completion_nsec &&
+		echo ${dev_blocksize} > blocksize &&
+		echo ${dev_size} > size &&
+		echo 1 > memory_backed || return $?
+
+	if ((conv_pcnt < 100)); then
+		echo 1 > zoned &&
+			echo "${zone_size}" > zone_size || return $?
+
+		if ((zone_capacity < zone_size)); then
+			if ((!zcap_supported)); then
+				echo "null_blk does not support zone capacity"
+				return 2
+			fi
+			echo "${zone_capacity}" > zone_capacity
+		fi
+		if ((conv_pcnt)); then
+			if ((!conv_supported)); then
+				echo "null_blk does not support conventional zones"
+				return 2
+			fi
+			nr_conv=$((dev_size/zone_size*conv_pcnt/100))
+			echo "${nr_conv}" > zone_nr_conv
+		fi
+	fi
+
+	echo 1 > power || return $?
+	return 0
+}
+
+function show_nullb_config()
+{
+	if ((conv_pcnt < 100)); then
+		echo "    $(printf "Zoned Device, %d%% Conventional Zones (%d)" \
+			  ${conv_pcnt} ${nr_conv})"
+		echo "    $(printf "Zone Size: %d MB" ${zone_size})"
+		echo "    $(printf "Zone Capacity: %d MB" ${zone_capacity})"
+		if ((max_open)); then
+			echo "    $(printf "Max Open: %d Zones" ${max_open})"
+		else
+			echo "    Max Open: Unlimited Zones"
+		fi
+	else
+		echo "    Non-zoned Device"
+	fi
+}
+
+#
+# Test sections.
+#
+# Fully conventional device.
+function section1
+{
+	conv_pcnt=100
+	max_open=0
+}
+
+# Zoned device with no conventional zones, ZCAP == ZSIZE, unlimited MaxOpen.
+function section2
+{
+	conv_pcnt=0
+	zone_size=1
+	zone_capacity=1
+	max_open=0
+}
+
+# Zoned device with no conventional zones, ZCAP < ZSIZE, unlimited MaxOpen.
+function section3
+{
+	conv_pcnt=0
+	zone_size=4
+	zone_capacity=3
+	max_open=0
+}
+
+# Zoned device with mostly sequential zones, ZCAP == ZSIZE, unlimited MaxOpen.
+function section4
+{
+	conv_pcnt=10
+	zone_size=1
+	zone_capacity=1
+	max_open=0
+}
+
+# Zoned device with mostly sequential zones, ZCAP < ZSIZE, unlimited MaxOpen.
+function section5
+{
+	conv_pcnt=10
+	zone_size=4
+	zone_capacity=3
+	max_open=0
+}
+
+# Zoned device with mostly conventional zones, ZCAP == ZSIZE, unlimited MaxOpen.
+function section6
+{
+	conv_pcnt=66
+	zone_size=1
+	zone_capacity=1
+	max_open=0
+}
+
+# Zoned device with mostly conventional zones, ZCAP < ZSIZE, unlimited MaxOpen.
+function section7
+{
+	dev_size=2048
+	conv_pcnt=66
+	zone_size=4
+	zone_capacity=3
+	max_open=0
+}
+
+# Zoned device with no conventional zones, ZCAP == ZSIZE, limited MaxOpen.
+function section8
+{
+	dev_size=1024
+	conv_pcnt=0
+	zone_size=1
+	zone_capacity=1
+	max_open=${set_max_open}
+	zbd_test_opts+=("-o ${max_open}")
+}
+
+# Zoned device with no conventional zones, ZCAP < ZSIZE, limited MaxOpen.
+function section8
+{
+	conv_pcnt=0
+	zone_size=4
+	zone_capacity=3
+	max_open=${set_max_open}
+	zbd_test_opts+=("-o ${max_open}")
+}
+
+# Zoned device with mostly sequential zones, ZCAP == ZSIZE, limited MaxOpen.
+function section10
+{
+	conv_pcnt=10
+	zone_size=1
+	zone_capacity=1
+	max_open=${set_max_open}
+	zbd_test_opts+=("-o ${max_open}")
+}
+
+# Zoned device with mostly sequential zones, ZCAP < ZSIZE, limited MaxOpen.
+function section11
+{
+	conv_pcnt=10
+	zone_size=4
+	zone_capacity=3
+	max_open=${set_max_open}
+	zbd_test_opts+=("-o ${max_open}")
+}
+
+# Zoned device with mostly conventional zones, ZCAP == ZSIZE, limited MaxOpen.
+function section12
+{
+	conv_pcnt=66
+	zone_size=1
+	zone_capacity=1
+	max_open=${set_max_open}
+	zbd_test_opts+=("-o ${max_open}")
+}
+
+# Zoned device with mostly conventional zones, ZCAP < ZSIZE, limited MaxOpen.
+function section13
+{
+	dev_size=2048
+	conv_pcnt=66
+	zone_size=4
+	zone_capacity=3
+	max_open=${set_max_open}
+	zbd_test_opts+=("-o ${max_open}")
+}
+
+#
+# Entry point.
+#
+SECONDS=0
+scriptdir="$(cd "$(dirname "$0")" && pwd)"
+sections=()
+zcap_supported=1
+conv_supported=1
+list_only=0
+dev_size=1024
+dev_blocksize=4096
+set_max_open=8
+zbd_test_opts=()
+libzbc=0
+num_of_runs=1
+
+while (($#)); do
+	case "$1" in
+		-s) sections+=("$2"); shift; shift;;
+		-o) set_max_open="${2}"; shift; shift;;
+		-L) list_only=1; shift;;
+		-r) cleanup_nullb; exit 0;;
+		-l) libzbc=1; shift;;
+		-n) num_of_runs="${2}"; shift; shift;;
+		-h) usage; break;;
+		--) shift; break;;
+		 *) usage; exit 1;;
+	esac
+done
+
+if [ "${#sections[@]}" = 0 ]; then
+	readarray -t sections < <(declare -F | grep "section[0-9]*" |  tr -c -d "[:digit:]\n" | sort -n)
+fi
+
+cleanup_nullb
+
+#
+# Test creating null_blk device and check if newer features are supported
+#
+if ! eval "create_nullb"; then
+	echo "can't create nullb"
+	exit 1
+fi
+if ! cat /sys/kernel/config/nullb/features | grep -q zone_capacity; then
+	zcap_supported=0
+fi
+if ! cat /sys/kernel/config/nullb/features | grep -q zone_nr_conv; then
+	conv_supported=0
+fi
+
+rc=0
+test_rc=0
+intr=0
+run_nr=1
+trap 'kill ${zbd_test_pid}; intr=1' SIGINT
+
+while ((run_nr <= $num_of_runs)); do
+	echo -e "\nRun #$run_nr:"
+	for section_number in "${sections[@]}"; do
+		cleanup_nullb
+		echo "---------- Section $(printf "%02d" $section_number) ----------"
+		if ! eval "create_nullb"; then
+			echo "error creating nullb"
+			exit 1
+		fi
+		zbd_test_opts=()
+		section$section_number
+		configure_nullb
+		rc=$?
+		((rc == 2)) && continue
+		if ((rc)); then
+			echo "can't set up nullb for section $(printf "%02d" $section_number)"
+			exit 1
+		fi
+		show_nullb_config
+		if ((libzbc)); then
+			if ((zone_capacity < zone_size)); then
+				echo "libzbc doesn't support zone capacity, skipping section $(printf "%02d" $section_number)"
+				continue
+			fi
+			if ((conv_pcnt == 100)); then
+				echo "libzbc only supports zoned devices, skipping section $(printf "%02d" $section_number)"
+				continue
+			fi
+			zbd_test_opts+=("-l")
+		fi
+		cd "${scriptdir}"
+		((intr)) && exit 1
+		((list_only)) && continue
+
+		./test-zbd-support ${zbd_test_opts[@]} /dev/nullb0 &
+		zbd_test_pid=$!
+		if kill -0 "${zbd_test_pid}"; then
+			wait "${zbd_test_pid}"
+			test_rc=$?
+		else
+			echo "can't run ZBD tests"
+			exit 1
+		fi
+		((intr)) && exit 1
+		(($test_rc)) && rc=1
+	done
+
+	run_nr=$((run_nr + 1))
+done
+
+if ((!list_only)); then
+	echo "--------------------------------"
+	echo "Total run time: $(TZ=UTC0 printf "%(%H:%M:%S)T\n" $(( SECONDS )) )"
+fi
+
+exit $rc
diff --git a/t/zbd/run-tests-against-regular-nullb b/t/zbd/run-tests-against-regular-nullb
deleted file mode 100755
index 5b7b4009..00000000
--- a/t/zbd/run-tests-against-regular-nullb
+++ /dev/null
@@ -1,27 +0,0 @@
-#!/bin/bash
-#
-# Copyright (C) 2018 Western Digital Corporation or its affiliates.
-#
-# This file is released under the GPL.
-
-scriptdir="$(cd "$(dirname "$0")" && pwd)"
-
-for d in /sys/kernel/config/nullb/*; do [ -d "$d" ] && rmdir "$d"; done
-modprobe -r null_blk
-modprobe null_blk nr_devices=0 || exit $?
-for d in /sys/kernel/config/nullb/*; do
-    [ -d "$d" ] && rmdir "$d"
-done
-modprobe -r null_blk
-[ -e /sys/module/null_blk ] && exit $?
-modprobe null_blk nr_devices=0 &&
-    cd /sys/kernel/config/nullb &&
-    mkdir nullb0 &&
-    cd nullb0 &&
-    echo 0 > completion_nsec &&
-    echo 4096 > blocksize &&
-    echo 1024 > size &&
-    echo 1 > memory_backed &&
-    echo 1 > power || exit $?
-
-"${scriptdir}"/test-zbd-support "$@" /dev/nullb0
diff --git a/t/zbd/run-tests-against-zoned-nullb b/t/zbd/run-tests-against-zoned-nullb
deleted file mode 100755
index f9c9530c..00000000
--- a/t/zbd/run-tests-against-zoned-nullb
+++ /dev/null
@@ -1,53 +0,0 @@
-#!/bin/bash
-#
-# Copyright (C) 2018 Western Digital Corporation or its affiliates.
-#
-# This file is released under the GPL.
-
-scriptdir="$(cd "$(dirname "$0")" && pwd)"
-
-zone_size=1
-zone_capacity=1
-if [[ ${1} == "-h" ]]; then
-    echo "Usage: ${0} [OPTIONS]"
-    echo "Options:"
-    echo -e "\t-h Show this message."
-    echo -e "\t-zone-cap Use null blk with zone capacity less than zone size."
-    echo -e "\tany option supported by test-zbd-support script."
-    exit 1
-elif [[ ${1} == "-zone-cap" ]]; then
-    zone_size=4
-    zone_capacity=3
-    shift
-fi
-
-for d in /sys/kernel/config/nullb/*; do [ -d "$d" ] && rmdir "$d"; done
-modprobe -r null_blk
-modprobe null_blk nr_devices=0 || exit $?
-for d in /sys/kernel/config/nullb/*; do
-    [ -d "$d" ] && rmdir "$d"
-done
-modprobe -r null_blk
-[ -e /sys/module/null_blk ] && exit $?
-modprobe null_blk nr_devices=0 &&
-    cd /sys/kernel/config/nullb &&
-    mkdir nullb0 &&
-    cd nullb0 || exit $?
-
-if ((zone_capacity < zone_size)); then
-    if [[ ! -w zone_capacity ]]; then
-        echo "null blk does not support zone capacity"
-        exit 1
-    fi
-    echo "${zone_capacity}" > zone_capacity
-fi
-
-echo 1 > zoned &&
-    echo "${zone_size}" > zone_size &&
-    echo 0 > completion_nsec &&
-    echo 4096 > blocksize &&
-    echo 1024 > size &&
-    echo 1 > memory_backed &&
-    echo 1 > power || exit $?
-
-"${scriptdir}"/test-zbd-support "$@" /dev/nullb0
-- 
2.28.0

Re: [PATCH v3 24/38] t/zbd: add run-tests-against-nullb script

[Shinichiro Kawasaki]

On Jan 07, 2021 / 06:57, Dmitry Fomichev wrote:
> This script combines the t/zbd/run-tests-against-zoned-nullb script
> functionality with t/zbd/run-tests-against-regular-nullb and adds
> more zoned device configurations to test. This considerably improves
> ZBD test coverage.
> 
> The added script makes the two old scripts named above obsolete,
> remove them. Modify t/run-fio-tests.py and Makefile to refer to the
> new script instead of the old one. Since the full test now runs
> significantly longer than the two old ones combined due to many more
> zoned configurations, only execute a few individual sections as a
> part of testing n "make fulltest" and run-fio-tests.py. One extra test
> section with 10% conventional zones is executed from the Makefile.
> The Python tests only exercise all-conventional and all-sequential
> configurations, exactly as before.
> 
> The script returns a non-zero return code if at least one of the
> executed sections had a failed test.
> 
> Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
> ---
>  Makefile                              |   5 +-
>  t/run-fio-tests.py                    |   8 +-
>  t/zbd/run-tests-against-nullb         | 338 ++++++++++++++++++++++++++
>  t/zbd/run-tests-against-regular-nullb |  27 --
>  t/zbd/run-tests-against-zoned-nullb   |  53 ----
>  5 files changed, 345 insertions(+), 86 deletions(-)
>  create mode 100755 t/zbd/run-tests-against-nullb
>  delete mode 100755 t/zbd/run-tests-against-regular-nullb
>  delete mode 100755 t/zbd/run-tests-against-zoned-nullb
> 
> diff --git a/Makefile b/Makefile
> index a838af9a..19bfb996 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -600,9 +600,10 @@ fulltest:
>  	   make -j &&						 	\
>  	   sudo make install)						\
>  	fi &&					 			\
> -	sudo t/zbd/run-tests-against-regular-nullb &&		 	\
> +	sudo t/zbd/run-tests-against-nullb -s 1 &&		 	\
>  	if [ -e /sys/module/null_blk/parameters/zoned ]; then		\
> -		sudo t/zbd/run-tests-against-zoned-nullb;	 	\
> +		sudo t/zbd/run-tests-against-nullb -s 2;	 	\
> +		sudo t/zbd/run-tests-against-nullb -s 4;	 	\
>  	fi
>  
>  install: $(PROGS) $(SCRIPTS) $(ENGS_OBJS) tools/plot/fio2gnuplot.1 FORCE
> diff --git a/t/run-fio-tests.py b/t/run-fio-tests.py
> index e5c2f17c..a59cdfe0 100755
> --- a/t/run-fio-tests.py
> +++ b/t/run-fio-tests.py
> @@ -879,8 +879,8 @@ TEST_LIST = [
>      {
>          'test_id':          1007,
>          'test_class':       FioExeTest,
> -        'exe':              't/zbd/run-tests-against-regular-nullb',
> -        'parameters':       None,
> +        'exe':              't/zbd/run-tests-against-nullb',
> +        'parameters':       ['-s', '1'],
>          'success':          SUCCESS_DEFAULT,
>          'requirements':     [Requirements.linux, Requirements.zbd,
>                               Requirements.root],
> @@ -888,8 +888,8 @@ TEST_LIST = [
>      {
>          'test_id':          1008,
>          'test_class':       FioExeTest,
> -        'exe':              't/zbd/run-tests-against-zoned-nullb',
> -        'parameters':       None,
> +        'exe':              't/zbd/run-tests-against-nullb',
> +        'parameters':       ['-s', '2'],
>          'success':          SUCCESS_DEFAULT,
>          'requirements':     [Requirements.linux, Requirements.zbd,
>                               Requirements.root, Requirements.zoned_nullb],
> diff --git a/t/zbd/run-tests-against-nullb b/t/zbd/run-tests-against-nullb
> new file mode 100755
> index 00000000..8d1eb005
> --- /dev/null
> +++ b/t/zbd/run-tests-against-nullb
> @@ -0,0 +1,338 @@
> +#!/bin/bash
> +#
> +# Copyright (C) 2020 Western Digital Corporation or its affiliates.
> +#
> +# This file is released under the GPL.
> +#
> +# Run t/zbd/test-zbd-support script against a variety of conventional,
> +# zoned and mixed zone configurations.
> +#
> +
> +function usage()

I would think bash function form "XXX()" is rather newer and better than
"function XXX()", for POSIX conformance and commonality with other scripts
in fio bash scripts.

> +{
> +	echo "This script runs the tests from t/zbd/test-zbd-support script"
> +        echo "against a nullb device in a variety of conventional and zoned"

A nit, spaces are used in place of a tab.

> +	echo "configurations."
> +	echo "Usage: ${0} [OPTIONS]"
> +	echo "Options:"
> +	echo -e "\t-h Show this message."
> +	echo -e "\t-L List the device layouts for every section without running"
> +	echo -e "\t   tests."
> +	echo -e "\t-s <#section> Only run the section with the given number."
> +	echo -e "\t-l Use libzbc ioengine to run the tests."
> +	echo -e "\t-o <max_open_zones> Specify MaxOpen value, (${set_max_open} by default)."
> +	echo -e "\t-n <#number of runs> Set the number of times to run the entire suite "
> +	echo -e "\t   or an individual section/test."
> +	echo -e "\t-r Remove the /dev/nullb0 device that may still exist after"
> +	echo -e "\t   running this script."
> +	exit 1
> +}
> +
> +function cleanup_nullb()
> +{
> +	for d in /sys/kernel/config/nullb/*; do [ -d "$d" ] && rmdir "$d"; done
> +	modprobe -r null_blk
> +	modprobe null_blk nr_devices=0 || exit $?
> +	for d in /sys/kernel/config/nullb/*; do
> +		[ -d "$d" ] && rmdir "$d"
> +	done
> +	modprobe -r null_blk
> +	[ -e /sys/module/null_blk ] && exit $?
> +}
> +
> +function create_nullb()
> +{
> +	modprobe null_blk nr_devices=0 &&
> +	cd /sys/kernel/config/nullb &&
> +	mkdir nullb0 &&
> +	cd nullb0 || return $?
> +}
> +
> +function configure_nullb()
> +{
> +	echo 0 > completion_nsec &&
> +		echo ${dev_blocksize} > blocksize &&
> +		echo ${dev_size} > size &&
> +		echo 1 > memory_backed || return $?
> +
> +	if ((conv_pcnt < 100)); then
> +		echo 1 > zoned &&
> +			echo "${zone_size}" > zone_size || return $?
> +
> +		if ((zone_capacity < zone_size)); then
> +			if ((!zcap_supported)); then
> +				echo "null_blk does not support zone capacity"
> +				return 2
> +			fi
> +			echo "${zone_capacity}" > zone_capacity
> +		fi
> +		if ((conv_pcnt)); then
> +			if ((!conv_supported)); then
> +				echo "null_blk does not support conventional zones"
> +				return 2
> +			fi
> +			nr_conv=$((dev_size/zone_size*conv_pcnt/100))
> +			echo "${nr_conv}" > zone_nr_conv
> +		fi
> +	fi
> +
> +	echo 1 > power || return $?
> +	return 0
> +}
> +
> +function show_nullb_config()
> +{
> +	if ((conv_pcnt < 100)); then
> +		echo "    $(printf "Zoned Device, %d%% Conventional Zones (%d)" \
> +			  ${conv_pcnt} ${nr_conv})"
> +		echo "    $(printf "Zone Size: %d MB" ${zone_size})"
> +		echo "    $(printf "Zone Capacity: %d MB" ${zone_capacity})"
> +		if ((max_open)); then
> +			echo "    $(printf "Max Open: %d Zones" ${max_open})"
> +		else
> +			echo "    Max Open: Unlimited Zones"
> +		fi
> +	else
> +		echo "    Non-zoned Device"
> +	fi
> +}
> +
> +#
> +# Test sections.
> +#
> +# Fully conventional device.
> +function section1

It is not mandatory, but I would like to see parentheses () for the
sectionX function definitions.

> +{
> +	conv_pcnt=100
> +	max_open=0
> +}
> +
> +# Zoned device with no conventional zones, ZCAP == ZSIZE, unlimited MaxOpen.
> +function section2
> +{
> +	conv_pcnt=0
> +	zone_size=1
> +	zone_capacity=1
> +	max_open=0
> +}
> +
> +# Zoned device with no conventional zones, ZCAP < ZSIZE, unlimited MaxOpen.
> +function section3
> +{
> +	conv_pcnt=0
> +	zone_size=4
> +	zone_capacity=3
> +	max_open=0
> +}
> +
> +# Zoned device with mostly sequential zones, ZCAP == ZSIZE, unlimited MaxOpen.
> +function section4
> +{
> +	conv_pcnt=10
> +	zone_size=1
> +	zone_capacity=1
> +	max_open=0
> +}
> +
> +# Zoned device with mostly sequential zones, ZCAP < ZSIZE, unlimited MaxOpen.
> +function section5
> +{
> +	conv_pcnt=10
> +	zone_size=4
> +	zone_capacity=3
> +	max_open=0
> +}
> +
> +# Zoned device with mostly conventional zones, ZCAP == ZSIZE, unlimited MaxOpen.
> +function section6
> +{
> +	conv_pcnt=66
> +	zone_size=1
> +	zone_capacity=1
> +	max_open=0
> +}
> +
> +# Zoned device with mostly conventional zones, ZCAP < ZSIZE, unlimited MaxOpen.
> +function section7
> +{
> +	dev_size=2048
> +	conv_pcnt=66
> +	zone_size=4
> +	zone_capacity=3
> +	max_open=0
> +}
> +
> +# Zoned device with no conventional zones, ZCAP == ZSIZE, limited MaxOpen.
> +function section8
> +{
> +	dev_size=1024
> +	conv_pcnt=0
> +	zone_size=1
> +	zone_capacity=1
> +	max_open=${set_max_open}
> +	zbd_test_opts+=("-o ${max_open}")
> +}
> +
> +# Zoned device with no conventional zones, ZCAP < ZSIZE, limited MaxOpen.
> +function section8

I think you meant section9, not 8.

Other than this, this patch looks good to me.

-- 
Best Regards,
Shin'ichiro Kawasaki

[PATCH v3 25/38] t/zbd: add -t option to run-tests-against-nullb

[Dmitry Fomichev]

From: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>

For debugging, it can be useful to run a single ZBD test case in all
zoned configurations defined in run-tests-against-nullb. Add -t option
to run-tests-against-nullb so that the single ZBD test case specified
in run-tests-against-nullb command line is executed in all sections
of the script.

Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
 t/zbd/run-tests-against-nullb | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/t/zbd/run-tests-against-nullb b/t/zbd/run-tests-against-nullb
index 8d1eb005..655025ed 100755
--- a/t/zbd/run-tests-against-nullb
+++ b/t/zbd/run-tests-against-nullb
@@ -20,6 +20,7 @@ function usage()
 	echo -e "\t   tests."
 	echo -e "\t-s <#section> Only run the section with the given number."
 	echo -e "\t-l Use libzbc ioengine to run the tests."
+	echo -e "\t-t <#test> Only run the test with the given number in every section."
 	echo -e "\t-o <max_open_zones> Specify MaxOpen value, (${set_max_open} by default)."
 	echo -e "\t-n <#number of runs> Set the number of times to run the entire suite "
 	echo -e "\t   or an individual section/test."
@@ -239,6 +240,7 @@ set_max_open=8
 zbd_test_opts=()
 libzbc=0
 num_of_runs=1
+test_case=0
 
 while (($#)); do
 	case "$1" in
@@ -248,6 +250,7 @@ while (($#)); do
 		-r) cleanup_nullb; exit 0;;
 		-l) libzbc=1; shift;;
 		-n) num_of_runs="${2}"; shift; shift;;
+		-t) test_case="${2}"; shift; shift;;
 		-h) usage; break;;
 		--) shift; break;;
 		 *) usage; exit 1;;
@@ -290,6 +293,9 @@ while ((run_nr <= $num_of_runs)); do
 			exit 1
 		fi
 		zbd_test_opts=()
+		if ((test_case)); then
+			zbd_test_opts+=("-t" "${test_case}")
+		fi
 		section$section_number
 		configure_nullb
 		rc=$?
-- 
2.28.0

[PATCH v3 26/38] t/zbd: skip tests when test prerequisites are not met

[Dmitry Fomichev]

From: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>

Some of the test cases in t/zbd/test-zbd-support require test target
devices to have certain features. When these prerequisites are not met,
they skip the actual test and report the test result to be "PASS".
This does not help users to understand the true test outcome.
As the tests expand to cover a wider variety of zoned devices and
layouts, reporting skipped tests becomes more and more beneficial.

Modify test-zbd-support script to report skipped test cases.
Introduce helper functions require_*() to check test target
prerequisites. If they are not met, set the variable SKIP_REASON and
return the constant SKIP_TESTCASE from the test function. In the main
loo, print "SKIP" status and SKIP_REASON if the test case is skipped.
Also, output the total number of skipped cases at the end of the test
script  run.

Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
 t/zbd/test-zbd-support | 93 ++++++++++++++++++++++++++++++------------
 1 file changed, 67 insertions(+), 26 deletions(-)

diff --git a/t/zbd/test-zbd-support b/t/zbd/test-zbd-support
index 652dddfc..d94b5125 100755
--- a/t/zbd/test-zbd-support
+++ b/t/zbd/test-zbd-support
@@ -190,6 +190,42 @@ prep_write() {
 		reset_zone "${dev}" -1
 }
 
+SKIP_TESTCASE=255
+
+require_scsi_dev() {
+	if ! is_scsi_device "$dev"; then
+		SKIP_REASON="$dev is not a SCSI device"
+		return 1
+	fi
+	return 0
+}
+
+require_conv_zone_bytes() {
+	local req_bytes=${1}
+
+	if ((req_bytes > first_sequential_zone_sector * 512)); then
+		SKIP_REASON="$dev does not have enough conventional zones"
+		return 1
+	fi
+	return 0
+}
+
+require_zbd() {
+	if [[ -z ${is_zbd} ]]; then
+		SKIP_REASON="$dev is not a zoned block device"
+		return 1
+	fi
+	return 0
+}
+
+require_regular_block_dev() {
+	if [[ -n ${is_zbd} ]]; then
+		SKIP_REASON="$dev is not a regular block device"
+		return 1
+	fi
+	return 0
+}
+
 # Check whether buffered writes are refused.
 test1() {
     run_fio --name=job1 --filename="$dev" --rw=write --direct=0 --bs=4K	\
@@ -327,10 +363,7 @@ test8() {
 test9() {
     local size
 
-    if ! is_scsi_device "$dev"; then
-	echo "$dev is not a SCSI device" >>"${logfile}.${test_number}"
-	return 0
-    fi
+    require_scsi_dev || return $SKIP_TESTCASE
 
     prep_write
     size=$((4 * zone_size))
@@ -346,10 +379,7 @@ test9() {
 test10() {
     local size
 
-    if ! is_scsi_device "$dev"; then
-	echo "$dev is not a SCSI device" >>"${logfile}.${test_number}"
-	return 0
-    fi
+    require_scsi_dev || return $SKIP_TESTCASE
 
     prep_write
     size=$((4 * zone_size))
@@ -413,11 +443,8 @@ test14() {
 
     prep_write
     size=$((16 * 2**20)) # 20 MB
-    if [ $size -gt $((first_sequential_zone_sector * 512)) ]; then
-	echo "$dev does not have enough sequential zones" \
-	     >>"${logfile}.${test_number}"
-	return 0
-    fi
+    require_conv_zone_bytes "${size}" || return $SKIP_TESTCASE
+
     run_one_fio_job "$(ioengine "libaio")" --iodepth=64 --rw=randwrite --bs=16K \
 		    --zonemode=zbd --zonesize="${zone_size}" --do_verify=1 \
 		    --verify=md5 --size=$size				   \
@@ -696,6 +723,8 @@ test31() {
 test32() {
     local off opts=() size
 
+    require_zbd || return $SKIP_TESTCASE
+
     prep_write
     off=$((first_sequential_zone_sector * 512))
     size=$((disk_size - off))
@@ -814,7 +843,7 @@ read_one_block() {
 
 # Check whether fio accepts --zonemode=none for zoned block devices.
 test39() {
-    [ -n "$is_zbd" ] || return 0
+    require_zbd || return $SKIP_TESTCASE
     read_one_block --zonemode=none >/dev/null || return $?
     check_read $((logical_block_size)) || return $?
 }
@@ -824,7 +853,7 @@ test40() {
     local bs
 
     bs=$((logical_block_size))
-    [ -n "$is_zbd" ] || return 0
+    require_zbd || return $SKIP_TESTCASE
     read_one_block --zonemode=strided |
 	grep -q 'fio: --zonesize must be specified when using --zonemode=strided' ||
 	return $?
@@ -834,21 +863,21 @@ test40() {
 
 # Check whether fio checks the zone size for zoned block devices.
 test41() {
-    [ -n "$is_zbd" ] || return 0
+    require_zbd || return $SKIP_TESTCASE
     read_one_block --zonemode=zbd --zonesize=$((2 * zone_size)) |
 	grep -q 'job parameter zonesize.*does not match disk zone size'
 }
 
 # Check whether fio handles --zonesize=0 correctly for regular block devices.
 test42() {
-    [ -n "$is_zbd" ] && return 0
+    require_regular_block_dev || return $SKIP_TESTCASE
     read_one_block --zonemode=zbd --zonesize=0 |
 	grep -q 'Specifying the zone size is mandatory for regular block devices with --zonemode=zbd'
 }
 
 # Check whether fio handles --zonesize=1 correctly for regular block devices.
 test43() {
-    [ -n "$is_zbd" ] && return 0
+    require_regular_block_dev || return $SKIP_TESTCASE
     read_one_block --zonemode=zbd --zonesize=1 |
 	grep -q 'zone size must be at least 512 bytes for --zonemode=zbd'
 }
@@ -862,7 +891,7 @@ test44() {
 test45() {
     local bs i
 
-    [ -z "$is_zbd" ] && return 0
+    require_zbd || return $SKIP_TESTCASE
     prep_write
     bs=$((logical_block_size))
     run_one_fio_job "$(ioengine "psync")" --iodepth=1 --rw=randwrite --bs=$bs\
@@ -901,6 +930,8 @@ test47() {
 test48() {
     local i jobs=16 off opts=()
 
+    require_zbd || return $SKIP_TESTCASE
+
     off=$((first_sequential_zone_sector * 512 + 64 * zone_size))
     size=$((16*zone_size))
     prep_write
@@ -930,11 +961,7 @@ test48() {
 # Check if fio handles --zonecapacity on a normal block device correctly
 test49() {
 
-    if [ -n "$is_zbd" ]; then
-	echo "$dev is not a regular block device" \
-	     >>"${logfile}.${test_number}"
-	return 0
-    fi
+    require_regular_block_dev || return $SKIP_TESTCASE
 
     size=$((2 * zone_size))
     capacity=$((zone_size * 3 / 4))
@@ -1087,10 +1114,12 @@ fi
 logfile=$0.log
 
 passed=0
+skipped=0
 failed=0
 if [ -t 1 ]; then
     red="\e[1;31m"
     green="\e[1;32m"
+    cyan="\e[1;36m"
     end="\e[m"
 else
     red=""
@@ -1101,14 +1130,23 @@ rc=0
 
 intr=0
 trap 'intr=1' SIGINT
+ret=0
 
 for test_number in "${tests[@]}"; do
     rm -f "${logfile}.${test_number}"
+    unset SKIP_REASON
     echo -n "Running test $(printf "%02d" $test_number) ... "
-    if eval "test$test_number" && check_log $test_number; then
+    eval "test$test_number"
+    ret=$?
+    if ((!ret)) && check_log $test_number; then
 	status="PASS"
 	cc_status="${green}${status}${end}"
 	((passed++))
+    elif ((ret==SKIP_TESTCASE)); then
+	status="SKIP"
+	echo "${SKIP_REASON}" >> "${logfile}.${test_number}"
+	cc_status="${cyan}${status}${end}    ${SKIP_REASON}"
+	((skipped++))
     else
 	status="FAIL"
 	cc_status="${red}${status}${end}"
@@ -1121,7 +1159,10 @@ for test_number in "${tests[@]}"; do
 done
 
 echo "$passed tests passed"
+if [ $skipped -gt 0 ]; then
+    echo " $skipped tests skipped"
+fi
 if [ $failed -gt 0 ]; then
-    echo " and $failed tests failed"
+    echo " $failed tests failed"
 fi
 exit $rc
-- 
2.28.0

[PATCH v3 27/38] t/zbd: skip tests that need too many sequential zones

[Dmitry Fomichev]

From: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>

Test cases #3, #4, #28, #29 and #48 require rather large numbers of
sequential zones to run properly and they fail if the test target
device has not enough of such zones in its zone configuration.

Check how many sequential zones are present on the test device and
skip any test cases for which this number is not enough.

Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
 t/zbd/test-zbd-support | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/t/zbd/test-zbd-support b/t/zbd/test-zbd-support
index d94b5125..fa6a279b 100755
--- a/t/zbd/test-zbd-support
+++ b/t/zbd/test-zbd-support
@@ -226,6 +226,17 @@ require_regular_block_dev() {
 	return 0
 }
 
+require_seq_zones() {
+	local req_seq_zones=${1}
+	local seq_bytes=$((disk_size - first_sequential_zone_sector * 512))
+
+	if ((req_seq_zones > seq_bytes / zone_size)); then
+		SKIP_REASON="$dev does not have $req_seq_zones sequential zones"
+		return 1
+	fi
+	return 0
+}
+
 # Check whether buffered writes are refused.
 test1() {
     run_fio --name=job1 --filename="$dev" --rw=write --direct=0 --bs=4K	\
@@ -265,6 +276,7 @@ test2() {
 test3() {
     local off opts=() rc
 
+    require_seq_zones 129 || return $SKIP_TESTCASE
     off=$((first_sequential_zone_sector * 512 + 128 * zone_size))
     size=$((zone_size))
     [ -n "$is_zbd" ] && reset_zone "$dev" $((off / 512))
@@ -282,6 +294,7 @@ test3() {
 test4() {
     local off opts=()
 
+    require_seq_zones 130 || return $SKIP_TESTCASE
     off=$((first_sequential_zone_sector * 512 + 129 * zone_size))
     size=$((zone_size))
     [ -n "$is_zbd" ] && reset_zone "$dev" $((off / 512))
@@ -631,6 +644,7 @@ test27() {
 test28() {
     local i jobs=16 off opts
 
+    require_seq_zones 65 || return $SKIP_TESTCASE
     off=$((first_sequential_zone_sector * 512 + 64 * zone_size))
     [ -n "$is_zbd" ] && reset_zone "$dev" $((off / 512))
     prep_write
@@ -655,6 +669,7 @@ test28() {
 test29() {
     local i jobs=16 off opts=()
 
+    require_seq_zones 80 || return $SKIP_TESTCASE
     off=$((first_sequential_zone_sector * 512 + 64 * zone_size))
     size=$((16*zone_size))
     prep_write
@@ -931,6 +946,7 @@ test48() {
     local i jobs=16 off opts=()
 
     require_zbd || return $SKIP_TESTCASE
+    require_seq_zones 80 || return $SKIP_TESTCASE
 
     off=$((first_sequential_zone_sector * 512 + 64 * zone_size))
     size=$((16*zone_size))
-- 
2.28.0

[PATCH v3 28/38] t/zbd: test that conventional zones are not locked during random i/o

[Dmitry Fomichev]

From: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>

A recently fixed bug was caused by an unexpected conventional zone lock
during random I/O adjustment. Only sequential zones are supposed to be
locked, but the conventional zone lock was observed with a random
workload against an I/O region with mixed conventional and sequential
zones.

Add two test cases with the same workload to ensure that no similar
regression happens in the future. One case tests reads and the other
is for writes. As a related change, add the helper function
require_conv_zones() to check that the test target device has enough
conventional zones available.

Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
 t/zbd/test-zbd-support | 56 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/t/zbd/test-zbd-support b/t/zbd/test-zbd-support
index fa6a279b..4ad55381 100755
--- a/t/zbd/test-zbd-support
+++ b/t/zbd/test-zbd-support
@@ -237,6 +237,17 @@ require_seq_zones() {
 	return 0
 }
 
+require_conv_zones() {
+	local req_c_zones=${1}
+	local conv_bytes=$((first_sequential_zone_sector * 512))
+
+	if ((req_c_zones > conv_bytes / zone_size)); then
+		SKIP_REASON="$dev does not have $req_c_zones conventional zones"
+		return 1
+	fi
+	return 0
+}
+
 # Check whether buffered writes are refused.
 test1() {
     run_fio --name=job1 --filename="$dev" --rw=write --direct=0 --bs=4K	\
@@ -991,6 +1002,51 @@ test49() {
     check_read $((capacity * 2)) || return $?
 }
 
+# Verify that conv zones are not locked and only seq zones are locked during
+# random read on conv-seq mixed zones.
+test50() {
+	local off
+
+	require_zbd || return $SKIP_TESTCASE
+	require_conv_zones 8 || return $SKIP_TESTCASE
+	require_seq_zones 8 || return $SKIP_TESTCASE
+
+	reset_zone "${dev}" -1
+
+	off=$((first_sequential_zone_sector * 512 - 8 * zone_size))
+	run_fio --name=job --filename=${dev} --offset=${off} --bs=64K \
+		--size=$((16 * zone_size)) "$(ioengine "libaio")" --rw=randread\
+		--time_based --runtime=3 --zonemode=zbd --zonesize=${zone_size}\
+		--direct=1 --group_reporting=1 ${job_var_opts[@]} \
+		>> "${logfile}.${test_number}" 2>&1 || return $?
+}
+
+# Verify that conv zones are neither locked nor opened during random write on
+# conv-seq mixed zones. Zone lock and zone open shall happen only on seq zones.
+test51() {
+	local off jobs=16
+	local -a opts
+
+	require_zbd || return $SKIP_TESTCASE
+	require_conv_zones 8 || return $SKIP_TESTCASE
+	require_seq_zones 8 || return $SKIP_TESTCASE
+
+	prep_write
+
+	off=$((first_sequential_zone_sector * 512 - 8 * zone_size))
+	opts+=("--size=$((16 * zone_size))" "$(ioengine "libaio")")
+	opts+=("--zonemode=zbd" "--direct=1" "--zonesize=${zone_size}")
+	opts+=("--max_open_zones=2" "--offset=$off")
+	opts+=("--thread=1" "--group_reporting=1")
+	opts+=("--time_based" "--runtime=30" "--rw=randwrite")
+	for ((i=0;i<jobs;i++)); do
+		opts+=("--name=job${i}" "--filename=$dev")
+		opts+=("--bs=$(((i+1)*16))K")
+		opts+=($(job_var_opts_exclude "--max_open_zones"))
+	done
+	run_fio "${opts[@]}" >> "${logfile}.${test_number}" 2>&1 || return $?
+}
+
 tests=()
 dynamic_analyzer=()
 reset_all_zones=
-- 
2.28.0

[PATCH v3 29/38] t/zbd: test that zone_reset_threshold calculation is correct

[Dmitry Fomichev]

From: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>

The option "zone_reset_threshold" specifies the ratio of logical blocks
with data to trigger zone resets. When the I/O range includes
conventional zones, only blocks in sequential zones must be used to
track this value. A recently fixed bug has uncovered that the number of
blocks in conventional zones were erroneously counted as the blocks
with data.

To prevent future regressions, add a test case to confirm that the
logical blocks accounting does not include conventional zones.

Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
 t/zbd/test-zbd-support | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/t/zbd/test-zbd-support b/t/zbd/test-zbd-support
index 4ad55381..217fdd10 100755
--- a/t/zbd/test-zbd-support
+++ b/t/zbd/test-zbd-support
@@ -1047,6 +1047,38 @@ test51() {
 	run_fio "${opts[@]}" >> "${logfile}.${test_number}" 2>&1 || return $?
 }
 
+# Verify that zone_reset_threshold only takes logical blocks from seq
+# zones into account, and logical blocks of conv zones are not counted.
+test52() {
+	local off io_size
+
+	require_zbd || return $SKIP_TESTCASE
+	require_conv_zones 8 || return $SKIP_TESTCASE
+	require_seq_zones 8 || return $SKIP_TESTCASE
+
+	reset_zone "${dev}" -1
+
+	# Total I/O size is 1/8 = 0.125 of the I/O range of cont + seq zones.
+	# Set zone_reset_threshold as 0.1. The threshold size is less than
+	# 0.125, then, reset count zero is expected.
+	# On the other hand, half of the I/O range is covered by conv zones.
+	# If fio would count the conv zones for zone_reset_threshold, the ratio
+	# were more than 0.5 and would trigger zone resets.
+
+	off=$((first_sequential_zone_sector * 512 - 8 * zone_size))
+	io_size=$((zone_size * 16 / 8))
+	run_fio --name=job --filename=$dev --rw=randwrite --bs=$((zone_size/16))\
+		--size=$((zone_size * 16)) --softrandommap=1 \
+		--io_size=$((io_size)) "$(ioengine "psync")" --offset=$off \
+		--zonemode=zbd --direct=1 --zonesize=${zone_size} \
+		--zone_reset_threshold=.1 --zone_reset_frequency=1.0 \
+		${job_var_opts[@]} --debug=zbd \
+		>> "${logfile}.${test_number}" 2>&1 || return $?
+
+	check_written ${io_size} || return $?
+	check_reset_count -eq 0 || return $?
+}
+
 tests=()
 dynamic_analyzer=()
 reset_all_zones=
-- 
2.28.0

[PATCH v3 30/38] t/zbd: test random I/O direction in all-conventional case

[Dmitry Fomichev]

From: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>

The number of 'sectors with data' is counted and used to determine the
direction of the first I/O of a random read/write ZBD workload. To
initialize the number, min_zone and max_zone fields in struct fio_file
are referred. There was a code bug that was recently fixed where
min_zone and max_zone fields were not initialized when all zones in I/O
region were conventional zones. This led to an uninitialized number of
sectors with data, and the write direction was always set for random
workloads.

Add a test case to perform random read/write workload on an I/O region
with both sequential and conventional zones. Check that both read and
write I/Os are executed.

Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
 t/zbd/test-zbd-support | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/t/zbd/test-zbd-support b/t/zbd/test-zbd-support
index 217fdd10..80763561 100755
--- a/t/zbd/test-zbd-support
+++ b/t/zbd/test-zbd-support
@@ -1079,6 +1079,34 @@ test52() {
 	check_reset_count -eq 0 || return $?
 }
 
+# Check both reads and writes are executed by random I/O to conventional zones.
+test53() {
+	local off capacity io read_b=0 written_b=0
+
+	require_zbd || return $SKIP_TESTCASE
+	require_conv_zones 4 || return $SKIP_TESTCASE
+
+	off=$((first_sequential_zone_sector * 512 - 4 * zone_size))
+	capacity=$(total_zone_capacity 4 $off $dev)
+	run_fio --name=job --filename=${dev} --rw=randrw --bs=64K \
+		--size=$((4 * zone_size)) "$(ioengine "psync")" --offset=${off}\
+		--zonemode=zbd --direct=1 --zonesize=${zone_size} \
+		${job_var_opts[@]} \
+		>> "${logfile}.${test_number}" 2>&1 || return $?
+
+	written_b=$(fio_written <"${logfile}.${test_number}")
+	read_b=$(fio_read <"${logfile}.${test_number}")
+	io=$((written_b + read_b))
+	echo "Number of bytes read: $read_b" >>"${logfile}.${test_number}"
+	echo "Number of bytes written: $written_b" >>"${logfile}.${test_number}"
+	echo "Total number of bytes read and written: $io <> $capacity" \
+	     >>"${logfile}.${test_number}"
+	if ((io==capacity && written_b != 0 && read_b != 0)); then
+		return 0
+	fi
+	return 1
+}
+
 tests=()
 dynamic_analyzer=()
 reset_all_zones=
-- 
2.28.0

[PATCH v3 31/38] t/zbd: fix wrong units in test case #37

[Dmitry Fomichev]

From: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>

The second argument of the function total_zone_capacity is expected to
be in bytes. However, the call in test case #37 provides this argument
in sectors and this results in a wrong capacity calculation. Make sure
that the value that is passed to this function is converted to bytes.

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

diff --git a/t/zbd/test-zbd-support b/t/zbd/test-zbd-support
index 80763561..4d8e905d 100755
--- a/t/zbd/test-zbd-support
+++ b/t/zbd/test-zbd-support
@@ -828,7 +828,7 @@ test37() {
     local bs off size capacity
 
     prep_write
-    capacity=$(total_zone_capacity 1 $first_sequential_zone_sector $dev)
+    capacity=$(total_zone_capacity 1 $((first_sequential_zone_sector*512)) $dev)
     if [ "$first_sequential_zone_sector" = 0 ]; then
 	off=0
     else
-- 
2.28.0

[PATCH v3 32/38] t/zbd: add an option to bail on a failed test

[Dmitry Fomichev]

Sometimes, it can be useful to inspect the state of the zones of the
test device, usually right after a test failure. Currently,
test-zbd-support script just keeps running and proper examination of
device zones can be difficult.

Add the -q option to test/zbd/support to quit immediately upon
encountering any test failure. Additionally, define the same option
in run-tests-against-nullb to propagate it to test/zbd/support.

Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
 t/zbd/run-tests-against-nullb | 12 +++++++++++-
 t/zbd/test-zbd-support        |  4 ++++
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/t/zbd/run-tests-against-nullb b/t/zbd/run-tests-against-nullb
index 655025ed..d6d5a814 100755
--- a/t/zbd/run-tests-against-nullb
+++ b/t/zbd/run-tests-against-nullb
@@ -24,6 +24,7 @@ function usage()
 	echo -e "\t-o <max_open_zones> Specify MaxOpen value, (${set_max_open} by default)."
 	echo -e "\t-n <#number of runs> Set the number of times to run the entire suite "
 	echo -e "\t   or an individual section/test."
+	echo -e "\t-q Quit t/zbd/test-zbd-support run after any failed test."
 	echo -e "\t-r Remove the /dev/nullb0 device that may still exist after"
 	echo -e "\t   running this script."
 	exit 1
@@ -241,6 +242,7 @@ zbd_test_opts=()
 libzbc=0
 num_of_runs=1
 test_case=0
+quit_on_err=0
 
 while (($#)); do
 	case "$1" in
@@ -251,6 +253,7 @@ while (($#)); do
 		-l) libzbc=1; shift;;
 		-n) num_of_runs="${2}"; shift; shift;;
 		-t) test_case="${2}"; shift; shift;;
+		-q) quit_on_err=1; shift;;
 		-h) usage; break;;
 		--) shift; break;;
 		 *) usage; exit 1;;
@@ -296,6 +299,9 @@ while ((run_nr <= $num_of_runs)); do
 		if ((test_case)); then
 			zbd_test_opts+=("-t" "${test_case}")
 		fi
+		if ((quit_on_err)); then
+			zbd_test_opts+=("-q")
+		fi
 		section$section_number
 		configure_nullb
 		rc=$?
@@ -330,9 +336,13 @@ while ((run_nr <= $num_of_runs)); do
 			exit 1
 		fi
 		((intr)) && exit 1
-		(($test_rc)) && rc=1
+		if (($test_rc)); then
+			rc=1
+			((quit_on_err)) && break
+		fi
 	done
 
+	((rc && quit_on_err)) && break
 	run_nr=$((run_nr + 1))
 done
 
diff --git a/t/zbd/test-zbd-support b/t/zbd/test-zbd-support
index 4d8e905d..93456ec6 100755
--- a/t/zbd/test-zbd-support
+++ b/t/zbd/test-zbd-support
@@ -14,6 +14,7 @@ usage() {
 	echo -e "\t-r Reset all zones before test start"
 	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"
 	echo -e "\t-z Run fio with debug=zbd option"
 }
 
@@ -1113,6 +1114,7 @@ reset_all_zones=
 use_libzbc=
 zbd_debug=
 max_open_zones_opt=
+quit_on_err=
 
 while [ "${1#-}" != "$1" ]; do
   case "$1" in
@@ -1127,6 +1129,7 @@ while [ "${1#-}" != "$1" ]; do
     -o) max_open_zones_opt="${2}"; shift; shift;;
     -v) dynamic_analyzer=(valgrind "--read-var-info=yes");
 	shift;;
+    -q) quit_on_err=1; shift;;
     -z) zbd_debug=1; shift;;
     --) shift; break;;
   esac
@@ -1288,6 +1291,7 @@ for test_number in "${tests[@]}"; do
     echo -e "$cc_status"
     echo "$status" >> "${logfile}.${test_number}"
     [ $intr -ne 0 ] && exit 1
+    [ -n "$quit_on_err" -a "$rc" -ne 0 ] && exit 1
 done
 
 echo "$passed tests passed"
-- 
2.28.0

Re: [PATCH v3 32/38] t/zbd: add an option to bail on a failed test

[Shinichiro Kawasaki]

On Jan 07, 2021 / 06:57, Dmitry Fomichev wrote:
> Sometimes, it can be useful to inspect the state of the zones of the
> test device, usually right after a test failure. Currently,
> test-zbd-support script just keeps running and proper examination of
> device zones can be difficult.
> 
> Add the -q option to test/zbd/support to quit immediately upon
> encountering any test failure. Additionally, define the same option
> in run-tests-against-nullb to propagate it to test/zbd/support.
> 
> Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>

Looks good to me.

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

-- 
Best Regards,
Shin'ichiro Kawasaki

[PATCH v3 33/38] t/zbd: prevent test #31 from looping

[Dmitry Fomichev]

The test 31 starts i/o to 128 zones in parallel.
There are two corner cases that are not properly handled in the
existing implementation -
1) If the total number of zones on the device is < 128, the test
will loop indefinitely because the loop increment is calculated as
zero by the script.
2) If the number of max_open_zones of the device is < 128, the
test will fail due to exceeding max_open_zones limit as the code
expects it to be >= 128.

Limit the number of open zones to the reported maximum
and skip the test if there is not enough zones on the device.

Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
 t/zbd/test-zbd-support | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/t/zbd/test-zbd-support b/t/zbd/test-zbd-support
index 93456ec6..033c2ebc 100755
--- a/t/zbd/test-zbd-support
+++ b/t/zbd/test-zbd-support
@@ -718,12 +718,18 @@ test31() {
     local bs inc nz off opts size
 
     prep_write
-    # Start with writing 128 KB to 128 sequential zones.
+    # Start with writing 128 KB to max_open_zones sequential zones.
     bs=128K
-    nz=128
+    nz=$((max_open_zones))
+    if [[ $nz -eq 0 ]]; then
+	nz=128
+    fi
     # shellcheck disable=SC2017
     inc=$(((disk_size - (first_sequential_zone_sector * 512)) / (nz * zone_size)
 	   * zone_size))
+    if [ "$inc" -eq 0 ]; then
+	require_seq_zones $nz || return $SKIP_TESTCASE
+    fi
     opts=()
     for ((off = first_sequential_zone_sector * 512; off < disk_size;
 	  off += inc)); do
-- 
2.28.0

Re: [PATCH v3 33/38] t/zbd: prevent test #31 from looping

[Shinichiro Kawasaki]

On Jan 07, 2021 / 06:57, Dmitry Fomichev wrote:
> The test 31 starts i/o to 128 zones in parallel.
> There are two corner cases that are not properly handled in the
> existing implementation -
> 1) If the total number of zones on the device is < 128, the test
> will loop indefinitely because the loop increment is calculated as
> zero by the script.
> 2) If the number of max_open_zones of the device is < 128, the
> test will fail due to exceeding max_open_zones limit as the code
> expects it to be >= 128.
> 
> Limit the number of open zones to the reported maximum
> and skip the test if there is not enough zones on the device.
> 
> Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>

Looks good to me.

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

-- 
Best Regards,
Shin'ichiro Kawasaki

[PATCH v3 34/38] t/zbd: add checks for offline zone condition

[Dmitry Fomichev]

Some tests, e.g. #39 an #40, try to read the first zone of the drive.
It is assumed that the first zone is readable. However, if the first
zone is offline, the read fails along with the entire test.

This commit adds two functions to perform zone report and find the
first and the last zones that are not offline. Several test cases
now call these functions to avoid test failures described above.

Fixes for two more test failures are included in this commit -

Test #14 tries to write to conventional zones if they are found at
the beginning of the LBA range of the drive, but it assumes that
these zones are online. This may not always be the case. Add "offset"
to avoid the i/o to be attempted to run against any preceding offline
zones.

Similarly, in test #17, the script tries to find the last zone.
Check for the case when the last zone is offline. The test doesn't
set the i/o file size, but it works OK in most of the cases because
typically this test operates on the last physical zone. With the
online lookup in place, this may not always be the case and if there
are any offline zones that trail the last non-offline zone,
then the i/o will try to access that zone and fail. Add the "size"
to avoid the i/o to be attempted to run against any trailing offline
zones.

Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
 t/zbd/functions        | 56 ++++++++++++++++++++++++++++++++++++++++--
 t/zbd/test-zbd-support | 31 +++++++++++++++++++----
 2 files changed, 80 insertions(+), 7 deletions(-)

diff --git a/t/zbd/functions b/t/zbd/functions
index 1a64a215..40ffe1de 100644
--- a/t/zbd/functions
+++ b/t/zbd/functions
@@ -71,7 +71,7 @@ first_sequential_zone() {
 
     if [ -n "${blkzone}" ] && [ ! -n "${use_libzbc}" ]; then
 	${blkzone} report "$dev" |
-	    sed -n 's/^[[:blank:]]*start:[[:blank:]]\([0-9a-zA-Z]*\),[[:blank:]]len[[:blank:]]\([0-9a-zA-Z]*\),.*type:[[:blank:]]2(.*/\1 \2/p' |
+	    sed -n 's/^[[:blank:]]*start:[[:blank:]]\([0-9a-zA-Z]*\),[[:blank:]]len[[:blank:]]\([0-9a-zA-Z]*\),.*zcond:\(14\|[[:blank:]][0-4]\)(.*type:[[:blank:]]\([2]\)(.*/\1 \2/p' |
 	    {
 		read -r starting_sector length &&
 		    # Convert from hex to decimal
@@ -79,7 +79,7 @@ first_sequential_zone() {
 	    }
     else
 	${zbc_report_zones} "$dev" |
-	    sed -n 's/^Zone [0-9]*: type 0x2 .*, sector \([0-9]*\), \([0-9]*\) sectors,.*$/\1 \2/p' |
+	    sed -n 's/^Zone [0-9]*: type 0x2 .*,[[:blank:]]cond[[:blank:]]0x[0-4e][[:blank:]].*, sector \([0-9]*\), \([0-9]*\) sectors.*$/\1 \2/p' |
 	    head -n1
     fi
 }
@@ -121,6 +121,58 @@ total_zone_capacity() {
 	echo $((capacity * 512))
 }
 
+# Reports the starting sector and length of the first zone of device $1
+# that is not in offline (or similar) condition.
+first_online_zone() {
+    local dev=$1
+
+    if [ -z "$is_zbd" ]; then
+	echo 0
+	return
+    fi
+
+    if [ -n "${blkzone}" ] && [ ! -n "${use_libzbc}" ]; then
+	${blkzone} report "$dev" |
+	    sed -n 's/^[[:blank:]]*start:[[:blank:]]\([0-9a-zA-Z]*\),[[:blank:]]len[[:blank:]]\([0-9a-zA-Z]*\),.*zcond:\(14\|[[:blank:]][0-4]\)(.*type:[[:blank:]][12](.*/\1/p' |
+	    head -n1 |
+	    {
+		read -r starting_sector &&
+		    # Convert from hex to decimal
+		    echo $((starting_sector))
+	    }
+    else
+	${zbc_report_zones} "$dev" |
+	    sed -n 's/^Zone[[:blank:]][0-9]*:[[:blank:]]type[[:blank:]]0x[12][[:blank:]].*,[[:blank:]]cond[[:blank:]]0x[0-4e][[:blank:]].*,[[:blank:]]sector[[:blank:]]\([0-9]*\),.*$/\1/p' |
+	    head -n1
+    fi
+}
+
+# Reports the starting sector and length of the last zone of device $1
+# that is not in offline (or similar) condition.
+last_online_zone() {
+    local dev=$1
+
+    if [ -z "$is_zbd" ]; then
+	echo 0
+	return
+    fi
+
+    if [ -n "${blkzone}" ] && [ ! -n "${use_libzbc}" ]; then
+	${blkzone} report "$dev" |
+	    sed -n 's/^[[:blank:]]*start:[[:blank:]]\([0-9a-zA-Z]*\),[[:blank:]]len[[:blank:]]\([0-9a-zA-Z]*\),.*zcond:\(14\|[[:blank:]][0-4]\)(.*type:[[:blank:]][12](.*/\1/p' |
+	    tail -1 |
+	    {
+		read -r starting_sector &&
+		    # Convert from hex to decimal
+		    echo $((starting_sector))
+	    }
+    else
+	${zbc_report_zones} "$dev" |
+	    sed -n 's/^Zone[[:blank:]][0-9]*:[[:blank:]]type[[:blank:]]0x[12][[:blank:]].*,[[:blank:]]cond[[:blank:]]0x[0-4e][[:blank:]].*,[[:blank:]]sector[[:blank:]]\([0-9]*\),.*$/\1/p' |
+	    tail -1
+    fi
+}
+
 max_open_zones() {
     local dev=$1
 
diff --git a/t/zbd/test-zbd-support b/t/zbd/test-zbd-support
index 033c2ebc..0b8015df 100755
--- a/t/zbd/test-zbd-support
+++ b/t/zbd/test-zbd-support
@@ -464,15 +464,20 @@ test13() {
 
 # Random write to conventional zones.
 test14() {
-    local size
+    local off size
 
+    if ! result=($(first_online_zone "$dev")); then
+	echo "Failed to determine first online zone"
+	exit 1
+    fi
+    off=${result[0]}
     prep_write
     size=$((16 * 2**20)) # 20 MB
     require_conv_zone_bytes "${size}" || return $SKIP_TESTCASE
 
     run_one_fio_job "$(ioengine "libaio")" --iodepth=64 --rw=randwrite --bs=16K \
 		    --zonemode=zbd --zonesize="${zone_size}" --do_verify=1 \
-		    --verify=md5 --size=$size				   \
+		    --verify=md5 --offset=$off --size=$size\
 		    >>"${logfile}.${test_number}" 2>&1 || return $?
     check_written $((size)) || return $?
     check_read $((size)) || return $?
@@ -529,17 +534,26 @@ test16() {
 
 # Random reads and writes in the last zone.
 test17() {
-    local io off read size written
+    local io off last read size written
 
     off=$(((disk_size / zone_size - 1) * zone_size))
     size=$((disk_size - off))
+    if ! last=($(last_online_zone "$dev")); then
+	echo "Failed to determine last online zone"
+	exit 1
+    fi
+    if [[ "$((last * 512))" -lt "$off" ]]; then
+	off=$((last * 512))
+	size=$zone_size
+    fi
     if [ -n "$is_zbd" ]; then
 	reset_zone "$dev" $((off / 512)) || return $?
     fi
     prep_write
     run_one_fio_job "$(ioengine "libaio")" --iodepth=8 --rw=randrw --bs=4K \
 		    --zonemode=zbd --zonesize="${zone_size}"		\
-		    --offset=$off --loops=2 --norandommap=1\
+		    --offset=$off --loops=2 --norandommap=1 \
+		    --size="$size"\
 		    >>"${logfile}.${test_number}" 2>&1 || return $?
     written=$(fio_written <"${logfile}.${test_number}")
     read=$(fio_read <"${logfile}.${test_number}")
@@ -867,10 +881,17 @@ test38() {
 
 # Read one block from a block device.
 read_one_block() {
+    local off
     local bs
 
+    if ! result=($(first_online_zone "$dev")); then
+	echo "Failed to determine first online zone"
+	exit 1
+    fi
+    off=${result[0]}
     bs=$((logical_block_size))
-    run_one_fio_job --rw=read "$(ioengine "psync")" --bs=$bs --size=$bs "$@" 2>&1 |
+    run_one_fio_job --rw=read "$(ioengine "psync")" --offset=$off --bs=$bs \
+		    --size=$bs "$@" 2>&1 |
 	tee -a "${logfile}.${test_number}"
 }
 
-- 
2.28.0

Re: [PATCH v3 34/38] t/zbd: add checks for offline zone condition

[Shinichiro Kawasaki]

On Jan 07, 2021 / 06:57, Dmitry Fomichev wrote:
> Some tests, e.g. #39 an #40, try to read the first zone of the drive.
> It is assumed that the first zone is readable. However, if the first
> zone is offline, the read fails along with the entire test.
> 
> This commit adds two functions to perform zone report and find the
> first and the last zones that are not offline. Several test cases
> now call these functions to avoid test failures described above.
> 
> Fixes for two more test failures are included in this commit -
> 
> Test #14 tries to write to conventional zones if they are found at
> the beginning of the LBA range of the drive, but it assumes that
> these zones are online. This may not always be the case. Add "offset"
> to avoid the i/o to be attempted to run against any preceding offline
> zones.
> 
> Similarly, in test #17, the script tries to find the last zone.
> Check for the case when the last zone is offline. The test doesn't
> set the i/o file size, but it works OK in most of the cases because
> typically this test operates on the last physical zone. With the
> online lookup in place, this may not always be the case and if there
> are any offline zones that trail the last non-offline zone,
> then the i/o will try to access that zone and fail. Add the "size"
> to avoid the i/o to be attempted to run against any trailing offline
> zones.
> 
> Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>

Looks good to me.

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

-- 
Best Regards,
Shin'ichiro Kawasaki

[PATCH v3 35/38] t/zbd: add test #54 to exercise ZBD verification

[Dmitry Fomichev]

Add a new test case to perform 75/25 read/write workload with varying
i/o size and verification on. It is very important to use a good random
generator for this test. Setting experimental_verify=1 is required for
this test to operate correctly.

Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
 t/zbd/test-zbd-support | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/t/zbd/test-zbd-support b/t/zbd/test-zbd-support
index 0b8015df..0230f1af 100755
--- a/t/zbd/test-zbd-support
+++ b/t/zbd/test-zbd-support
@@ -1135,6 +1135,24 @@ test53() {
 	return 1
 }
 
+# Test read/write mix with verify.
+test54() {
+	require_zbd || return $SKIP_TESTCASE
+	require_seq_zones 8 || return $SKIP_TESTCASE
+
+	run_fio --name=job --filename=${dev} "$(ioengine "libaio")" \
+		--time_based=1 --runtime=30s --continue_on_error=0 \
+		--offset=$((first_sequential_zone_sector * 512)) \
+		--size=$((8*zone_size)) --direct=1 --iodepth=1 \
+		--rw=randrw:2 --rwmixwrite=25 --bsrange=4k-${zone_size} \
+		--zonemode=zbd --zonesize=${zone_size} \
+		--verify=crc32c --do_verify=1 --verify_backlog=2 \
+		--experimental_verify=1 \
+		--alloc-size=65536 --random_generator=tausworthe64 \
+		${job_var_opts[@]} --debug=zbd \
+		>> "${logfile}.${test_number}" 2>&1 || return $?
+}
+
 tests=()
 dynamic_analyzer=()
 reset_all_zones=
-- 
2.28.0

Re: [PATCH v3 35/38] t/zbd: add test #54 to exercise ZBD verification

[Shinichiro Kawasaki]

On Jan 07, 2021 / 06:57, Dmitry Fomichev wrote:
> Add a new test case to perform 75/25 read/write workload with varying
> i/o size and verification on. It is very important to use a good random
> generator for this test. Setting experimental_verify=1 is required for
> this test to operate correctly.
> 
> Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>

Looks good to me.

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

-- 
Best Regards,
Shin'ichiro Kawasaki

[PATCH v3 36/38] t/zbd: show elapsed time in test-zbd-support

[Dmitry Fomichev]

This script may take quite a lot of time to run against large
zoned HDDs. At the end of every run, show exactly how much time
it took.

Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
 t/zbd/test-zbd-support | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/t/zbd/test-zbd-support b/t/zbd/test-zbd-support
index 0230f1af..f9427981 100755
--- a/t/zbd/test-zbd-support
+++ b/t/zbd/test-zbd-support
@@ -1153,6 +1153,7 @@ test54() {
 		>> "${logfile}.${test_number}" 2>&1 || return $?
 }
 
+SECONDS=0
 tests=()
 dynamic_analyzer=()
 reset_all_zones=
@@ -1346,4 +1347,5 @@ fi
 if [ $failed -gt 0 ]; then
     echo " $failed tests failed"
 fi
+echo "Run time: $(TZ=UTC0 printf "%(%H:%M:%S)T\n" $(( SECONDS )) )"
 exit $rc
-- 
2.28.0

Re: [PATCH v3 36/38] t/zbd: show elapsed time in test-zbd-support

[Shinichiro Kawasaki]

On Jan 07, 2021 / 06:57, Dmitry Fomichev wrote:
> This script may take quite a lot of time to run against large
> zoned HDDs. At the end of every run, show exactly how much time
> it took.
> 
> Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>

Looks good to me.

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

-- 
Best Regards,
Shin'ichiro Kawasaki

[PATCH v3 37/38] t/zbd: increase timeout in test #48

[Dmitry Fomichev]

Test #48 runs some i/o to the test device for 30 seconds and then waits
45 seconds for fio to finish. If this wait times out, the test assumes
that fio is hung because of a zone locking issue and fails. It is
observed that 45s may not be enough for some HDDs, especially the ones
running specialized firmware.

Increase the timeout to 180 seconds to avoid any false positives.
There is no change in test duration for the most common devices.
The test will wait for the full 180 seconds only if it fails, otherwise
it will finish very soon after the 30 second i/o period ends.

Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
 t/zbd/test-zbd-support | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/zbd/test-zbd-support b/t/zbd/test-zbd-support
index f9427981..05c00900 100755
--- a/t/zbd/test-zbd-support
+++ b/t/zbd/test-zbd-support
@@ -1008,7 +1008,7 @@ test48() {
 
     { echo; echo "fio ${opts[*]}"; echo; } >>"${logfile}.${test_number}"
 
-    timeout -v -s KILL 45s \
+    timeout -v -s KILL 180s \
 	    "${dynamic_analyzer[@]}" "$fio" "${opts[@]}" \
 	    >> "${logfile}.${test_number}" 2>&1 || return $?
 }
-- 
2.28.0

Re: [PATCH v3 37/38] t/zbd: increase timeout in test #48

[Shinichiro Kawasaki]

On Jan 07, 2021 / 06:57, Dmitry Fomichev wrote:
> Test #48 runs some i/o to the test device for 30 seconds and then waits
> 45 seconds for fio to finish. If this wait times out, the test assumes
> that fio is hung because of a zone locking issue and fails. It is
> observed that 45s may not be enough for some HDDs, especially the ones
> running specialized firmware.
> 
> Increase the timeout to 180 seconds to avoid any false positives.
> There is no change in test duration for the most common devices.
> The test will wait for the full 180 seconds only if it fails, otherwise
> it will finish very soon after the 30 second i/o period ends.
> 
> Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>

Looks good to me.

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

-- 
Best Regards,
Shin'ichiro Kawasaki

[PATCH v3 38/38] t/zbd: avoid looping on invalid command line options

[Dmitry Fomichev]

t/zbd/test-zbd-support loops indefinitely if an unrecognized option
is specified in the command line. Add a switch case to display usage
and exit the script.

Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
 t/zbd/test-zbd-support | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/zbd/test-zbd-support b/t/zbd/test-zbd-support
index 05c00900..1658dc25 100755
--- a/t/zbd/test-zbd-support
+++ b/t/zbd/test-zbd-support
@@ -1178,6 +1178,7 @@ while [ "${1#-}" != "$1" ]; do
     -q) quit_on_err=1; shift;;
     -z) zbd_debug=1; shift;;
     --) shift; break;;
+     *) usage; exit 1;;
   esac
 done
 
-- 
2.28.0

Re: [PATCH v3 38/38] t/zbd: avoid looping on invalid command line options

[Shinichiro Kawasaki]

On Jan 07, 2021 / 06:57, Dmitry Fomichev wrote:
> t/zbd/test-zbd-support loops indefinitely if an unrecognized option
> is specified in the command line. Add a switch case to display usage
> and exit the script.
> 
> Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>

Looks good to me.

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

-- 
Best Regards,
Shin'ichiro Kawasaki

Re: [PATCH v3 00/38] ZBD fixes and improvements

[Shinichiro Kawasaki]

On Jan 07, 2021 / 06:57, Dmitry Fomichev wrote:
> v2 -> v3:
> 
>  - fix two libzbc-related bugs in test scripts (found by Shinichiro).
>  - modify libzbc ioengine code to allow fio to operate successfully
>    against null_blk devices while using this ioengine.
>  - add -l option to run-tests-against-nullb script to allow running
>    the tests while using libzbc ioengine.
>  - add -n option to run-tests-against-nullb to allow running the
>    specified tests multiple times. This option is the most useful
>    in combination with the existing -s and -t options.
>  - fix sporadic failed assertion in test #51 that is easily triggered
>    by running run-tests-against-nullb script with -l and -n options.
>  - make minor coding style improvements in run-tests-against-nullb.
> 
> v1 -> v2:
> 
>  - replace both run-tests-against-conventional-nullb and 
>    run-tests-against-conventional-nullb with a single script,
>    run-tests-against-nullb, that runs test-zbd-support unit tests
>    over a variety on different zoned configurations.
>  - add five new test cases to test-zbd-support to cover the code
>    changes made in zbd.c as a part of this series.
>  - mark all test cases in test-zbd-support script that are not
>    applicable for the device configuration as SKIP instead of
>    reporting them as passed.
>  - properly handle writes to conventional zones that cross over
>    to sequential zones.
>  - make additional improvements in zone locking parts of zbd.c.
>  - implement miscellaneous test script enhancements.
> 
> This patch series contains bug fixes and refactoring changes
> related to support for Zoned Block Devices (ZBD) in fio.
> The highlights:
> 
>  - fix several errors related to running workloads that span
>    a mix of conventional zones and write pointer zones.
>  - improve counting of sectors with data (SWD).
>  - remove dependencies on particular zone types in the code.
>  - add code to gracefully handle offline zones.
> 
> Aravind Ramesh (1):
>   zbd: initialize sectors with data at start time
> 
> Dmitry Fomichev (25):
>   zbd: return ENOMEM if zone buffer allocation fails
>   zbd: use zbd_zone_nr() more actively in the code
>   zbd: add get_zone() helper function
>   zbd: introduce zone_unlock()
>   zbd: engines/libzbc: don't fail on assert for offline zones
>   zbd: remove dependency on zone type during i/o
>   zbd: skip offline zones in zbd_convert_to_open_zone()
>   zbd: avoid zone buffer overrun
>   zbd: don't unlock zone mutex after verify replay
>   zbd: use zone_lock() in zbd_process_swd()
>   zbd: don't log "zone nnnn is not open" message
>   zbd: handle conventional start zone in zbd_convert_to_open_zone()
>   zbd: improve replay range validation
>   engines/libzbc: enable block backend
>   zbd: avoid failing assertion in zbd_convert_to_open_zone()
>   zbd: set thread errors in zbd_adjust_block()
>   t/zbd: check for error in test #2
>   t/zbd: add run-tests-against-nullb script

Dmitry, thank you for posting this series. I reviewed all of your patches in it.
Overall, they are all good and I believe they worth up-streaming. One thing, I
think the patch above needs update. Please find my comments on it in other mail.

>   t/zbd: add an option to bail on a failed test
>   t/zbd: prevent test #31 from looping
>   t/zbd: add checks for offline zone condition
>   t/zbd: add test #54 to exercise ZBD verification
>   t/zbd: show elapsed time in test-zbd-support
>   t/zbd: increase timeout in test #48
>   t/zbd: avoid looping on invalid command line options
> 
> Shin'ichiro Kawasaki (12):
>   zbd: do not lock conventional zones on I/O adjustment
>   zbd: do not set zbd handlers for conventional zones
>   zbd: count sectors with data for write pointer zones

Regarding my patches, you kindly reviewed them and added your SOB tags,
except this patch above. Could you check it again?

Thanks!

-- 
Best Regards,
Shin'ichiro Kawasaki

RE: [PATCH v3 00/38] ZBD fixes and improvements

[Dmitry Fomichev]

Shinichiro,

Thank you for reviewing! I am going to send a v4 with the changes to
t/zbd/run-tests-against-nullb script that you have suggested, extended
commit message in "zbd: improve replay range validation" patch and
added sign-off for " zbd: count sectors with data for write pointer zones"
patch.

Best regards,
Dmitry

> -----Original Message-----
> From: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> Sent: Friday, January 22, 2021 4:25 AM
> To: Dmitry Fomichev <Dmitry.Fomichev@wdc.com>
> Cc: Jens Axboe <axboe@kernel.dk>; fio@vger.kernel.org; Aravind Ramesh
> <Aravind.Ramesh@wdc.com>; Bart Van Assche <bvanassche@acm.org>;
> Naohiro Aota <Naohiro.Aota@wdc.com>; Niklas Cassel
> <Niklas.Cassel@wdc.com>; Damien Le Moal <Damien.LeMoal@wdc.com>
> Subject: Re: [PATCH v3 00/38] ZBD fixes and improvements
> 
> On Jan 07, 2021 / 06:57, Dmitry Fomichev wrote:
> > v2 -> v3:
> >
> >  - fix two libzbc-related bugs in test scripts (found by Shinichiro).
> >  - modify libzbc ioengine code to allow fio to operate successfully
> >    against null_blk devices while using this ioengine.
> >  - add -l option to run-tests-against-nullb script to allow running
> >    the tests while using libzbc ioengine.
> >  - add -n option to run-tests-against-nullb to allow running the
> >    specified tests multiple times. This option is the most useful
> >    in combination with the existing -s and -t options.
> >  - fix sporadic failed assertion in test #51 that is easily triggered
> >    by running run-tests-against-nullb script with -l and -n options.
> >  - make minor coding style improvements in run-tests-against-nullb.
> >
> > v1 -> v2:
> >
> >  - replace both run-tests-against-conventional-nullb and
> >    run-tests-against-conventional-nullb with a single script,
> >    run-tests-against-nullb, that runs test-zbd-support unit tests
> >    over a variety on different zoned configurations.
> >  - add five new test cases to test-zbd-support to cover the code
> >    changes made in zbd.c as a part of this series.
> >  - mark all test cases in test-zbd-support script that are not
> >    applicable for the device configuration as SKIP instead of
> >    reporting them as passed.
> >  - properly handle writes to conventional zones that cross over
> >    to sequential zones.
> >  - make additional improvements in zone locking parts of zbd.c.
> >  - implement miscellaneous test script enhancements.
> >
> > This patch series contains bug fixes and refactoring changes
> > related to support for Zoned Block Devices (ZBD) in fio.
> > The highlights:
> >
> >  - fix several errors related to running workloads that span
> >    a mix of conventional zones and write pointer zones.
> >  - improve counting of sectors with data (SWD).
> >  - remove dependencies on particular zone types in the code.
> >  - add code to gracefully handle offline zones.
> >
> > Aravind Ramesh (1):
> >   zbd: initialize sectors with data at start time
> >
> > Dmitry Fomichev (25):
> >   zbd: return ENOMEM if zone buffer allocation fails
> >   zbd: use zbd_zone_nr() more actively in the code
> >   zbd: add get_zone() helper function
> >   zbd: introduce zone_unlock()
> >   zbd: engines/libzbc: don't fail on assert for offline zones
> >   zbd: remove dependency on zone type during i/o
> >   zbd: skip offline zones in zbd_convert_to_open_zone()
> >   zbd: avoid zone buffer overrun
> >   zbd: don't unlock zone mutex after verify replay
> >   zbd: use zone_lock() in zbd_process_swd()
> >   zbd: don't log "zone nnnn is not open" message
> >   zbd: handle conventional start zone in zbd_convert_to_open_zone()
> >   zbd: improve replay range validation
> >   engines/libzbc: enable block backend
> >   zbd: avoid failing assertion in zbd_convert_to_open_zone()
> >   zbd: set thread errors in zbd_adjust_block()
> >   t/zbd: check for error in test #2
> >   t/zbd: add run-tests-against-nullb script
> 
> Dmitry, thank you for posting this series. I reviewed all of your patches in it.
> Overall, they are all good and I believe they worth up-streaming. One thing, I
> think the patch above needs update. Please find my comments on it in other
> mail.
> 
> >   t/zbd: add an option to bail on a failed test
> >   t/zbd: prevent test #31 from looping
> >   t/zbd: add checks for offline zone condition
> >   t/zbd: add test #54 to exercise ZBD verification
> >   t/zbd: show elapsed time in test-zbd-support
> >   t/zbd: increase timeout in test #48
> >   t/zbd: avoid looping on invalid command line options
> >
> > Shin'ichiro Kawasaki (12):
> >   zbd: do not lock conventional zones on I/O adjustment
> >   zbd: do not set zbd handlers for conventional zones
> >   zbd: count sectors with data for write pointer zones
> 
> Regarding my patches, you kindly reviewed them and added your SOB tags,
> except this patch above. Could you check it again?
> 
> Thanks!
> 
> --
> Best Regards,
> Shin'ichiro Kawasaki