From 67282020ce1b9427f296af1a449ea0d6895530bc Mon Sep 17 00:00:00 2001 From: Shin'ichiro Kawasaki Date: Thu, 8 Jun 2023 16:06:05 +0900 Subject: [PATCH] zbd: fix write zone accounting of trim workload The commit e3be810bf0fd ("zbd: Support zone reset by trim") supported trim for zonemode=zbd by introducing the function zbd_do_io_u_trim(), which calls zbd_reset_zone(). However, it did not call zbd_write_zone_put() to the trim target zone, then trim operation resulted in wrong accounting of write zones. To fix the issue, call zbd_write_zone_put() from zbd_reset_zone(). To cover the case to reset zones without a zbd_write_zone_put() call, prepare another function __zbd_reset_zone(). While at it, simplify zbd_reset_zones() by calling the modified zbd_reset_zone(). Of note is that the constifier of the argument td of do_io_u_trim() is removed since zbd_write_zone_put() requires changes in that argument. Fixes: e3be810bf0fd ("zbd: Support zone reset by trim") Suggested-by: Niklas Cassel Signed-off-by: Shin'ichiro Kawasaki Reviewed-by: Niklas Cassel Signed-off-by: Vincent Fu --- engines/io_uring.c | 2 +- io_u.c | 2 +- io_u.h | 2 +- zbd.c | 39 +++++++++++++++++++++++++++++++-------- zbd.h | 2 +- 5 files changed, 35 insertions(+), 12 deletions(-) diff --git a/engines/io_uring.c b/engines/io_uring.c index ff64fc9f..73e4a27a 100644 --- a/engines/io_uring.c +++ b/engines/io_uring.c @@ -561,7 +561,7 @@ static inline void fio_ioring_cmdprio_prep(struct thread_data *td, ld->sqes[io_u->index].ioprio = io_u->ioprio; } -static int fio_ioring_cmd_io_u_trim(const struct thread_data *td, +static int fio_ioring_cmd_io_u_trim(struct thread_data *td, struct io_u *io_u) { struct fio_file *f = io_u->file; diff --git a/io_u.c b/io_u.c index 6f5fc94d..faf512e5 100644 --- a/io_u.c +++ b/io_u.c @@ -2379,7 +2379,7 @@ int do_io_u_sync(const struct thread_data *td, struct io_u *io_u) return ret; } -int do_io_u_trim(const struct thread_data *td, struct io_u *io_u) +int do_io_u_trim(struct thread_data *td, struct io_u *io_u) { #ifndef FIO_HAVE_TRIM io_u->error = EINVAL; diff --git a/io_u.h b/io_u.h index 55b4d083..b432a540 100644 --- a/io_u.h +++ b/io_u.h @@ -162,7 +162,7 @@ void io_u_mark_submit(struct thread_data *, unsigned int); bool queue_full(const struct thread_data *); int do_io_u_sync(const struct thread_data *, struct io_u *); -int do_io_u_trim(const struct thread_data *, struct io_u *); +int do_io_u_trim(struct thread_data *, struct io_u *); #ifdef FIO_INC_DEBUG static inline void dprint_io_u(struct io_u *io_u, const char *p) diff --git a/zbd.c b/zbd.c index 36b68d62..9455140a 100644 --- a/zbd.c +++ b/zbd.c @@ -254,7 +254,7 @@ static int zbd_reset_wp(struct thread_data *td, struct fio_file *f, } /** - * zbd_reset_zone - reset the write pointer of a single zone + * __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. @@ -263,8 +263,8 @@ static int zbd_reset_wp(struct thread_data *td, struct fio_file *f, * * The caller must hold z->mutex. */ -static int zbd_reset_zone(struct thread_data *td, struct fio_file *f, - struct fio_zone_info *z) +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; @@ -339,6 +339,32 @@ static void zbd_write_zone_put(struct thread_data *td, const struct fio_file *f, z->write = 0; } +/** + * zbd_reset_zone - reset the write pointer of a single zone and remove the zone + * from the array of write zones. + * @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) +{ + int ret; + + ret = __zbd_reset_zone(td, f, z); + if (ret) + return ret; + + pthread_mutex_lock(&f->zbd_info->mutex); + zbd_write_zone_put(td, f, z); + pthread_mutex_unlock(&f->zbd_info->mutex); + return 0; +} + /** * zbd_finish_zone - finish the specified zone * @td: FIO thread data. @@ -404,9 +430,6 @@ static int zbd_reset_zones(struct thread_data *td, struct fio_file *f, continue; zone_lock(td, f, z); - pthread_mutex_lock(&f->zbd_info->mutex); - zbd_write_zone_put(td, f, z); - pthread_mutex_unlock(&f->zbd_info->mutex); if (z->wp != z->start) { dprint(FD_ZBD, "%s: resetting zone %u\n", @@ -2048,7 +2071,7 @@ retry: */ io_u_quiesce(td); zb->reset_zone = 0; - if (zbd_reset_zone(td, f, zb) < 0) + if (__zbd_reset_zone(td, f, zb) < 0) goto eof; if (zb->capacity < min_bs) { @@ -2167,7 +2190,7 @@ char *zbd_write_status(const struct thread_stat *ts) * Return io_u_completed when reset zone succeeds. Return 0 when the target zone * does not have write pointer. On error, return negative errno. */ -int zbd_do_io_u_trim(const struct thread_data *td, struct io_u *io_u) +int zbd_do_io_u_trim(struct thread_data *td, struct io_u *io_u) { struct fio_file *f = io_u->file; struct fio_zone_info *z; diff --git a/zbd.h b/zbd.h index 25a3e0f1..f0ac9876 100644 --- a/zbd.h +++ b/zbd.h @@ -100,7 +100,7 @@ enum fio_ddir zbd_adjust_ddir(struct thread_data *td, struct io_u *io_u, enum fio_ddir ddir); enum io_u_action zbd_adjust_block(struct thread_data *td, struct io_u *io_u); char *zbd_write_status(const struct thread_stat *ts); -int zbd_do_io_u_trim(const struct thread_data *td, struct io_u *io_u); +int zbd_do_io_u_trim(struct thread_data *td, struct io_u *io_u); static inline void zbd_close_file(struct fio_file *f) { -- 2.25.1