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.
> + 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.
> + if (fscanf(fp, "%s", offline_cpus_str) < 0) {
This needs something equivalent to
46058430fc5d ("selftests/resctrl: Protect against array overflow when reading strings")
> + 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.
> + */
> +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();
> return 0;
> }
>
Reinette
