On 2020/12/01 20:44, Krishna Kanth Reddy wrote:
> From: Ankit Kumar <ankit.kumar@xxxxxxxxxxx>
>
> Added a check so that source and destination zones don't overlap.
> Source and destination offsets are aligned to zone start.
> The source range zone data is copied sequentially to the destination
> zones.
> Added a function to reset the destination zones. Source zones won't
> be reset.
I do not see how this can work correctly without relying on the zone locking
mechanism. Other jobs may be writing to the same zones too. That will either
result in the copy failing or the writes failing due to the zone wp unexpectedly
changing.
>
> Signed-off-by: Krishna Kanth Reddy <krish.reddy@xxxxxxxxxxx>
> ---
> file.h | 4 +++
> filesetup.c | 18 +++++++++++
> init.c | 5 +++
> io_u.c | 2 +-
> zbd.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++++-----
> 5 files changed, 109 insertions(+), 8 deletions(-)
>
> diff --git a/file.h b/file.h
> index f5a794e4..23012753 100644
> --- a/file.h
> +++ b/file.h
> @@ -110,6 +110,10 @@ struct fio_file {
> uint32_t min_zone; /* inclusive */
> uint32_t max_zone; /* exclusive */
>
> + /* zonemode=zbd copy destination working area */
> + uint32_t min_dest_zone; /* inclusive */
> + uint32_t max_dest_zone; /* exclusive */
> +
> /*
> * Track last end and last start of IO for a given data direction
> */
> diff --git a/filesetup.c b/filesetup.c
> index 68a21fac..54752511 100644
> --- a/filesetup.c
> +++ b/filesetup.c
> @@ -1231,6 +1231,24 @@ int setup_files(struct thread_data *td)
>
> if (td_copy(td))
> f->file_dest_offset = get_dest_offset(td, f);
> +
> + if (td_copy(td) && (td->o.zone_mode == ZONE_MODE_ZBD)) {
> + if (f->file_offset > f->file_dest_offset) {
> + if (f->file_offset - f->file_dest_offset < f->io_size) {
> + log_err("%s: For copy operation on ZBD device "
> + "source and destination area shouldn't overlap\n",
> + o->name);
> + goto err_out;
> + }
> + } else {
> + if (f->file_dest_offset - f->file_offset < f->io_size) {
> + log_err("%s: For copy operation on ZBD device "
> + "source and destination area shouldn't overlap\n",
> + o->name);
> + goto err_out;
> + }
> + }
> + }
> }
>
> if (td->o.block_error_hist) {
> diff --git a/init.c b/init.c
> index e5835b7b..b5db65af 100644
> --- a/init.c
> +++ b/init.c
> @@ -671,6 +671,11 @@ static int fixup_options(struct thread_data *td)
> if (o->zone_mode == ZONE_MODE_STRIDED && !o->zone_range)
> o->zone_range = o->zone_size;
>
> + if (o->zone_mode == ZONE_MODE_ZBD && td_copy(td) && td_random(td)) {
> + log_err("fio: --zonemode=zbd supports copy operation only in sequential mode.\n");
> + ret |= 1;
> + }
> +
> /*
> * Reads and copies can do overwrites, we always need to pre-create the file
> */
> diff --git a/io_u.c b/io_u.c
> index 83c7960a..2de91f2a 100644
> --- a/io_u.c
> +++ b/io_u.c
> @@ -1003,7 +1003,7 @@ static int fill_io_u(struct thread_data *td, struct io_u *io_u)
> offset = io_u->offset;
> }
>
> - if (td->o.zone_mode == ZONE_MODE_ZBD) {
> + if ((td->o.zone_mode == ZONE_MODE_ZBD) && !(td_copy(td))) {
> ret = zbd_adjust_block(td, io_u);
> if (ret == io_u_eof)
> return 1;
> diff --git a/zbd.c b/zbd.c
> index 58fed98e..8201665b 100644
> --- a/zbd.c
> +++ b/zbd.c
> @@ -246,11 +246,11 @@ static bool zbd_is_seq_job(struct fio_file *f)
> */
> static bool zbd_verify_sizes(void)
> {
> - const struct fio_zone_info *z;
> + const struct fio_zone_info *z, *zd;
> struct thread_data *td;
> struct fio_file *f;
> uint64_t new_offset, new_end;
> - uint32_t zone_idx;
> + uint32_t zone_idx, zone_didx;
> int i, j;
>
> for_each_td(td, i) {
> @@ -259,6 +259,9 @@ static bool zbd_verify_sizes(void)
> continue;
> if (f->file_offset >= f->real_file_size)
> continue;
> + if ((td->o.td_ddir == TD_DDIR_COPY) &&
> + (f->file_dest_offset >= f->real_file_size))
> + continue;
> if (!zbd_is_seq_job(f))
> continue;
>
> @@ -301,6 +304,15 @@ static bool zbd_verify_sizes(void)
> f->io_size -= (new_offset - f->file_offset);
> f->file_offset = new_offset;
> }
> + if (td->o.td_ddir == TD_DDIR_COPY) {
> + zone_didx = zbd_zone_idx(f, f->file_dest_offset);
> + zd = &f->zbd_info->zone_info[zone_didx];
> + if (f->file_dest_offset != zd->start) {
> + new_offset = zbd_zone_end(zd);
> + f->file_dest_offset = new_offset;
> + }
> + }
> +
> zone_idx = zbd_zone_idx(f, f->file_offset + f->io_size);
> z = &f->zbd_info->zone_info[zone_idx];
> new_end = z->start;
> @@ -320,6 +332,12 @@ static bool zbd_verify_sizes(void)
> f->min_zone = zbd_zone_idx(f, f->file_offset);
> f->max_zone = zbd_zone_idx(f, f->file_offset + f->io_size);
> assert(f->min_zone < f->max_zone);
> +
> + if (td->o.td_ddir == TD_DDIR_COPY) {
> + f->min_dest_zone = zbd_zone_idx(f, f->file_dest_offset);
> + f->max_dest_zone = zbd_zone_idx(f, f->file_dest_offset + f->io_size);
> + assert(f->min_dest_zone < f->max_dest_zone);
> + }
> }
> }
>
> @@ -823,6 +841,42 @@ static int zbd_reset_zones(struct thread_data *td, struct fio_file *f,
> return res;
> }
>
> +/*
> + * Reset a range of zones.
> + * @td: fio thread data.
> + * @f: fio file for which to reset zones
> + */
> +static void zbd_reset_dest_zones(struct thread_data *td, struct fio_file *f)
> +{
> + struct fio_zone_info *z, *zb, *ze;
> + int ret = 0;
> + uint64_t offset, length;
> +
> + zb = &f->zbd_info->zone_info[f->min_dest_zone];
> + ze = &f->zbd_info->zone_info[f->max_dest_zone];
> +
> + for (z = zb; z < ze; z++) {
> + offset = z->start;
> + length = (z+1)->start - offset;
> +
> + dprint(FD_ZBD, "%s: resetting wp of zone %u.\n", f->file_name,
> + zbd_zone_nr(f->zbd_info, z));
> + switch (f->zbd_info->model) {
> + case ZBD_HOST_AWARE:
> + case ZBD_HOST_MANAGED:
> + ret = zbd_reset_wp(td, f, offset, length);
> + break;
> + default:
> + break;
> + }
> +
> + if (ret < 0)
> + continue;
> +
> + td->ts.nr_zone_resets++;
> + }
> +}
> +
> /*
> * Reset zbd_info.write_cnt, the counter that counts down towards the next
> * zone reset.
> @@ -924,9 +978,14 @@ void zbd_file_reset(struct thread_data *td, struct fio_file *f)
> {
> struct fio_zone_info *zb, *ze;
>
> - if (!f->zbd_info || !td_write(td))
> + if (!f->zbd_info || !(td_write(td) || td_copy(td)))
> return;
>
> + if (td_copy(td)) {
> + zbd_reset_dest_zones(td, f);
> + return;
> + }
> +
> zb = &f->zbd_info->zone_info[f->min_zone];
> ze = &f->zbd_info->zone_info[f->max_zone];
> zbd_init_swd(f);
> @@ -1410,8 +1469,8 @@ 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;
> + struct fio_zone_info *z, *zd;
> + uint32_t zone_idx, zone_didx;
>
> assert(td->o.zone_mode == ZONE_MODE_ZBD);
> assert(td->o.zone_size);
> @@ -1419,13 +1478,18 @@ void setup_zbd_zone_mode(struct thread_data *td, struct io_u *io_u)
> zone_idx = zbd_zone_idx(f, f->last_pos[ddir]);
> z = &f->zbd_info->zone_info[zone_idx];
>
> + if (ddir == DDIR_COPY) {
> + zone_didx = zbd_zone_idx(f, f->last_pos_dest[ddir]);
> + zd = &f->zbd_info->zone_info[zone_didx];
> + }
> +
> /*
> * When the zone capacity is smaller than the zone size and the I/O is
> - * sequential write, skip to zone end if the latest position is at the
> + * sequential write or copy, 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 &&
> + (ddir == DDIR_WRITE || ddir == DDIR_COPY) &&
> f->last_pos[ddir] >= zbd_zone_capacity_end(z)) {
> dprint(FD_ZBD,
> "%s: Jump from zone capacity limit to zone end:"
> @@ -1436,6 +1500,8 @@ void setup_zbd_zone_mode(struct thread_data *td, struct io_u *io_u)
> (unsigned long long) z->capacity);
> td->io_skip_bytes += zbd_zone_end(z) - f->last_pos[ddir];
> f->last_pos[ddir] = zbd_zone_end(z);
> + if (ddir == DDIR_COPY)
> + f->last_pos_dest[ddir] = zbd_zone_end(zd);
> }
>
> /*
> @@ -1461,13 +1527,21 @@ void setup_zbd_zone_mode(struct thread_data *td, struct io_u *io_u)
> td->zone_bytes = 0;
> f->file_offset += td->o.zone_size + td->o.zone_skip;
>
> + if (ddir == DDIR_COPY)
> + f->file_dest_offset += td->o.zone_size + td->o.zone_skip;
> /*
> * Wrap from the beginning, if we exceed the file size
> */
> if (f->file_offset >= f->real_file_size)
> f->file_offset = get_start_offset(td, f);
>
> + if ((ddir == DDIR_COPY) && f->file_dest_offset >= f->real_file_size)
> + f->file_dest_offset = get_dest_offset(td, f);
> +
> f->last_pos[ddir] = f->file_offset;
> + if (ddir == DDIR_COPY)
> + f->last_pos_dest[io_u->ddir] = f->file_dest_offset;
> +
> td->io_skip_bytes += td->o.zone_skip;
> }
> }
>
--
Damien Le Moal
Western Digital Research
