All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 00/12] Cleanup zbd code
@ 2021-12-14  1:24 Damien Le Moal
  2021-12-14  1:24 ` [PATCH v2 01/12] fio: Improve documentation of ignore_zone_limits option Damien Le Moal
                   ` (12 more replies)
  0 siblings, 13 replies; 18+ messages in thread
From: Damien Le Moal @ 2021-12-14  1:24 UTC (permalink / raw)
  To: fio, Jens Axboe; +Cc: Niklas Cassel, Shinichiro Kawasaki

The patch series cleans up zbd.c code, improving code readability.
Patches 1 to 11 do not introduce any functional change. Patch 12 fixes
a problem with the zbd test script.

Changes from v1:
* Renamed zbd_verify_file_sizes() to zbd_zone_align_file_sizes() in
  patch 5
* Change function names as suggested by Niklas in patches 10 and 11
* Added Niklas Reviewed-by tag
* Added patch 12

Damien Le Moal (11):
  fio: Improve documentation of ignore_zone_limits option
  zbd: define local functions as static
  zbd: move and cleanup code
  zbd: remove is_zone_open() helper
  zbd: introduce zbd_zone_align_file_sizes() helper
  zbd: fix code style issues
  zbd: simplify zbd_close_zone()
  zbd: simplify zbd_open_zone()
  zbd: rename zbd_zone_idx() and zbd_zone_nr()
  zbd: rename get_zone()
  zbd: introduce zbd_offset_to_zone() helper

Shin'ichiro Kawasaki (1):
  t/zbd: Avoid inappropriate blkzone command call in zone_cap_bs

 HOWTO           |   6 +
 fio.1           |   6 +-
 t/zbd/functions |   6 +-
 zbd.c           | 963 ++++++++++++++++++++++++++----------------------
 4 files changed, 531 insertions(+), 450 deletions(-)

-- 
2.31.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2 01/12] fio: Improve documentation of ignore_zone_limits option
  2021-12-14  1:24 [PATCH v2 00/12] Cleanup zbd code Damien Le Moal
@ 2021-12-14  1:24 ` Damien Le Moal
  2021-12-14  1:24 ` [PATCH v2 02/12] zbd: define local functions as static Damien Le Moal
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 18+ messages in thread
From: Damien Le Moal @ 2021-12-14  1:24 UTC (permalink / raw)
  To: fio, Jens Axboe; +Cc: Niklas Cassel, Shinichiro Kawasaki

In the manual pages, change the description of the option
ignore_zone_limits to its action when set, instead of the confusing text
describing what happens when it is not set. Also add the description
of this option in the HOWTO file as it is missing.

Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com>
---
 HOWTO | 6 ++++++
 fio.1 | 6 +++---
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/HOWTO b/HOWTO
index 8c9e4135..2956e50d 100644
--- a/HOWTO
+++ b/HOWTO
@@ -1063,6 +1063,12 @@ Target file/device
 	Limit on the number of simultaneously opened zones per single
 	thread/process.
 
+.. option:: ignore_zone_limits=bool
+	If this option is used, fio will ignore the maximum number of open
+	zones limit of the zoned block device in use, thus allowing the
+	option :option:`max_open_zones` value to be larger than the device
+	reported limit. Default: false.
+
 .. option:: zone_reset_threshold=float
 
 	A number between zero and one that indicates the ratio of logical
diff --git a/fio.1 b/fio.1
index a3ebb67d..e0458c22 100644
--- a/fio.1
+++ b/fio.1
@@ -838,9 +838,9 @@ threads/processes.
 Limit on the number of simultaneously opened zones per single thread/process.
 .TP
 .BI ignore_zone_limits \fR=\fPbool
-If this isn't set, fio will query the max open zones limit from the zoned block
-device, and exit if the specified \fBmax_open_zones\fR value is larger than the
-limit reported by the device. Default: false.
+If this option is used, fio will ignore the maximum number of open zones limit
+of the zoned block device in use, thus allowing the option \fBmax_open_zones\fR
+value to be larger than the device reported limit. Default: false.
 .TP
 .BI zone_reset_threshold \fR=\fPfloat
 A number between zero and one that indicates the ratio of logical blocks with
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 02/12] zbd: define local functions as static
  2021-12-14  1:24 [PATCH v2 00/12] Cleanup zbd code Damien Le Moal
  2021-12-14  1:24 ` [PATCH v2 01/12] fio: Improve documentation of ignore_zone_limits option Damien Le Moal
@ 2021-12-14  1:24 ` Damien Le Moal
  2021-12-14  1:24 ` [PATCH v2 03/12] zbd: move and cleanup code Damien Le Moal
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 18+ messages in thread
From: Damien Le Moal @ 2021-12-14  1:24 UTC (permalink / raw)
  To: fio, Jens Axboe; +Cc: Niklas Cassel, Shinichiro Kawasaki

Define zbd_get_zoned_model(), zbd_report_zones(), zbd_reset_wp() and
zbd_get_max_open_zones() as static since these functions are used
locally only.

No functional changes.

Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com>
---
 zbd.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/zbd.c b/zbd.c
index c18998c4..44e77227 100644
--- a/zbd.c
+++ b/zbd.c
@@ -27,8 +27,8 @@
  * @td: FIO thread data
  * @f: FIO file for which to get model information
  */
-int zbd_get_zoned_model(struct thread_data *td, struct fio_file *f,
-			enum zbd_zoned_model *model)
+static int zbd_get_zoned_model(struct thread_data *td, struct fio_file *f,
+			       enum zbd_zoned_model *model)
 {
 	int ret;
 
@@ -71,9 +71,9 @@ int zbd_get_zoned_model(struct thread_data *td, struct fio_file *f,
  * upon failure. If the zone report is empty, always assume an error (device
  * problem) and return -EIO.
  */
-int zbd_report_zones(struct thread_data *td, struct fio_file *f,
-		     uint64_t offset, struct zbd_zone *zones,
-		     unsigned int nr_zones)
+static int zbd_report_zones(struct thread_data *td, struct fio_file *f,
+			    uint64_t offset, struct zbd_zone *zones,
+			    unsigned int nr_zones)
 {
 	int ret;
 
@@ -105,8 +105,8 @@ int zbd_report_zones(struct thread_data *td, struct fio_file *f,
  * Reset the write pointer of all zones in the range @offset...@offset+@length.
  * Returns 0 upon success and a negative error code upon failure.
  */
-int zbd_reset_wp(struct thread_data *td, struct fio_file *f,
-		 uint64_t offset, uint64_t length)
+static int zbd_reset_wp(struct thread_data *td, struct fio_file *f,
+			uint64_t offset, uint64_t length)
 {
 	int ret;
 
@@ -133,8 +133,8 @@ int zbd_reset_wp(struct thread_data *td, struct fio_file *f,
  *
  * Returns 0 upon success and a negative error code upon failure.
  */
-int zbd_get_max_open_zones(struct thread_data *td, struct fio_file *f,
-			   unsigned int *max_open_zones)
+static int zbd_get_max_open_zones(struct thread_data *td, struct fio_file *f,
+				  unsigned int *max_open_zones)
 {
 	int ret;
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 03/12] zbd: move and cleanup code
  2021-12-14  1:24 [PATCH v2 00/12] Cleanup zbd code Damien Le Moal
  2021-12-14  1:24 ` [PATCH v2 01/12] fio: Improve documentation of ignore_zone_limits option Damien Le Moal
  2021-12-14  1:24 ` [PATCH v2 02/12] zbd: define local functions as static Damien Le Moal
@ 2021-12-14  1:24 ` Damien Le Moal
  2021-12-14  1:24 ` [PATCH v2 04/12] zbd: remove is_zone_open() helper Damien Le Moal
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 18+ messages in thread
From: Damien Le Moal @ 2021-12-14  1:24 UTC (permalink / raw)
  To: fio, Jens Axboe; +Cc: Niklas Cassel, Shinichiro Kawasaki

Move zone manipulation helper functions at the beginning of the zbd.c
file to avoid forward declarations and to group these functions
together apart from the IO manipulation functions.
Also fix function comments.

No functional changes.

Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com>
---
 zbd.c | 582 ++++++++++++++++++++++++++++++----------------------------
 1 file changed, 301 insertions(+), 281 deletions(-)

diff --git a/zbd.c b/zbd.c
index 44e77227..20d53b15 100644
--- a/zbd.c
+++ b/zbd.c
@@ -22,6 +22,112 @@
 #include "pshared.h"
 #include "zbd.h"
 
+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 unsigned int zbd_zone_nr(const struct fio_file *f,
+				       struct fio_zone_info *zone)
+{
+	return zone - f->zbd_info->zone_info;
+}
+
+/**
+ * zbd_zone_idx - convert an offset into a zone number
+ * @f: file pointer.
+ * @offset: offset in bytes. If this offset is in the first zone_size bytes
+ *	    past the disk size then the index of the sentinel is returned.
+ */
+static uint32_t zbd_zone_idx(const struct fio_file *f, uint64_t offset)
+{
+	uint32_t zone_idx;
+
+	if (f->zbd_info->zone_size_log2 > 0)
+		zone_idx = offset >> f->zbd_info->zone_size_log2;
+	else
+		zone_idx = offset / f->zbd_info->zone_size;
+
+	return min(zone_idx, f->zbd_info->nr_zones);
+}
+
+/**
+ * zbd_zone_end - Return zone end location
+ * @z: zone info pointer.
+ */
+static inline uint64_t zbd_zone_end(const struct fio_zone_info *z)
+{
+	return (z+1)->start;
+}
+
+/**
+ * zbd_zone_capacity_end - Return zone capacity limit end location
+ * @z: zone info pointer.
+ */
+static inline uint64_t zbd_zone_capacity_end(const struct fio_zone_info *z)
+{
+	return z->start + z->capacity;
+}
+
+/**
+ * zbd_zone_full - verify whether a minimum number of bytes remain in a zone
+ * @f: file pointer.
+ * @z: zone info pointer.
+ * @required: minimum number of bytes that must remain in a zone.
+ *
+ * The caller must hold z->mutex.
+ */
+static bool zbd_zone_full(const struct fio_file *f, struct fio_zone_info *z,
+			  uint64_t required)
+{
+	assert((required & 511) == 0);
+
+	return z->has_wp &&
+		z->wp + required > zbd_zone_capacity_end(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;
+
+	/* 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.
+	 * To avoid multiple jobs doing asynchronous I/Os from deadlocking each
+	 * other waiting for zone locks when building an io_u batch, first
+	 * only trylock the zone. If the zone is already locked by another job,
+	 * process the currently queued I/Os so that I/O progress is made and
+	 * zones unlocked.
+	 */
+	if (pthread_mutex_trylock(&z->mutex) != 0) {
+		if (!td_ioengine_flagged(td, FIO_SYNCIO))
+			io_u_quiesce(td);
+		pthread_mutex_lock(&z->mutex);
+	}
+}
+
+static inline void zone_unlock(struct fio_zone_info *z)
+{
+	int ret;
+
+	assert(z->has_wp);
+	ret = pthread_mutex_unlock(&z->mutex);
+	assert(!ret);
+}
+
+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];
+}
+
 /**
  * zbd_get_zoned_model - Get a device zoned model
  * @td: FIO thread data
@@ -123,6 +229,126 @@ static int zbd_reset_wp(struct thread_data *td, struct fio_file *f,
 	return ret;
 }
 
+/**
+ * zbd_reset_zone - reset the write pointer of a single zone
+ * @td: FIO thread data.
+ * @f: FIO file associated with the disk for which to reset a write pointer.
+ * @z: Zone to reset.
+ *
+ * Returns 0 upon success and a negative error code upon failure.
+ *
+ * The caller must hold z->mutex.
+ */
+static int zbd_reset_zone(struct thread_data *td, struct fio_file *f,
+			  struct fio_zone_info *z)
+{
+	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 (!data_in_zone)
+		return 0;
+
+	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, z));
+	switch (f->zbd_info->model) {
+	case ZBD_HOST_AWARE:
+	case ZBD_HOST_MANAGED:
+		ret = zbd_reset_wp(td, f, offset, length);
+		if (ret < 0)
+			return ret;
+		break;
+	default:
+		break;
+	}
+
+	pthread_mutex_lock(&f->zbd_info->mutex);
+	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;
+
+	td->ts.nr_zone_resets++;
+
+	return ret;
+}
+
+/**
+ * zbd_close_zone - Remove a zone from the open zones array.
+ * @td: FIO thread data.
+ * @f: FIO file associated with the disk for which to reset a write pointer.
+ * @zone_idx: Index of the zone to remove.
+ *
+ * The caller must hold f->zbd_info->mutex.
+ */
+static void zbd_close_zone(struct thread_data *td, const struct fio_file *f,
+			   unsigned int zone_idx)
+{
+	uint32_t open_zone_idx = 0;
+
+	for (; open_zone_idx < f->zbd_info->num_open_zones; open_zone_idx++) {
+		if (f->zbd_info->open_zones[open_zone_idx] == zone_idx)
+			break;
+	}
+	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,
+		f->zbd_info->open_zones + open_zone_idx + 1,
+		(ZBD_MAX_OPEN_ZONES - (open_zone_idx + 1)) *
+		sizeof(f->zbd_info->open_zones[0]));
+	f->zbd_info->num_open_zones--;
+	td->num_open_zones--;
+	get_zone(f, zone_idx)->open = 0;
+}
+
+/**
+ * zbd_reset_zones - Reset a range of zones.
+ * @td: fio thread data.
+ * @f: fio file for which to reset zones
+ * @zb: first zone to reset.
+ * @ze: first zone not to reset.
+ *
+ * Returns 0 upon success and 1 upon failure.
+ */
+static int zbd_reset_zones(struct thread_data *td, struct fio_file *f,
+			   struct fio_zone_info *const zb,
+			   struct fio_zone_info *const ze)
+{
+	struct fio_zone_info *z;
+	const uint64_t min_bs = td->o.min_bs[DDIR_WRITE];
+	int res = 0;
+
+	assert(min_bs);
+
+	dprint(FD_ZBD, "%s: examining zones %u .. %u\n", f->file_name,
+		zbd_zone_nr(f, zb), zbd_zone_nr(f, ze));
+	for (z = zb; z < ze; z++) {
+		uint32_t nz = zbd_zone_nr(f, z);
+
+		if (!z->has_wp)
+			continue;
+		zone_lock(td, f, z);
+		pthread_mutex_lock(&f->zbd_info->mutex);
+		zbd_close_zone(td, f, nz);
+		pthread_mutex_unlock(&f->zbd_info->mutex);
+		if (z->wp != z->start) {
+			dprint(FD_ZBD, "%s: resetting zone %u\n",
+			       f->file_name, zbd_zone_nr(f, z));
+			if (zbd_reset_zone(td, f, z) < 0)
+				res = 1;
+		}
+		zone_unlock(z);
+	}
+
+	return res;
+}
+
 /**
  * zbd_get_max_open_zones - Get the maximum number of open zones
  * @td: FIO thread data
@@ -152,103 +378,99 @@ static int zbd_get_max_open_zones(struct thread_data *td, struct fio_file *f,
 }
 
 /**
- * zbd_zone_idx - convert an offset into a zone number
- * @f: file pointer.
- * @offset: offset in bytes. If this offset is in the first zone_size bytes
- *	    past the disk size then the index of the sentinel is returned.
+ * is_zone_open - Test if a zone is already in the array of open zones.
+ * @td: fio thread data.
+ * @f: fio file for which to test zones.
+ * @zone_idx: Index of the zone to check.
+ *
+ * The caller must hold f->zbd_info->mutex.
  */
-static uint32_t zbd_zone_idx(const struct fio_file *f, uint64_t offset)
+static bool is_zone_open(const struct thread_data *td, const struct fio_file *f,
+			 unsigned int zone_idx)
 {
-	uint32_t zone_idx;
-
-	if (f->zbd_info->zone_size_log2 > 0)
-		zone_idx = offset >> f->zbd_info->zone_size_log2;
-	else
-		zone_idx = offset / f->zbd_info->zone_size;
+	struct zoned_block_device_info *zbdi = f->zbd_info;
+	int i;
 
-	return min(zone_idx, f->zbd_info->nr_zones);
-}
+	/*
+	 * This function should never be called when zbdi->max_open_zones == 0.
+	 */
+	assert(zbdi->max_open_zones);
+	assert(td->o.job_max_open_zones == 0 ||
+	       td->num_open_zones <= td->o.job_max_open_zones);
+	assert(td->o.job_max_open_zones <= zbdi->max_open_zones);
+	assert(zbdi->num_open_zones <= zbdi->max_open_zones);
 
-/**
- * zbd_zone_end - Return zone end location
- * @z: zone info pointer.
- */
-static inline uint64_t zbd_zone_end(const struct fio_zone_info *z)
-{
-	return (z+1)->start;
-}
+	for (i = 0; i < zbdi->num_open_zones; i++)
+		if (zbdi->open_zones[i] == zone_idx)
+			return true;
 
-/**
- * zbd_zone_capacity_end - Return zone capacity limit end location
- * @z: zone info pointer.
- */
-static inline uint64_t zbd_zone_capacity_end(const struct fio_zone_info *z)
-{
-	return z->start + z->capacity;
+	return false;
 }
 
 /**
- * zbd_zone_full - verify whether a minimum number of bytes remain in a zone
- * @f: file pointer.
- * @z: zone info pointer.
- * @required: minimum number of bytes that must remain in a zone.
+ * zbd_open_zone - Add a zone to the array of open zones.
+ * @td: fio thread data.
+ * @f: fio file that has the open zones to add.
+ * @zone_idx: Index of the zone to add.
  *
- * The caller must hold z->mutex.
+ * Open a ZBD zone if it is not already open. Returns true if either the zone
+ * was already open or if the zone was successfully added to the array of open
+ * zones without exceeding the maximum number of open zones. Returns false if
+ * the zone was not already open and opening the zone would cause the zone limit
+ * to be exceeded.
  */
-static bool zbd_zone_full(const struct fio_file *f, struct fio_zone_info *z,
-			  uint64_t required)
-{
-	assert((required & 511) == 0);
-
-	return z->has_wp &&
-		z->wp + required > zbd_zone_capacity_end(z);
-}
-
-static void zone_lock(struct thread_data *td, const struct fio_file *f,
-		      struct fio_zone_info *z)
+static bool zbd_open_zone(struct thread_data *td, const struct fio_file *f,
+			  uint32_t zone_idx)
 {
-	struct zoned_block_device_info *zbd = f->zbd_info;
-	uint32_t nz = z - zbd->zone_info;
-
-	/* A thread should never lock zones outside its working area. */
-	assert(f->min_zone <= nz && nz < f->max_zone);
+	const uint64_t min_bs = td->o.min_bs[DDIR_WRITE];
+	struct zoned_block_device_info *zbdi = f->zbd_info;
+	struct fio_zone_info *z = get_zone(f, zone_idx);
+	bool res = true;
 
-	assert(z->has_wp);
+	if (z->cond == ZBD_ZONE_COND_OFFLINE)
+		return false;
 
 	/*
-	 * 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.
-	 * To avoid multiple jobs doing asynchronous I/Os from deadlocking each
-	 * other waiting for zone locks when building an io_u batch, first
-	 * only trylock the zone. If the zone is already locked by another job,
-	 * process the currently queued I/Os so that I/O progress is made and
-	 * zones unlocked.
+	 * Skip full zones with data verification enabled because resetting a
+	 * zone causes data loss and hence causes verification to fail.
 	 */
-	if (pthread_mutex_trylock(&z->mutex) != 0) {
-		if (!td_ioengine_flagged(td, FIO_SYNCIO))
-			io_u_quiesce(td);
-		pthread_mutex_lock(&z->mutex);
-	}
-}
-
-static inline void zone_unlock(struct fio_zone_info *z)
-{
-	int ret;
+	if (td->o.verify != VERIFY_NONE && zbd_zone_full(f, z, min_bs))
+		return false;
 
-	assert(z->has_wp);
-	ret = pthread_mutex_unlock(&z->mutex);
-	assert(!ret);
-}
+	/*
+	 * zbdi->max_open_zones == 0 means that there is no limit on the maximum
+	 * number of open zones. In this case, do no track open zones in
+	 * zbdi->open_zones array.
+	 */
+	if (!zbdi->max_open_zones)
+		return true;
 
-static bool is_valid_offset(const struct fio_file *f, uint64_t offset)
-{
-	return (uint64_t)(offset - f->file_offset) < f->io_size;
-}
+	pthread_mutex_lock(&zbdi->mutex);
+	if (is_zone_open(td, f, zone_idx)) {
+		/*
+		 * If the zone is already open and going to be full by writes
+		 * in-flight, handle it as a full zone instead of an open zone.
+		 */
+		if (z->wp >= zbd_zone_capacity_end(z))
+			res = false;
+		goto out;
+	}
+	res = false;
+	/* Zero means no limit */
+	if (td->o.job_max_open_zones > 0 &&
+	    td->num_open_zones >= td->o.job_max_open_zones)
+		goto out;
+	if (zbdi->num_open_zones >= zbdi->max_open_zones)
+		goto out;
+	dprint(FD_ZBD, "%s: opening zone %d\n", f->file_name, zone_idx);
+	zbdi->open_zones[zbdi->num_open_zones++] = zone_idx;
+	td->num_open_zones++;
+	z->open = 1;
+	res = true;
 
-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];
+out:
+	pthread_mutex_unlock(&zbdi->mutex);
+	return res;
 }
 
 /* Verify whether direct I/O is used for all host-managed zoned drives. */
@@ -751,11 +973,6 @@ static int zbd_init_zone_info(struct thread_data *td, struct fio_file *file)
 	return ret;
 }
 
-static bool zbd_open_zone(struct thread_data *td, const struct fio_file *f,
-			  uint32_t zone_idx);
-static int zbd_reset_zone(struct thread_data *td, struct fio_file *f,
-			  struct fio_zone_info *z);
-
 int zbd_init_files(struct thread_data *td)
 {
 	struct fio_file *f;
@@ -879,123 +1096,6 @@ int zbd_setup_files(struct thread_data *td)
 	return 0;
 }
 
-static inline unsigned int zbd_zone_nr(const struct fio_file *f,
-				       struct fio_zone_info *zone)
-{
-	return zone - f->zbd_info->zone_info;
-}
-
-/**
- * zbd_reset_zone - reset the write pointer of a single zone
- * @td: FIO thread data.
- * @f: FIO file associated with the disk for which to reset a write pointer.
- * @z: Zone to reset.
- *
- * Returns 0 upon success and a negative error code upon failure.
- *
- * The caller must hold z->mutex.
- */
-static int zbd_reset_zone(struct thread_data *td, struct fio_file *f,
-			  struct fio_zone_info *z)
-{
-	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 (!data_in_zone)
-		return 0;
-
-	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, z));
-	switch (f->zbd_info->model) {
-	case ZBD_HOST_AWARE:
-	case ZBD_HOST_MANAGED:
-		ret = zbd_reset_wp(td, f, offset, length);
-		if (ret < 0)
-			return ret;
-		break;
-	default:
-		break;
-	}
-
-	pthread_mutex_lock(&f->zbd_info->mutex);
-	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;
-
-	td->ts.nr_zone_resets++;
-
-	return ret;
-}
-
-/* The caller must hold f->zbd_info->mutex */
-static void zbd_close_zone(struct thread_data *td, const struct fio_file *f,
-			   unsigned int zone_idx)
-{
-	uint32_t open_zone_idx = 0;
-
-	for (; open_zone_idx < f->zbd_info->num_open_zones; open_zone_idx++) {
-		if (f->zbd_info->open_zones[open_zone_idx] == zone_idx)
-			break;
-	}
-	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,
-		f->zbd_info->open_zones + open_zone_idx + 1,
-		(ZBD_MAX_OPEN_ZONES - (open_zone_idx + 1)) *
-		sizeof(f->zbd_info->open_zones[0]));
-	f->zbd_info->num_open_zones--;
-	td->num_open_zones--;
-	get_zone(f, zone_idx)->open = 0;
-}
-
-/*
- * Reset a range of zones. Returns 0 upon success and 1 upon failure.
- * @td: fio thread data.
- * @f: fio file for which to reset zones
- * @zb: first zone to reset.
- * @ze: first zone not to reset.
- */
-static int zbd_reset_zones(struct thread_data *td, struct fio_file *f,
-			   struct fio_zone_info *const zb,
-			   struct fio_zone_info *const ze)
-{
-	struct fio_zone_info *z;
-	const uint64_t min_bs = td->o.min_bs[DDIR_WRITE];
-	int res = 0;
-
-	assert(min_bs);
-
-	dprint(FD_ZBD, "%s: examining zones %u .. %u\n", f->file_name,
-		zbd_zone_nr(f, zb), zbd_zone_nr(f, ze));
-	for (z = zb; z < ze; z++) {
-		uint32_t nz = zbd_zone_nr(f, z);
-
-		if (!z->has_wp)
-			continue;
-		zone_lock(td, f, z);
-		pthread_mutex_lock(&f->zbd_info->mutex);
-		zbd_close_zone(td, f, nz);
-		pthread_mutex_unlock(&f->zbd_info->mutex);
-		if (z->wp != z->start) {
-			dprint(FD_ZBD, "%s: resetting zone %u\n",
-			       f->file_name, zbd_zone_nr(f, z));
-			if (zbd_reset_zone(td, f, z) < 0)
-				res = 1;
-		}
-		zone_unlock(z);
-	}
-
-	return res;
-}
-
 /*
  * Reset zbd_info.write_cnt, the counter that counts down towards the next
  * zone reset.
@@ -1112,86 +1212,6 @@ void zbd_file_reset(struct thread_data *td, struct fio_file *f)
 	zbd_reset_write_cnt(td, f);
 }
 
-/* The caller must hold f->zbd_info->mutex. */
-static bool is_zone_open(const struct thread_data *td, const struct fio_file *f,
-			 unsigned int zone_idx)
-{
-	struct zoned_block_device_info *zbdi = f->zbd_info;
-	int i;
-
-	/* This function should never be called when zbdi->max_open_zones == 0 */
-	assert(zbdi->max_open_zones);
-	assert(td->o.job_max_open_zones == 0 || td->num_open_zones <= td->o.job_max_open_zones);
-	assert(td->o.job_max_open_zones <= zbdi->max_open_zones);
-	assert(zbdi->num_open_zones <= zbdi->max_open_zones);
-
-	for (i = 0; i < zbdi->num_open_zones; i++)
-		if (zbdi->open_zones[i] == zone_idx)
-			return true;
-
-	return false;
-}
-
-/*
- * Open a ZBD zone if it was not yet open. Returns true if either the zone was
- * already open or if opening a new zone is allowed. Returns false if the zone
- * was not yet open and opening a new zone would cause the zone limit to be
- * exceeded.
- */
-static bool zbd_open_zone(struct thread_data *td, const struct fio_file *f,
-			  uint32_t zone_idx)
-{
-	const uint64_t min_bs = td->o.min_bs[DDIR_WRITE];
-	struct zoned_block_device_info *zbdi = f->zbd_info;
-	struct fio_zone_info *z = get_zone(f, zone_idx);
-	bool res = true;
-
-	if (z->cond == ZBD_ZONE_COND_OFFLINE)
-		return false;
-
-	/*
-	 * Skip full zones with data verification enabled because resetting a
-	 * zone causes data loss and hence causes verification to fail.
-	 */
-	if (td->o.verify != VERIFY_NONE && zbd_zone_full(f, z, min_bs))
-		return false;
-
-	/*
-	 * zbdi->max_open_zones == 0 means that there is no limit on the maximum
-	 * number of open zones. In this case, do no track open zones in
-	 * zbdi->open_zones array.
-	 */
-	if (!zbdi->max_open_zones)
-		return true;
-
-	pthread_mutex_lock(&zbdi->mutex);
-	if (is_zone_open(td, f, zone_idx)) {
-		/*
-		 * If the zone is already open and going to be full by writes
-		 * in-flight, handle it as a full zone instead of an open zone.
-		 */
-		if (z->wp >= zbd_zone_capacity_end(z))
-			res = false;
-		goto out;
-	}
-	res = false;
-	/* Zero means no limit */
-	if (td->o.job_max_open_zones > 0 &&
-	    td->num_open_zones >= td->o.job_max_open_zones)
-		goto out;
-	if (zbdi->num_open_zones >= zbdi->max_open_zones)
-		goto out;
-	dprint(FD_ZBD, "%s: opening zone %d\n", f->file_name, zone_idx);
-	zbdi->open_zones[zbdi->num_open_zones++] = zone_idx;
-	td->num_open_zones++;
-	z->open = 1;
-	res = true;
-
-out:
-	pthread_mutex_unlock(&zbdi->mutex);
-	return res;
-}
-
 /* Return random zone index for one of the open zones. */
 static uint32_t pick_random_zone_idx(const struct fio_file *f,
 				     const struct io_u *io_u)
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 04/12] zbd: remove is_zone_open() helper
  2021-12-14  1:24 [PATCH v2 00/12] Cleanup zbd code Damien Le Moal
                   ` (2 preceding siblings ...)
  2021-12-14  1:24 ` [PATCH v2 03/12] zbd: move and cleanup code Damien Le Moal
@ 2021-12-14  1:24 ` Damien Le Moal
  2021-12-14  1:24 ` [PATCH v2 05/12] zbd: introduce zbd_zone_align_file_sizes() helper Damien Le Moal
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 18+ messages in thread
From: Damien Le Moal @ 2021-12-14  1:24 UTC (permalink / raw)
  To: fio, Jens Axboe; +Cc: Niklas Cassel, Shinichiro Kawasaki

The helper function is_zone_open() is useless as a each zone has an open
flag indicating if it is part of the array of open zones. Remove this
function code and use the zone open flag in zbd_open_zone().

Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com>
---
 zbd.c | 38 +++++---------------------------------
 1 file changed, 5 insertions(+), 33 deletions(-)

diff --git a/zbd.c b/zbd.c
index 20d53b15..70afdd82 100644
--- a/zbd.c
+++ b/zbd.c
@@ -377,36 +377,6 @@ static int zbd_get_max_open_zones(struct thread_data *td, struct fio_file *f,
 	return ret;
 }
 
-/**
- * is_zone_open - Test if a zone is already in the array of open zones.
- * @td: fio thread data.
- * @f: fio file for which to test zones.
- * @zone_idx: Index of the zone to check.
- *
- * The caller must hold f->zbd_info->mutex.
- */
-static bool is_zone_open(const struct thread_data *td, const struct fio_file *f,
-			 unsigned int zone_idx)
-{
-	struct zoned_block_device_info *zbdi = f->zbd_info;
-	int i;
-
-	/*
-	 * This function should never be called when zbdi->max_open_zones == 0.
-	 */
-	assert(zbdi->max_open_zones);
-	assert(td->o.job_max_open_zones == 0 ||
-	       td->num_open_zones <= td->o.job_max_open_zones);
-	assert(td->o.job_max_open_zones <= zbdi->max_open_zones);
-	assert(zbdi->num_open_zones <= zbdi->max_open_zones);
-
-	for (i = 0; i < zbdi->num_open_zones; i++)
-		if (zbdi->open_zones[i] == zone_idx)
-			return true;
-
-	return false;
-}
-
 /**
  * zbd_open_zone - Add a zone to the array of open zones.
  * @td: fio thread data.
@@ -446,10 +416,12 @@ static bool zbd_open_zone(struct thread_data *td, const struct fio_file *f,
 		return true;
 
 	pthread_mutex_lock(&zbdi->mutex);
-	if (is_zone_open(td, f, zone_idx)) {
+
+	if (z->open) {
 		/*
-		 * If the zone is already open and going to be full by writes
-		 * in-flight, handle it as a full zone instead of an open zone.
+		 * If the zone is going to be completely filled by writes
+		 * already in-flight, handle it as a full zone instead of an
+		 * open zone.
 		 */
 		if (z->wp >= zbd_zone_capacity_end(z))
 			res = false;
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 05/12] zbd: introduce zbd_zone_align_file_sizes() helper
  2021-12-14  1:24 [PATCH v2 00/12] Cleanup zbd code Damien Le Moal
                   ` (3 preceding siblings ...)
  2021-12-14  1:24 ` [PATCH v2 04/12] zbd: remove is_zone_open() helper Damien Le Moal
@ 2021-12-14  1:24 ` Damien Le Moal
  2021-12-14  8:33   ` Niklas Cassel
  2021-12-14  1:24 ` [PATCH v2 06/12] zbd: fix code style issues Damien Le Moal
                   ` (7 subsequent siblings)
  12 siblings, 1 reply; 18+ messages in thread
From: Damien Le Moal @ 2021-12-14  1:24 UTC (permalink / raw)
  To: fio, Jens Axboe; +Cc: Niklas Cassel, Shinichiro Kawasaki

Move the code for the innermost loop of the function zbd_verify_sizes()
to the new helper function zbd_zone_align_file_sizes(). This helper
avoids large indentation of the code in zbd_verify_sizes() and makes
the code easier to read.

No functional changes.

Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
---
 zbd.c | 138 ++++++++++++++++++++++++++++++++--------------------------
 1 file changed, 77 insertions(+), 61 deletions(-)

diff --git a/zbd.c b/zbd.c
index 70afdd82..11c15c62 100644
--- a/zbd.c
+++ b/zbd.c
@@ -482,79 +482,95 @@ static bool zbd_is_seq_job(struct fio_file *f)
 	return false;
 }
 
+/*
+ * Verify whether the file offset and size parameters are aligned with zone
+ * boundaries. If the file offset is not aligned, align it down to the start of
+ * the zone containing the start offset and align up the file io_size parameter.
+ */
+static bool zbd_zone_align_file_sizes(struct thread_data *td,
+				      struct fio_file *f)
+{
+	const struct fio_zone_info *z;
+	uint64_t new_offset, new_end;
+	uint32_t zone_idx;
+
+	if (!f->zbd_info)
+		return true;
+	if (f->file_offset >= f->real_file_size)
+		return true;
+	if (!zbd_is_seq_job(f))
+		return true;
+
+	if (!td->o.zone_size) {
+		td->o.zone_size = f->zbd_info->zone_size;
+		if (!td->o.zone_size) {
+			log_err("%s: invalid 0 zone size\n",
+				f->file_name);
+			return false;
+		}
+	} else if (td->o.zone_size != f->zbd_info->zone_size) {
+		log_err("%s: zonesize %llu does not match the device zone size %"PRIu64".\n",
+			f->file_name, td->o.zone_size,
+			f->zbd_info->zone_size);
+		return false;
+	}
+
+	if (td->o.zone_skip % td->o.zone_size) {
+		log_err("%s: zoneskip %llu is not a multiple of the device zone size %llu.\n",
+			f->file_name, td->o.zone_skip,
+			td->o.zone_size);
+		return false;
+	}
+
+	zone_idx = zbd_zone_idx(f, f->file_offset);
+	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);
+		if (new_offset >= f->file_offset + f->io_size) {
+			log_info("%s: io_size must be at least one zone\n",
+				 f->file_name);
+			return false;
+		}
+		log_info("%s: rounded up offset from %"PRIu64" to %"PRIu64"\n",
+			 f->file_name, f->file_offset,
+			 new_offset);
+		f->io_size -= (new_offset - f->file_offset);
+		f->file_offset = new_offset;
+	}
+
+	zone_idx = zbd_zone_idx(f, f->file_offset + f->io_size);
+	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)) {
+		if (new_end <= f->file_offset) {
+			log_info("%s: io_size must be at least one zone\n",
+				 f->file_name);
+			return false;
+		}
+		log_info("%s: rounded down io_size from %"PRIu64" to %"PRIu64"\n",
+			 f->file_name, f->io_size,
+			 new_end - f->file_offset);
+		f->io_size = new_end - f->file_offset;
+	}
+
+	return true;
+}
+
 /*
  * Verify whether offset and size parameters are aligned with zone boundaries.
  */
 static bool zbd_verify_sizes(void)
 {
-	const struct fio_zone_info *z;
 	struct thread_data *td;
 	struct fio_file *f;
-	uint64_t new_offset, new_end;
-	uint32_t zone_idx;
 	int i, j;
 
 	for_each_td(td, i) {
 		for_each_file(td, f, j) {
-			if (!f->zbd_info)
-				continue;
-			if (f->file_offset >= f->real_file_size)
-				continue;
-			if (!zbd_is_seq_job(f))
-				continue;
-
-			if (!td->o.zone_size) {
-				td->o.zone_size = f->zbd_info->zone_size;
-				if (!td->o.zone_size) {
-					log_err("%s: invalid 0 zone size\n",
-						f->file_name);
-					return false;
-				}
-			} else if (td->o.zone_size != f->zbd_info->zone_size) {
-				log_err("%s: job parameter zonesize %llu does not match disk zone size %"PRIu64".\n",
-					f->file_name, td->o.zone_size,
-					f->zbd_info->zone_size);
+			if (!zbd_zone_align_file_sizes(td, f))
 				return false;
-			}
-
-			if (td->o.zone_skip % td->o.zone_size) {
-				log_err("%s: zoneskip %llu is not a multiple of the device zone size %llu.\n",
-					f->file_name, td->o.zone_skip,
-					td->o.zone_size);
-				return false;
-			}
-
-			zone_idx = zbd_zone_idx(f, f->file_offset);
-			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);
-				if (new_offset >= f->file_offset + f->io_size) {
-					log_info("%s: io_size must be at least one zone\n",
-						 f->file_name);
-					return false;
-				}
-				log_info("%s: rounded up offset from %"PRIu64" to %"PRIu64"\n",
-					 f->file_name, f->file_offset,
-					 new_offset);
-				f->io_size -= (new_offset - f->file_offset);
-				f->file_offset = new_offset;
-			}
-			zone_idx = zbd_zone_idx(f, f->file_offset + f->io_size);
-			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)) {
-				if (new_end <= f->file_offset) {
-					log_info("%s: io_size must be at least one zone\n",
-						 f->file_name);
-					return false;
-				}
-				log_info("%s: rounded down io_size from %"PRIu64" to %"PRIu64"\n",
-					 f->file_name, f->io_size,
-					 new_end - f->file_offset);
-				f->io_size = new_end - f->file_offset;
-			}
 		}
 	}
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 06/12] zbd: fix code style issues
  2021-12-14  1:24 [PATCH v2 00/12] Cleanup zbd code Damien Le Moal
                   ` (4 preceding siblings ...)
  2021-12-14  1:24 ` [PATCH v2 05/12] zbd: introduce zbd_zone_align_file_sizes() helper Damien Le Moal
@ 2021-12-14  1:24 ` Damien Le Moal
  2021-12-14  1:24 ` [PATCH v2 07/12] zbd: simplify zbd_close_zone() Damien Le Moal
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 18+ messages in thread
From: Damien Le Moal @ 2021-12-14  1:24 UTC (permalink / raw)
  To: fio, Jens Axboe; +Cc: Niklas Cassel, Shinichiro Kawasaki

Avoid overly long lines, remove unnecessary curly brackets and add blank
lines to make the code more readable.

No functional changes.

Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com>
---
 zbd.c | 177 +++++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 125 insertions(+), 52 deletions(-)

diff --git a/zbd.c b/zbd.c
index 11c15c62..b87fefc4 100644
--- a/zbd.c
+++ b/zbd.c
@@ -252,8 +252,9 @@ 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, z));
+	dprint(FD_ZBD, "%s: resetting wp of zone %u.\n",
+	       f->file_name, zbd_zone_nr(f, z));
+
 	switch (f->zbd_info->model) {
 	case ZBD_HOST_AWARE:
 	case ZBD_HOST_MANAGED:
@@ -269,6 +270,7 @@ static int zbd_reset_zone(struct thread_data *td, struct fio_file *f,
 	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;
 
@@ -297,11 +299,14 @@ static void zbd_close_zone(struct thread_data *td, const struct fio_file *f,
 	if (open_zone_idx == f->zbd_info->num_open_zones)
 		return;
 
-	dprint(FD_ZBD, "%s: closing zone %d\n", f->file_name, zone_idx);
+	dprint(FD_ZBD, "%s: closing zone %d\n",
+	       f->file_name, zone_idx);
+
 	memmove(f->zbd_info->open_zones + open_zone_idx,
 		f->zbd_info->open_zones + open_zone_idx + 1,
 		(ZBD_MAX_OPEN_ZONES - (open_zone_idx + 1)) *
 		sizeof(f->zbd_info->open_zones[0]));
+
 	f->zbd_info->num_open_zones--;
 	td->num_open_zones--;
 	get_zone(f, zone_idx)->open = 0;
@@ -326,23 +331,27 @@ 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, zb), zbd_zone_nr(f, ze));
+	dprint(FD_ZBD, "%s: examining zones %u .. %u\n",
+	       f->file_name, zbd_zone_nr(f, zb), zbd_zone_nr(f, ze));
+
 	for (z = zb; z < ze; z++) {
 		uint32_t nz = zbd_zone_nr(f, z);
 
 		if (!z->has_wp)
 			continue;
+
 		zone_lock(td, f, z);
 		pthread_mutex_lock(&f->zbd_info->mutex);
 		zbd_close_zone(td, f, nz);
 		pthread_mutex_unlock(&f->zbd_info->mutex);
+
 		if (z->wp != z->start) {
 			dprint(FD_ZBD, "%s: resetting zone %u\n",
 			       f->file_name, zbd_zone_nr(f, z));
 			if (zbd_reset_zone(td, f, z) < 0)
 				res = 1;
 		}
+
 		zone_unlock(z);
 	}
 
@@ -427,6 +436,7 @@ static bool zbd_open_zone(struct thread_data *td, const struct fio_file *f,
 			res = false;
 		goto out;
 	}
+
 	res = false;
 	/* Zero means no limit */
 	if (td->o.job_max_open_zones > 0 &&
@@ -434,7 +444,10 @@ static bool zbd_open_zone(struct thread_data *td, const struct fio_file *f,
 		goto out;
 	if (zbdi->num_open_zones >= zbdi->max_open_zones)
 		goto out;
-	dprint(FD_ZBD, "%s: opening zone %d\n", f->file_name, zone_idx);
+
+	dprint(FD_ZBD, "%s: opening zone %d\n",
+	       f->file_name, zone_idx);
+
 	zbdi->open_zones[zbdi->num_open_zones++] = zone_idx;
 	td->num_open_zones++;
 	z->open = 1;
@@ -471,8 +484,10 @@ static bool zbd_is_seq_job(struct fio_file *f)
 	uint32_t zone_idx, zone_idx_b, zone_idx_e;
 
 	assert(f->zbd_info);
+
 	if (f->io_size == 0)
 		return false;
+
 	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++)
@@ -595,6 +610,7 @@ static bool zbd_verify_bs(void)
 
 			if (!f->zbd_info)
 				continue;
+
 			zone_size = f->zbd_info->zone_size;
 			if (td_trim(td) && td->o.bs[DDIR_TRIM] != zone_size) {
 				log_info("%s: trim block size %llu is not the zone size %"PRIu64"\n",
@@ -739,8 +755,8 @@ static int parse_zone_info(struct thread_data *td, struct fio_file *f)
 		goto out;
 	}
 
-	dprint(FD_ZBD, "Device %s has %d zones of size %"PRIu64" KB\n", f->file_name,
-	       nr_zones, zone_size / 1024);
+	dprint(FD_ZBD, "Device %s has %d zones of size %"PRIu64" KB\n",
+	       f->file_name, nr_zones, zone_size / 1024);
 
 	zbd_info = scalloc(1, sizeof(*zbd_info) +
 			   (nr_zones + 1) * sizeof(zbd_info->zone_info[0]));
@@ -756,6 +772,7 @@ static int parse_zone_info(struct thread_data *td, struct fio_file *f)
 						     PTHREAD_MUTEX_RECURSIVE);
 			p->start = z->start;
 			p->capacity = z->capacity;
+
 			switch (z->cond) {
 			case ZBD_ZONE_COND_NOT_WP:
 			case ZBD_ZONE_COND_FULL:
@@ -789,6 +806,7 @@ 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,
 				       min((uint32_t)(nr_zones - j),
 					   ZBD_REPORT_MAX_ZONES));
@@ -856,7 +874,8 @@ out:
 	/* Ensure that the limit is not larger than FIO's internal limit */
 	if (zbd->max_open_zones > ZBD_MAX_OPEN_ZONES) {
 		td_verror(td, EINVAL, "'max_open_zones' value is too large");
-		log_err("'max_open_zones' value is larger than %u\n", ZBD_MAX_OPEN_ZONES);
+		log_err("'max_open_zones' value is larger than %u\n",
+			ZBD_MAX_OPEN_ZONES);
 		return -EINVAL;
 	}
 
@@ -958,6 +977,7 @@ static int zbd_init_zone_info(struct thread_data *td, struct fio_file *file)
 	ret = zbd_create_zone_info(td, file);
 	if (ret < 0)
 		td_verror(td, -ret, "zbd_create_zone_info() failed");
+
 	return ret;
 }
 
@@ -970,6 +990,7 @@ int zbd_init_files(struct thread_data *td)
 		if (zbd_init_zone_info(td, f))
 			return 1;
 	}
+
 	return 0;
 }
 
@@ -980,27 +1001,24 @@ void zbd_recalc_options_with_zone_granularity(struct thread_data *td)
 
 	for_each_file(td, f, i) {
 		struct zoned_block_device_info *zbd = f->zbd_info;
-		// zonemode=strided doesn't get per-file zone size.
-		uint64_t zone_size = zbd ? zbd->zone_size : td->o.zone_size;
+		uint64_t zone_size;
 
+		/* zonemode=strided doesn't get per-file zone size. */
+		zone_size = zbd ? zbd->zone_size : td->o.zone_size;
 		if (zone_size == 0)
 			continue;
 
-		if (td->o.size_nz > 0) {
+		if (td->o.size_nz > 0)
 			td->o.size = td->o.size_nz * zone_size;
-		}
-		if (td->o.io_size_nz > 0) {
+		if (td->o.io_size_nz > 0)
 			td->o.io_size = td->o.io_size_nz * zone_size;
-		}
-		if (td->o.start_offset_nz > 0) {
+		if (td->o.start_offset_nz > 0)
 			td->o.start_offset = td->o.start_offset_nz * zone_size;
-		}
-		if (td->o.offset_increment_nz > 0) {
-			td->o.offset_increment = td->o.offset_increment_nz * zone_size;
-		}
-		if (td->o.zone_skip_nz > 0) {
+		if (td->o.offset_increment_nz > 0)
+			td->o.offset_increment =
+				td->o.offset_increment_nz * zone_size;
+		if (td->o.zone_skip_nz > 0)
 			td->o.zone_skip = td->o.zone_skip_nz * zone_size;
-		}
 	}
 }
 
@@ -1143,6 +1161,7 @@ static uint64_t zbd_process_swd(struct thread_data *td,
 		}
 		swd += z->wp - z->start;
 	}
+
 	pthread_mutex_lock(&f->zbd_info->mutex);
 	switch (a) {
 	case CHECK_SWD:
@@ -1155,6 +1174,7 @@ static uint64_t zbd_process_swd(struct thread_data *td,
 		break;
 	}
 	pthread_mutex_unlock(&f->zbd_info->mutex);
+
 	for (z = zb; z < ze; z++)
 		if (z->has_wp)
 			zone_unlock(z);
@@ -1188,8 +1208,10 @@ void zbd_file_reset(struct thread_data *td, struct fio_file *f)
 	zb = get_zone(f, f->min_zone);
 	ze = get_zone(f, f->max_zone);
 	swd = zbd_process_swd(td, f, SET_SWD);
-	dprint(FD_ZBD, "%s(%s): swd = %" PRIu64 "\n", __func__, f->file_name,
-	       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
@@ -1204,8 +1226,8 @@ void zbd_file_reset(struct thread_data *td, struct fio_file *f)
 static uint32_t pick_random_zone_idx(const struct fio_file *f,
 				     const struct io_u *io_u)
 {
-	return (io_u->offset - f->file_offset) * f->zbd_info->num_open_zones /
-		f->io_size;
+	return (io_u->offset - f->file_offset) *
+		f->zbd_info->num_open_zones / f->io_size;
 }
 
 static bool any_io_in_flight(void)
@@ -1258,7 +1280,9 @@ static struct fio_zone_info *zbd_convert_to_open_zone(struct thread_data *td,
 		zone_idx = f->min_zone;
 	else if (zone_idx >= f->max_zone)
 		zone_idx = f->max_zone - 1;
-	dprint(FD_ZBD, "%s(%s): starting from zone %d (offset %lld, buflen %lld)\n",
+
+	dprint(FD_ZBD,
+	       "%s(%s): starting from zone %d (offset %lld, buflen %lld)\n",
 	       __func__, f->file_name, zone_idx, io_u->offset, io_u->buflen);
 
 	/*
@@ -1273,10 +1297,13 @@ static struct fio_zone_info *zbd_convert_to_open_zone(struct thread_data *td,
 		z = get_zone(f, zone_idx);
 		if (z->has_wp)
 			zone_lock(td, f, z);
+
 		pthread_mutex_lock(&zbdi->mutex);
+
 		if (z->has_wp) {
 			if (z->cond != ZBD_ZONE_COND_OFFLINE &&
-			    zbdi->max_open_zones == 0 && td->o.job_max_open_zones == 0)
+			    zbdi->max_open_zones == 0 &&
+			    td->o.job_max_open_zones == 0)
 				goto examine_zone;
 			if (zbdi->num_open_zones == 0) {
 				dprint(FD_ZBD, "%s(%s): no zones are open\n",
@@ -1286,14 +1313,15 @@ static struct fio_zone_info *zbd_convert_to_open_zone(struct thread_data *td,
 		}
 
 		/*
-		 * List of opened zones is per-device, shared across all threads.
-		 * Start with quasi-random candidate zone.
-		 * Ignore zones which don't belong to thread's offset/size area.
+		 * List of opened zones is per-device, shared across all
+		 * threads. Start with quasi-random candidate zone. 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 ||
 		       open_zone_idx < zbdi->num_open_zones);
 		tmp_idx = open_zone_idx;
+
 		for (i = 0; i < zbdi->num_open_zones; i++) {
 			uint32_t tmpz;
 
@@ -1310,9 +1338,12 @@ 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(&zbdi->mutex);
+
 		if (z->has_wp)
 			zone_unlock(z);
+
 		return NULL;
 
 found_candidate_zone:
@@ -1320,7 +1351,9 @@ found_candidate_zone:
 		if (new_zone_idx == zone_idx)
 			break;
 		zone_idx = new_zone_idx;
+
 		pthread_mutex_unlock(&zbdi->mutex);
+
 		if (z->has_wp)
 			zone_unlock(z);
 	}
@@ -1351,7 +1384,8 @@ open_other_zone:
 	 * zone close before opening a new zone.
 	 */
 	if (wait_zone_close) {
-		dprint(FD_ZBD, "%s(%s): quiesce to allow open zones to close\n",
+		dprint(FD_ZBD,
+		       "%s(%s): quiesce to allow open zones to close\n",
 		       __func__, f->file_name);
 		io_u_quiesce(td);
 	}
@@ -1404,7 +1438,8 @@ retry:
 	 */
 	in_flight = any_io_in_flight();
 	if (in_flight || should_retry) {
-		dprint(FD_ZBD, "%s(%s): wait zone close and retry open zones\n",
+		dprint(FD_ZBD,
+		       "%s(%s): wait zone close and retry open zones\n",
 		       __func__, f->file_name);
 		pthread_mutex_unlock(&zbdi->mutex);
 		zone_unlock(z);
@@ -1415,17 +1450,22 @@ retry:
 	}
 
 	pthread_mutex_unlock(&zbdi->mutex);
+
 	zone_unlock(z);
-	dprint(FD_ZBD, "%s(%s): did not open another zone\n", __func__,
-	       f->file_name);
+
+	dprint(FD_ZBD, "%s(%s): did not open another zone\n",
+	       __func__, f->file_name);
+
 	return NULL;
 
 out:
-	dprint(FD_ZBD, "%s(%s): returning zone %d\n", __func__, f->file_name,
-	       zone_idx);
+	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;
 }
 
@@ -1444,18 +1484,20 @@ static struct fio_zone_info *zbd_replay_write_order(struct thread_data *td,
 	}
 
 	if (z->verify_block * min_bs >= z->capacity) {
-		log_err("%s: %d * %"PRIu64" >= %"PRIu64"\n", f->file_name, z->verify_block,
-			min_bs, z->capacity);
+		log_err("%s: %d * %"PRIu64" >= %"PRIu64"\n",
+			f->file_name, z->verify_block, min_bs, z->capacity);
 		/*
 		 * If the assertion below fails during a test run, adding
 		 * "--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 >= %"PRIu64"\n", f->file_name, io_u->offset,
-			io_u->buflen, zbd_zone_capacity_end(z));
+		log_err("%s: %llu + %llu >= %"PRIu64"\n",
+			f->file_name, io_u->offset, io_u->buflen,
+			zbd_zone_capacity_end(z));
 		assert(false);
 	}
 	z->verify_block += io_u->buflen / min_bs;
@@ -1493,6 +1535,7 @@ zbd_find_zone(struct thread_data *td, struct io_u *io_u, uint64_t min_bytes,
 		} else if (!td_random(td)) {
 			break;
 		}
+
 		if (td_random(td) && z2 >= zf &&
 		    z2->cond != ZBD_ZONE_COND_OFFLINE) {
 			if (z2->has_wp)
@@ -1503,8 +1546,11 @@ zbd_find_zone(struct thread_data *td, struct io_u *io_u, uint64_t min_bytes,
 				zone_unlock(z2);
 		}
 	}
-	dprint(FD_ZBD, "%s: no zone has %"PRIu64" bytes of readable data\n",
+
+	dprint(FD_ZBD,
+	       "%s: no zone has %"PRIu64" bytes of readable data\n",
 	       f->file_name, min_bytes);
+
 	return NULL;
 }
 
@@ -1567,11 +1613,12 @@ static void zbd_queue_io(struct thread_data *td, struct io_u *io_u, int q,
 	case DDIR_WRITE:
 		zone_end = min((uint64_t)(io_u->offset + io_u->buflen),
 			       zbd_zone_capacity_end(z));
-		pthread_mutex_lock(&zbd_info->mutex);
+
 		/*
 		 * z->wp > zone_end means that one or more I/O errors
 		 * have occurred.
 		 */
+		pthread_mutex_lock(&zbd_info->mutex);
 		if (z->wp <= zone_end) {
 			zbd_info->sectors_with_data += zone_end - z->wp;
 			zbd_info->wp_sectors_with_data += zone_end - z->wp;
@@ -1671,8 +1718,8 @@ void setup_zbd_zone_mode(struct thread_data *td, struct io_u *io_u)
 	 * sequential write, skip to zone end if the latest position is at the
 	 * zone capacity limit.
 	 */
-	if (z->capacity < f->zbd_info->zone_size && !td_random(td) &&
-	    ddir == DDIR_WRITE &&
+	if (z->capacity < f->zbd_info->zone_size &&
+	    !td_random(td) && ddir == DDIR_WRITE &&
 	    f->last_pos[ddir] >= zbd_zone_capacity_end(z)) {
 		dprint(FD_ZBD,
 		       "%s: Jump from zone capacity limit to zone end:"
@@ -1770,6 +1817,7 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 	assert(min_bs);
 	assert(is_valid_offset(f, io_u->offset));
 	assert(io_u->buflen);
+
 	zone_idx_b = zbd_zone_idx(f, io_u->offset);
 	zb = get_zone(f, zone_idx_b);
 	orig_zb = zb;
@@ -1778,6 +1826,7 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 		/* 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.
@@ -1791,12 +1840,16 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 			       "%s: off=%llu + min_bs=%"PRIu64" > next zone %"PRIu64"\n",
 			       f->file_name, io_u->offset,
 			       min_bs, (zb + 1)->start);
-			io_u->offset = zb->start + (zb + 1)->start - io_u->offset;
-			new_len = min(io_u->buflen, (zb + 1)->start - io_u->offset);
+			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;
 	}
 
@@ -1818,6 +1871,7 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 			zb = zbd_replay_write_order(td, io_u, zb);
 			goto accept;
 		}
+
 		/*
 		 * Check that there is enough written data in the zone to do an
 		 * I/O of at least min_bs B. If there isn't, find a new zone for
@@ -1847,6 +1901,7 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 			if (!td_random(td))
 				io_u->offset = zb->start;
 		}
+
 		/*
 		 * Make sure the I/O is within the zone valid data range while
 		 * maximizing the I/O size and preserving randomness.
@@ -1857,12 +1912,14 @@ 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.
 		 */
@@ -1874,9 +1931,12 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 			dprint(FD_IO, "Changed length from %u into %llu\n",
 			       orig_len, io_u->buflen);
 		}
+
 		assert(zb->start <= io_u->offset);
 		assert(io_u->offset + io_u->buflen <= zb->wp);
+
 		goto accept;
+
 	case DDIR_WRITE:
 		if (io_u->buflen > zbdi->zone_size) {
 			td_verror(td, EINVAL, "I/O buflen exceeds zone size");
@@ -1885,6 +1945,7 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 			       f->file_name, io_u->buflen, zbdi->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);
@@ -1894,14 +1955,14 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 				goto eof;
 			}
 		}
+
 		/* Check whether the zone reset threshold has been exceeded */
 		if (td->o.zrf.u.f) {
-			if (zbdi->wp_sectors_with_data >=
-			    f->io_size * td->o.zrt.u.f &&
-			    zbd_dec_and_reset_write_cnt(td, f)) {
+			if (zbdi->wp_sectors_with_data >= f->io_size * td->o.zrt.u.f &&
+			    zbd_dec_and_reset_write_cnt(td, f))
 				zb->reset_zone = 1;
-			}
 		}
+
 		/* Reset the zone pointer if necessary */
 		if (zb->reset_zone || zbd_zone_full(f, zb, min_bs)) {
 			assert(td->o.verify == VERIFY_NONE);
@@ -1924,6 +1985,7 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 				goto eof;
 			}
 		}
+
 		/* Make writes occur at the write pointer */
 		assert(!zbd_zone_full(f, zb, min_bs));
 		io_u->offset = zb->wp;
@@ -1933,6 +1995,7 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 			       f->file_name, io_u->offset);
 			goto eof;
 		}
+
 		/*
 		 * Make sure that the buflen is a multiple of the minimal
 		 * block size. Give up if shrinking would make the request too
@@ -1949,10 +2012,13 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 			       orig_len, io_u->buflen);
 			goto accept;
 		}
+
 		td_verror(td, EIO, "zone remainder too small");
 		log_err("zone remainder %lld smaller than min block size %"PRIu64"\n",
 			(zbd_zone_capacity_end(zb) - io_u->offset), min_bs);
+
 		goto eof;
+
 	case DDIR_TRIM:
 		/* Check random trim targets a non-empty zone */
 		if (!td_random(td) || zb->wp > zb->start)
@@ -1968,7 +2034,9 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 			       f->file_name, io_u->offset);
 			goto accept;
 		}
+
 		goto eof;
+
 	case DDIR_SYNC:
 		/* fall-through */
 	case DDIR_DATASYNC:
@@ -1986,19 +2054,23 @@ accept:
 	assert(zb->cond != ZBD_ZONE_COND_OFFLINE);
 	assert(!io_u->zbd_queue_io);
 	assert(!io_u->zbd_put_io);
+
 	io_u->zbd_queue_io = zbd_queue_io;
 	io_u->zbd_put_io = zbd_put_io;
+
 	/*
 	 * Since we return with the zone lock still held,
 	 * add an annotation to let Coverity know that it
 	 * is intentional.
 	 */
 	/* coverity[missing_unlock] */
+
 	return io_u_accept;
 
 eof:
 	if (zb && zb->has_wp)
 		zone_unlock(zb);
+
 	return io_u_eof;
 }
 
@@ -2036,7 +2108,8 @@ int zbd_do_io_u_trim(const struct thread_data *td, struct io_u *io_u)
 		return 0;
 
 	if (io_u->offset != z->start) {
-		log_err("Trim offset not at zone start (%lld)\n", io_u->offset);
+		log_err("Trim offset not at zone start (%lld)\n",
+			io_u->offset);
 		return -EINVAL;
 	}
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 07/12] zbd: simplify zbd_close_zone()
  2021-12-14  1:24 [PATCH v2 00/12] Cleanup zbd code Damien Le Moal
                   ` (5 preceding siblings ...)
  2021-12-14  1:24 ` [PATCH v2 06/12] zbd: fix code style issues Damien Le Moal
@ 2021-12-14  1:24 ` Damien Le Moal
  2021-12-14  1:24 ` [PATCH v2 08/12] zbd: simplify zbd_open_zone() Damien Le Moal
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 18+ messages in thread
From: Damien Le Moal @ 2021-12-14  1:24 UTC (permalink / raw)
  To: fio, Jens Axboe; +Cc: Niklas Cassel, Shinichiro Kawasaki

Change the interface of zbd_close_zone() to directly use a pointer to a
zone information structure as all callers already have this information.
Also do nothing for zones that are not marked as open instead of
figuring out this fact by searching the array of open zones.

No functional changes.

Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com>
---
 zbd.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/zbd.c b/zbd.c
index b87fefc4..26387335 100644
--- a/zbd.c
+++ b/zbd.c
@@ -288,28 +288,31 @@ static int zbd_reset_zone(struct thread_data *td, struct fio_file *f,
  * The caller must hold f->zbd_info->mutex.
  */
 static void zbd_close_zone(struct thread_data *td, const struct fio_file *f,
-			   unsigned int zone_idx)
+			   struct fio_zone_info *z)
 {
-	uint32_t open_zone_idx = 0;
+	uint32_t ozi;
 
-	for (; open_zone_idx < f->zbd_info->num_open_zones; open_zone_idx++) {
-		if (f->zbd_info->open_zones[open_zone_idx] == zone_idx)
+	if (!z->open)
+		return;
+
+	for (ozi = 0; ozi < f->zbd_info->num_open_zones; ozi++) {
+		if (get_zone(f, f->zbd_info->open_zones[ozi]) == z)
 			break;
 	}
-	if (open_zone_idx == f->zbd_info->num_open_zones)
+	if (ozi == f->zbd_info->num_open_zones)
 		return;
 
-	dprint(FD_ZBD, "%s: closing zone %d\n",
-	       f->file_name, zone_idx);
+	dprint(FD_ZBD, "%s: closing zone %u\n",
+	       f->file_name, zbd_zone_nr(f, z));
 
-	memmove(f->zbd_info->open_zones + open_zone_idx,
-		f->zbd_info->open_zones + open_zone_idx + 1,
-		(ZBD_MAX_OPEN_ZONES - (open_zone_idx + 1)) *
+	memmove(f->zbd_info->open_zones + ozi,
+		f->zbd_info->open_zones + ozi + 1,
+		(ZBD_MAX_OPEN_ZONES - (ozi + 1)) *
 		sizeof(f->zbd_info->open_zones[0]));
 
 	f->zbd_info->num_open_zones--;
 	td->num_open_zones--;
-	get_zone(f, zone_idx)->open = 0;
+	z->open = 0;
 }
 
 /**
@@ -335,14 +338,12 @@ static int zbd_reset_zones(struct thread_data *td, struct fio_file *f,
 	       f->file_name, zbd_zone_nr(f, zb), zbd_zone_nr(f, ze));
 
 	for (z = zb; z < ze; z++) {
-		uint32_t nz = zbd_zone_nr(f, z);
-
 		if (!z->has_wp)
 			continue;
 
 		zone_lock(td, f, z);
 		pthread_mutex_lock(&f->zbd_info->mutex);
-		zbd_close_zone(td, f, nz);
+		zbd_close_zone(td, f, z);
 		pthread_mutex_unlock(&f->zbd_info->mutex);
 
 		if (z->wp != z->start) {
@@ -1571,7 +1572,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, zbd_zone_nr(f, z));
+		zbd_close_zone(td, f, z);
 		pthread_mutex_unlock(&f->zbd_info->mutex);
 	}
 }
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 08/12] zbd: simplify zbd_open_zone()
  2021-12-14  1:24 [PATCH v2 00/12] Cleanup zbd code Damien Le Moal
                   ` (6 preceding siblings ...)
  2021-12-14  1:24 ` [PATCH v2 07/12] zbd: simplify zbd_close_zone() Damien Le Moal
@ 2021-12-14  1:24 ` Damien Le Moal
  2021-12-14  1:24 ` [PATCH v2 09/12] zbd: rename zbd_zone_idx() and zbd_zone_nr() Damien Le Moal
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 18+ messages in thread
From: Damien Le Moal @ 2021-12-14  1:24 UTC (permalink / raw)
  To: fio, Jens Axboe; +Cc: Niklas Cassel, Shinichiro Kawasaki

Similarly to zbd_close_zone(), directly pass a pointer to a zone
information structure to zbd_open_zone() instead of a zone number.

No functional changes.

Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com>
---
 zbd.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/zbd.c b/zbd.c
index 26387335..b917fa42 100644
--- a/zbd.c
+++ b/zbd.c
@@ -400,11 +400,11 @@ static int zbd_get_max_open_zones(struct thread_data *td, struct fio_file *f,
  * to be exceeded.
  */
 static bool zbd_open_zone(struct thread_data *td, const struct fio_file *f,
-			  uint32_t zone_idx)
+			  struct fio_zone_info *z)
 {
 	const uint64_t min_bs = td->o.min_bs[DDIR_WRITE];
 	struct zoned_block_device_info *zbdi = f->zbd_info;
-	struct fio_zone_info *z = get_zone(f, zone_idx);
+	uint32_t zone_idx = zbd_zone_nr(f, z);
 	bool res = true;
 
 	if (z->cond == ZBD_ZONE_COND_OFFLINE)
@@ -446,7 +446,7 @@ static bool zbd_open_zone(struct thread_data *td, const struct fio_file *f,
 	if (zbdi->num_open_zones >= zbdi->max_open_zones)
 		goto out;
 
-	dprint(FD_ZBD, "%s: opening zone %d\n",
+	dprint(FD_ZBD, "%s: opening zone %u\n",
 	       f->file_name, zone_idx);
 
 	zbdi->open_zones[zbdi->num_open_zones++] = zone_idx;
@@ -1087,7 +1087,7 @@ int zbd_setup_files(struct thread_data *td)
 			if (z->cond != ZBD_ZONE_COND_IMP_OPEN &&
 			    z->cond != ZBD_ZONE_COND_EXP_OPEN)
 				continue;
-			if (zbd_open_zone(td, f, zi))
+			if (zbd_open_zone(td, f, z))
 				continue;
 			/*
 			 * If the number of open zones exceeds specified limits,
@@ -1409,7 +1409,7 @@ retry:
 		zone_lock(td, f, z);
 		if (z->open)
 			continue;
-		if (zbd_open_zone(td, f, zone_idx))
+		if (zbd_open_zone(td, f, z))
 			goto out;
 	}
 
@@ -1478,7 +1478,7 @@ static struct fio_zone_info *zbd_replay_write_order(struct thread_data *td,
 	const struct fio_file *f = io_u->file;
 	const uint64_t min_bs = td->o.min_bs[DDIR_WRITE];
 
-	if (!zbd_open_zone(td, f, zbd_zone_nr(f, z))) {
+	if (!zbd_open_zone(td, f, z)) {
 		zone_unlock(z);
 		z = zbd_convert_to_open_zone(td, io_u);
 		assert(z);
@@ -1947,7 +1947,7 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 			goto eof;
 		}
 
-		if (!zbd_open_zone(td, f, zone_idx_b)) {
+		if (!zbd_open_zone(td, f, zb)) {
 			zone_unlock(zb);
 			zb = zbd_convert_to_open_zone(td, io_u);
 			if (!zb) {
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 09/12] zbd: rename zbd_zone_idx() and zbd_zone_nr()
  2021-12-14  1:24 [PATCH v2 00/12] Cleanup zbd code Damien Le Moal
                   ` (7 preceding siblings ...)
  2021-12-14  1:24 ` [PATCH v2 08/12] zbd: simplify zbd_open_zone() Damien Le Moal
@ 2021-12-14  1:24 ` Damien Le Moal
  2021-12-14  8:33   ` Niklas Cassel
  2021-12-14  1:24 ` [PATCH v2 10/12] zbd: rename get_zone() Damien Le Moal
                   ` (3 subsequent siblings)
  12 siblings, 1 reply; 18+ messages in thread
From: Damien Le Moal @ 2021-12-14  1:24 UTC (permalink / raw)
  To: fio, Jens Axboe; +Cc: Niklas Cassel, Shinichiro Kawasaki

Rename zbd_zone_idx() to zbd_offset_to_zone_idx() to make it clear that
the argument determining the zone is a file offset. To be consistent,
rename zbd_zone_nr() to zbd_zone_idx() to avoid confusion with a number
of zones. While at it, have both functions return value be of the same
unsigned int type.

No functional changes.

Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
---
 zbd.c | 45 ++++++++++++++++++++++++---------------------
 1 file changed, 24 insertions(+), 21 deletions(-)

diff --git a/zbd.c b/zbd.c
index b917fa42..592f7c03 100644
--- a/zbd.c
+++ b/zbd.c
@@ -27,19 +27,20 @@ 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 unsigned int zbd_zone_nr(const struct fio_file *f,
-				       struct fio_zone_info *zone)
+static inline unsigned int zbd_zone_idx(const struct fio_file *f,
+					struct fio_zone_info *zone)
 {
 	return zone - f->zbd_info->zone_info;
 }
 
 /**
- * zbd_zone_idx - convert an offset into a zone number
+ * zbd_offset_to_zone_idx - convert an offset into a zone number
  * @f: file pointer.
  * @offset: offset in bytes. If this offset is in the first zone_size bytes
  *	    past the disk size then the index of the sentinel is returned.
  */
-static uint32_t zbd_zone_idx(const struct fio_file *f, uint64_t offset)
+static unsigned int zbd_offset_to_zone_idx(const struct fio_file *f,
+					   uint64_t offset)
 {
 	uint32_t zone_idx;
 
@@ -253,7 +254,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, z));
+	       f->file_name, zbd_zone_idx(f, z));
 
 	switch (f->zbd_info->model) {
 	case ZBD_HOST_AWARE:
@@ -303,7 +304,7 @@ static void zbd_close_zone(struct thread_data *td, const struct fio_file *f,
 		return;
 
 	dprint(FD_ZBD, "%s: closing zone %u\n",
-	       f->file_name, zbd_zone_nr(f, z));
+	       f->file_name, zbd_zone_idx(f, z));
 
 	memmove(f->zbd_info->open_zones + ozi,
 		f->zbd_info->open_zones + ozi + 1,
@@ -335,7 +336,7 @@ 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, zb), zbd_zone_nr(f, ze));
+	       f->file_name, zbd_zone_idx(f, zb), zbd_zone_idx(f, ze));
 
 	for (z = zb; z < ze; z++) {
 		if (!z->has_wp)
@@ -348,7 +349,7 @@ static int zbd_reset_zones(struct thread_data *td, struct fio_file *f,
 
 		if (z->wp != z->start) {
 			dprint(FD_ZBD, "%s: resetting zone %u\n",
-			       f->file_name, zbd_zone_nr(f, z));
+			       f->file_name, zbd_zone_idx(f, z));
 			if (zbd_reset_zone(td, f, z) < 0)
 				res = 1;
 		}
@@ -404,7 +405,7 @@ static bool zbd_open_zone(struct thread_data *td, const struct fio_file *f,
 {
 	const uint64_t min_bs = td->o.min_bs[DDIR_WRITE];
 	struct zoned_block_device_info *zbdi = f->zbd_info;
-	uint32_t zone_idx = zbd_zone_nr(f, z);
+	uint32_t zone_idx = zbd_zone_idx(f, z);
 	bool res = true;
 
 	if (z->cond == ZBD_ZONE_COND_OFFLINE)
@@ -489,8 +490,9 @@ static bool zbd_is_seq_job(struct fio_file *f)
 	if (f->io_size == 0)
 		return false;
 
-	zone_idx_b = zbd_zone_idx(f, f->file_offset);
-	zone_idx_e = zbd_zone_idx(f, f->file_offset + f->io_size - 1);
+	zone_idx_b = zbd_offset_to_zone_idx(f, f->file_offset);
+	zone_idx_e =
+		zbd_offset_to_zone_idx(f, f->file_offset + f->io_size - 1);
 	for (zone_idx = zone_idx_b; zone_idx <= zone_idx_e; zone_idx++)
 		if (get_zone(f, zone_idx)->has_wp)
 			return true;
@@ -538,7 +540,7 @@ static bool zbd_zone_align_file_sizes(struct thread_data *td,
 		return false;
 	}
 
-	zone_idx = zbd_zone_idx(f, f->file_offset);
+	zone_idx = zbd_offset_to_zone_idx(f, f->file_offset);
 	z = get_zone(f, zone_idx);
 	if ((f->file_offset != z->start) &&
 	    (td->o.td_ddir != TD_DDIR_READ)) {
@@ -555,7 +557,7 @@ static bool zbd_zone_align_file_sizes(struct thread_data *td,
 		f->file_offset = new_offset;
 	}
 
-	zone_idx = zbd_zone_idx(f, f->file_offset + f->io_size);
+	zone_idx = zbd_offset_to_zone_idx(f, f->file_offset + f->io_size);
 	z = get_zone(f, zone_idx);
 	new_end = z->start;
 	if ((td->o.td_ddir != TD_DDIR_READ) &&
@@ -1046,8 +1048,9 @@ int zbd_setup_files(struct thread_data *td)
 
 		assert(zbd);
 
-		f->min_zone = zbd_zone_idx(f, f->file_offset);
-		f->max_zone = zbd_zone_idx(f, f->file_offset + f->io_size);
+		f->min_zone = zbd_offset_to_zone_idx(f, f->file_offset);
+		f->max_zone =
+			zbd_offset_to_zone_idx(f, f->file_offset + f->io_size);
 
 		/*
 		 * When all zones in the I/O range are conventional, io_size
@@ -1275,7 +1278,7 @@ static struct fio_zone_info *zbd_convert_to_open_zone(struct thread_data *td,
 		 */
 		zone_idx = zbdi->open_zones[pick_random_zone_idx(f, io_u)];
 	} else {
-		zone_idx = zbd_zone_idx(f, io_u->offset);
+		zone_idx = zbd_offset_to_zone_idx(f, io_u->offset);
 	}
 	if (zone_idx < f->min_zone)
 		zone_idx = f->min_zone;
@@ -1597,7 +1600,7 @@ static void zbd_queue_io(struct thread_data *td, struct io_u *io_u, int q,
 
 	assert(zbd_info);
 
-	zone_idx = zbd_zone_idx(f, io_u->offset);
+	zone_idx = zbd_offset_to_zone_idx(f, io_u->offset);
 	assert(zone_idx < zbd_info->nr_zones);
 	z = get_zone(f, zone_idx);
 
@@ -1655,7 +1658,7 @@ static void zbd_put_io(struct thread_data *td, const struct io_u *io_u)
 
 	assert(zbd_info);
 
-	zone_idx = zbd_zone_idx(f, io_u->offset);
+	zone_idx = zbd_offset_to_zone_idx(f, io_u->offset);
 	assert(zone_idx < zbd_info->nr_zones);
 	z = get_zone(f, zone_idx);
 
@@ -1711,7 +1714,7 @@ void setup_zbd_zone_mode(struct thread_data *td, struct io_u *io_u)
 	assert(td->o.zone_size);
 	assert(f->zbd_info);
 
-	zone_idx = zbd_zone_idx(f, f->last_pos[ddir]);
+	zone_idx = zbd_offset_to_zone_idx(f, f->last_pos[ddir]);
 	z = get_zone(f, zone_idx);
 
 	/*
@@ -1819,7 +1822,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);
+	zone_idx_b = zbd_offset_to_zone_idx(f, io_u->offset);
 	zb = get_zone(f, zone_idx_b);
 	orig_zb = zb;
 
@@ -2102,7 +2105,7 @@ int zbd_do_io_u_trim(const struct thread_data *td, struct io_u *io_u)
 	uint32_t zone_idx;
 	int ret;
 
-	zone_idx = zbd_zone_idx(f, io_u->offset);
+	zone_idx = zbd_offset_to_zone_idx(f, io_u->offset);
 	z = get_zone(f, zone_idx);
 
 	if (!z->has_wp)
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 10/12] zbd: rename get_zone()
  2021-12-14  1:24 [PATCH v2 00/12] Cleanup zbd code Damien Le Moal
                   ` (8 preceding siblings ...)
  2021-12-14  1:24 ` [PATCH v2 09/12] zbd: rename zbd_zone_idx() and zbd_zone_nr() Damien Le Moal
@ 2021-12-14  1:24 ` Damien Le Moal
  2021-12-14  1:24 ` [PATCH v2 11/12] zbd: introduce zbd_offset_to_zone() helper Damien Le Moal
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 18+ messages in thread
From: Damien Le Moal @ 2021-12-14  1:24 UTC (permalink / raw)
  To: fio, Jens Axboe; +Cc: Niklas Cassel, Shinichiro Kawasaki

Rename get_zone() to zbd_get_zone() to be consistent with the naming
pattern of most zbd functions.

No functional changes.

Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com>
---
 zbd.c | 44 ++++++++++++++++++++++----------------------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/zbd.c b/zbd.c
index 592f7c03..095a6bad 100644
--- a/zbd.c
+++ b/zbd.c
@@ -123,10 +123,10 @@ static inline void zone_unlock(struct fio_zone_info *z)
 	assert(!ret);
 }
 
-static inline struct fio_zone_info *get_zone(const struct fio_file *f,
-					     unsigned int zone_nr)
+static inline struct fio_zone_info *zbd_get_zone(const struct fio_file *f,
+						 unsigned int zone_idx)
 {
-	return &f->zbd_info->zone_info[zone_nr];
+	return &f->zbd_info->zone_info[zone_idx];
 }
 
 /**
@@ -297,7 +297,7 @@ static void zbd_close_zone(struct thread_data *td, const struct fio_file *f,
 		return;
 
 	for (ozi = 0; ozi < f->zbd_info->num_open_zones; ozi++) {
-		if (get_zone(f, f->zbd_info->open_zones[ozi]) == z)
+		if (zbd_get_zone(f, f->zbd_info->open_zones[ozi]) == z)
 			break;
 	}
 	if (ozi == f->zbd_info->num_open_zones)
@@ -494,7 +494,7 @@ static bool zbd_is_seq_job(struct fio_file *f)
 	zone_idx_e =
 		zbd_offset_to_zone_idx(f, f->file_offset + f->io_size - 1);
 	for (zone_idx = zone_idx_b; zone_idx <= zone_idx_e; zone_idx++)
-		if (get_zone(f, zone_idx)->has_wp)
+		if (zbd_get_zone(f, zone_idx)->has_wp)
 			return true;
 
 	return false;
@@ -541,7 +541,7 @@ static bool zbd_zone_align_file_sizes(struct thread_data *td,
 	}
 
 	zone_idx = zbd_offset_to_zone_idx(f, f->file_offset);
-	z = get_zone(f, zone_idx);
+	z = zbd_get_zone(f, zone_idx);
 	if ((f->file_offset != z->start) &&
 	    (td->o.td_ddir != TD_DDIR_READ)) {
 		new_offset = zbd_zone_end(z);
@@ -558,7 +558,7 @@ static bool zbd_zone_align_file_sizes(struct thread_data *td,
 	}
 
 	zone_idx = zbd_offset_to_zone_idx(f, f->file_offset + f->io_size);
-	z = get_zone(f, zone_idx);
+	z = zbd_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)) {
@@ -1156,8 +1156,8 @@ static uint64_t zbd_process_swd(struct thread_data *td,
 	uint64_t swd = 0;
 	uint64_t wp_swd = 0;
 
-	zb = get_zone(f, f->min_zone);
-	ze = get_zone(f, f->max_zone);
+	zb = zbd_get_zone(f, f->min_zone);
+	ze = zbd_get_zone(f, f->max_zone);
 	for (z = zb; z < ze; z++) {
 		if (z->has_wp) {
 			zone_lock(td, f, z);
@@ -1209,8 +1209,8 @@ void zbd_file_reset(struct thread_data *td, struct fio_file *f)
 	if (!f->zbd_info || !td_write(td))
 		return;
 
-	zb = get_zone(f, f->min_zone);
-	ze = get_zone(f, f->max_zone);
+	zb = zbd_get_zone(f, f->min_zone);
+	ze = zbd_get_zone(f, f->max_zone);
 	swd = zbd_process_swd(td, f, SET_SWD);
 
 	dprint(FD_ZBD, "%s(%s): swd = %" PRIu64 "\n",
@@ -1298,7 +1298,7 @@ static struct fio_zone_info *zbd_convert_to_open_zone(struct thread_data *td,
 	for (;;) {
 		uint32_t tmp_idx;
 
-		z = get_zone(f, zone_idx);
+		z = zbd_get_zone(f, zone_idx);
 		if (z->has_wp)
 			zone_lock(td, f, z);
 
@@ -1404,7 +1404,7 @@ retry:
 		if (!is_valid_offset(f, z->start)) {
 			/* Wrap-around. */
 			zone_idx = f->min_zone;
-			z = get_zone(f, zone_idx);
+			z = zbd_get_zone(f, zone_idx);
 		}
 		assert(is_valid_offset(f, z->start));
 		if (!z->has_wp)
@@ -1427,7 +1427,7 @@ retry:
 		pthread_mutex_unlock(&zbdi->mutex);
 		zone_unlock(z);
 
-		z = get_zone(f, zone_idx);
+		z = zbd_get_zone(f, zone_idx);
 
 		zone_lock(td, f, z);
 		if (z->wp + min_bs <= zbd_zone_capacity_end(z))
@@ -1522,7 +1522,7 @@ zbd_find_zone(struct thread_data *td, struct io_u *io_u, uint64_t min_bytes,
 {
 	struct fio_file *f = io_u->file;
 	struct fio_zone_info *z1, *z2;
-	const struct fio_zone_info *const zf = get_zone(f, f->min_zone);
+	const struct fio_zone_info *const zf = zbd_get_zone(f, f->min_zone);
 
 	/*
 	 * Skip to the next non-empty zone in case of sequential I/O and to
@@ -1602,7 +1602,7 @@ static void zbd_queue_io(struct thread_data *td, struct io_u *io_u, int q,
 
 	zone_idx = zbd_offset_to_zone_idx(f, io_u->offset);
 	assert(zone_idx < zbd_info->nr_zones);
-	z = get_zone(f, zone_idx);
+	z = zbd_get_zone(f, zone_idx);
 
 	assert(z->has_wp);
 
@@ -1660,7 +1660,7 @@ static void zbd_put_io(struct thread_data *td, const struct io_u *io_u)
 
 	zone_idx = zbd_offset_to_zone_idx(f, io_u->offset);
 	assert(zone_idx < zbd_info->nr_zones);
-	z = get_zone(f, zone_idx);
+	z = zbd_get_zone(f, zone_idx);
 
 	assert(z->has_wp);
 
@@ -1715,7 +1715,7 @@ void setup_zbd_zone_mode(struct thread_data *td, struct io_u *io_u)
 	assert(f->zbd_info);
 
 	zone_idx = zbd_offset_to_zone_idx(f, f->last_pos[ddir]);
-	z = get_zone(f, zone_idx);
+	z = zbd_get_zone(f, zone_idx);
 
 	/*
 	 * When the zone capacity is smaller than the zone size and the I/O is
@@ -1823,7 +1823,7 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 	assert(io_u->buflen);
 
 	zone_idx_b = zbd_offset_to_zone_idx(f, io_u->offset);
-	zb = get_zone(f, zone_idx_b);
+	zb = zbd_get_zone(f, zone_idx_b);
 	orig_zb = zb;
 
 	if (!zb->has_wp) {
@@ -1886,7 +1886,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))) {
 			zone_unlock(zb);
-			zl = get_zone(f, f->max_zone);
+			zl = zbd_get_zone(f, f->max_zone);
 			zb = zbd_find_zone(td, io_u, min_bs, zb, zl);
 			if (!zb) {
 				dprint(FD_ZBD,
@@ -2030,7 +2030,7 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 
 		/* Find out a non-empty zone to trim */
 		zone_unlock(zb);
-		zl = get_zone(f, f->max_zone);
+		zl = zbd_get_zone(f, f->max_zone);
 		zb = zbd_find_zone(td, io_u, 1, zb, zl);
 		if (zb) {
 			io_u->offset = zb->start;
@@ -2106,7 +2106,7 @@ int zbd_do_io_u_trim(const struct thread_data *td, struct io_u *io_u)
 	int ret;
 
 	zone_idx = zbd_offset_to_zone_idx(f, io_u->offset);
-	z = get_zone(f, zone_idx);
+	z = zbd_get_zone(f, zone_idx);
 
 	if (!z->has_wp)
 		return 0;
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 11/12] zbd: introduce zbd_offset_to_zone() helper
  2021-12-14  1:24 [PATCH v2 00/12] Cleanup zbd code Damien Le Moal
                   ` (9 preceding siblings ...)
  2021-12-14  1:24 ` [PATCH v2 10/12] zbd: rename get_zone() Damien Le Moal
@ 2021-12-14  1:24 ` Damien Le Moal
  2021-12-14  8:34   ` Niklas Cassel
  2021-12-14  1:24 ` [PATCH v2 12/12] t/zbd: Avoid inappropriate blkzone command call in zone_cap_bs Damien Le Moal
  2021-12-14 13:48 ` [PATCH v2 00/12] Cleanup zbd code Jens Axboe
  12 siblings, 1 reply; 18+ messages in thread
From: Damien Le Moal @ 2021-12-14  1:24 UTC (permalink / raw)
  To: fio, Jens Axboe; +Cc: Niklas Cassel, Shinichiro Kawasaki

Introduce the helper function zbd_offset_to_zone() to get a zone
structure using a file offset. In many functions, this replaces the
two line code pattern:

	zone_idx = zbd_offset_to_zone_idx(f, offset);
	z = zbd_get_zone(f, zone_idx);

with a single line of code:

	z = zbd_offset_to_zone(f, offset);

Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
---
 zbd.c | 44 ++++++++++++++++----------------------------
 1 file changed, 16 insertions(+), 28 deletions(-)

diff --git a/zbd.c b/zbd.c
index 095a6bad..b1fd6b4b 100644
--- a/zbd.c
+++ b/zbd.c
@@ -129,6 +129,12 @@ static inline struct fio_zone_info *zbd_get_zone(const struct fio_file *f,
 	return &f->zbd_info->zone_info[zone_idx];
 }
 
+static inline struct fio_zone_info *
+zbd_offset_to_zone(const struct fio_file *f,  uint64_t offset)
+{
+	return zbd_get_zone(f, zbd_offset_to_zone_idx(f, offset));
+}
+
 /**
  * zbd_get_zoned_model - Get a device zoned model
  * @td: FIO thread data
@@ -510,7 +516,6 @@ static bool zbd_zone_align_file_sizes(struct thread_data *td,
 {
 	const struct fio_zone_info *z;
 	uint64_t new_offset, new_end;
-	uint32_t zone_idx;
 
 	if (!f->zbd_info)
 		return true;
@@ -540,8 +545,7 @@ static bool zbd_zone_align_file_sizes(struct thread_data *td,
 		return false;
 	}
 
-	zone_idx = zbd_offset_to_zone_idx(f, f->file_offset);
-	z = zbd_get_zone(f, zone_idx);
+	z = zbd_offset_to_zone(f, f->file_offset);
 	if ((f->file_offset != z->start) &&
 	    (td->o.td_ddir != TD_DDIR_READ)) {
 		new_offset = zbd_zone_end(z);
@@ -557,8 +561,7 @@ static bool zbd_zone_align_file_sizes(struct thread_data *td,
 		f->file_offset = new_offset;
 	}
 
-	zone_idx = zbd_offset_to_zone_idx(f, f->file_offset + f->io_size);
-	z = zbd_get_zone(f, zone_idx);
+	z = zbd_offset_to_zone(f, f->file_offset + f->io_size);
 	new_end = z->start;
 	if ((td->o.td_ddir != TD_DDIR_READ) &&
 	    (f->file_offset + f->io_size != new_end)) {
@@ -1595,15 +1598,11 @@ static void zbd_queue_io(struct thread_data *td, struct io_u *io_u, int q,
 	const struct fio_file *f = io_u->file;
 	struct zoned_block_device_info *zbd_info = f->zbd_info;
 	struct fio_zone_info *z;
-	uint32_t zone_idx;
 	uint64_t zone_end;
 
 	assert(zbd_info);
 
-	zone_idx = zbd_offset_to_zone_idx(f, io_u->offset);
-	assert(zone_idx < zbd_info->nr_zones);
-	z = zbd_get_zone(f, zone_idx);
-
+	z = zbd_offset_to_zone(f, io_u->offset);
 	assert(z->has_wp);
 
 	if (!success)
@@ -1611,7 +1610,7 @@ static void zbd_queue_io(struct thread_data *td, struct io_u *io_u, int q,
 
 	dprint(FD_ZBD,
 	       "%s: queued I/O (%lld, %llu) for zone %u\n",
-	       f->file_name, io_u->offset, io_u->buflen, zone_idx);
+	       f->file_name, io_u->offset, io_u->buflen, zbd_zone_idx(f, z));
 
 	switch (io_u->ddir) {
 	case DDIR_WRITE:
@@ -1654,19 +1653,15 @@ static void zbd_put_io(struct thread_data *td, const struct io_u *io_u)
 	const struct fio_file *f = io_u->file;
 	struct zoned_block_device_info *zbd_info = f->zbd_info;
 	struct fio_zone_info *z;
-	uint32_t zone_idx;
 
 	assert(zbd_info);
 
-	zone_idx = zbd_offset_to_zone_idx(f, io_u->offset);
-	assert(zone_idx < zbd_info->nr_zones);
-	z = zbd_get_zone(f, zone_idx);
-
+	z = zbd_offset_to_zone(f, io_u->offset);
 	assert(z->has_wp);
 
 	dprint(FD_ZBD,
 	       "%s: terminate I/O (%lld, %llu) for zone %u\n",
-	       f->file_name, io_u->offset, io_u->buflen, zone_idx);
+	       f->file_name, io_u->offset, io_u->buflen, zbd_zone_idx(f, z));
 
 	zbd_end_zone_io(td, io_u, z);
 
@@ -1708,14 +1703,12 @@ void setup_zbd_zone_mode(struct thread_data *td, struct io_u *io_u)
 	struct fio_file *f = io_u->file;
 	enum fio_ddir ddir = io_u->ddir;
 	struct fio_zone_info *z;
-	uint32_t zone_idx;
 
 	assert(td->o.zone_mode == ZONE_MODE_ZBD);
 	assert(td->o.zone_size);
 	assert(f->zbd_info);
 
-	zone_idx = zbd_offset_to_zone_idx(f, f->last_pos[ddir]);
-	z = zbd_get_zone(f, zone_idx);
+	z = zbd_offset_to_zone(f, f->last_pos[ddir]);
 
 	/*
 	 * When the zone capacity is smaller than the zone size and the I/O is
@@ -1729,7 +1722,7 @@ void setup_zbd_zone_mode(struct thread_data *td, struct io_u *io_u)
 		       "%s: Jump from zone capacity limit to zone end:"
 		       " (%"PRIu64" -> %"PRIu64") for zone %u (%"PRIu64")\n",
 		       f->file_name, f->last_pos[ddir],
-		       zbd_zone_end(z), zone_idx, z->capacity);
+		       zbd_zone_end(z), zbd_zone_idx(f, z), z->capacity);
 		td->io_skip_bytes += zbd_zone_end(z) - f->last_pos[ddir];
 		f->last_pos[ddir] = zbd_zone_end(z);
 	}
@@ -1810,7 +1803,6 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 {
 	struct fio_file *f = io_u->file;
 	struct zoned_block_device_info *zbdi = f->zbd_info;
-	uint32_t zone_idx_b;
 	struct fio_zone_info *zb, *zl, *orig_zb;
 	uint32_t orig_len = io_u->buflen;
 	uint64_t min_bs = td->o.min_bs[io_u->ddir];
@@ -1822,8 +1814,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_offset_to_zone_idx(f, io_u->offset);
-	zb = zbd_get_zone(f, zone_idx_b);
+	zb = zbd_offset_to_zone(f, io_u->offset);
 	orig_zb = zb;
 
 	if (!zb->has_wp) {
@@ -2102,12 +2093,9 @@ int zbd_do_io_u_trim(const struct thread_data *td, struct io_u *io_u)
 {
 	struct fio_file *f = io_u->file;
 	struct fio_zone_info *z;
-	uint32_t zone_idx;
 	int ret;
 
-	zone_idx = zbd_offset_to_zone_idx(f, io_u->offset);
-	z = zbd_get_zone(f, zone_idx);
-
+	z = zbd_offset_to_zone(f, io_u->offset);
 	if (!z->has_wp)
 		return 0;
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 12/12] t/zbd: Avoid inappropriate blkzone command call in zone_cap_bs
  2021-12-14  1:24 [PATCH v2 00/12] Cleanup zbd code Damien Le Moal
                   ` (10 preceding siblings ...)
  2021-12-14  1:24 ` [PATCH v2 11/12] zbd: introduce zbd_offset_to_zone() helper Damien Le Moal
@ 2021-12-14  1:24 ` Damien Le Moal
  2021-12-14  8:34   ` Niklas Cassel
  2021-12-14 13:48 ` [PATCH v2 00/12] Cleanup zbd code Jens Axboe
  12 siblings, 1 reply; 18+ messages in thread
From: Damien Le Moal @ 2021-12-14  1:24 UTC (permalink / raw)
  To: fio, Jens Axboe; +Cc: Niklas Cassel, Shinichiro Kawasaki

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

When the script test-zbd-support is run for regular block devices or
SG nodes, blkzone command shall not be called. However, zone_cap_bs()
helper function calls the command regardless of the zone model or
device type, and results in error messages such as "unable to determine
zone size" or "not a block device". Avoid the command call by returning
the zone size argument passed to this function when the test device is
a regular block device or a SG node.

Fixes: 1ae82d673cf5 ("t/zbd: Align block size to zone capacity")
Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
---
 t/zbd/functions | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/t/zbd/functions b/t/zbd/functions
index e4e248b9..7cff18fd 100644
--- a/t/zbd/functions
+++ b/t/zbd/functions
@@ -72,9 +72,11 @@ zone_cap_bs() {
 	local sed_str='s/.*len \([0-9A-Za-z]*\), cap \([0-9A-Za-z]*\).*/\1 \2/p'
 	local cap bs="$zone_size"
 
-	# When blkzone is not available or blkzone does not report capacity,
+	# When blkzone command is neither available nor relevant to the
+	# test device, or when blkzone command does not report capacity,
 	# assume that zone capacity is same as zone size for all zones.
-	if [ -z "${blkzone}" ] || ! blkzone_reports_capacity "${dev}"; then
+	if [ -z "${blkzone}" ] || [ -z "$is_zbd" ] || [ -c "$dev" ] ||
+		   ! blkzone_reports_capacity "${dev}"; then
 		echo "$zone_size"
 		return
 	fi
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 05/12] zbd: introduce zbd_zone_align_file_sizes() helper
  2021-12-14  1:24 ` [PATCH v2 05/12] zbd: introduce zbd_zone_align_file_sizes() helper Damien Le Moal
@ 2021-12-14  8:33   ` Niklas Cassel
  0 siblings, 0 replies; 18+ messages in thread
From: Niklas Cassel @ 2021-12-14  8:33 UTC (permalink / raw)
  To: Damien Le Moal; +Cc: fio, Jens Axboe, Shinichiro Kawasaki

On Tue, Dec 14, 2021 at 10:24:06AM +0900, Damien Le Moal wrote:
> Move the code for the innermost loop of the function zbd_verify_sizes()
> to the new helper function zbd_zone_align_file_sizes(). This helper
> avoids large indentation of the code in zbd_verify_sizes() and makes
> the code easier to read.
> 
> No functional changes.
> 
> Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> ---

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 09/12] zbd: rename zbd_zone_idx() and zbd_zone_nr()
  2021-12-14  1:24 ` [PATCH v2 09/12] zbd: rename zbd_zone_idx() and zbd_zone_nr() Damien Le Moal
@ 2021-12-14  8:33   ` Niklas Cassel
  0 siblings, 0 replies; 18+ messages in thread
From: Niklas Cassel @ 2021-12-14  8:33 UTC (permalink / raw)
  To: Damien Le Moal; +Cc: fio, Jens Axboe, Shinichiro Kawasaki

On Tue, Dec 14, 2021 at 10:24:10AM +0900, Damien Le Moal wrote:
> Rename zbd_zone_idx() to zbd_offset_to_zone_idx() to make it clear that
> the argument determining the zone is a file offset. To be consistent,
> rename zbd_zone_nr() to zbd_zone_idx() to avoid confusion with a number
> of zones. While at it, have both functions return value be of the same
> unsigned int type.
> 
> No functional changes.
> 
> Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> ---

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 11/12] zbd: introduce zbd_offset_to_zone() helper
  2021-12-14  1:24 ` [PATCH v2 11/12] zbd: introduce zbd_offset_to_zone() helper Damien Le Moal
@ 2021-12-14  8:34   ` Niklas Cassel
  0 siblings, 0 replies; 18+ messages in thread
From: Niklas Cassel @ 2021-12-14  8:34 UTC (permalink / raw)
  To: Damien Le Moal; +Cc: fio, Jens Axboe, Shinichiro Kawasaki

On Tue, Dec 14, 2021 at 10:24:12AM +0900, Damien Le Moal wrote:
> Introduce the helper function zbd_offset_to_zone() to get a zone
> structure using a file offset. In many functions, this replaces the
> two line code pattern:
> 
> 	zone_idx = zbd_offset_to_zone_idx(f, offset);
> 	z = zbd_get_zone(f, zone_idx);
> 
> with a single line of code:
> 
> 	z = zbd_offset_to_zone(f, offset);
> 
> Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> ---

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 12/12] t/zbd: Avoid inappropriate blkzone command call in zone_cap_bs
  2021-12-14  1:24 ` [PATCH v2 12/12] t/zbd: Avoid inappropriate blkzone command call in zone_cap_bs Damien Le Moal
@ 2021-12-14  8:34   ` Niklas Cassel
  0 siblings, 0 replies; 18+ messages in thread
From: Niklas Cassel @ 2021-12-14  8:34 UTC (permalink / raw)
  To: Damien Le Moal; +Cc: fio, Jens Axboe, Shinichiro Kawasaki

On Tue, Dec 14, 2021 at 10:24:13AM +0900, Damien Le Moal wrote:
> From: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> 
> When the script test-zbd-support is run for regular block devices or
> SG nodes, blkzone command shall not be called. However, zone_cap_bs()
> helper function calls the command regardless of the zone model or
> device type, and results in error messages such as "unable to determine
> zone size" or "not a block device". Avoid the command call by returning
> the zone size argument passed to this function when the test device is
> a regular block device or a SG node.
> 
> Fixes: 1ae82d673cf5 ("t/zbd: Align block size to zone capacity")
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> ---

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 00/12] Cleanup zbd code
  2021-12-14  1:24 [PATCH v2 00/12] Cleanup zbd code Damien Le Moal
                   ` (11 preceding siblings ...)
  2021-12-14  1:24 ` [PATCH v2 12/12] t/zbd: Avoid inappropriate blkzone command call in zone_cap_bs Damien Le Moal
@ 2021-12-14 13:48 ` Jens Axboe
  12 siblings, 0 replies; 18+ messages in thread
From: Jens Axboe @ 2021-12-14 13:48 UTC (permalink / raw)
  To: Damien Le Moal, fio; +Cc: Niklas Cassel, Shinichiro Kawasaki

On Tue, 14 Dec 2021 10:24:01 +0900, Damien Le Moal wrote:
> The patch series cleans up zbd.c code, improving code readability.
> Patches 1 to 11 do not introduce any functional change. Patch 12 fixes
> a problem with the zbd test script.
> 
> Changes from v1:
> * Renamed zbd_verify_file_sizes() to zbd_zone_align_file_sizes() in
>   patch 5
> * Change function names as suggested by Niklas in patches 10 and 11
> * Added Niklas Reviewed-by tag
> * Added patch 12
> 
> [...]

Applied, thanks!

[01/12] fio: Improve documentation of ignore_zone_limits option
        commit: 12324d569861b83fb76e874122ddfcf28ee8d485
[02/12] zbd: define local functions as static
        commit: 38334c1347e624237118d08f467aff35a8fcafe6
[03/12] zbd: move and cleanup code
        commit: 410a071c59d7992968af7073bf39546df18542ff
[04/12] zbd: remove is_zone_open() helper
        commit: b5a0f7ce303d5fa5cca51af0652b7f7cf9bc5d61
[05/12] zbd: introduce zbd_zone_align_file_sizes() helper
        commit: 0bf93a1a7ed700b24177edb6db6c4c42e93ca7b2
[06/12] zbd: fix code style issues
        commit: 139d8dc666e5e4a05eaa9f5e603fc3453c5fe43f
[07/12] zbd: simplify zbd_close_zone()
        commit: a23411bb2e1c7de1e97c505ca2ec7270a35f7be1
[08/12] zbd: simplify zbd_open_zone()
        commit: aad7c276b01304d3fa128c84dc885eb557944b29
[09/12] zbd: rename zbd_zone_idx() and zbd_zone_nr()
        commit: dc8a3d629310d1ac9143d62c9f37ff16331737d5
[10/12] zbd: rename get_zone()
        commit: 39e06ee779cae55cb7d02eb11a5f12ca7f21a337
[11/12] zbd: introduce zbd_offset_to_zone() helper
        commit: 53aa61719b906bdf8e058c94ca93e537b90806b1
[12/12] t/zbd: Avoid inappropriate blkzone command call in zone_cap_bs
        commit: 9ffe433d729101a34d9709030d7d4dd2444347ef

Best regards,
-- 
Jens Axboe



^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2021-12-14 13:48 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-14  1:24 [PATCH v2 00/12] Cleanup zbd code Damien Le Moal
2021-12-14  1:24 ` [PATCH v2 01/12] fio: Improve documentation of ignore_zone_limits option Damien Le Moal
2021-12-14  1:24 ` [PATCH v2 02/12] zbd: define local functions as static Damien Le Moal
2021-12-14  1:24 ` [PATCH v2 03/12] zbd: move and cleanup code Damien Le Moal
2021-12-14  1:24 ` [PATCH v2 04/12] zbd: remove is_zone_open() helper Damien Le Moal
2021-12-14  1:24 ` [PATCH v2 05/12] zbd: introduce zbd_zone_align_file_sizes() helper Damien Le Moal
2021-12-14  8:33   ` Niklas Cassel
2021-12-14  1:24 ` [PATCH v2 06/12] zbd: fix code style issues Damien Le Moal
2021-12-14  1:24 ` [PATCH v2 07/12] zbd: simplify zbd_close_zone() Damien Le Moal
2021-12-14  1:24 ` [PATCH v2 08/12] zbd: simplify zbd_open_zone() Damien Le Moal
2021-12-14  1:24 ` [PATCH v2 09/12] zbd: rename zbd_zone_idx() and zbd_zone_nr() Damien Le Moal
2021-12-14  8:33   ` Niklas Cassel
2021-12-14  1:24 ` [PATCH v2 10/12] zbd: rename get_zone() Damien Le Moal
2021-12-14  1:24 ` [PATCH v2 11/12] zbd: introduce zbd_offset_to_zone() helper Damien Le Moal
2021-12-14  8:34   ` Niklas Cassel
2021-12-14  1:24 ` [PATCH v2 12/12] t/zbd: Avoid inappropriate blkzone command call in zone_cap_bs Damien Le Moal
2021-12-14  8:34   ` Niklas Cassel
2021-12-14 13:48 ` [PATCH v2 00/12] Cleanup zbd code Jens Axboe

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.