Hi, On 2023-08-30 at 13:51:29 -0700, Reinette Chatre wrote: >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. I'll fix it. >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 ...". Thanks for the tip, will apply it from now on. >> >> 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. It'll be fixed next time. >> Signed-off-by: Wieczor-Retman, Maciej <maciej.wieczor-retman@xxxxxxxxx> >> --- >> 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? Thanks for noticing. Yes, from my side it seems turning it into static is okay. I tried it out on Ilpo's branches that I know he's currently working on and there were no errors either. -- Kind regards Maciej Wieczór-Retman
