| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <3978b8f7-1d80-d7-926a-72b2cfd923bd@linux.intel.com>
Date: Thu, 24 Aug 2023 15:56:25 +0300 (EEST)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: "Wieczor-Retman, Maciej" <maciej.wieczor-retman@...el.com>
cc: LKML <linux-kernel@...r.kernel.org>,
Reinette Chatre <reinette.chatre@...el.com>,
fenghua.yu@...el.com
Subject: Re: [PATCH 2/3] selftests/resctrl: Move run_benchmark() to a more
fitting file
On Thu, 24 Aug 2023, Wieczor-Retman, Maciej wrote:
Thanks for this patch, the new location is much more appropriate and
logical (more than once I've tried to look for this from the wrong file).
> Resctrlfs.c file contains mostly functions that interact in some way
> with resctrl FS entries while functions inside resctrl_val.c deal with
> measurements and benchmarking
>
> Run_benchmark() function is located in resctrlfs.c file even though it's
> purpose is not interacting with the resctrl FS but to execute cache
> checking logic
>
> Move run_benchmark() to resctrl_val.c just before resctrl_val() function
> that makes use of run_benchmark()
Please terminate your sentences in changelog with . like in normal
writing.
> Signed-off-by: Wieczor-Retman, Maciej <maciej.wieczor-retman@...el.com>
> ---
> tools/testing/selftests/resctrl/resctrl_val.c | 52 +++++++++++++++++++
> tools/testing/selftests/resctrl/resctrlfs.c | 52 -------------------
> 2 files changed, 52 insertions(+), 52 deletions(-)
>
> diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c
> index f0f6c5f6e98b..667542c084eb 100644
> --- a/tools/testing/selftests/resctrl/resctrl_val.c
> +++ b/tools/testing/selftests/resctrl/resctrl_val.c
> @@ -621,6 +621,58 @@ measure_vals(struct resctrl_val_param *param, unsigned long *bw_resc_start)
> return 0;
> }
>
> +/*
> + * run_benchmark - Run a specified benchmark or fill_buf (default benchmark)
> + * in specified signal. Direct benchmark stdio to /dev/null.
> + * @signum: signal number
> + * @info: signal info
> + * @ucontext: user context in signal handling
> + *
> + * Return: void
This Return: void feels waste of screen space as if it wouldn't be
obvious from the function signature.
> + */
> +void run_benchmark(int signum, siginfo_t *info, void *ucontext)
> +{
> + int operation, ret, memflush;
> + char **benchmark_cmd;
> + size_t span;
> + bool once;
> + FILE *fp;
> +
> + benchmark_cmd = info->si_ptr;
> +
> + /*
> + * Direct stdio of child to /dev/null, so that only parent writes to
> + * stdio (console)
> + */
> + fp = freopen("/dev/null", "w", stdout);
> + if (!fp)
> + PARENT_EXIT("Unable to direct benchmark status to /dev/null");
> +
> + if (strcmp(benchmark_cmd[0], "fill_buf") == 0) {
> + /* Execute default fill_buf benchmark */
> + span = strtoul(benchmark_cmd[1], NULL, 10);
> + memflush = atoi(benchmark_cmd[2]);
> + operation = atoi(benchmark_cmd[3]);
> + if (!strcmp(benchmark_cmd[4], "true"))
> + once = true;
> + else if (!strcmp(benchmark_cmd[4], "false"))
> + once = false;
> + else
> + PARENT_EXIT("Invalid once parameter");
> +
> + if (run_fill_buf(span, memflush, operation, once))
> + fprintf(stderr, "Error in running fill buffer\n");
> + } else {
> + /* Execute specified benchmark */
> + ret = execvp(benchmark_cmd[0], benchmark_cmd);
> + if (ret)
> + perror("wrong\n");
> + }
> +
> + fclose(stdout);
> + PARENT_EXIT("Unable to run specified benchmark");
> +}
> +
> /*
> * resctrl_val: execute benchmark and measure memory bandwidth on
> * the benchmark
> diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c
> index a6d0b632cbc6..e3c94614c086 100644
> --- a/tools/testing/selftests/resctrl/resctrlfs.c
> +++ b/tools/testing/selftests/resctrl/resctrlfs.c
> @@ -291,58 +291,6 @@ int taskset_benchmark(pid_t bm_pid, int cpu_no)
> return 0;
> }
>
> -/*
> - * run_benchmark - Run a specified benchmark or fill_buf (default benchmark)
> - * in specified signal. Direct benchmark stdio to /dev/null.
> - * @signum: signal number
> - * @info: signal info
> - * @ucontext: user context in signal handling
> - *
> - * Return: void
> - */
> -void run_benchmark(int signum, siginfo_t *info, void *ucontext)
> -{
> - int operation, ret, memflush;
> - char **benchmark_cmd;
> - size_t span;
> - bool once;
> - FILE *fp;
> -
> - benchmark_cmd = info->si_ptr;
> -
> - /*
> - * Direct stdio of child to /dev/null, so that only parent writes to
> - * stdio (console)
> - */
> - fp = freopen("/dev/null", "w", stdout);
> - if (!fp)
> - PARENT_EXIT("Unable to direct benchmark status to /dev/null");
> -
> - if (strcmp(benchmark_cmd[0], "fill_buf") == 0) {
> - /* Execute default fill_buf benchmark */
> - span = strtoul(benchmark_cmd[1], NULL, 10);
> - memflush = atoi(benchmark_cmd[2]);
> - operation = atoi(benchmark_cmd[3]);
> - if (!strcmp(benchmark_cmd[4], "true"))
> - once = true;
> - else if (!strcmp(benchmark_cmd[4], "false"))
> - once = false;
> - else
> - PARENT_EXIT("Invalid once parameter");
> -
> - if (run_fill_buf(span, memflush, operation, once))
> - fprintf(stderr, "Error in running fill buffer\n");
> - } else {
> - /* Execute specified benchmark */
> - ret = execvp(benchmark_cmd[0], benchmark_cmd);
> - if (ret)
> - perror("wrong\n");
> - }
> -
> - fclose(stdout);
> - PARENT_EXIT("Unable to run specified benchmark");
> -}
> -
> /*
> * create_grp - Create a group only if one doesn't exist
> * @grp_name: Name of the group
>
--
i.
Powered by blists - more mailing lists