From 385360fe8144462a12d4c69c49ca8d2c1232c282 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Fri, 10 Jan 2025 17:25:38 +0100 Subject: [PATCH] MINOR: cpu-topo: ignore single-core clusters Some platforms (several armv7, intel 14900 etc) report one distinct cluster per core. This is problematic as it cannot let clusters be used to distinguish real groups of cores, and cannot be used to build thread groups. Let's just compare the cluster cpus to the siblings, and ignore it if they exactly match. We must also take care of not falling back to core_cpus_list, which can enumerate cores that already have their cluster assigned (e.g. intel 14900 has 4 4-Ecore clusters in addition to the 8 Pcores). --- src/cpu_topo.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/cpu_topo.c b/src/cpu_topo.c index b464f09d7..9c405d786 100644 --- a/src/cpu_topo.c +++ b/src/cpu_topo.c @@ -445,6 +445,7 @@ int cpu_detect_topology(void) /* detect the presence of some kernel-specific fields */ no_cache = no_topo = no_capa = no_clust = no_pkg = no_freq = no_cppc = -1; for (cpu = 0; cpu <= cpu_topo_lastcpu; cpu++) { + struct hap_cpuset siblings_list = { }; struct hap_cpuset cpus_list; int next_level = 1; // assume L1 if unknown int idx, level; @@ -525,18 +526,19 @@ int cpu_detect_topology(void) * share the same cores, and also to tell apart cores that * support SMT from those which do not. When mixed, generally * the ones with SMT are big cores and the ones without are the - * small ones. + * small ones. We also read the entry if the cluster_id is not + * known because we'll have to compare both values. */ - if (ha_cpu_topo[cpu].ts_id < 0 && + if ((ha_cpu_topo[cpu].ts_id < 0 || ha_cpu_topo[cpu].cl_gid < 0) && read_line_to_trash(NUMA_DETECT_SYSTEM_SYSFS_PATH "/cpu/cpu%d/topology/thread_siblings_list", cpu) >= 0) { parse_cpu_set_args[0] = trash.area; parse_cpu_set_args[1] = "\0"; - if (parse_cpu_set(parse_cpu_set_args, &cpus_list, NULL) == 0) { + if (parse_cpu_set(parse_cpu_set_args, &siblings_list, NULL) == 0) { int sib_id = 0; - cpu_id.th_cnt = ha_cpuset_count(&cpus_list); + cpu_id.th_cnt = ha_cpuset_count(&siblings_list); for (cpu2 = 0; cpu2 <= cpu_topo_lastcpu; cpu2++) { - if (ha_cpuset_isset(&cpus_list, cpu2)) { + if (ha_cpuset_isset(&siblings_list, cpu2)) { ha_cpu_topo[cpu2].ts_id = cpu_id.ts_id; ha_cpu_topo[cpu2].th_cnt = cpu_id.th_cnt; ha_cpu_topo[cpu2].th_id = sib_id++; @@ -552,6 +554,9 @@ int cpu_detect_topology(void) * above as a fallback for package but we don't care here. We * only consider these values if there's more than one CPU per * cluster (some kernels such as 6.1 report one cluster per CPU). + * Note that we purposely ignore clusters that are reportedly + * equal to the siblings list, because some machines report one + * distinct cluster per *core* (e.g. some armv7 and intel 14900). */ if (no_clust < 0) { no_clust = !is_file_present(NUMA_DETECT_SYSTEM_SYSFS_PATH "/cpu/cpu%d/topology/cluster_cpus_list", cpu) && @@ -563,7 +568,8 @@ int cpu_detect_topology(void) read_line_to_trash(NUMA_DETECT_SYSTEM_SYSFS_PATH "/cpu/cpu%d/topology/core_siblings_list", cpu) >= 0)) { parse_cpu_set_args[0] = trash.area; parse_cpu_set_args[1] = "\0"; - if (parse_cpu_set(parse_cpu_set_args, &cpus_list, NULL) == 0 && ha_cpuset_count(&cpus_list) > 1) { + if (parse_cpu_set(parse_cpu_set_args, &cpus_list, NULL) == 0 && ha_cpuset_count(&cpus_list) > 1 && + (memcmp(&cpus_list, &siblings_list, sizeof(cpus_list)) != 0)) { for (cpu2 = 0; cpu2 <= cpu_topo_lastcpu; cpu2++) { if (ha_cpuset_isset(&cpus_list, cpu2)) { ha_cpu_topo[cpu2].cl_lid = cpu_id.cl_lid;