Hello!
On 2024-01-26 at 13:08:19 -0800, Reinette Chatre wrote:
Hi Maciej,
On 1/25/2024 3:10 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>.
Signed-off-by: Maciej Wieczor-Retman maciej.wieczor-retman@intel.com
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 | 39 +++++++++++++++++++++ 2 files changed, 40 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..cb5147c5f9a9 100644 --- a/tools/testing/selftests/resctrl/resctrlfs.c +++ b/tools/testing/selftests/resctrl/resctrlfs.c @@ -249,6 +249,45 @@ static int get_bit_mask(const char *filename, unsigned long *mask) return 0; } +/*
By not starting with /** the following will not be interpreted as kernel-doc but the formatting does appear to follow the syntax, but not consistently so. I think it would be more readable if the kernel-doc syntax is followed consistently.
Sure, I'll change it.
- resource_info_unsigned_get - Read an unsigned value from a file in
- /sys/fs/resctrl/info/RESOURCE/FILENAME
"Read an unsigned value from /sys/fs/resctrl/info/RESOURCE/FILENAME"?
Okay
- @resource: Resource name that matches directory names in
names? (plural)
Right, it doesn't make much sense, I'll move it to singular.
/sys/fs/resctrl/info
- @filename: Filename of a file located in a directory specified with the
'resource' variable.I think this can be shortened to "File in /sys/fs/resctrl/info/@resource"
Sure, thanks
- @val: Variable where the read value is saved on success.
"Contains read value on success."
(no need to refer to it as a variable/parameter, it is implied by syntax).
Right, I'll change it.
- Return: = 0 on success, < 0 on failure. On success the read value is saved into the 'val'
- variable.
"saved into the 'val' variable" -> "saved into @val" (since syntax indicates it is the parameter there is no need to elaborate).
Sure, thanks
Also please let lines in comments be of consistent length.
Okay, I'll keep it to 80 characters.
- */
+int resource_info_unsigned_get(const char *resource, const char *filename,
unsigned int *val)+{
- char reason[128], 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) {
snprintf(reason, sizeof(reason), "Error in opening %s file\n", filename);(apart from other discussions). "file" in message seems redundant. It can just be "Error opening %s". It may also be useful to print file_path instead of filename to be specific of what the code tried to open.
Okay, I'll change it to file_path.
ksft_perror(reason);return -1;- }
- if (fscanf(fp, "%u", val) <= 0) {
snprintf(reason, sizeof(reason), "Could not get %s's contents\n", filename);ksft_perror(reason);filename -> file_path ?
Same as above.
fclose(fp);return -1;- }
- fclose(fp);
- return 0;
+}
Reinette