From: Niklas Cassel <Niklas.Cassel@wdc.com>
To: Damien Le Moal <dlemoal@kernel.org>
Cc: "fio@vger.kernel.org" <fio@vger.kernel.org>,
Vincent Fu <vincentfu@gmail.com>, Jens Axboe <axboe@kernel.dk>
Subject: Re: [PATCH v3 2/6] cmdprio: Introduce generic option definitions
Date: Fri, 21 Jul 2023 11:32:00 +0000 [thread overview]
Message-ID: <ZLpsr2wcVLvoi6ac@x1-carbon> (raw)
In-Reply-To: <20230721110510.44772-3-dlemoal@kernel.org>
On Fri, Jul 21, 2023 at 08:05:06PM +0900, Damien Le Moal wrote:
> The definition of the per-I/O priority options for the io_uring and
> libaio I/O engines are almost identical, differing only by the option
> group and option data structure used.
>
> Introduce the CMDPRIO_OPTIONS macro in engines/cmdprio.h to generically
> define these options in the io_uring and libaio engines to simplify the
> code.
>
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> ---
> engines/cmdprio.h | 84 ++++++++++++++++++++++++++++++++++++++++++++++
> engines/io_uring.c | 82 +-------------------------------------------
> engines/libaio.c | 82 +-------------------------------------------
> 3 files changed, 86 insertions(+), 162 deletions(-)
>
> diff --git a/engines/cmdprio.h b/engines/cmdprio.h
> index 755da8d0..2c9d87bc 100644
> --- a/engines/cmdprio.h
> +++ b/engines/cmdprio.h
> @@ -7,6 +7,7 @@
> #define FIO_CMDPRIO_H
>
> #include "../fio.h"
> +#include "../optgroup.h"
>
> /* read and writes only, no trim */
> #define CMDPRIO_RWDIR_CNT 2
> @@ -42,6 +43,89 @@ struct cmdprio_options {
> char *bssplit_str;
> };
>
> +#ifdef FIO_HAVE_IOPRIO_CLASS
> +#define CMDPRIO_OPTIONS(opt_struct, opt_group) \
> + { \
> + .name = "cmdprio_percentage", \
> + .lname = "high priority percentage", \
> + .type = FIO_OPT_INT, \
> + .off1 = offsetof(opt_struct, \
> + cmdprio_options.percentage[DDIR_READ]), \
> + .off2 = offsetof(opt_struct, \
> + cmdprio_options.percentage[DDIR_WRITE]), \
> + .minval = 0, \
> + .maxval = 100, \
> + .help = "Send high priority I/O this percentage of the time", \
> + .category = FIO_OPT_C_ENGINE, \
> + .group = opt_group, \
> + }, \
> + { \
> + .name = "cmdprio_class", \
> + .lname = "Asynchronous I/O priority class", \
> + .type = FIO_OPT_INT, \
> + .off1 = offsetof(opt_struct, \
> + cmdprio_options.class[DDIR_READ]), \
> + .off2 = offsetof(opt_struct, \
> + cmdprio_options.class[DDIR_WRITE]), \
> + .help = "Set asynchronous IO priority class", \
> + .minval = IOPRIO_MIN_PRIO_CLASS + 1, \
> + .maxval = IOPRIO_MAX_PRIO_CLASS, \
> + .interval = 1, \
> + .category = FIO_OPT_C_ENGINE, \
> + .group = opt_group, \
> + }, \
> + { \
> + .name = "cmdprio", \
> + .lname = "Asynchronous I/O priority level", \
> + .type = FIO_OPT_INT, \
> + .off1 = offsetof(opt_struct, \
> + cmdprio_options.level[DDIR_READ]), \
> + .off2 = offsetof(opt_struct, \
> + cmdprio_options.level[DDIR_WRITE]), \
> + .help = "Set asynchronous IO priority level", \
> + .minval = IOPRIO_MIN_PRIO, \
> + .maxval = IOPRIO_MAX_PRIO, \
> + .interval = 1, \
> + .category = FIO_OPT_C_ENGINE, \
> + .group = opt_group, \
> + }, \
> + { \
> + .name = "cmdprio_bssplit", \
> + .lname = "Priority percentage block size split", \
> + .type = FIO_OPT_STR_STORE, \
> + .off1 = offsetof(opt_struct, cmdprio_options.bssplit_str), \
> + .help = "Set priority percentages for different block sizes", \
> + .category = FIO_OPT_C_ENGINE, \
> + .group = opt_group, \
> + }
> +#else
> +#define CMDPRIO_OPTIONS(opt_struct, opt_group) \
> + { \
> + .name = "cmdprio_percentage", \
> + .lname = "high priority percentage", \
> + .type = FIO_OPT_UNSUPPORTED, \
> + .help = "Platform does not support I/O priority classes", \
> + }, \
> + { \
> + .name = "cmdprio_class", \
> + .lname = "Asynchronous I/O priority class", \
> + .type = FIO_OPT_UNSUPPORTED, \
> + .help = "Platform does not support I/O priority classes", \
> + }, \
> + { \
> + .name = "cmdprio", \
> + .lname = "Asynchronous I/O priority level", \
> + .type = FIO_OPT_UNSUPPORTED, \
> + .help = "Platform does not support I/O priority classes", \
> + }, \
> + { \
> + .name = "cmdprio_bssplit", \
> + .lname = "Priority percentage block size split", \
> + .type = FIO_OPT_UNSUPPORTED, \
> + .help = "Platform does not support I/O priority classes", \
> + }
> +#endif
> +
> struct cmdprio {
> struct cmdprio_options *options;
> struct cmdprio_prio perc_entry[CMDPRIO_RWDIR_CNT];
> diff --git a/engines/io_uring.c b/engines/io_uring.c
> index f30a3c00..5613c4c6 100644
> --- a/engines/io_uring.c
> +++ b/engines/io_uring.c
> @@ -127,87 +127,6 @@ static struct fio_option options[] = {
> .category = FIO_OPT_C_ENGINE,
> .group = FIO_OPT_G_IOURING,
> },
> -#ifdef FIO_HAVE_IOPRIO_CLASS
> - {
> - .name = "cmdprio_percentage",
> - .lname = "high priority percentage",
> - .type = FIO_OPT_INT,
> - .off1 = offsetof(struct ioring_options,
> - cmdprio_options.percentage[DDIR_READ]),
> - .off2 = offsetof(struct ioring_options,
> - cmdprio_options.percentage[DDIR_WRITE]),
> - .minval = 0,
> - .maxval = 100,
> - .help = "Send high priority I/O this percentage of the time",
> - .category = FIO_OPT_C_ENGINE,
> - .group = FIO_OPT_G_IOURING,
> - },
> - {
> - .name = "cmdprio_class",
> - .lname = "Asynchronous I/O priority class",
> - .type = FIO_OPT_INT,
> - .off1 = offsetof(struct ioring_options,
> - cmdprio_options.class[DDIR_READ]),
> - .off2 = offsetof(struct ioring_options,
> - cmdprio_options.class[DDIR_WRITE]),
> - .help = "Set asynchronous IO priority class",
> - .minval = IOPRIO_MIN_PRIO_CLASS + 1,
> - .maxval = IOPRIO_MAX_PRIO_CLASS,
> - .interval = 1,
> - .category = FIO_OPT_C_ENGINE,
> - .group = FIO_OPT_G_IOURING,
> - },
> - {
> - .name = "cmdprio",
> - .lname = "Asynchronous I/O priority level",
> - .type = FIO_OPT_INT,
> - .off1 = offsetof(struct ioring_options,
> - cmdprio_options.level[DDIR_READ]),
> - .off2 = offsetof(struct ioring_options,
> - cmdprio_options.level[DDIR_WRITE]),
> - .help = "Set asynchronous IO priority level",
> - .minval = IOPRIO_MIN_PRIO,
> - .maxval = IOPRIO_MAX_PRIO,
> - .interval = 1,
> - .category = FIO_OPT_C_ENGINE,
> - .group = FIO_OPT_G_IOURING,
> - },
> - {
> - .name = "cmdprio_bssplit",
> - .lname = "Priority percentage block size split",
> - .type = FIO_OPT_STR_STORE,
> - .off1 = offsetof(struct ioring_options,
> - cmdprio_options.bssplit_str),
> - .help = "Set priority percentages for different block sizes",
> - .category = FIO_OPT_C_ENGINE,
> - .group = FIO_OPT_G_IOURING,
> - },
> -#else
> - {
> - .name = "cmdprio_percentage",
> - .lname = "high priority percentage",
> - .type = FIO_OPT_UNSUPPORTED,
> - .help = "Your platform does not support I/O priority classes",
> - },
> - {
> - .name = "cmdprio_class",
> - .lname = "Asynchronous I/O priority class",
> - .type = FIO_OPT_UNSUPPORTED,
> - .help = "Your platform does not support I/O priority classes",
> - },
> - {
> - .name = "cmdprio",
> - .lname = "Asynchronous I/O priority level",
> - .type = FIO_OPT_UNSUPPORTED,
> - .help = "Your platform does not support I/O priority classes",
> - },
> - {
> - .name = "cmdprio_bssplit",
> - .lname = "Priority percentage block size split",
> - .type = FIO_OPT_UNSUPPORTED,
> - .help = "Your platform does not support I/O priority classes",
> - },
> -#endif
> {
> .name = "fixedbufs",
> .lname = "Fixed (pre-mapped) IO buffers",
> @@ -297,6 +216,7 @@ static struct fio_option options[] = {
> .category = FIO_OPT_C_ENGINE,
> .group = FIO_OPT_G_IOURING,
> },
> + CMDPRIO_OPTIONS(struct ioring_options, FIO_OPT_G_IOURING),
> {
> .name = NULL,
> },
> diff --git a/engines/libaio.c b/engines/libaio.c
> index 6a0745aa..aaccc7ce 100644
> --- a/engines/libaio.c
> +++ b/engines/libaio.c
> @@ -72,87 +72,6 @@ static struct fio_option options[] = {
> .category = FIO_OPT_C_ENGINE,
> .group = FIO_OPT_G_LIBAIO,
> },
> -#ifdef FIO_HAVE_IOPRIO_CLASS
> - {
> - .name = "cmdprio_percentage",
> - .lname = "high priority percentage",
> - .type = FIO_OPT_INT,
> - .off1 = offsetof(struct libaio_options,
> - cmdprio_options.percentage[DDIR_READ]),
> - .off2 = offsetof(struct libaio_options,
> - cmdprio_options.percentage[DDIR_WRITE]),
> - .minval = 0,
> - .maxval = 100,
> - .help = "Send high priority I/O this percentage of the time",
> - .category = FIO_OPT_C_ENGINE,
> - .group = FIO_OPT_G_LIBAIO,
> - },
> - {
> - .name = "cmdprio_class",
> - .lname = "Asynchronous I/O priority class",
> - .type = FIO_OPT_INT,
> - .off1 = offsetof(struct libaio_options,
> - cmdprio_options.class[DDIR_READ]),
> - .off2 = offsetof(struct libaio_options,
> - cmdprio_options.class[DDIR_WRITE]),
> - .help = "Set asynchronous IO priority class",
> - .minval = IOPRIO_MIN_PRIO_CLASS + 1,
> - .maxval = IOPRIO_MAX_PRIO_CLASS,
> - .interval = 1,
> - .category = FIO_OPT_C_ENGINE,
> - .group = FIO_OPT_G_LIBAIO,
> - },
> - {
> - .name = "cmdprio",
> - .lname = "Asynchronous I/O priority level",
> - .type = FIO_OPT_INT,
> - .off1 = offsetof(struct libaio_options,
> - cmdprio_options.level[DDIR_READ]),
> - .off2 = offsetof(struct libaio_options,
> - cmdprio_options.level[DDIR_WRITE]),
> - .help = "Set asynchronous IO priority level",
> - .minval = IOPRIO_MIN_PRIO,
> - .maxval = IOPRIO_MAX_PRIO,
> - .interval = 1,
> - .category = FIO_OPT_C_ENGINE,
> - .group = FIO_OPT_G_LIBAIO,
> - },
> - {
> - .name = "cmdprio_bssplit",
> - .lname = "Priority percentage block size split",
> - .type = FIO_OPT_STR_STORE,
> - .off1 = offsetof(struct libaio_options,
> - cmdprio_options.bssplit_str),
> - .help = "Set priority percentages for different block sizes",
> - .category = FIO_OPT_C_ENGINE,
> - .group = FIO_OPT_G_LIBAIO,
> - },
> -#else
> - {
> - .name = "cmdprio_percentage",
> - .lname = "high priority percentage",
> - .type = FIO_OPT_UNSUPPORTED,
> - .help = "Your platform does not support I/O priority classes",
> - },
> - {
> - .name = "cmdprio_class",
> - .lname = "Asynchronous I/O priority class",
> - .type = FIO_OPT_UNSUPPORTED,
> - .help = "Your platform does not support I/O priority classes",
> - },
> - {
> - .name = "cmdprio",
> - .lname = "Asynchronous I/O priority level",
> - .type = FIO_OPT_UNSUPPORTED,
> - .help = "Your platform does not support I/O priority classes",
> - },
> - {
> - .name = "cmdprio_bssplit",
> - .lname = "Priority percentage block size split",
> - .type = FIO_OPT_UNSUPPORTED,
> - .help = "Your platform does not support I/O priority classes",
> - },
> -#endif
> {
> .name = "nowait",
> .lname = "RWF_NOWAIT",
> @@ -162,6 +81,7 @@ static struct fio_option options[] = {
> .category = FIO_OPT_C_ENGINE,
> .group = FIO_OPT_G_LIBAIO,
> },
> + CMDPRIO_OPTIONS(struct libaio_options, FIO_OPT_G_LIBAIO),
> {
> .name = NULL,
> },
> --
> 2.41.0
>
Nice cleanup!
Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com>
next prev parent reply other threads:[~2023-07-21 11:32 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-21 11:05 [PATCH v3 0/6] Add support for I/O priority hints Damien Le Moal
2023-07-21 11:05 ` [PATCH v3 1/6] os-linux: Cleanup IO priority class and value macros Damien Le Moal
2023-07-21 11:05 ` [PATCH v3 2/6] cmdprio: Introduce generic option definitions Damien Le Moal
2023-07-21 11:32 ` Niklas Cassel [this message]
2023-07-21 11:05 ` [PATCH v3 3/6] os-linux: add initial support for IO priority hints Damien Le Moal
2023-07-21 11:05 ` [PATCH v3 4/6] options: add priohint option Damien Le Moal
2023-07-21 11:05 ` [PATCH v3 5/6] cmdprio: Add support for per I/O priority hint Damien Le Moal
2023-07-21 11:32 ` Niklas Cassel
2023-07-21 11:05 ` [PATCH v3 6/6] stats: Add hint information to per priority level stats Damien Le Moal
2023-07-21 21:23 ` [PATCH v3 0/6] Add support for I/O priority hints Jens Axboe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZLpsr2wcVLvoi6ac@x1-carbon \
--to=niklas.cassel@wdc.com \
--cc=axboe@kernel.dk \
--cc=dlemoal@kernel.org \
--cc=fio@vger.kernel.org \
--cc=vincentfu@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox