All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] v1 Patchset : Zone Explicit Open support
       [not found] <CGME20200929130315epcas5p4e6a11cc77fb06a64c31e57740d2e0e31@epcas5p4.samsung.com>
@ 2020-09-29 12:59 ` Krishna Kanth Reddy
       [not found]   ` <CGME20200929130320epcas5p3fe001cbdeacc1f49a6f237c31c34e93d@epcas5p3.samsung.com>
                     ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Krishna Kanth Reddy @ 2020-09-29 12:59 UTC (permalink / raw)
  To: axboe; +Cc: fio, Krishna Kanth Reddy

1. Added a new FIO option zone_open.
	When it is enabled, Zones will be opened explicitly before sending I/Os.
2. Add support for open_zones in libzbc I/O engine
3. t/zbd: Add support to verify Zone Explicit Open

This is a RFC.
Feedback / Comments will help in improving this further.

Ankit Kumar (2):
  Add support for Explicit open zones
  Add support for open_zones in libzbc I/O engine.

Krishna Kanth Reddy (1):
  t/zbd: Add support to verify Zone Explicit Open

 HOWTO                       | 13 ++++++++
 engines/libzbc.c            | 42 +++++++++++++++++++++++++
 engines/skeleton_external.c | 12 ++++++++
 fio.1                       | 11 +++++++
 ioengines.h                 |  2 ++
 options.c                   | 20 ++++++++++++
 oslib/blkzoned.h            |  7 +++++
 oslib/linux-blkzoned.c      | 26 ++++++++++++++++
 t/zbd/test-zbd-support      | 27 ++++++++++++++++
 thread_options.h            |  7 +++++
 zbd.c                       | 61 +++++++++++++++++++++++++++++++++++++
 11 files changed, 228 insertions(+)

-- 
2.17.1



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

* [PATCH 1/3] Add support for Explicit open zones
       [not found]   ` <CGME20200929130320epcas5p3fe001cbdeacc1f49a6f237c31c34e93d@epcas5p3.samsung.com>
@ 2020-09-29 12:59     ` Krishna Kanth Reddy
  0 siblings, 0 replies; 7+ messages in thread
From: Krishna Kanth Reddy @ 2020-09-29 12:59 UTC (permalink / raw)
  To: axboe; +Cc: fio, Ankit Kumar, Krishna Kanth Reddy

From: Ankit Kumar <ankit.kumar@samsung.com>

Added a new FIO option zone_open. When it is enabled,
Zones will be opened explicitly before sending I/Os.

Signed-off-by: Krishna Kanth Reddy <krish.reddy@samsung.com>
---
 HOWTO                       | 13 ++++++++
 engines/skeleton_external.c | 12 ++++++++
 fio.1                       | 11 +++++++
 ioengines.h                 |  2 ++
 options.c                   | 20 ++++++++++++
 oslib/blkzoned.h            |  7 +++++
 oslib/linux-blkzoned.c      | 26 ++++++++++++++++
 thread_options.h            |  7 +++++
 zbd.c                       | 61 +++++++++++++++++++++++++++++++++++++
 9 files changed, 159 insertions(+)

diff --git a/HOWTO b/HOWTO
index 2d8c7a02..320a1083 100644
--- a/HOWTO
+++ b/HOWTO
@@ -1022,6 +1022,19 @@ Target file/device
 	:option:`zonesize` bytes of data have been transferred. This parameter
 	must be zero for :option:`zonemode` =zbd.
 
+.. option:: zone_open=str
+
+	This parameter applies to :option:`zonemode` =zbd only.
+
+	Accepted values are:
+
+		**implicit**
+				Zones are opened implicitly before sending
+				the I/Os.
+		**explicit**
+				Zones are opened explicitly before sending
+				the I/Os.
+
 .. option:: read_beyond_wp=bool
 
 	This parameter applies to :option:`zonemode` =zbd only.
diff --git a/engines/skeleton_external.c b/engines/skeleton_external.c
index 7f3e4cb3..828668b8 100644
--- a/engines/skeleton_external.c
+++ b/engines/skeleton_external.c
@@ -193,6 +193,17 @@ static int fio_skeleton_reset_wp(struct thread_data *td, struct fio_file *f,
 	return 0;
 }
 
+/*
+ * Hook called for explicitly opening zones of a ZBD_HOST_AWARE or
+ * ZBD_HOST_MANAGED zoned block device. All the zones in the range
+ * @offset..@offset + @length must be opened.
+ */
+static int fio_skeleton_open_zones(struct thread_data *td, struct fio_file *f,
+				   uint64_t offset, uint64_t length)
+{
+	return 0;
+}
+
 /*
  * Note that the structure is exported, so that fio can get it via
  * dlsym(..., "ioengine"); for (and only for) external engines.
@@ -212,6 +223,7 @@ struct ioengine_ops ioengine = {
 	.get_zoned_model = fio_skeleton_get_zoned_model,
 	.report_zones	= fio_skeleton_report_zones,
 	.reset_wp	= fio_skeleton_reset_wp,
+	.open_zones	= fio_skeleton_open_zones,
 	.options	= options,
 	.option_struct_size	= sizeof(struct fio_skeleton_options),
 };
diff --git a/fio.1 b/fio.1
index a881277c..de69f7af 100644
--- a/fio.1
+++ b/fio.1
@@ -789,6 +789,17 @@ once a zone is fully written (write workloads) or all written data in the
 zone have been read (read workloads). This parameter is valid only for
 sequential workloads and ignored for random workloads. For read workloads,
 see also \fBread_beyond_wp\fR.
+.TP
+.BI zone_open\fR=\fPstr
+For \fBzonemode\fR=zbd the accepted values are
+.RS
+.TP
+.B explicit
+Zones are opened Explicitly before sending I/Os.
+.TP
+.B implicit
+Zones are opened Implicitly before sending I/Os.
+.RE
 
 .TP
 .BI read_beyond_wp \fR=\fPbool
diff --git a/ioengines.h b/ioengines.h
index 54dadba2..8caac373 100644
--- a/ioengines.h
+++ b/ioengines.h
@@ -57,6 +57,8 @@ struct ioengine_ops {
 			    uint64_t, struct zbd_zone *, unsigned int);
 	int (*reset_wp)(struct thread_data *, struct fio_file *,
 			uint64_t, uint64_t);
+	int (*open_zones)(struct thread_data *, const struct fio_file *,
+			  uint64_t, uint64_t);
 	int option_struct_size;
 	struct fio_option *options;
 };
diff --git a/options.c b/options.c
index b497d973..2b2bbb6e 100644
--- a/options.c
+++ b/options.c
@@ -3332,6 +3332,26 @@ struct fio_option fio_options[FIO_MAX_OPTS] = {
 			   },
 		},
 	},
+	{
+		.name	= "zone_open",
+		.lname	= "zone_open",
+		.type	= FIO_OPT_STR,
+		.off1	= offsetof(struct thread_options, zone_open),
+		.help	= "Zone Open Mode (Implicit / Explicit) for Zoned Block Devices",
+		.def	= "implicit",
+		.category = FIO_OPT_C_IO,
+		.group	= FIO_OPT_G_ZONE,
+		.posval	= {
+			   { .ival = "implicit",
+			     .oval = ZONE_OPEN_IMPLICIT,
+			     .help = "Open Zones Implicitly",
+			   },
+			   { .ival = "explicit",
+			     .oval = ZONE_OPEN_EXPLICIT,
+			     .help = "Open Zones Explicitly",
+			   },
+		},
+	},
 	{
 		.name	= "zonesize",
 		.lname	= "Zone size",
diff --git a/oslib/blkzoned.h b/oslib/blkzoned.h
index 4cc071dc..9c1422d3 100644
--- a/oslib/blkzoned.h
+++ b/oslib/blkzoned.h
@@ -16,6 +16,8 @@ extern int blkzoned_report_zones(struct thread_data *td,
 				struct zbd_zone *zones, unsigned int nr_zones);
 extern int blkzoned_reset_wp(struct thread_data *td, struct fio_file *f,
 				uint64_t offset, uint64_t length);
+extern int blkzoned_open_zone(struct thread_data *td, const struct fio_file *f,
+				uint64_t offset, uint64_t length);
 #else
 /*
  * Define stubs for systems that do not have zoned block device support.
@@ -44,6 +46,11 @@ static inline int blkzoned_reset_wp(struct thread_data *td, struct fio_file *f,
 {
 	return -EIO;
 }
+static inline int blkzoned_open_zone(struct thread_data *td, const struct fio_file *f,
+				     uint64_t offset, uint64_t length)
+{
+	return -EIO;
+}
 #endif
 
 #endif /* FIO_BLKZONED_H */
diff --git a/oslib/linux-blkzoned.c b/oslib/linux-blkzoned.c
index 0a8a577a..f8cdce3b 100644
--- a/oslib/linux-blkzoned.c
+++ b/oslib/linux-blkzoned.c
@@ -240,3 +240,29 @@ int blkzoned_reset_wp(struct thread_data *td, struct fio_file *f,
 
 	return ret;
 }
+
+int blkzoned_open_zone(struct thread_data *td, const struct fio_file *f,
+		       uint64_t offset, uint64_t length)
+{
+	struct blk_zone_range zr = {
+		.sector         = offset >> 9,
+		.nr_sectors     = length >> 9,
+	};
+	int fd, ret = 0;
+
+	/* If the file is not yet opened, open it for this function. */
+	fd = f->fd;
+	if (fd < 0) {
+		fd = open(f->file_name, O_RDWR | O_LARGEFILE);
+		if (fd < 0)
+			return -errno;
+	}
+
+	if (ioctl(fd, BLKOPENZONE, &zr) < 0)
+		return -errno;
+
+	if (f->fd < 0)
+		close(fd);
+
+	return ret;
+}
diff --git a/thread_options.h b/thread_options.h
index 97c400fe..d8f352b7 100644
--- a/thread_options.h
+++ b/thread_options.h
@@ -18,6 +18,11 @@ enum fio_zone_mode {
 	ZONE_MODE_ZBD		= 3,
 };
 
+enum fio_zone_open {
+	ZONE_OPEN_IMPLICIT	= 0, /* Open Zones Implicitly */
+	ZONE_OPEN_EXPLICIT	= 1, /* Open Zones Explicitly */
+};
+
 /*
  * What type of allocation to use for io buffers
  */
@@ -197,6 +202,7 @@ struct thread_options {
 	unsigned long long zone_capacity;
 	unsigned long long zone_skip;
 	enum fio_zone_mode zone_mode;
+	enum fio_zone_open zone_open;
 	unsigned long long lockmem;
 	enum fio_memtype mem_type;
 	unsigned int mem_align;
@@ -640,6 +646,7 @@ struct thread_options_pack {
 	uint32_t allow_mounted_write;
 
 	uint32_t zone_mode;
+	uint32_t zone_open;
 } __attribute__((packed));
 
 extern void convert_thread_options_to_cpu(struct thread_options *o, struct thread_options_pack *top);
diff --git a/zbd.c b/zbd.c
index 905c0c2b..70836e38 100644
--- a/zbd.c
+++ b/zbd.c
@@ -738,12 +738,53 @@ static int zbd_reset_zone(struct thread_data *td, struct fio_file *f,
 	pthread_mutex_unlock(&f->zbd_info->mutex);
 	z->wp = z->start;
 	z->verify_block = 0;
+	z->cond = ZBD_ZONE_COND_EMPTY;
 
 	td->ts.nr_zone_resets++;
 
 	return ret;
 }
 
+/**
+ * zbd_explicit_open_zone - Open a zone explicitly
+ * @td: FIO thread data.
+ * @f: FIO file associated with the disk for which to open the zone.
+ * @z: Zone to open.
+ *
+ * Returns 0 upon success and a negative error code upon failure.
+ *
+ * The caller must hold z->mutex.
+ */
+static int zbd_explicit_open_zone(struct thread_data *td, const struct fio_file *f,
+				  struct fio_zone_info *z)
+{
+	uint64_t offset = z->start;
+	uint64_t length = (z+1)->start - offset;
+	int ret = 0;
+
+	assert(is_valid_offset(f, offset + length - 1));
+
+	switch (f->zbd_info->model) {
+	case ZBD_HOST_AWARE:
+	case ZBD_HOST_MANAGED:
+		if (td->io_ops && td->io_ops->open_zones)
+			ret = td->io_ops->open_zones(td, f, z->start, f->zbd_info->zone_size);
+		else
+			ret = blkzoned_open_zone(td, f, z->start, f->zbd_info->zone_size);
+		if (ret < 0) {
+			td_verror(td, errno, "opening zone failed");
+			log_err("%s: Explicit open for zone :%d failed (%d).\n",
+				f->file_name, zbd_zone_nr(f->zbd_info, z), errno);
+		}
+		z->cond = ZBD_ZONE_COND_EXP_OPEN;
+		break;
+	default:
+		break;
+	}
+
+	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)
@@ -986,6 +1027,11 @@ static bool zbd_open_zone(struct thread_data *td, const struct fio_file *f,
 		 */
 		if (z->wp >= zbd_zone_capacity_end(z))
 			res = false;
+		if (td->o.zone_open == ZONE_OPEN_EXPLICIT &&
+		    z->cond == ZBD_ZONE_COND_IMP_OPEN)
+			if (zbd_explicit_open_zone(td, f, z))
+				res = false;
+
 		goto out;
 	}
 	res = false;
@@ -996,6 +1042,16 @@ static bool zbd_open_zone(struct thread_data *td, const struct fio_file *f,
 	if (f->zbd_info->num_open_zones >= f->zbd_info->max_open_zones)
 		goto out;
 	dprint(FD_ZBD, "%s: opening zone %d\n", f->file_name, zone_idx);
+	if (td->o.zone_open == ZONE_OPEN_EXPLICIT) {
+		if (z->cond == ZBD_ZONE_COND_FULL) {
+			res = true;
+			goto out;
+		}
+		if (zbd_explicit_open_zone(td, f, z))
+			goto out;
+
+	} else
+		z->cond = ZBD_ZONE_COND_IMP_OPEN;
 	f->zbd_info->open_zones[f->zbd_info->num_open_zones++] = zone_idx;
 	td->num_open_zones++;
 	z->open = 1;
@@ -1268,6 +1324,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)) {
+		z->cond = ZBD_ZONE_COND_FULL;
 		pthread_mutex_lock(&f->zbd_info->mutex);
 		zbd_close_zone(td, f, z - f->zbd_info->zone_info);
 		pthread_mutex_unlock(&f->zbd_info->mutex);
@@ -1635,6 +1692,10 @@ enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u)
 			if (zbd_reset_zone(td, f, zb) < 0)
 				goto eof;
 
+			if ((td->o.zone_open == ZONE_OPEN_EXPLICIT) &&
+			    zbd_explicit_open_zone(td, f, zb))
+				goto eof;
+
 			if (zb->capacity < min_bs) {
 				log_err("zone capacity %llu smaller than minimum block size %d\n",
 					(unsigned long long)zb->capacity,
-- 
2.17.1



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

* [PATCH 2/3] Add support for open_zones in libzbc I/O engine.
       [not found]   ` <CGME20200929130325epcas5p285128c2026c1bb8743967f15b0657ec7@epcas5p2.samsung.com>
@ 2020-09-29 12:59     ` Krishna Kanth Reddy
  0 siblings, 0 replies; 7+ messages in thread
From: Krishna Kanth Reddy @ 2020-09-29 12:59 UTC (permalink / raw)
  To: axboe; +Cc: fio, Ankit Kumar, Krishna Kanth Reddy

From: Ankit Kumar <ankit.kumar@samsung.com>

Signed-off-by: Krishna Kanth Reddy <krish.reddy@samsung.com>
---
 engines/libzbc.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/engines/libzbc.c b/engines/libzbc.c
index 4b900233..4b742d24 100644
--- a/engines/libzbc.c
+++ b/engines/libzbc.c
@@ -334,6 +334,47 @@ err:
 	return -ret;
 }
 
+static int libzbc_open_zones(struct thread_data *td, const struct fio_file *f,
+			     uint64_t offset, uint64_t length)
+{
+	struct libzbc_data *ld = td->io_ops_data;
+	uint64_t sector = offset >> 9;
+	uint64_t end_sector = (offset + length) >> 9;
+	unsigned int nr_zones;
+	struct zbc_errno err;
+	int i, ret;
+
+	assert(ld);
+	assert(ld->zdev);
+
+	nr_zones = (length + td->o.zone_size - 1) / td->o.zone_size;
+	if (!sector && end_sector >= ld->nr_sectors) {
+		/* Open all zones */
+		ret = zbc_open_zone(ld->zdev, 0, ZBC_OP_ALL_ZONES);
+		if (ret)
+			goto err;
+
+		return 0;
+	}
+
+	for (i = 0; i < nr_zones; i++, sector += td->o.zone_size >> 9) {
+		ret = zbc_open_zone(ld->zdev, sector, 0);
+		if (ret)
+			goto err;
+	}
+
+	return 0;
+
+err:
+	zbc_errno(ld->zdev, &err);
+	td_verror(td, errno, "zbc_open_zone failed");
+	if (err.sk)
+		log_err("%s: open failed %s:%s\n",
+			f->file_name,
+			zbc_sk_str(err.sk), zbc_asc_ascq_str(err.asc_ascq));
+	return -ret;
+}
+
 ssize_t libzbc_rw(struct thread_data *td, struct io_u *io_u)
 {
 	struct libzbc_data *ld = td->io_ops_data;
@@ -413,6 +454,7 @@ FIO_STATIC struct ioengine_ops ioengine = {
 	.get_zoned_model	= libzbc_get_zoned_model,
 	.report_zones		= libzbc_report_zones,
 	.reset_wp		= libzbc_reset_wp,
+	.open_zones		= libzbc_open_zones,
 	.queue			= libzbc_queue,
 	.flags			= FIO_SYNCIO | FIO_NOEXTEND | FIO_RAWIO,
 };
-- 
2.17.1



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

* [PATCH 3/3] t/zbd: Add support to verify Zone Explicit Open
       [not found]   ` <CGME20200929130329epcas5p1c170f7010a73df8c1cdb79ccf6662428@epcas5p1.samsung.com>
@ 2020-09-29 12:59     ` Krishna Kanth Reddy
  0 siblings, 0 replies; 7+ messages in thread
From: Krishna Kanth Reddy @ 2020-09-29 12:59 UTC (permalink / raw)
  To: axboe; +Cc: fio, Krishna Kanth Reddy, Ankit Kumar

Modify the test-zbd-support script to verify the Zone Explicit Open.
Added a new FIO option zone_open.
When zone_open option is set to explicit, zones are opened explicitly.
If it is set to implicit, zones are opened implicitly.
Default value is implicit for the zone_open option.

Signed-off-by: Ankit Kumar <ankit.kumar@samsung.com>
---
 t/zbd/test-zbd-support | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/t/zbd/test-zbd-support b/t/zbd/test-zbd-support
index 248423bb..c1f35165 100755
--- a/t/zbd/test-zbd-support
+++ b/t/zbd/test-zbd-support
@@ -948,6 +948,33 @@ test49() {
     check_read $((capacity * 2)) || return $?
 }
 
+# Explicit zone open to sequential zones, libaio, 1 job, queue depth 1
+test50() {
+    local size capacity
+
+    size=$((8 * zone_size))
+    off=$((first_sequential_zone_sector * 512))
+    capacity=$(total_zone_capacity 8 $off $dev)
+
+    run_fio_on_seq --ioengine=libaio --iodepth=1 --rw=write --zone_open=explicit \
+                   --bs=65536 --size=$size --do_verify=1 --verify=md5 \
+                   >>"${logfile}.${test_number}" 2>&1 || return $?
+    check_written $capacity || return $?
+    check_read $capacity || return $?
+}
+
+# Random zone write to sequential zones, libaio, 8 jobs, queue depth 64 per job, Explicit open
+test51() {
+    local capacity
+
+    off=$((first_sequential_zone_sector * 512))
+    capacity=$(total_zone_capacity 4 $off $dev)
+    run_fio_on_seq --ioengine=libaio --iodepth=64 --rw=randwrite \
+		   --bs=4096 --group_reporting=1 --numjobs=8 --zone_open=explicit \
+                   >> "${logfile}.${test_number}" 2>&1 || return $?
+    check_written $((capacity * 8)) || return $?
+}
+
 tests=()
 dynamic_analyzer=()
 reset_all_zones=
-- 
2.17.1



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

* Re: [PATCH 0/3] v1 Patchset : Zone Explicit Open support
  2020-09-29 12:59 ` [PATCH 0/3] v1 Patchset : Zone Explicit Open support Krishna Kanth Reddy
                     ` (2 preceding siblings ...)
       [not found]   ` <CGME20200929130329epcas5p1c170f7010a73df8c1cdb79ccf6662428@epcas5p1.samsung.com>
@ 2020-09-29 16:32   ` Damien Le Moal
  2020-10-06  4:32     ` Krishna Kanth Reddy
  3 siblings, 1 reply; 7+ messages in thread
From: Damien Le Moal @ 2020-09-29 16:32 UTC (permalink / raw)
  To: Krishna Kanth Reddy, axboe; +Cc: fio

On 2020/09/29 22:03, Krishna Kanth Reddy wrote:
> 1. Added a new FIO option zone_open.
> 	When it is enabled, Zones will be opened explicitly before sending I/Os.
> 2. Add support for open_zones in libzbc I/O engine
> 3. t/zbd: Add support to verify Zone Explicit Open
> 
> This is a RFC.
> Feedback / Comments will help in improving this further.

My main question is why ? There is no performance benefit on the drive side from
doing explicit zone open. On the contrary, performance may go down due to the
added command overhead. So why do this ? What is your goal ?

Using the max_open_zones=N option, fio writes will be limited to a set of N
zones, implicitly open, until the zones are full (not open anymore) and new
zones chosen (becoming implicitly open with writes). Adding explicit zone open
will not give you anything more with this option used. And if you do not use it,
performance with random writes will significantly go down.

So please try to give convincing reasons for this.

> 
> Ankit Kumar (2):
>   Add support for Explicit open zones
>   Add support for open_zones in libzbc I/O engine.
> 
> Krishna Kanth Reddy (1):
>   t/zbd: Add support to verify Zone Explicit Open
> 
>  HOWTO                       | 13 ++++++++
>  engines/libzbc.c            | 42 +++++++++++++++++++++++++
>  engines/skeleton_external.c | 12 ++++++++
>  fio.1                       | 11 +++++++
>  ioengines.h                 |  2 ++
>  options.c                   | 20 ++++++++++++
>  oslib/blkzoned.h            |  7 +++++
>  oslib/linux-blkzoned.c      | 26 ++++++++++++++++
>  t/zbd/test-zbd-support      | 27 ++++++++++++++++
>  thread_options.h            |  7 +++++
>  zbd.c                       | 61 +++++++++++++++++++++++++++++++++++++
>  11 files changed, 228 insertions(+)
> 


-- 
Damien Le Moal
Western Digital Research


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

* Re: [PATCH 0/3] v1 Patchset : Zone Explicit Open support
  2020-09-29 16:32   ` [PATCH 0/3] v1 Patchset : Zone Explicit Open support Damien Le Moal
@ 2020-10-06  4:32     ` Krishna Kanth Reddy
  2020-10-06  5:18       ` Damien Le Moal
  0 siblings, 1 reply; 7+ messages in thread
From: Krishna Kanth Reddy @ 2020-10-06  4:32 UTC (permalink / raw)
  To: Damien Le Moal; +Cc: axboe, fio

[-- Attachment #1: Type: text/plain, Size: 2152 bytes --]

On Tue, Sep 29, 2020 at 04:32:09PM +0000, Damien Le Moal wrote:
>On 2020/09/29 22:03, Krishna Kanth Reddy wrote:
>> 1. Added a new FIO option zone_open.
>> 	When it is enabled, Zones will be opened explicitly before sending I/Os.
>> 2. Add support for open_zones in libzbc I/O engine
>> 3. t/zbd: Add support to verify Zone Explicit Open
>>
>> This is a RFC.
>> Feedback / Comments will help in improving this further.
>
>My main question is why ? There is no performance benefit on the drive side from
>doing explicit zone open. On the contrary, performance may go down due to the
>added command overhead. So why do this ? What is your goal ?
>
>Using the max_open_zones=N option, fio writes will be limited to a set of N
>zones, implicitly open, until the zones are full (not open anymore) and new
>zones chosen (becoming implicitly open with writes). Adding explicit zone open
>will not give you anything more with this option used. And if you do not use it,
>performance with random writes will significantly go down.
>
>So please try to give convincing reasons for this.
>
It is a building block for future TPs.
We would like to do the pre-work and add the right option abstraction.
Zone open is 'implicit' by default.
Hence there won't be a performance drop with the newly added code.

>>
>> Ankit Kumar (2):
>>   Add support for Explicit open zones
>>   Add support for open_zones in libzbc I/O engine.
>>
>> Krishna Kanth Reddy (1):
>>   t/zbd: Add support to verify Zone Explicit Open
>>
>>  HOWTO                       | 13 ++++++++
>>  engines/libzbc.c            | 42 +++++++++++++++++++++++++
>>  engines/skeleton_external.c | 12 ++++++++
>>  fio.1                       | 11 +++++++
>>  ioengines.h                 |  2 ++
>>  options.c                   | 20 ++++++++++++
>>  oslib/blkzoned.h            |  7 +++++
>>  oslib/linux-blkzoned.c      | 26 ++++++++++++++++
>>  t/zbd/test-zbd-support      | 27 ++++++++++++++++
>>  thread_options.h            |  7 +++++
>>  zbd.c                       | 61 +++++++++++++++++++++++++++++++++++++
>>  11 files changed, 228 insertions(+)
>>
>
>
>-- 
>Damien Le Moal
>Western Digital Research
>

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



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

* Re: [PATCH 0/3] v1 Patchset : Zone Explicit Open support
  2020-10-06  4:32     ` Krishna Kanth Reddy
@ 2020-10-06  5:18       ` Damien Le Moal
  0 siblings, 0 replies; 7+ messages in thread
From: Damien Le Moal @ 2020-10-06  5:18 UTC (permalink / raw)
  To: Krishna Kanth Reddy; +Cc: axboe, fio

On 2020/10/06 13:37, Krishna Kanth Reddy wrote:
> On Tue, Sep 29, 2020 at 04:32:09PM +0000, Damien Le Moal wrote:
>> On 2020/09/29 22:03, Krishna Kanth Reddy wrote:
>>> 1. Added a new FIO option zone_open.
>>> 	When it is enabled, Zones will be opened explicitly before sending I/Os.
>>> 2. Add support for open_zones in libzbc I/O engine
>>> 3. t/zbd: Add support to verify Zone Explicit Open
>>>
>>> This is a RFC.
>>> Feedback / Comments will help in improving this further.
>>
>> My main question is why ? There is no performance benefit on the drive side from
>> doing explicit zone open. On the contrary, performance may go down due to the
>> added command overhead. So why do this ? What is your goal ?
>>
>> Using the max_open_zones=N option, fio writes will be limited to a set of N
>> zones, implicitly open, until the zones are full (not open anymore) and new
>> zones chosen (becoming implicitly open with writes). Adding explicit zone open
>> will not give you anything more with this option used. And if you do not use it,
>> performance with random writes will significantly go down.
>>
>> So please try to give convincing reasons for this.
>>
> It is a building block for future TPs.

Which one ? Without that justification, I do not see the point of introducing
this change right now.

> We would like to do the pre-work and add the right option abstraction.
> Zone open is 'implicit' by default.
> Hence there won't be a performance drop with the newly added code.

Sure, but there will be a drop for regular zone block devices with some
workloads when that option is used. So if explicit zone open is needed by some
future TP feature, it may be better to integrate explicit open with that feature
rather than making it a generic fio option that users may mistakenly enable and
end up wondering why they see lower performance with their device...


> 
>>>
>>> Ankit Kumar (2):
>>>   Add support for Explicit open zones
>>>   Add support for open_zones in libzbc I/O engine.
>>>
>>> Krishna Kanth Reddy (1):
>>>   t/zbd: Add support to verify Zone Explicit Open
>>>
>>>  HOWTO                       | 13 ++++++++
>>>  engines/libzbc.c            | 42 +++++++++++++++++++++++++
>>>  engines/skeleton_external.c | 12 ++++++++
>>>  fio.1                       | 11 +++++++
>>>  ioengines.h                 |  2 ++
>>>  options.c                   | 20 ++++++++++++
>>>  oslib/blkzoned.h            |  7 +++++
>>>  oslib/linux-blkzoned.c      | 26 ++++++++++++++++
>>>  t/zbd/test-zbd-support      | 27 ++++++++++++++++
>>>  thread_options.h            |  7 +++++
>>>  zbd.c                       | 61 +++++++++++++++++++++++++++++++++++++
>>>  11 files changed, 228 insertions(+)
>>>
>>
>>
>> -- 
>> Damien Le Moal
>> Western Digital Research
>>


-- 
Damien Le Moal
Western Digital Research


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

end of thread, other threads:[~2020-10-06  5:18 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20200929130315epcas5p4e6a11cc77fb06a64c31e57740d2e0e31@epcas5p4.samsung.com>
2020-09-29 12:59 ` [PATCH 0/3] v1 Patchset : Zone Explicit Open support Krishna Kanth Reddy
     [not found]   ` <CGME20200929130320epcas5p3fe001cbdeacc1f49a6f237c31c34e93d@epcas5p3.samsung.com>
2020-09-29 12:59     ` [PATCH 1/3] Add support for Explicit open zones Krishna Kanth Reddy
     [not found]   ` <CGME20200929130325epcas5p285128c2026c1bb8743967f15b0657ec7@epcas5p2.samsung.com>
2020-09-29 12:59     ` [PATCH 2/3] Add support for open_zones in libzbc I/O engine Krishna Kanth Reddy
     [not found]   ` <CGME20200929130329epcas5p1c170f7010a73df8c1cdb79ccf6662428@epcas5p1.samsung.com>
2020-09-29 12:59     ` [PATCH 3/3] t/zbd: Add support to verify Zone Explicit Open Krishna Kanth Reddy
2020-09-29 16:32   ` [PATCH 0/3] v1 Patchset : Zone Explicit Open support Damien Le Moal
2020-10-06  4:32     ` Krishna Kanth Reddy
2020-10-06  5:18       ` Damien Le Moal

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.