On 2021/05/13 7:37, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@xxxxxxx>
>
> Define a new IO engine operation to get the maximum number of open zones.
> Like the existing IO engine operations: .get_zoned_model, .report_zones,
> and .reset_wp, this new IO engine operation is only valid for zoned block
> devices.
>
> Similarly to the other zbd IO engine operations, also provide a default
> implementation inside oslib/linux-blkzoned.c that will be used if the
> ioengine does not override it.
>
> The default Linux oslib implementation is implemented similarly to
> blkzoned_get_zoned_model(), i.e. it will return a successful error code
> even when the sysfs attribute does not exist.
> This is because the sysfs max_open_zones attribute was introduced first
> in Linux v5.9.
> All error handling is still there, so an ioengine that provides its own
> implementation will still have its error code respected properly.
>
> Signed-off-by: Niklas Cassel <niklas.cassel@xxxxxxx>
> ---
> engines/skeleton_external.c | 13 ++++++
> ioengines.h | 4 +-
> oslib/blkzoned.h | 7 +++
> oslib/linux-blkzoned.c | 22 ++++++++++
> zbd.c | 87 ++++++++++++++++++++++++++++++++++---
> 5 files changed, 127 insertions(+), 6 deletions(-)
>
> diff --git a/engines/skeleton_external.c b/engines/skeleton_external.c
> index 7f3e4cb3..c79b6f11 100644
> --- a/engines/skeleton_external.c
> +++ b/engines/skeleton_external.c
> @@ -193,6 +193,18 @@ static int fio_skeleton_reset_wp(struct thread_data *td, struct fio_file *f,
> return 0;
> }
>
> +/*
> + * Hook called for getting the maximum number of open zones for a
> + * ZBD_HOST_MANAGED zoned block device.
> + * A @max_open_zones value set to zero means no limit.
> + */
> +static int fio_skeleton_get_max_open_zones(struct thread_data *td,
> + struct fio_file *f,
> + unsigned int *max_open_zones)
> +{
> + 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 +224,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,
> + .get_max_open_zones = fio_skeleton_get_max_open_zones,
> .options = options,
> .option_struct_size = sizeof(struct fio_skeleton_options),
> };
> diff --git a/ioengines.h b/ioengines.h
> index 1d01ab0a..b3f755b4 100644
> --- a/ioengines.h
> +++ b/ioengines.h
> @@ -8,7 +8,7 @@
> #include "io_u.h"
> #include "zbd_types.h"
>
> -#define FIO_IOOPS_VERSION 29
> +#define FIO_IOOPS_VERSION 30
>
> #ifndef CONFIG_DYNAMIC_ENGINES
> #define FIO_STATIC static
> @@ -59,6 +59,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 (*get_max_open_zones)(struct thread_data *, struct fio_file *,
> + unsigned int *);
> int option_struct_size;
> struct fio_option *options;
> };
> diff --git a/oslib/blkzoned.h b/oslib/blkzoned.h
> index 4cc071dc..719b041d 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_get_max_open_zones(struct thread_data *td, struct fio_file *f,
> + unsigned int *max_open_zones);
> #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_get_max_open_zones(struct thread_data *td, struct fio_file *f,
> + unsigned int *max_open_zones)
> +{
> + return -EIO;
> +}
> #endif
>
> #endif /* FIO_BLKZONED_H */
> diff --git a/oslib/linux-blkzoned.c b/oslib/linux-blkzoned.c
> index 127069ca..82b662a2 100644
> --- a/oslib/linux-blkzoned.c
> +++ b/oslib/linux-blkzoned.c
> @@ -160,6 +160,28 @@ int blkzoned_get_zoned_model(struct thread_data *td, struct fio_file *f,
> return 0;
> }
>
> +int blkzoned_get_max_open_zones(struct thread_data *td, struct fio_file *f,
> + unsigned int *max_open_zones)
> +{
> + char *max_open_str;
> +
> + if (f->filetype != FIO_TYPE_BLOCK) {
> + return -EIO;
> + }
No need for the curly brackets.
> +
> + max_open_str = blkzoned_get_sysfs_attr(f->file_name, "queue/max_open_zones");
> + if (!max_open_str)
> + return 0;
> +
> + dprint(FD_ZBD, "%s: max open zones supported by device: %s\n",
> + f->file_name, max_open_str);
> + *max_open_zones = atoll(max_open_str);
> +
> + free(max_open_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 8ed8f195..f23bbbac 100644
> --- a/zbd.c
> +++ b/zbd.c
> @@ -113,6 +113,34 @@ int zbd_reset_wp(struct thread_data *td, struct fio_file *f,
> return ret;
> }
>
> +/**
> + * zbd_get_max_open_zones - Get the maximum number of open zones
> + * @td: FIO thread data
> + * @f: FIO file for which to get max open zones
> + * @max_open_zones: Upon success, result will be stored here.
> + *
> + * A @max_open_zones value set to zero means no limit.
> + *
> + * 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)
> +{
> + int ret;
> +
> + 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);
> + if (ret < 0) {
> + td_verror(td, errno, "get max open zones failed");
> + log_err("%s: get max open zones failed (%d).\n",
> + f->file_name, errno);
> + }
> +
> + return ret;
> +}
> +
> /**
> * zbd_zone_idx - convert an offset into a zone number
> * @f: file pointer.
> @@ -554,6 +582,48 @@ out:
> return ret;
> }
>
> +static int zbd_set_max_open_zones(struct thread_data *td, struct fio_file *f)
> +{
> + struct zoned_block_device_info *zbd = f->zbd_info;
> + unsigned int max_open_zones;
> + int ret;
> +
> + if (zbd->model != ZBD_HOST_MANAGED) {
> + /* Only host-managed devices have a max open limit */
> + zbd->max_open_zones = td->o.max_open_zones;
> + goto out;
> + }
> +
> + /* If host-managed, get the max open limit */
> + ret = zbd_get_max_open_zones(td, f, &max_open_zones);
> + if (ret)
> + return ret;
> +
> + if (td->o.max_open_zones > 0 && max_open_zones > 0 &&
> + td->o.max_open_zones > max_open_zones) {
> + /* User requested a limit, but limit is too large */
> + td_verror(td, EINVAL,
> + "Specified --max_open_zones is too large");
> + log_err("Specified --max_open_zones (%d) is larger than max (%u)\n",
> + td->o.max_open_zones, max_open_zones);
> + return -EINVAL;
> + } else if (td->o.max_open_zones > 0) {
> + /* User requested a limit, limit is not too large */
> + zbd->max_open_zones = td->o.max_open_zones;
> + } else {
> + /* User did not request a limit. Set limit to max supported */
> + zbd->max_open_zones = max_open_zones;
> + }
Here, you could reverse the order of the if/else if/else. That would simplify
the conditions:
if (!td->o.max_open_zones) {
/* User did not request a limit. Set limit to max supported */
zbd->max_open_zones = max_open_zones;
} else if (td->o.max_open_zones < max_open_zones) {
/* User requested a limit, limit is not too large */
zbd->max_open_zones = td->o.max_open_zones;
} else {
/* User requested a limit, but limit is too large */
...
return -EINVAL;
}
> +
> +out:
> + /* Ensure that the limit is not larger than FIO's internal limit */
> + zbd->max_open_zones = zbd->max_open_zones ?: ZBD_MAX_OPEN_ZONES;
> + dprint(FD_ZBD, "%s: using max open zones limit: %"PRIu32"\n",
> + f->file_name, zbd->max_open_zones);
It may be good to have a log_info() too here, no ?
> +
> + return 0;
> +}
> +
> /*
> * Allocate zone information and store it into f->zbd_info if zonemode=zbd.
> *
> @@ -576,9 +646,13 @@ static int zbd_create_zone_info(struct thread_data *td, struct fio_file *f)
> case ZBD_HOST_AWARE:
> case ZBD_HOST_MANAGED:
> ret = parse_zone_info(td, f);
> + if (ret)
> + return ret;
> break;
> case ZBD_NONE:
> ret = init_zone_info(td, f);
> + if (ret)
> + return ret;
> break;
> default:
> td_verror(td, EINVAL, "Unsupported zoned model");
> @@ -586,12 +660,15 @@ static int zbd_create_zone_info(struct thread_data *td, struct fio_file *f)
> return -EINVAL;
> }
>
> - if (ret == 0) {
> - f->zbd_info->model = zbd_model;
> - f->zbd_info->max_open_zones =
> - td->o.max_open_zones ?: ZBD_MAX_OPEN_ZONES;
> + f->zbd_info->model = zbd_model;
> +
> + ret = zbd_set_max_open_zones(td, f);
> + if (ret) {
> + zbd_free_zone_info(f);
> + return ret;
> }
> - return ret;
> +
> + return 0;
> }
>
> void zbd_free_zone_info(struct fio_file *f)
>
--
Damien Le Moal
Western Digital Research
