On 2021/11/09 9:28, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@xxxxxxx>
>
> Move cmdprio function definitions from the cmdprio.h header file to a new
> cmdprio.c file, such that we can add new static functions to cmdprio.c.
>
> A follow up patch will add new cmdprio functions which do not need to be
> directly accessible by ioengines.
>
> Signed-off-by: Niklas Cassel <niklas.cassel@xxxxxxx>
Looks good.
Reviewed-by: Damien Le Moal <damien.lemoal@xxxxxxxxxxxxxxxxxx>
> ---
> Makefile | 6 +++
> engines/cmdprio.c | 130 ++++++++++++++++++++++++++++++++++++++++++++++
> engines/cmdprio.h | 127 ++------------------------------------------
> 3 files changed, 141 insertions(+), 122 deletions(-)
> create mode 100644 engines/cmdprio.c
>
> diff --git a/Makefile b/Makefile
> index 4ae5a371..e9028dce 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -98,6 +98,7 @@ else ifdef CONFIG_32BIT
> endif
> ifdef CONFIG_LIBAIO
> libaio_SRCS = engines/libaio.c
> + cmdprio_SRCS = engines/cmdprio.c
> libaio_LIBS = -laio
> ENGINES += libaio
> endif
> @@ -225,6 +226,7 @@ endif
> ifeq ($(CONFIG_TARGET_OS), Linux)
> SOURCE += diskutil.c fifo.c blktrace.c cgroup.c trim.c engines/sg.c \
> oslib/linux-dev-lookup.c engines/io_uring.c
> + cmdprio_SRCS = engines/cmdprio.c
> ifdef CONFIG_HAS_BLKZONED
> SOURCE += oslib/linux-blkzoned.c
> endif
> @@ -281,6 +283,10 @@ ifneq (,$(findstring CYGWIN,$(CONFIG_TARGET_OS)))
> FIO_CFLAGS += -DPSAPI_VERSION=1 -Ios/windows/posix/include -Wno-format
> endif
>
> +ifdef cmdprio_SRCS
> + SOURCE += $(cmdprio_SRCS)
> +endif
> +
> ifdef CONFIG_DYNAMIC_ENGINES
> DYNAMIC_ENGS := $(ENGINES)
> define engine_template =
> diff --git a/engines/cmdprio.c b/engines/cmdprio.c
> new file mode 100644
> index 00000000..f5b7342f
> --- /dev/null
> +++ b/engines/cmdprio.c
> @@ -0,0 +1,130 @@
> +/*
> + * IO priority handling helper functions common to the libaio and io_uring
> + * engines.
> + */
> +
> +#include "cmdprio.h"
> +
> +static int fio_cmdprio_bssplit_ddir(struct thread_options *to, void *cb_arg,
> + enum fio_ddir ddir, char *str, bool data)
> +{
> + struct cmdprio *cmdprio = cb_arg;
> + struct split split;
> + unsigned int i;
> +
> + if (ddir == DDIR_TRIM)
> + return 0;
> +
> + memset(&split, 0, sizeof(split));
> +
> + if (split_parse_ddir(to, &split, str, data, BSSPLIT_MAX))
> + return 1;
> + if (!split.nr)
> + return 0;
> +
> + cmdprio->bssplit_nr[ddir] = split.nr;
> + cmdprio->bssplit[ddir] = malloc(split.nr * sizeof(struct bssplit));
> + if (!cmdprio->bssplit[ddir])
> + return 1;
> +
> + for (i = 0; i < split.nr; i++) {
> + cmdprio->bssplit[ddir][i].bs = split.val1[i];
> + if (split.val2[i] == -1U) {
> + cmdprio->bssplit[ddir][i].perc = 0;
> + } else {
> + if (split.val2[i] > 100)
> + cmdprio->bssplit[ddir][i].perc = 100;
> + else
> + cmdprio->bssplit[ddir][i].perc = split.val2[i];
> + }
> + }
> +
> + return 0;
> +}
> +
> +int fio_cmdprio_bssplit_parse(struct thread_data *td, const char *input,
> + struct cmdprio *cmdprio)
> +{
> + char *str, *p;
> + int i, ret = 0;
> +
> + p = str = strdup(input);
> +
> + strip_blank_front(&str);
> + strip_blank_end(str);
> +
> + ret = str_split_parse(td, str, fio_cmdprio_bssplit_ddir, cmdprio, false);
> +
> + if (parse_dryrun()) {
> + for (i = 0; i < DDIR_RWDIR_CNT; i++) {
> + free(cmdprio->bssplit[i]);
> + cmdprio->bssplit[i] = NULL;
> + cmdprio->bssplit_nr[i] = 0;
> + }
> + }
> +
> + free(p);
> + return ret;
> +}
> +
> +int fio_cmdprio_percentage(struct cmdprio *cmdprio, struct io_u *io_u)
> +{
> + enum fio_ddir ddir = io_u->ddir;
> + unsigned int p = cmdprio->percentage[ddir];
> + int i;
> +
> + /*
> + * If cmdprio_percentage option was specified, then use that
> + * percentage. Otherwise, use cmdprio_bssplit percentages depending
> + * on the IO size.
> + */
> + if (p)
> + return p;
> +
> + for (i = 0; i < cmdprio->bssplit_nr[ddir]; i++) {
> + if (cmdprio->bssplit[ddir][i].bs == io_u->buflen)
> + return cmdprio->bssplit[ddir][i].perc;
> + }
> +
> + return 0;
> +}
> +
> +int fio_cmdprio_init(struct thread_data *td, struct cmdprio *cmdprio,
> + bool *has_cmdprio)
> +{
> + struct thread_options *to = &td->o;
> + bool has_cmdprio_percentage = false;
> + bool has_cmdprio_bssplit = false;
> + int i;
> +
> + /*
> + * If cmdprio_percentage/cmdprio_bssplit is set and cmdprio_class
> + * is not set, default to RT priority class.
> + */
> + for (i = 0; i < DDIR_RWDIR_CNT; i++) {
> + if (cmdprio->percentage[i]) {
> + if (!cmdprio->class[i])
> + cmdprio->class[i] = IOPRIO_CLASS_RT;
> + has_cmdprio_percentage = true;
> + }
> + if (cmdprio->bssplit_nr[i]) {
> + if (!cmdprio->class[i])
> + cmdprio->class[i] = IOPRIO_CLASS_RT;
> + has_cmdprio_bssplit = true;
> + }
> + }
> +
> + /*
> + * Check for option conflicts
> + */
> + if (has_cmdprio_percentage && has_cmdprio_bssplit) {
> + log_err("%s: cmdprio_percentage and cmdprio_bssplit options "
> + "are mutually exclusive\n",
> + to->name);
> + return 1;
> + }
> +
> + *has_cmdprio = has_cmdprio_percentage || has_cmdprio_bssplit;
> +
> + return 0;
> +}
> diff --git a/engines/cmdprio.h b/engines/cmdprio.h
> index 0edc4365..33a8f5b9 100644
> --- a/engines/cmdprio.h
> +++ b/engines/cmdprio.h
> @@ -16,129 +16,12 @@ struct cmdprio {
> struct bssplit *bssplit[DDIR_RWDIR_CNT];
> };
>
> -static int fio_cmdprio_bssplit_ddir(struct thread_options *to, void *cb_arg,
> - enum fio_ddir ddir, char *str, bool data)
> -{
> - struct cmdprio *cmdprio = cb_arg;
> - struct split split;
> - unsigned int i;
> +int fio_cmdprio_bssplit_parse(struct thread_data *td, const char *input,
> + struct cmdprio *cmdprio);
>
> - if (ddir == DDIR_TRIM)
> - return 0;
> +int fio_cmdprio_percentage(struct cmdprio *cmdprio, struct io_u *io_u);
>
> - memset(&split, 0, sizeof(split));
> -
> - if (split_parse_ddir(to, &split, str, data, BSSPLIT_MAX))
> - return 1;
> - if (!split.nr)
> - return 0;
> -
> - cmdprio->bssplit_nr[ddir] = split.nr;
> - cmdprio->bssplit[ddir] = malloc(split.nr * sizeof(struct bssplit));
> - if (!cmdprio->bssplit[ddir])
> - return 1;
> -
> - for (i = 0; i < split.nr; i++) {
> - cmdprio->bssplit[ddir][i].bs = split.val1[i];
> - if (split.val2[i] == -1U) {
> - cmdprio->bssplit[ddir][i].perc = 0;
> - } else {
> - if (split.val2[i] > 100)
> - cmdprio->bssplit[ddir][i].perc = 100;
> - else
> - cmdprio->bssplit[ddir][i].perc = split.val2[i];
> - }
> - }
> -
> - return 0;
> -}
> -
> -static int fio_cmdprio_bssplit_parse(struct thread_data *td, const char *input,
> - struct cmdprio *cmdprio)
> -{
> - char *str, *p;
> - int i, ret = 0;
> -
> - p = str = strdup(input);
> -
> - strip_blank_front(&str);
> - strip_blank_end(str);
> -
> - ret = str_split_parse(td, str, fio_cmdprio_bssplit_ddir, cmdprio, false);
> -
> - if (parse_dryrun()) {
> - for (i = 0; i < DDIR_RWDIR_CNT; i++) {
> - free(cmdprio->bssplit[i]);
> - cmdprio->bssplit[i] = NULL;
> - cmdprio->bssplit_nr[i] = 0;
> - }
> - }
> -
> - free(p);
> - return ret;
> -}
> -
> -static inline int fio_cmdprio_percentage(struct cmdprio *cmdprio,
> - struct io_u *io_u)
> -{
> - enum fio_ddir ddir = io_u->ddir;
> - unsigned int p = cmdprio->percentage[ddir];
> - int i;
> -
> - /*
> - * If cmdprio_percentage option was specified, then use that
> - * percentage. Otherwise, use cmdprio_bssplit percentages depending
> - * on the IO size.
> - */
> - if (p)
> - return p;
> -
> - for (i = 0; i < cmdprio->bssplit_nr[ddir]; i++) {
> - if (cmdprio->bssplit[ddir][i].bs == io_u->buflen)
> - return cmdprio->bssplit[ddir][i].perc;
> - }
> -
> - return 0;
> -}
> -
> -static int fio_cmdprio_init(struct thread_data *td, struct cmdprio *cmdprio,
> - bool *has_cmdprio)
> -{
> - struct thread_options *to = &td->o;
> - bool has_cmdprio_percentage = false;
> - bool has_cmdprio_bssplit = false;
> - int i;
> -
> - /*
> - * If cmdprio_percentage/cmdprio_bssplit is set and cmdprio_class
> - * is not set, default to RT priority class.
> - */
> - for (i = 0; i < DDIR_RWDIR_CNT; i++) {
> - if (cmdprio->percentage[i]) {
> - if (!cmdprio->class[i])
> - cmdprio->class[i] = IOPRIO_CLASS_RT;
> - has_cmdprio_percentage = true;
> - }
> - if (cmdprio->bssplit_nr[i]) {
> - if (!cmdprio->class[i])
> - cmdprio->class[i] = IOPRIO_CLASS_RT;
> - has_cmdprio_bssplit = true;
> - }
> - }
> -
> - /*
> - * Check for option conflicts
> - */
> - if (has_cmdprio_percentage && has_cmdprio_bssplit) {
> - log_err("%s: cmdprio_percentage and cmdprio_bssplit options "
> - "are mutually exclusive\n",
> - to->name);
> - return 1;
> - }
> -
> - *has_cmdprio = has_cmdprio_percentage || has_cmdprio_bssplit;
> -
> - return 0;
> -}
> +int fio_cmdprio_init(struct thread_data *td, struct cmdprio *cmdprio,
> + bool *has_cmdprio);
>
> #endif
>
--
Damien Le Moal
Western Digital Research
