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 <[email protected]>
>> ---
>> 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