On Jul 18, 2023 / 13:55, Niklas Cassel wrote:
> On Fri, Jul 07, 2023 at 12:14:53PM +0900, Shin'ichiro Kawasaki wrote:
> > As a preparation to improve open zones accounting for devices with the
> > max_active_zones limit, refer max_active_zones sysfs attribute. Then
> > keep the limit value in the struct zoned_block_device_info.
> >
> > Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@xxxxxxx>
> > ---
> > oslib/blkzoned.h | 9 +++++++++
> > oslib/linux-blkzoned.c | 23 +++++++++++++++++++++++
> > zbd.c | 8 ++++++++
> > zbd.h | 4 ++++
> > 4 files changed, 44 insertions(+)
> >
> > diff --git a/oslib/blkzoned.h b/oslib/blkzoned.h
> > index 29fb034f..e598bd4f 100644
> > --- a/oslib/blkzoned.h
> > +++ b/oslib/blkzoned.h
> > @@ -18,6 +18,9 @@ extern int blkzoned_reset_wp(struct thread_data *td, struct fio_file *f,
> > uint64_t offset, uint64_t length);
> > extern int blkzoned_get_max_open_zones(struct thread_data *td, struct fio_file *f,
> > unsigned int *max_open_zones);
> > +extern int blkzoned_get_max_active_zones(struct thread_data *td,
> > + struct fio_file *f,
> > + unsigned int *max_active_zones);
> > extern int blkzoned_finish_zone(struct thread_data *td, struct fio_file *f,
> > uint64_t offset, uint64_t length);
> > #else
> > @@ -53,6 +56,12 @@ static inline int blkzoned_get_max_open_zones(struct thread_data *td, struct fio
> > {
> > return -EIO;
> > }
> > +static inline int blkzoned_get_max_active_zones(struct thread_data *td,
> > + struct fio_file *f,
> > + unsigned int *max_open_zones)
> > +{
> > + return -EIO;
> > +}
> > static inline int blkzoned_finish_zone(struct thread_data *td,
> > struct fio_file *f,
> > uint64_t offset, uint64_t length)
> > diff --git a/oslib/linux-blkzoned.c b/oslib/linux-blkzoned.c
> > index 722e0992..2c3ecf33 100644
> > --- a/oslib/linux-blkzoned.c
> > +++ b/oslib/linux-blkzoned.c
> > @@ -186,6 +186,29 @@ int blkzoned_get_max_open_zones(struct thread_data *td, struct fio_file *f,
> > return 0;
> > }
> >
> > +int blkzoned_get_max_active_zones(struct thread_data *td, struct fio_file *f,
> > + unsigned int *max_active_zones)
> > +{
> > + char *max_active_str;
> > +
> > + if (f->filetype != FIO_TYPE_BLOCK)
> > + return -EIO;
> > +
> > + max_active_str = blkzoned_get_sysfs_attr(f->file_name, "queue/max_active_zones");
> > + if (!max_active_str) {
> > + *max_active_zones = 0;
> > + return 0;
> > + }
> > +
> > + dprint(FD_ZBD, "%s: max active zones supported by device: %s\n",
> > + f->file_name, max_active_str);
> > + *max_active_zones = atoll(max_active_str);
> > +
> > + free(max_active_str);
> > +
> > + return 0;
> > +}
> > +
> > static uint64_t zone_capacity(struct blk_zone_report *hdr,
> > struct blk_zone *blkz)
> > {
> > diff --git a/zbd.c b/zbd.c
> > index d4565215..9743b60e 100644
> > --- a/zbd.c
> > +++ b/zbd.c
> > @@ -922,6 +922,14 @@ static int parse_zone_info(struct thread_data *td, struct fio_file *f)
> > /* a sentinel */
> > zbd_info->zone_info[nr_zones].start = offset;
> >
> > + ret = blkzoned_get_max_active_zones(td, f,
> > + &zbd_info->max_active_zones);
>
>
> For max_open_zones, we have a get_max_open_zones function pointer in struct
> ioengine_ops.
>
> And then we have a function:
> static int zbd_get_max_open_zones(struct thread_data *td, struct fio_file *f,
> unsigned int *max_open_zones)
> {
> if (td->io_ops && td->io_ops->get_max_open_zones)
> ret = td->io_ops->get_max_open_zones(td, f, max_open_zones);
> else
> ret = blkzoned_get_max_open_zones(td, f, max_open_zones);
>
> }
> in zbd.c.
>
>
> I do realize that the engines/libzbc.c ioengine will not provide a custom
> implementation for get_max_active_zones, since max active zones does not
> exist in ZBC/ZAC, however, perhaps we should be nice/consistent and provide
> a .get_max_active_zones function pointer anyway, I can imagine that the
> in-tree ioengine engines/xnvme.c might want to implement it, and I could
> also see the out-of-tree SPDK ioengine plugin(s) wanting to implement it.
> (Even if introducing a new function pointer means that we need to bump
> FIO_IOOPS_VERSION.)
Thanks for the review. I agree that this idea will be nice and consistent.
Another IO engine to support .get_max_active_zones will be io_uring engine.
I will rework this patch to add the callback to struct ioengine_ops.
