Re: [PATCH v4 2/5] selftests/resctrl: Add helpers for the non-contiguous test

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