Hello Reinette, On 2024-01-26 at 13:08:52 -0800, Reinette Chatre wrote: >Hi Maciej, > >On 1/25/2024 3:12 AM, Maciej Wieczor-Retman wrote: >> Feature checking done by resctrl_mon_feature_exists() covers features >> represented by the feature name presence inside the 'mon_features' file >> in /sys/fs/resctrl/info/L3_MON directory. There exists a different way >> to represent feature support and that is by the presence of 0 or 1 in a >> single file in the info/resource directory. In this case the filename >> represents what feature support is being indicated. >> >> Add a generic function to check file presence in the >> /sys/fs/resctrl/info/<RESOURCE> directory. >> >> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@xxxxxxxxx> >> --- >> Changelog v3: >> - Split off the new function into this patch. (Reinette) >> >> Changelog v2: >> - Add this patch. >> >> tools/testing/selftests/resctrl/resctrl.h | 2 ++ >> tools/testing/selftests/resctrl/resctrlfs.c | 26 +++++++++++++++++++++ >> 2 files changed, 28 insertions(+) >> >> diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h >> index 4603b215b97e..c39105f46da9 100644 >> --- a/tools/testing/selftests/resctrl/resctrl.h >> +++ b/tools/testing/selftests/resctrl/resctrl.h >> @@ -138,6 +138,8 @@ int umount_resctrlfs(void); >> int validate_bw_report_request(char *bw_report); >> bool resctrl_resource_exists(const char *resource); >> bool resctrl_mon_feature_exists(const char *feature); >> +bool resource_info_file_exists(const char *resource, >> + const char *feature); > >In addition what Ilpo commented about other line, this too can be >on one line. Thanks! > >> bool test_resource_feature_check(const struct resctrl_test *test); >> char *fgrep(FILE *inf, const char *str); >> int taskset_benchmark(pid_t bm_pid, int cpu_no, cpu_set_t *old_affinity); >> diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c >> index e4ba8614fb7b..a6427732e0ad 100644 >> --- a/tools/testing/selftests/resctrl/resctrlfs.c >> +++ b/tools/testing/selftests/resctrl/resctrlfs.c >> @@ -763,6 +763,32 @@ bool resctrl_mon_feature_exists(const char *feature) >> return !!res; >> } >> >> +/* >> + * resource_info_file_exists - Check if a file is present inside >> + * /sys/fs/resctrl/info/RESOURCE. > >As confirmed in the changelog this is intended to be a generic function that >tests if a file exists ... > >> + * @resource: Required resource (Eg: MB, L3, L2, etc.) >> + * @feature: Required feature. > >... so assuming a use of this by referring to the file as "feature" is unexpected. > >This function jumps between "file" and "feature" in comments and code, please >just stick to goal of making it a generic utility that checks for a file and >refer to it consistently so. Okay, I'll stick to 'file'. > >Reinette -- Kind regards Maciej Wieczór-Retman
