[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <16eccb6b-a968-c6ee-dfb3-221ed8ec023c@intel.com>
Date: Wed, 30 Aug 2023 13:51:29 -0700
From: Reinette Chatre <reinette.chatre@...el.com>
To: "Wieczor-Retman, Maciej" <maciej.wieczor-retman@...el.com>,
<linux-kernel@...r.kernel.org>, <linux-kselftest@...r.kernel.org>,
<shuah@...nel.org>, <fenghua.yu@...el.com>
CC: <ilpo.jarvinen@...ux.intel.com>
Subject: Re: [PATCH v2 2/2] selftests/resctrl: Move run_benchmark() to a more
fitting file
Hi Maciej,
On 8/28/2023 2:56 AM, Wieczor-Retman, Maciej wrote:
> 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.
It looks like your editor may be automatically capitalize first words
of sentences like Resctrlfs.c and Run_benchmark() above.
Also please note that when using () to indicate a function it is not
necessary to say it is a function. For example above can just be:
"run_benchmark() is located ..." ... similarly you can just say
"resctrlfs.c contains ...".
>
> Move run_benchmark() to resctrl_val.c just before resctrl_val() function
> that makes use of run_benchmark().
>
> Remove return comment from kernel-doc since the function is type void.
>
> Changelog v2:
> - Add dots at the end of patch msg sentences.
> - Remove "Return: void" from run_benchmark() kernel-doc comment.
>
same comment about changelog.
> Signed-off-by: Wieczor-Retman, Maciej <maciej.wieczor-retman@...el.com>
> ---
> tools/testing/selftests/resctrl/resctrl_val.c | 50 ++++++++++++++++++
> tools/testing/selftests/resctrl/resctrlfs.c | 52 -------------------
> 2 files changed, 50 insertions(+), 52 deletions(-)
>
> diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c
> index f0f6c5f6e98b..5c8dc0a7bab9 100644
> --- a/tools/testing/selftests/resctrl/resctrl_val.c
> +++ b/tools/testing/selftests/resctrl/resctrl_val.c
> @@ -621,6 +621,56 @@ 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
> + */
> +void run_benchmark(int signum, siginfo_t *info, void *ucontext)
Can run_benchmark() now be made static and its declaration removed from
the header file?
Reinette
Powered by blists - more mailing lists