| Message ID | 89b8965d563e4e61b95b20be55c26475d830b245.1707130307.git.maciej.wieczor-retman@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | selftests/resctrl: Add non-contiguous CBMs in Intel CAT selftest | expand |
On Mon, 5 Feb 2024, Maciej Wieczor-Retman wrote: > The CAT non-contiguous selftests have to read the file responsible for > reporting support of non-contiguous CBMs in kernel (resctrl). Then the > test compares if that information matches what is reported by CPUID > output. > > Add a generic helper function to read an unsigned number from a file in > /sys/fs/resctrl/info/<RESOURCE>/<FILE>. > > Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com> > --- > Changelog v4: > - Rewrite function comment. > - Redo ksft_perror() as ksft_print_msg(). (Reinette) > > Changelog v3: > - Rewrite patch message. > - Add documentation and rewrote the function. (Reinette) > > Changelog v2: > - Add this patch. > > tools/testing/selftests/resctrl/resctrl.h | 1 + > tools/testing/selftests/resctrl/resctrlfs.c | 36 +++++++++++++++++++++ > 2 files changed, 37 insertions(+) > > diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h > index a1462029998e..5116ea082d03 100644 > --- a/tools/testing/selftests/resctrl/resctrl.h > +++ b/tools/testing/selftests/resctrl/resctrl.h > @@ -162,6 +162,7 @@ unsigned int count_contiguous_bits(unsigned long val, unsigned int *start); > int get_full_cbm(const char *cache_type, unsigned long *mask); > int get_mask_no_shareable(const char *cache_type, unsigned long *mask); > int get_cache_size(int cpu_no, const char *cache_type, unsigned long *cache_size); > +int resource_info_unsigned_get(const char *resource, const char *filename, unsigned int *val); > void ctrlc_handler(int signum, siginfo_t *info, void *ptr); > int signal_handler_register(void); > void signal_handler_unregister(void); > diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c > index 5750662cce57..e0fbc46a917a 100644 > --- a/tools/testing/selftests/resctrl/resctrlfs.c > +++ b/tools/testing/selftests/resctrl/resctrlfs.c > @@ -249,6 +249,42 @@ static int get_bit_mask(const char *filename, unsigned long *mask) > return 0; > } > > +/** > + * resource_info_unsigned_get - Read an unsigned value from > + * /sys/fs/resctrl/info/RESOURCE/FILENAME > + * @resource: Resource name that matches directory name in > + * /sys/fs/resctrl/info > + * @filename: File in /sys/fs/resctrl/info/@resource > + * @val: Contains read value on success. > + * > + * Return: = 0 on success, < 0 on failure. On success the read > + * value is saved into the @val. > + */ > +int resource_info_unsigned_get(const char *resource, const char *filename, > + unsigned int *val) > +{ > + char file_path[PATH_MAX]; > + FILE *fp; > + > + snprintf(file_path, sizeof(file_path), "%s/%s/%s", INFO_PATH, resource, > + filename); > + > + fp = fopen(file_path, "r"); > + if (!fp) { > + ksft_print_msg("Error in opening %s\n: %m\n", file_path); Adding the extra \n in between there will likely mess up the test output formatting (the expected prefixes will not appear). Therefore, manually adding newlines should be avoided. > + return -1; > + } > + > + if (fscanf(fp, "%u", val) <= 0) { > + ksft_print_msg("Could not get contents of %s\n: %m\n", file_path); Ditto. With those two fixed, Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
On 2024-02-05 at 15:16:27 +0200, Ilpo Järvinen wrote: >On Mon, 5 Feb 2024, Maciej Wieczor-Retman wrote: > >> The CAT non-contiguous selftests have to read the file responsible for >> reporting support of non-contiguous CBMs in kernel (resctrl). Then the >> test compares if that information matches what is reported by CPUID >> output. >> >> Add a generic helper function to read an unsigned number from a file in >> /sys/fs/resctrl/info/<RESOURCE>/<FILE>. >> >> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com> >> --- >> Changelog v4: >> - Rewrite function comment. >> - Redo ksft_perror() as ksft_print_msg(). (Reinette) >> >> Changelog v3: >> - Rewrite patch message. >> - Add documentation and rewrote the function. (Reinette) >> >> Changelog v2: >> - Add this patch. >> >> tools/testing/selftests/resctrl/resctrl.h | 1 + >> tools/testing/selftests/resctrl/resctrlfs.c | 36 +++++++++++++++++++++ >> 2 files changed, 37 insertions(+) >> >> diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h >> index a1462029998e..5116ea082d03 100644 >> --- a/tools/testing/selftests/resctrl/resctrl.h >> +++ b/tools/testing/selftests/resctrl/resctrl.h >> @@ -162,6 +162,7 @@ unsigned int count_contiguous_bits(unsigned long val, unsigned int *start); >> int get_full_cbm(const char *cache_type, unsigned long *mask); >> int get_mask_no_shareable(const char *cache_type, unsigned long *mask); >> int get_cache_size(int cpu_no, const char *cache_type, unsigned long *cache_size); >> +int resource_info_unsigned_get(const char *resource, const char *filename, unsigned int *val); >> void ctrlc_handler(int signum, siginfo_t *info, void *ptr); >> int signal_handler_register(void); >> void signal_handler_unregister(void); >> diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c >> index 5750662cce57..e0fbc46a917a 100644 >> --- a/tools/testing/selftests/resctrl/resctrlfs.c >> +++ b/tools/testing/selftests/resctrl/resctrlfs.c >> @@ -249,6 +249,42 @@ static int get_bit_mask(const char *filename, unsigned long *mask) >> return 0; >> } >> >> +/** >> + * resource_info_unsigned_get - Read an unsigned value from >> + * /sys/fs/resctrl/info/RESOURCE/FILENAME >> + * @resource: Resource name that matches directory name in >> + * /sys/fs/resctrl/info >> + * @filename: File in /sys/fs/resctrl/info/@resource >> + * @val: Contains read value on success. >> + * >> + * Return: = 0 on success, < 0 on failure. On success the read >> + * value is saved into the @val. >> + */ >> +int resource_info_unsigned_get(const char *resource, const char *filename, >> + unsigned int *val) >> +{ >> + char file_path[PATH_MAX]; >> + FILE *fp; >> + >> + snprintf(file_path, sizeof(file_path), "%s/%s/%s", INFO_PATH, resource, >> + filename); >> + >> + fp = fopen(file_path, "r"); >> + if (!fp) { >> + ksft_print_msg("Error in opening %s\n: %m\n", file_path); > >Adding the extra \n in between there will likely mess up the test output >formatting (the expected prefixes will not appear). Therefore, manually >adding newlines should be avoided. > >> + return -1; >> + } >> + >> + if (fscanf(fp, "%u", val) <= 0) { >> + ksft_print_msg("Could not get contents of %s\n: %m\n", file_path); > >Ditto. I thought I've seen some other error checking paths where '%m' was on the next line but I couldn't find it again right now so I'll remove the newline. > >With those two fixed, > >Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> > > >-- > i.
Hi Maciej, The subject mentions "helpers" (plural) that may not be accurate anymore. On 2/5/2024 4:08 AM, Maciej Wieczor-Retman wrote: > The CAT non-contiguous selftests have to read the file responsible for > reporting support of non-contiguous CBMs in kernel (resctrl). Then the > test compares if that information matches what is reported by CPUID > output. > > Add a generic helper function to read an unsigned number from a file in > /sys/fs/resctrl/info/<RESOURCE>/<FILE>. The "a file in" above can be dropped or it should read "from a file in /sys/fs/resctrl/info/<RESOURCE>". > > Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com> > --- > Changelog v4: > - Rewrite function comment. > - Redo ksft_perror() as ksft_print_msg(). (Reinette) > > Changelog v3: > - Rewrite patch message. > - Add documentation and rewrote the function. (Reinette) > > Changelog v2: > - Add this patch. > > tools/testing/selftests/resctrl/resctrl.h | 1 + > tools/testing/selftests/resctrl/resctrlfs.c | 36 +++++++++++++++++++++ > 2 files changed, 37 insertions(+) > > diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h > index a1462029998e..5116ea082d03 100644 > --- a/tools/testing/selftests/resctrl/resctrl.h > +++ b/tools/testing/selftests/resctrl/resctrl.h > @@ -162,6 +162,7 @@ unsigned int count_contiguous_bits(unsigned long val, unsigned int *start); > int get_full_cbm(const char *cache_type, unsigned long *mask); > int get_mask_no_shareable(const char *cache_type, unsigned long *mask); > int get_cache_size(int cpu_no, const char *cache_type, unsigned long *cache_size); > +int resource_info_unsigned_get(const char *resource, const char *filename, unsigned int *val); > void ctrlc_handler(int signum, siginfo_t *info, void *ptr); > int signal_handler_register(void); > void signal_handler_unregister(void); > diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c > index 5750662cce57..e0fbc46a917a 100644 > --- a/tools/testing/selftests/resctrl/resctrlfs.c > +++ b/tools/testing/selftests/resctrl/resctrlfs.c > @@ -249,6 +249,42 @@ static int get_bit_mask(const char *filename, unsigned long *mask) > return 0; > } > > +/** Apologies for not being clear in my previous comment. I do not think we want to start exposing resctrl selftest comments to the kernel-doc tool using this change. The tests are using kernel-doc style but this is done outside from the kernel-doc tool's view. > + * resource_info_unsigned_get - Read an unsigned value from > + * /sys/fs/resctrl/info/RESOURCE/FILENAME > + * @resource: Resource name that matches directory name in > + * /sys/fs/resctrl/info > + * @filename: File in /sys/fs/resctrl/info/@resource > + * @val: Contains read value on success. > + * > + * Return: = 0 on success, < 0 on failure. On success the read > + * value is saved into the @val. It can just be "saved into @val" > + */ > +int resource_info_unsigned_get(const char *resource, const char *filename, > + unsigned int *val) > +{ > + char file_path[PATH_MAX]; > + FILE *fp; > + > + snprintf(file_path, sizeof(file_path), "%s/%s/%s", INFO_PATH, resource, > + filename); > + > + fp = fopen(file_path, "r"); > + if (!fp) { > + ksft_print_msg("Error in opening %s\n: %m\n", file_path); > + return -1; > + } Apart from Ilpo's comment this can also just be "Error opening %s: ..." > + > + if (fscanf(fp, "%u", val) <= 0) { > + ksft_print_msg("Could not get contents of %s\n: %m\n", file_path); > + fclose(fp); > + return -1; > + } > + > + fclose(fp); > + return 0; > +} > + > /* > * create_bit_mask- Create bit mask from start, len pair > * @start: LSB of the mask Reinette
diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h index a1462029998e..5116ea082d03 100644 --- a/tools/testing/selftests/resctrl/resctrl.h +++ b/tools/testing/selftests/resctrl/resctrl.h @@ -162,6 +162,7 @@ unsigned int count_contiguous_bits(unsigned long val, unsigned int *start); int get_full_cbm(const char *cache_type, unsigned long *mask); int get_mask_no_shareable(const char *cache_type, unsigned long *mask); int get_cache_size(int cpu_no, const char *cache_type, unsigned long *cache_size); +int resource_info_unsigned_get(const char *resource, const char *filename, unsigned int *val); void ctrlc_handler(int signum, siginfo_t *info, void *ptr); int signal_handler_register(void); void signal_handler_unregister(void); diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c index 5750662cce57..e0fbc46a917a 100644 --- a/tools/testing/selftests/resctrl/resctrlfs.c +++ b/tools/testing/selftests/resctrl/resctrlfs.c @@ -249,6 +249,42 @@ static int get_bit_mask(const char *filename, unsigned long *mask) return 0; } +/** + * resource_info_unsigned_get - Read an unsigned value from + * /sys/fs/resctrl/info/RESOURCE/FILENAME + * @resource: Resource name that matches directory name in + * /sys/fs/resctrl/info + * @filename: File in /sys/fs/resctrl/info/@resource + * @val: Contains read value on success. + * + * Return: = 0 on success, < 0 on failure. On success the read + * value is saved into the @val. + */ +int resource_info_unsigned_get(const char *resource, const char *filename, + unsigned int *val) +{ + char file_path[PATH_MAX]; + FILE *fp; + + snprintf(file_path, sizeof(file_path), "%s/%s/%s", INFO_PATH, resource, + filename); + + fp = fopen(file_path, "r"); + if (!fp) { + ksft_print_msg("Error in opening %s\n: %m\n", file_path); + return -1; + } + + if (fscanf(fp, "%u", val) <= 0) { + ksft_print_msg("Could not get contents of %s\n: %m\n", file_path); + fclose(fp); + return -1; + } + + fclose(fp); + return 0; +} + /* * create_bit_mask- Create bit mask from start, len pair * @start: LSB of the mask
The CAT non-contiguous selftests have to read the file responsible for reporting support of non-contiguous CBMs in kernel (resctrl). Then the test compares if that information matches what is reported by CPUID output. Add a generic helper function to read an unsigned number from a file in /sys/fs/resctrl/info/<RESOURCE>/<FILE>. Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com> --- Changelog v4: - Rewrite function comment. - Redo ksft_perror() as ksft_print_msg(). (Reinette) Changelog v3: - Rewrite patch message. - Add documentation and rewrote the function. (Reinette) Changelog v2: - Add this patch. tools/testing/selftests/resctrl/resctrl.h | 1 + tools/testing/selftests/resctrl/resctrlfs.c | 36 +++++++++++++++++++++ 2 files changed, 37 insertions(+)