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@xxxxxxxxx>
>> ---
>> 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@xxxxxxxxxxxxxxx>
>
>
>--
> i.
--
Kind regards
Maciej Wieczór-Retman
