On 2024-12-03 at 15:26:41 -0800, Reinette Chatre wrote:
Hi Maciej,
On 12/2/24 3:08 AM, Maciej Wieczor-Retman wrote:
diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c index d38d6dd90be4..50561993d37c 100644 --- a/tools/testing/selftests/resctrl/resctrlfs.c +++ b/tools/testing/selftests/resctrl/resctrlfs.c @@ -13,6 +13,8 @@ #include "resctrl.h" +int snc_unreliable;
static int find_resctrl_mount(char *buffer) { FILE *mounts; @@ -156,6 +158,90 @@ int get_domain_id(const char *resource, int cpu_no, int *domain_id) return 0; } +/*
- Count number of CPUs in a /sys bitmap
- */
+static unsigned int count_sys_bitmap_bits(char *name) +{
- FILE *fp = fopen(name, "r");
- int count = 0, c;
- if (!fp)
return 0;- while ((c = fgetc(fp)) != EOF) {
if (!isxdigit(c))continue;switch (c) {case 'f':count++;case '7': case 'b': case 'd': case 'e':count++;case '3': case '5': case '6': case '9': case 'a': case 'c':count++;case '1': case '2': case '4': case '8':count++;}- }
- fclose(fp);
running this through a syntax checker triggers a couple of complaints due to the missing break statements. I think this can be made more robust by making use of "fallthrough" and "break". It looks like this can be obtained by including linux/compiler.h ... but from what I can tell care should be taken to set the include directory _after_ includink lib.mk so that top_srcdir is set correctly.
Sure, I'll look into it
- return count;
+}
+static bool cpus_offline_empty(void) +{
- char offline_cpus_str[64];
- FILE *fp;
- fp = fopen("/sys/devices/system/cpu/offline", "r");
Please check fp before use.
Will do
- if (fscanf(fp, "%s", offline_cpus_str) < 0) {
This needs something equivalent to 46058430fc5d ("selftests/resctrl: Protect against array overflow when reading strings")
Thanks, I'll add that protection here.
if (!errno) {fclose(fp);return 1;}ksft_perror("Could not read /sys/devices/system/cpu/offline");- }
- fclose(fp);
- return 0;
+}
+/*
- Detect SNC by comparing #CPUs in node0 with #CPUs sharing LLC with CPU0.
- If any CPUs are offline declare the detection as unreliable and skip the
- tests.
nit: "and skip the tests" can be dropped since the function need not make assumption about how callers will use it.
Right, sorry, will remove it.
- */
+int snc_nodes_per_l3_cache(void) +{
- int node_cpus, cache_cpus;
- static int snc_mode;
- if (!snc_mode) {
snc_mode = 1;if (!cpus_offline_empty()) {ksft_print_msg("Runtime SNC detection unreliable due to offline CPUs.\n");ksft_print_msg("Setting SNC mode to disabled.\n");snc_unreliable = 1;return snc_mode;}node_cpus = count_sys_bitmap_bits("/sys/devices/system/node/node0/cpumap");cache_cpus = count_sys_bitmap_bits("/sys/devices/system/cpu/cpu0/cache/index3/shared_cpu_map");if (!node_cpus || !cache_cpus) {ksft_print_msg("Could not determine Sub-NUMA Cluster mode.\n");snc_unreliable = 1;return snc_mode;}snc_mode = cache_cpus / node_cpus;if (snc_mode > 1)ksft_print_msg("SNC-%d mode discovered.\n", snc_mode);- }
- return snc_mode;
+}
/*
- get_cache_size - Get cache size for a specified CPU
- @cpu_no: CPU number
@@ -211,6 +297,17 @@ int get_cache_size(int cpu_no, const char *cache_type, unsigned long *cache_size break; }
- /*
* The amount of cache represented by each bit in the masks* in the schemata file is reduced by a factor equal to SNC* nodes per L3 cache.* E.g. on a SNC-2 system with a 100MB L3 cache a test that* allocates memory from its local SNC node (default behavior* without using libnuma) will only see 50 MB llc_occupancy* with a fully populated L3 mask in the schemata file.*/- if (cache_num == 3)
*cache_size /= snc_nodes_per_l3_cache();}
Reinette