From 06a8520328193e3c663007ede386ef2aa345f65d Mon Sep 17 00:00:00 2001 From: Len Brown Date: Thu, 30 Apr 2026 21:28:19 -0400 Subject: tools/power turbostat: Fix fd_perf leak on reset When turbostat notices a topology change, it frees perf file descriptors for each CPU to prepare to re-initialize. Fix an off-by-one error in that per-cpu loop that resulted in a file descriptor leak of the file for the highest numbered CPU in the system. Fixes: 67bab430f4e7 ("tools/power turbostat: Group SMI counter with APERF and MPERF") Assisted-by: GitHub Copilot (claude-sonnet-4.6) Signed-off-by: Len Brown --- tools/power/x86/turbostat/turbostat.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c index 920694c3c1ec..0d40525171c1 100644 --- a/tools/power/x86/turbostat/turbostat.c +++ b/tools/power/x86/turbostat/turbostat.c @@ -5915,7 +5915,7 @@ void free_fd_msr(void) if (!msr_counter_info) return; - for (int cpu = 0; cpu < topo.max_cpu_num; ++cpu) { + for (int cpu = 0; cpu <= topo.max_cpu_num; ++cpu) { if (msr_counter_info[cpu].fd_perf != -1) close(msr_counter_info[cpu].fd_perf); } -- cgit v1.2.3 From 217fef8a26da7527b2b81c5fd285666393c4e00f Mon Sep 17 00:00:00 2001 From: Len Brown Date: Thu, 30 Apr 2026 21:40:36 -0400 Subject: tools/power turbostat: Fix CWF PMT off-by-one Fix off-by-one error causing last CPU on CWF to never get a pmt counter allocated. Fixes: 5ce1e9bbb2a1 ("tools/power turbostat: Add CPU%c1e BIC for CWF") Assisted-by: GitHub Copilot (claude-sonnet-4.6) Signed-off-by: Len Brown --- tools/power/x86/turbostat/turbostat.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c index 0d40525171c1..17c2e3d2163f 100644 --- a/tools/power/x86/turbostat/turbostat.c +++ b/tools/power/x86/turbostat/turbostat.c @@ -10439,7 +10439,7 @@ void pmt_init(void) mod_num = 0; /* Relative module number for current PMT file. */ /* Open the counter for each CPU. */ - for (cpu_num = 0; cpu_num < topo.max_cpu_num;) { + for (cpu_num = 0; cpu_num <= topo.max_cpu_num;) { if (cpu_is_not_allowed(cpu_num)) goto next_loop_iter; -- cgit v1.2.3 From 1b2903a459718e12fb81b41db900d719eb953f02 Mon Sep 17 00:00:00 2001 From: Len Brown Date: Sun, 3 May 2026 15:23:25 -0400 Subject: tools/power turbostat: Fix PMT error path fd handling It is possible to leak a file descriptor if a diretory open succeeds, but the scan fails. Close the fd on error. Also, on cleanup, check for NULL fd. Assisted-by: Claude: Claude Sonnet 4.6 Fixes: 4265a86582eaa2 ("tools/power turbostat: Add PMT directory iterator helper") Signed-off-by: Len Brown --- tools/power/x86/turbostat/turbostat.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c index 17c2e3d2163f..f158c60c2c45 100644 --- a/tools/power/x86/turbostat/turbostat.c +++ b/tools/power/x86/turbostat/turbostat.c @@ -2034,8 +2034,11 @@ const struct dirent *pmt_diriter_begin(struct pmt_diriter_t *iter, const char *p return NULL; num_names = scandir(pmt_root_path, &iter->namelist, pmt_telemdir_filter, pmt_telemdir_sort); - if (num_names == -1) + if (num_names == -1) { + closedir(iter->dir); + iter->dir = NULL; return NULL; + } } iter->current_name_idx = 0; @@ -2063,8 +2066,10 @@ void pmt_diriter_remove(struct pmt_diriter_t *iter) iter->num_names = 0; iter->current_name_idx = 0; - closedir(iter->dir); - iter->dir = NULL; + if (iter->dir) { + closedir(iter->dir); + iter->dir = NULL; + } } unsigned int pmt_counter_get_width(const struct pmt_counter *p) -- cgit v1.2.3 From 993912732cceb22b68f6261e2a0112b1c4a3d457 Mon Sep 17 00:00:00 2001 From: Len Brown Date: Sun, 3 May 2026 14:17:54 -0400 Subject: tools/power turbostat: Fix add_counter issue if > 24 counters. We allocate space for added counters ahead of time, and we arbitrarily chose 24 added counters of each type. If a future code flow hits this limit and continues, and then removes counters, we do a phantom increment and lose track of the number of added counters. Increment the count only if there is no error. Fixes: 388e9c8134be6b ("tools/power turbostat: Make extensible via the --add parameter") Assisted-by: Claude: Claude Sonnet 4.6 Signed-off-by: Len Brown --- tools/power/x86/turbostat/turbostat.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c index f158c60c2c45..c3e190d91a8d 100644 --- a/tools/power/x86/turbostat/turbostat.c +++ b/tools/power/x86/turbostat/turbostat.c @@ -10673,10 +10673,11 @@ int add_counter(unsigned int msr_num, char *path, char *name, fprintf(stderr, "%s: %s FOUND\n", __func__, name); break; } - if (sys.added_thread_counters++ >= MAX_ADDED_THREAD_COUNTERS) { + if (sys.added_thread_counters >= MAX_ADDED_THREAD_COUNTERS) { warnx("ignoring thread counter %s", name); return -1; } + sys.added_thread_counters++; break; case SCOPE_CORE: msrp = find_msrp_by_name(sys.cp, name); @@ -10685,10 +10686,11 @@ int add_counter(unsigned int msr_num, char *path, char *name, fprintf(stderr, "%s: %s FOUND\n", __func__, name); break; } - if (sys.added_core_counters++ >= MAX_ADDED_CORE_COUNTERS) { + if (sys.added_core_counters >= MAX_ADDED_CORE_COUNTERS) { warnx("ignoring core counter %s", name); return -1; } + sys.added_core_counters++; break; case SCOPE_PACKAGE: msrp = find_msrp_by_name(sys.pp, name); @@ -10697,10 +10699,11 @@ int add_counter(unsigned int msr_num, char *path, char *name, fprintf(stderr, "%s: %s FOUND\n", __func__, name); break; } - if (sys.added_package_counters++ >= MAX_ADDED_PACKAGE_COUNTERS) { + if (sys.added_package_counters >= MAX_ADDED_PACKAGE_COUNTERS) { warnx("ignoring package counter %s", name); return -1; } + sys.added_package_counters++; break; default: warnx("ignoring counter %s with unknown scope", name); -- cgit v1.2.3 From 53ffddf66f4efbd8836bd117decb234630854c47 Mon Sep 17 00:00:00 2001 From: Len Brown Date: Tue, 28 Apr 2026 18:02:39 -0400 Subject: tools/power turbostat: Cleanup: Delete unused flags No functional change. Assisted-by: Claude:claude-haiku-4-5 Signed-off-by: Len Brown --- tools/power/x86/turbostat/turbostat.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c index c3e190d91a8d..307a629ab0b7 100644 --- a/tools/power/x86/turbostat/turbostat.c +++ b/tools/power/x86/turbostat/turbostat.c @@ -138,8 +138,6 @@ struct msr_counter { enum counter_format format; struct msr_counter *next; unsigned int flags; -#define FLAGS_HIDE (1 << 0) -#define FLAGS_SHOW (1 << 1) #define SYSFS_PERCPU (1 << 1) }; static int use_android_msr_path; -- cgit v1.2.3 From b8121832a67c931c6b4a7c72b5a73780aca5870b Mon Sep 17 00:00:00 2001 From: Len Brown Date: Tue, 28 Apr 2026 20:48:56 -0400 Subject: tools/power turbostat: Rename: Differntiate counter and CPU sets Sets of Built-In-Counters (BICs) are implemented using cpu_set_t. Since cpu_set_t is also used for sets of CPU's, invent the name "bic_set_t" to make it more clear which is which. No functional change. Signed-off-by: Len Brown --- tools/power/x86/turbostat/turbostat.c | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c index 307a629ab0b7..9f783b74e75b 100644 --- a/tools/power/x86/turbostat/turbostat.c +++ b/tools/power/x86/turbostat/turbostat.c @@ -290,7 +290,9 @@ enum bic_names { MAX_BIC }; -void print_bic_set(char *s, cpu_set_t *set) +#define bic_set_t cpu_set_t /* implement bic_set_t using cpu_set_t */ + +void print_bic_set(char *s, bic_set_t *set) { int i; @@ -306,17 +308,17 @@ void print_bic_set(char *s, cpu_set_t *set) putchar('\n'); } -static cpu_set_t bic_group_topology; -static cpu_set_t bic_group_thermal_pwr; -static cpu_set_t bic_group_frequency; -static cpu_set_t bic_group_hw_idle; -static cpu_set_t bic_group_sw_idle; -static cpu_set_t bic_group_idle; -static cpu_set_t bic_group_cache; -static cpu_set_t bic_group_other; -static cpu_set_t bic_group_disabled_by_default; -static cpu_set_t bic_enabled; -static cpu_set_t bic_present; +static bic_set_t bic_group_topology; +static bic_set_t bic_group_thermal_pwr; +static bic_set_t bic_group_frequency; +static bic_set_t bic_group_hw_idle; +static bic_set_t bic_group_sw_idle; +static bic_set_t bic_group_idle; +static bic_set_t bic_group_cache; +static bic_set_t bic_group_other; +static bic_set_t bic_group_disabled_by_default; +static bic_set_t bic_enabled; +static bic_set_t bic_present; /* modify */ #define BIC_INIT(set) CPU_ZERO(set) @@ -332,7 +334,7 @@ static cpu_set_t bic_present; #define DO_BIC_READ(COUNTER_NUMBER) CPU_ISSET(COUNTER_NUMBER, &bic_present) #define DO_BIC(COUNTER_NUMBER) (CPU_ISSET(COUNTER_NUMBER, &bic_enabled) && CPU_ISSET(COUNTER_NUMBER, &bic_present)) -static void bic_set_all(cpu_set_t *set) +static void bic_set_all(bic_set_t *set) { int i; @@ -346,7 +348,7 @@ static void bic_set_all(cpu_set_t *set) * bic_clear_bits() * clear all the bits from "clr" in "dst" */ -static void bic_clear_bits(cpu_set_t *dst, cpu_set_t *clr) +static void bic_clear_bits(bic_set_t *dst, bic_set_t *clr) { int i; @@ -2783,7 +2785,7 @@ void help(void) * for all the strings in comma separate name_list, * set the approprate bit in return value. */ -void bic_lookup(cpu_set_t *ret_set, char *name_list, enum show_hide_mode mode) +void bic_lookup(bic_set_t *ret_set, char *name_list, enum show_hide_mode mode) { unsigned int i; @@ -11564,7 +11566,7 @@ void cmdline(int argc, char **argv) * multiple invocations simply clear more bits in enabled mask */ { - cpu_set_t bic_group_hide; + bic_set_t bic_group_hide; BIC_INIT(&bic_group_hide); -- cgit v1.2.3 From ddaeccdb1378b02ecd0e446db3319eab082a8cea Mon Sep 17 00:00:00 2001 From: Len Brown Date: Tue, 28 Apr 2026 21:49:46 -0400 Subject: tools/power turbostat: Cleanup: Remove hard-coded 8192 CPU limit The --cpu-set cmdline option was parsed before topology_probe() set max_cpu_num, and so it used a hard-coded cpu limit of 8192. Probe max_cpu_num earlier and allocate the set for --cpu-set using the same size as all other cpu sets, sufficiently large enough to hold max possible (but not bigger). No functional change, unless you count fixing some error paths. Signed-off-by: Len Brown --- tools/power/x86/turbostat/turbostat.c | 75 ++++++++++++++++++++--------------- 1 file changed, 44 insertions(+), 31 deletions(-) diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c index 9f783b74e75b..4291067c3e54 100644 --- a/tools/power/x86/turbostat/turbostat.c +++ b/tools/power/x86/turbostat/turbostat.c @@ -1407,7 +1407,6 @@ void init_perf_model_support(unsigned int family, unsigned int model) int backwards_count; char *progname; -#define CPU_SUBSET_MAXCPUS 8192 /* need to use before probe... */ cpu_set_t *cpu_present_set, *cpu_possible_set, *cpu_effective_set, *cpu_allowed_set, *cpu_affinity_set, *cpu_subset; cpu_set_t *perf_pcore_set, *perf_ecore_set, *perf_lcore_set; size_t cpu_present_setsize, cpu_possible_setsize, cpu_effective_setsize, cpu_allowed_setsize, cpu_affinity_setsize, cpu_subset_size; @@ -6170,7 +6169,7 @@ int get_physical_node_id(struct cpu_topology *thiscpu) static int parse_cpu_str(char *cpu_str, cpu_set_t *cpu_set, int cpu_set_size) { - unsigned int start, end; + int start, end; char *next = cpu_str; while (next && *next) { @@ -6183,7 +6182,7 @@ static int parse_cpu_str(char *cpu_str, cpu_set_t *cpu_set, int cpu_set_size) start = strtoul(next, &next, 10); - if (start >= CPU_SUBSET_MAXCPUS) + if (start > topo.max_cpu_num) return 1; CPU_SET_S(start, cpu_set_size, cpu_set); @@ -6210,7 +6209,7 @@ static int parse_cpu_str(char *cpu_str, cpu_set_t *cpu_set, int cpu_set_size) return 1; while (++start <= end) { - if (start >= CPU_SUBSET_MAXCPUS) + if (start > topo.max_cpu_num) return 1; CPU_SET_S(start, cpu_set_size, cpu_set); } @@ -6378,26 +6377,6 @@ void re_initialize(void) fprintf(outf, "turbostat: re-initialized with num_cpus %d, allowed_cpus %d\n", topo.num_cpus, topo.allowed_cpus); } -void set_max_cpu_num(void) -{ - FILE *filep; - int current_cpu; - unsigned long dummy; - char pathname[64]; - - current_cpu = sched_getcpu(); - if (current_cpu < 0) - err(1, "cannot find calling cpu ID"); - sprintf(pathname, "/sys/devices/system/cpu/cpu%d/topology/thread_siblings", current_cpu); - - filep = fopen_or_die(pathname, "r"); - topo.max_cpu_num = 0; - while (fscanf(filep, "%lx,", &dummy) == 1) - topo.max_cpu_num += BITMASK_SIZE; - fclose(filep); - topo.max_cpu_num--; /* 0 based */ -} - /* * count_cpus() * remember the last one seen, it will be the max @@ -9526,6 +9505,35 @@ int set_thread_siblings(struct cpu_topology *thiscpu) return (ht_id - 1); } +/* + * get_max_cpu_num(path) + * return the last number of the cpu list in this file + */ +static int get_max_cpu_num(char *path) +{ + FILE *filep; + int retval, num; + char c; + + filep = fopen(path, "r"); + if (filep == NULL) { + warn("%s", path); + return 255; + } + + while ((retval = fscanf(filep, "%d%c", &num, &c)) == 2) { + if (c == '\n') + break; + if (c != ',' && c != '-') + errx(1, "Bad Format '%s'", path); + } + if (retval != 2) + errx(1, "Bad format '%s'", path); + + fclose(filep); + return num; +} + void topology_probe(bool startup) { int i; @@ -9533,8 +9541,7 @@ void topology_probe(bool startup) int max_package_id = 0; int max_siblings = 0; - /* Initialize num_cpus, max_cpu_num */ - set_max_cpu_num(); + /* Initialize num_cpus */ topo.num_cpus = 0; for_all_proc_cpus(count_cpus); if (!summary_only) @@ -9601,7 +9608,7 @@ void topology_probe(bool startup) * * cpu_allowed_set is the intersection of cpu_present_set/cpu_effective_set/cpu_subset. */ - for (i = 0; i < CPU_SUBSET_MAXCPUS; ++i) { + for (i = 0; i <= topo.max_cpu_num; ++i) { if (cpu_subset && !CPU_ISSET_S(i, cpu_subset_size, cpu_subset)) continue; @@ -9609,7 +9616,7 @@ void topology_probe(bool startup) if (cpu_subset) { /* cpus in cpu_subset must be in cpu_present_set during startup */ if (startup) - err(1, "cpu%d not present", i); + errx(1, "cpu%d not present", i); else fprintf(stderr, "cpu%d not present\n", i); } @@ -9629,7 +9636,7 @@ void topology_probe(bool startup) print_cpu_set("allowed set", cpu_allowed_set); if (!CPU_COUNT_S(cpu_allowed_setsize, cpu_allowed_set)) - err(-ENODEV, "No valid cpus found"); + errx(-ENODEV, "No valid cpus found"); sched_setaffinity(0, cpu_allowed_setsize, cpu_allowed_set); /* @@ -11464,15 +11471,17 @@ void parse_cpu_command(char *optarg) if (show_core_only || show_pkg_only) goto error; - cpu_subset = CPU_ALLOC(CPU_SUBSET_MAXCPUS); + cpu_subset = CPU_ALLOC(topo.max_cpu_num + 1); if (cpu_subset == NULL) err(3, "CPU_ALLOC"); - cpu_subset_size = CPU_ALLOC_SIZE(CPU_SUBSET_MAXCPUS); + cpu_subset_size = CPU_ALLOC_SIZE(topo.max_cpu_num + 1); CPU_ZERO_S(cpu_subset_size, cpu_subset); if (parse_cpu_str(optarg, cpu_subset, cpu_subset_size)) goto error; + if (debug) + print_cpu_set("cpu subsetset", cpu_subset); return; @@ -11684,6 +11693,10 @@ int main(int argc, char **argv) close(fd); skip_cgroup_setting: + + /* probe max_cpu_num early for use by cmdline() */ + topo.max_cpu_num = get_max_cpu_num("/sys/devices/system/cpu/possible"); + outf = stderr; cmdline(argc, argv); -- cgit v1.2.3 From 34fa44d6842b8f7c455c5676d210333d357b54f6 Mon Sep 17 00:00:00 2001 From: Len Brown Date: Tue, 28 Apr 2026 22:19:41 -0400 Subject: tools/power turbostat: Cleanup: Use one cpu_setsize for all purposes Use a single cpu_setsize to describe every cpu_set_t. Size it based on the maximum possible cpu number. No functional change. Signed-off-by: Len Brown --- tools/power/x86/turbostat/turbostat.c | 77 ++++++++++++++++------------------- 1 file changed, 34 insertions(+), 43 deletions(-) diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c index 4291067c3e54..e475e5f263be 100644 --- a/tools/power/x86/turbostat/turbostat.c +++ b/tools/power/x86/turbostat/turbostat.c @@ -1409,7 +1409,7 @@ char *progname; cpu_set_t *cpu_present_set, *cpu_possible_set, *cpu_effective_set, *cpu_allowed_set, *cpu_affinity_set, *cpu_subset; cpu_set_t *perf_pcore_set, *perf_ecore_set, *perf_lcore_set; -size_t cpu_present_setsize, cpu_possible_setsize, cpu_effective_setsize, cpu_allowed_setsize, cpu_affinity_setsize, cpu_subset_size; +size_t cpu_setsize; #define MAX_ADDED_THREAD_COUNTERS 24 #define MAX_ADDED_CORE_COUNTERS 8 #define MAX_ADDED_PACKAGE_COUNTERS 16 @@ -2440,7 +2440,7 @@ int cpu_is_not_present(int cpu) if (cpu < 0) return 1; - return !CPU_ISSET_S(cpu, cpu_present_setsize, cpu_present_set); + return !CPU_ISSET_S(cpu, cpu_setsize, cpu_present_set); } int cpu_is_not_allowed(int cpu) @@ -2448,7 +2448,7 @@ int cpu_is_not_allowed(int cpu) if (cpu < 0) return 1; - return !CPU_ISSET_S(cpu, cpu_allowed_setsize, cpu_allowed_set); + return !CPU_ISSET_S(cpu, cpu_setsize, cpu_allowed_set); } #define GLOBAL_CORE_ID(core_id, pkg_id) (core_id + pkg_id * (topo.max_core_id + 1)) @@ -2541,9 +2541,9 @@ int is_cpu_first_thread_in_package(struct thread_data *t, struct core_data *c, s int cpu_migrate(int cpu) { - CPU_ZERO_S(cpu_affinity_setsize, cpu_affinity_set); - CPU_SET_S(cpu, cpu_affinity_setsize, cpu_affinity_set); - if (sched_setaffinity(0, cpu_affinity_setsize, cpu_affinity_set) == -1) + CPU_ZERO_S(cpu_setsize, cpu_affinity_set); + CPU_SET_S(cpu, cpu_setsize, cpu_affinity_set); + if (sched_setaffinity(0, cpu_setsize, cpu_affinity_set) == -1) return -1; else return 0; @@ -3371,7 +3371,7 @@ int format_counters(PER_THREAD_PARAMS) return 0; /*if not summary line and --cpu is used */ - if ((t != average.threads) && (cpu_subset && !CPU_ISSET_S(t->cpu_id, cpu_subset_size, cpu_subset))) + if ((t != average.threads) && (cpu_subset && !CPU_ISSET_S(t->cpu_id, cpu_setsize, cpu_subset))) return 0; if (DO_BIC(BIC_USEC)) { @@ -5982,19 +5982,15 @@ void free_all_buffers(void) CPU_FREE(cpu_present_set); cpu_present_set = NULL; - cpu_present_setsize = 0; CPU_FREE(cpu_effective_set); cpu_effective_set = NULL; - cpu_effective_setsize = 0; CPU_FREE(cpu_allowed_set); cpu_allowed_set = NULL; - cpu_allowed_setsize = 0; CPU_FREE(cpu_affinity_set); cpu_affinity_set = NULL; - cpu_affinity_setsize = 0; if (perf_pcore_set) { CPU_FREE(perf_pcore_set); @@ -6349,7 +6345,7 @@ static void update_effective_set(bool startup) { update_effective_str(startup); - if (parse_cpu_str(cpu_effective_str, cpu_effective_set, cpu_effective_setsize)) + if (parse_cpu_str(cpu_effective_str, cpu_effective_set, cpu_setsize)) err(1, "%s: cpu str malformat %s", PATH_EFFECTIVE_CPUS, cpu_effective_str); } @@ -6391,7 +6387,7 @@ int count_cpus(int cpu) int mark_cpu_present(int cpu) { - CPU_SET_S(cpu, cpu_present_setsize, cpu_present_set); + CPU_SET_S(cpu, cpu_setsize, cpu_present_set); return 0; } @@ -8531,7 +8527,7 @@ int initialize_cpu_set_from_sysfs(cpu_set_t *cpu_set, char *sysfs_path, char *sy warn("read %s", sysfs_path); goto err; } - if (parse_cpu_str(cpuset_buf, cpu_set, cpu_possible_setsize)) { + if (parse_cpu_str(cpuset_buf, cpu_set, cpu_setsize)) { warnx("%s: cpu str malformat %s\n", sysfs_path, cpu_effective_str); goto err; } @@ -8567,7 +8563,7 @@ void linux_perf_init_hybrid_cpus(void) perf_pcore_set = CPU_ALLOC((topo.max_cpu_num + 1)); if (perf_pcore_set == NULL) err(3, "CPU_ALLOC"); - CPU_ZERO_S(cpu_possible_setsize, perf_pcore_set); + CPU_ZERO_S(cpu_setsize, perf_pcore_set); initialize_cpu_set_from_sysfs(perf_pcore_set, perf_cpu_pcore_path, "cpus"); if (debug) print_cpu_set("perf pcores", perf_pcore_set); @@ -8579,7 +8575,7 @@ void linux_perf_init_hybrid_cpus(void) perf_ecore_set = CPU_ALLOC((topo.max_cpu_num + 1)); if (perf_ecore_set == NULL) err(3, "CPU_ALLOC"); - CPU_ZERO_S(cpu_possible_setsize, perf_ecore_set); + CPU_ZERO_S(cpu_setsize, perf_ecore_set); initialize_cpu_set_from_sysfs(perf_ecore_set, perf_cpu_ecore_path, "cpus"); if (debug) print_cpu_set("perf ecores", perf_ecore_set); @@ -8591,7 +8587,7 @@ void linux_perf_init_hybrid_cpus(void) perf_lcore_set = CPU_ALLOC((topo.max_cpu_num + 1)); if (perf_lcore_set == NULL) err(3, "CPU_ALLOC"); - CPU_ZERO_S(cpu_possible_setsize, perf_lcore_set); + CPU_ZERO_S(cpu_setsize, perf_lcore_set); initialize_cpu_set_from_sysfs(perf_lcore_set, perf_cpu_lcore_path, "cpus"); if (debug) print_cpu_set("perf lcores", perf_lcore_set); @@ -9416,7 +9412,7 @@ void perf_l2_init(void) } continue; } - if (perf_pcore_set && CPU_ISSET_S(cpu, cpu_possible_setsize, perf_pcore_set)) { + if (perf_pcore_set && CPU_ISSET_S(cpu, cpu_setsize, perf_pcore_set)) { fd_l2_percpu[cpu] = open_perf_counter(cpu, perf_pmu_types.pcore, perf_model_support->first.refs, -1, PERF_FORMAT_GROUP); if (fd_l2_percpu[cpu] == -1) { warnx("%s(cpu%d, 0x%x, 0x%llx) REFS", __func__, cpu, perf_pmu_types.pcore, perf_model_support->first.refs); @@ -9429,7 +9425,7 @@ void perf_l2_init(void) free_fd_l2_percpu(); return; } - } else if (perf_ecore_set && CPU_ISSET_S(cpu, cpu_possible_setsize, perf_ecore_set)) { + } else if (perf_ecore_set && CPU_ISSET_S(cpu, cpu_setsize, perf_ecore_set)) { fd_l2_percpu[cpu] = open_perf_counter(cpu, perf_pmu_types.ecore, perf_model_support->second.refs, -1, PERF_FORMAT_GROUP); if (fd_l2_percpu[cpu] == -1) { warnx("%s(cpu%d, 0x%x, 0x%llx) REFS", __func__, cpu, perf_pmu_types.ecore, perf_model_support->second.refs); @@ -9442,7 +9438,7 @@ void perf_l2_init(void) free_fd_l2_percpu(); return; } - } else if (perf_lcore_set && CPU_ISSET_S(cpu, cpu_possible_setsize, perf_lcore_set)) { + } else if (perf_lcore_set && CPU_ISSET_S(cpu, cpu_setsize, perf_lcore_set)) { fd_l2_percpu[cpu] = open_perf_counter(cpu, perf_pmu_types.lcore, perf_model_support->third.refs, -1, PERF_FORMAT_GROUP); if (fd_l2_percpu[cpu] == -1) { warnx("%s(cpu%d, 0x%x, 0x%llx) REFS", __func__, cpu, perf_pmu_types.lcore, perf_model_support->third.refs); @@ -9560,8 +9556,7 @@ void topology_probe(bool startup) cpu_present_set = CPU_ALLOC((topo.max_cpu_num + 1)); if (cpu_present_set == NULL) err(3, "CPU_ALLOC"); - cpu_present_setsize = CPU_ALLOC_SIZE((topo.max_cpu_num + 1)); - CPU_ZERO_S(cpu_present_setsize, cpu_present_set); + CPU_ZERO_S(cpu_setsize, cpu_present_set); for_all_proc_cpus(mark_cpu_present); if (debug) print_cpu_set("present set", cpu_present_set); @@ -9572,8 +9567,7 @@ void topology_probe(bool startup) cpu_possible_set = CPU_ALLOC((topo.max_cpu_num + 1)); if (cpu_possible_set == NULL) err(3, "CPU_ALLOC"); - cpu_possible_setsize = CPU_ALLOC_SIZE((topo.max_cpu_num + 1)); - CPU_ZERO_S(cpu_possible_setsize, cpu_possible_set); + CPU_ZERO_S(cpu_setsize, cpu_possible_set); initialize_cpu_set_from_sysfs(cpu_possible_set, "/sys/devices/system/cpu", "possible"); if (debug) print_cpu_set("possible set", cpu_possible_set); @@ -9584,8 +9578,7 @@ void topology_probe(bool startup) cpu_effective_set = CPU_ALLOC((topo.max_cpu_num + 1)); if (cpu_effective_set == NULL) err(3, "CPU_ALLOC"); - cpu_effective_setsize = CPU_ALLOC_SIZE((topo.max_cpu_num + 1)); - CPU_ZERO_S(cpu_effective_setsize, cpu_effective_set); + CPU_ZERO_S(cpu_setsize, cpu_effective_set); update_effective_set(startup); if (debug) print_cpu_set("effective set", cpu_effective_set); @@ -9596,8 +9589,7 @@ void topology_probe(bool startup) cpu_allowed_set = CPU_ALLOC((topo.max_cpu_num + 1)); if (cpu_allowed_set == NULL) err(3, "CPU_ALLOC"); - cpu_allowed_setsize = CPU_ALLOC_SIZE((topo.max_cpu_num + 1)); - CPU_ZERO_S(cpu_allowed_setsize, cpu_allowed_set); + CPU_ZERO_S(cpu_setsize, cpu_allowed_set); /* * Validate and update cpu_allowed_set. @@ -9609,10 +9601,10 @@ void topology_probe(bool startup) * cpu_allowed_set is the intersection of cpu_present_set/cpu_effective_set/cpu_subset. */ for (i = 0; i <= topo.max_cpu_num; ++i) { - if (cpu_subset && !CPU_ISSET_S(i, cpu_subset_size, cpu_subset)) + if (cpu_subset && !CPU_ISSET_S(i, cpu_setsize, cpu_subset)) continue; - if (!CPU_ISSET_S(i, cpu_present_setsize, cpu_present_set)) { + if (!CPU_ISSET_S(i, cpu_setsize, cpu_present_set)) { if (cpu_subset) { /* cpus in cpu_subset must be in cpu_present_set during startup */ if (startup) @@ -9623,21 +9615,21 @@ void topology_probe(bool startup) continue; } - if (CPU_COUNT_S(cpu_effective_setsize, cpu_effective_set)) { - if (!CPU_ISSET_S(i, cpu_effective_setsize, cpu_effective_set)) { + if (CPU_COUNT_S(cpu_setsize, cpu_effective_set)) { + if (!CPU_ISSET_S(i, cpu_setsize, cpu_effective_set)) { fprintf(stderr, "cpu%d not effective\n", i); continue; } } - CPU_SET_S(i, cpu_allowed_setsize, cpu_allowed_set); + CPU_SET_S(i, cpu_setsize, cpu_allowed_set); } if (debug) print_cpu_set("allowed set", cpu_allowed_set); - if (!CPU_COUNT_S(cpu_allowed_setsize, cpu_allowed_set)) + if (!CPU_COUNT_S(cpu_setsize, cpu_allowed_set)) errx(-ENODEV, "No valid cpus found"); - sched_setaffinity(0, cpu_allowed_setsize, cpu_allowed_set); + sched_setaffinity(0, cpu_setsize, cpu_allowed_set); /* * Allocate and initialize cpu_affinity_set @@ -9645,8 +9637,7 @@ void topology_probe(bool startup) cpu_affinity_set = CPU_ALLOC((topo.max_cpu_num + 1)); if (cpu_affinity_set == NULL) err(3, "CPU_ALLOC"); - cpu_affinity_setsize = CPU_ALLOC_SIZE((topo.max_cpu_num + 1)); - CPU_ZERO_S(cpu_affinity_setsize, cpu_affinity_set); + CPU_ZERO_S(cpu_setsize, cpu_affinity_set); for_all_proc_cpus(clear_ht_id); @@ -10537,11 +10528,11 @@ void turbostat_init() void affinitize_child(void) { /* Prefer cpu_possible_set, if available */ - if (sched_setaffinity(0, cpu_possible_setsize, cpu_possible_set)) { + if (sched_setaffinity(0, cpu_setsize, cpu_possible_set)) { warn("sched_setaffinity cpu_possible_set"); /* Otherwise, allow child to run on same cpu set as turbostat */ - if (sched_setaffinity(0, cpu_allowed_setsize, cpu_allowed_set)) + if (sched_setaffinity(0, cpu_setsize, cpu_allowed_set)) warn("sched_setaffinity cpu_allowed_set"); } } @@ -11474,11 +11465,10 @@ void parse_cpu_command(char *optarg) cpu_subset = CPU_ALLOC(topo.max_cpu_num + 1); if (cpu_subset == NULL) err(3, "CPU_ALLOC"); - cpu_subset_size = CPU_ALLOC_SIZE(topo.max_cpu_num + 1); - CPU_ZERO_S(cpu_subset_size, cpu_subset); + CPU_ZERO_S(cpu_setsize, cpu_subset); - if (parse_cpu_str(optarg, cpu_subset, cpu_subset_size)) + if (parse_cpu_str(optarg, cpu_subset, cpu_setsize)) goto error; if (debug) print_cpu_set("cpu subsetset", cpu_subset); @@ -11694,8 +11684,9 @@ int main(int argc, char **argv) skip_cgroup_setting: - /* probe max_cpu_num early for use by cmdline() */ + /* probe max_cpu_num and set cpu_setsize early for use by cmdline() */ topo.max_cpu_num = get_max_cpu_num("/sys/devices/system/cpu/possible"); + cpu_setsize = CPU_ALLOC_SIZE((topo.max_cpu_num + 1)); outf = stderr; cmdline(argc, argv); -- cgit v1.2.3 From ca3015b3f244d07b11adade000b59b23b5f44ca4 Mon Sep 17 00:00:00 2001 From: Len Brown Date: Tue, 28 Apr 2026 22:53:38 -0400 Subject: tools/power turbostat: Allow mulitple --cpu on cmdline If --cpu is on the cmdline more than once, previous invocations are forgotton (and their cpu_set is leaked). Allow multiple invocations of --cpu on the cmline. Or them together, reusing the same same cpu_set. Signed-off-by: Len Brown --- tools/power/x86/turbostat/turbostat.8 | 2 +- tools/power/x86/turbostat/turbostat.c | 12 +++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/tools/power/x86/turbostat/turbostat.8 b/tools/power/x86/turbostat/turbostat.8 index 344ede2f8546..9b81dca7032f 100644 --- a/tools/power/x86/turbostat/turbostat.8 +++ b/tools/power/x86/turbostat/turbostat.8 @@ -92,7 +92,7 @@ name as necessary to disambiguate it from others is necessary. Note that option '%x' hex identifier of the PMT MMIO region. .fi .PP -\fB--cpu cpu-set\fP limit output to system summary plus the specified cpu-set. If cpu-set is the string "core", then the system summary plus the first CPU in each core are printed -- eg. subsequent HT siblings are not printed. Or if cpu-set is the string "package", then the system summary plus the first CPU in each package is printed. Otherwise, the system summary plus the specified set of CPUs are printed. The cpu-set is ordered from low to high, comma delimited with ".." and "-" permitted to denote a range. eg. 1,2,8,14..17,21-44 +\fB--cpu cpu-set\fP limit output to system summary plus the specified cpu-set. If cpu-set is the string "core", then the system summary plus the first CPU in each core are printed -- eg. subsequent HT siblings are not printed. Or if cpu-set is the string "package", then the system summary plus the first CPU in each package is printed. Otherwise, the system summary plus the specified set of CPUs are printed. The cpu-set is ordered from low to high, comma delimited with ".." and "-" permitted to denote a range. eg. 1,2,8,14..17,21-44 Multiple invocations of --cpu are OR'd together. .PP \fB--hide column\fP do not show the specified built-in columns. May be invoked multiple times, or with a comma-separated list of column names. .PP diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c index e475e5f263be..02685fdd5218 100644 --- a/tools/power/x86/turbostat/turbostat.c +++ b/tools/power/x86/turbostat/turbostat.c @@ -11462,16 +11462,18 @@ void parse_cpu_command(char *optarg) if (show_core_only || show_pkg_only) goto error; - cpu_subset = CPU_ALLOC(topo.max_cpu_num + 1); - if (cpu_subset == NULL) - err(3, "CPU_ALLOC"); + if (!cpu_subset) { + cpu_subset = CPU_ALLOC(topo.max_cpu_num + 1); + if (cpu_subset == NULL) + err(3, "CPU_ALLOC"); - CPU_ZERO_S(cpu_setsize, cpu_subset); + CPU_ZERO_S(cpu_setsize, cpu_subset); + } if (parse_cpu_str(optarg, cpu_subset, cpu_setsize)) goto error; if (debug) - print_cpu_set("cpu subsetset", cpu_subset); + print_cpu_set("--cpu", cpu_subset); return; -- cgit v1.2.3 From 3603f5e52c75974003a4ffacfa8baec2ba933abc Mon Sep 17 00:00:00 2001 From: Len Brown Date: Tue, 28 Apr 2026 19:39:40 -0400 Subject: tools/power turbostat: Cleanup: Delete duplicate table entry No functional change. Assisted-by: Claude:claude-haiku-4-5 Signed-off-by: Len Brown --- tools/power/x86/turbostat/turbostat.c | 1 - 1 file changed, 1 deletion(-) diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c index 02685fdd5218..e72a6e744eca 100644 --- a/tools/power/x86/turbostat/turbostat.c +++ b/tools/power/x86/turbostat/turbostat.c @@ -1312,7 +1312,6 @@ static struct perf_model_support turbostat_perf_model_support[] = { { INTEL_ATOM_CRESTMONT, { PCE(0x00, 0x00), PCE(0x00, 0x02)}, {}, {} }, /* GRR */ { INTEL_ATOM_DARKMONT_X, { PCE(0x01, 0xFF), PCE(0x01, 0xBF)}, {}, {} }, /* CWF */ - { INTEL_ALDERLAKE, { PCE(0x00, 0xFF), PCE(0x00, 0xDF)}, { PCE(0x00, 0x00), PCE(0x00, 0x02)}, {} }, { INTEL_ALDERLAKE, { PCE(0x00, 0xFF), PCE(0x00, 0xDF)}, { PCE(0x00, 0x00), PCE(0x00, 0x02)}, {} }, { INTEL_ALDERLAKE_L, { PCE(0x00, 0xFF), PCE(0x00, 0xDF)}, { PCE(0x00, 0x00), PCE(0x00, 0x02)}, {} }, { INTEL_RAPTORLAKE, { PCE(0x00, 0xFF), PCE(0x00, 0xDF)}, { PCE(0x00, 0x00), PCE(0x00, 0x02)}, {} }, -- cgit v1.2.3 From 23be2513a3d7199dee134c3adfcb76c5a5581866 Mon Sep 17 00:00:00 2001 From: Len Brown Date: Tue, 28 Apr 2026 22:57:34 -0400 Subject: tools/power turbostat: Rename cpu_subset to cpuset_cmdline The cpu_set supporting "--cpu" on the cmdline is special. Unlike all other cpu_sets, it is allocated, re-used, and never freed. Add a comment noting this and name it "cpuset_cmdline" to clarify its use. No functional change. Signed-off-by: Len Brown --- tools/power/x86/turbostat/turbostat.c | 39 ++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c index e72a6e744eca..2e613f8dfc49 100644 --- a/tools/power/x86/turbostat/turbostat.c +++ b/tools/power/x86/turbostat/turbostat.c @@ -1406,7 +1406,7 @@ void init_perf_model_support(unsigned int family, unsigned int model) int backwards_count; char *progname; -cpu_set_t *cpu_present_set, *cpu_possible_set, *cpu_effective_set, *cpu_allowed_set, *cpu_affinity_set, *cpu_subset; +cpu_set_t *cpu_present_set, *cpu_possible_set, *cpu_effective_set, *cpu_allowed_set, *cpu_affinity_set, *cpuset_cmdline; cpu_set_t *perf_pcore_set, *perf_ecore_set, *perf_lcore_set; size_t cpu_setsize; #define MAX_ADDED_THREAD_COUNTERS 24 @@ -3370,7 +3370,7 @@ int format_counters(PER_THREAD_PARAMS) return 0; /*if not summary line and --cpu is used */ - if ((t != average.threads) && (cpu_subset && !CPU_ISSET_S(t->cpu_id, cpu_setsize, cpu_subset))) + if ((t != average.threads) && (cpuset_cmdline && !CPU_ISSET_S(t->cpu_id, cpu_setsize, cpuset_cmdline))) return 0; if (DO_BIC(BIC_USEC)) { @@ -9593,19 +9593,19 @@ void topology_probe(bool startup) /* * Validate and update cpu_allowed_set. * - * Make sure all cpus in cpu_subset are also in cpu_present_set during startup. - * Give a warning when cpus in cpu_subset become unavailable at runtime. + * Make sure all cpus in cpuset_cmdline are also in cpu_present_set during startup. + * Give a warning when cpus in cpuset_cmdline become unavailable at runtime. * Give a warning when cpus are not effective because of cgroup setting. * - * cpu_allowed_set is the intersection of cpu_present_set/cpu_effective_set/cpu_subset. + * cpu_allowed_set is the intersection of cpu_present_set/cpu_effective_set/cpuset_cmdline. */ for (i = 0; i <= topo.max_cpu_num; ++i) { - if (cpu_subset && !CPU_ISSET_S(i, cpu_setsize, cpu_subset)) + if (cpuset_cmdline && !CPU_ISSET_S(i, cpu_setsize, cpuset_cmdline)) continue; if (!CPU_ISSET_S(i, cpu_setsize, cpu_present_set)) { - if (cpu_subset) { - /* cpus in cpu_subset must be in cpu_present_set during startup */ + if (cpuset_cmdline) { + /* cpus in cpuset_cmdline must be in cpu_present_set during startup */ if (startup) errx(1, "cpu%d not present", i); else @@ -11442,18 +11442,18 @@ void probe_cpuidle_counts(void) /* * parse cpuset with following syntax - * 1,2,4..6,8-10 and set bits in cpu_subset + * 1,2,4..6,8-10 and set bits in cpuset_cmdline */ void parse_cpu_command(char *optarg) { if (!strcmp(optarg, "core")) { - if (cpu_subset) + if (cpuset_cmdline) goto error; show_core_only++; return; } if (!strcmp(optarg, "package")) { - if (cpu_subset) + if (cpuset_cmdline) goto error; show_pkg_only++; return; @@ -11461,18 +11461,23 @@ void parse_cpu_command(char *optarg) if (show_core_only || show_pkg_only) goto error; - if (!cpu_subset) { - cpu_subset = CPU_ALLOC(topo.max_cpu_num + 1); - if (cpu_subset == NULL) + /* + * cpuset_cmdline is allocated on the first invocation of --cpu + * It is re-used for subsequent invocations. + * It is never freed. + */ + if (!cpuset_cmdline) { + cpuset_cmdline = CPU_ALLOC(topo.max_cpu_num + 1); + if (cpuset_cmdline == NULL) err(3, "CPU_ALLOC"); - CPU_ZERO_S(cpu_setsize, cpu_subset); + CPU_ZERO_S(cpu_setsize, cpuset_cmdline); } - if (parse_cpu_str(optarg, cpu_subset, cpu_setsize)) + if (parse_cpu_str(optarg, cpuset_cmdline, cpu_setsize)) goto error; if (debug) - print_cpu_set("--cpu", cpu_subset); + print_cpu_set("--cpu", cpuset_cmdline); return; -- cgit v1.2.3 From f8ce9e11bf73cd8c10020619f9c4b71866329af9 Mon Sep 17 00:00:00 2001 From: Len Brown Date: Tue, 28 Apr 2026 23:24:24 -0400 Subject: tools/power turbostat: Cleanup: Remove useless assert() No functional change. Assisted-by: Claude:claude-haiku-4-5 Signed-off-by: Len Brown --- tools/power/x86/turbostat/turbostat.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c index 2e613f8dfc49..57d342464fdf 100644 --- a/tools/power/x86/turbostat/turbostat.c +++ b/tools/power/x86/turbostat/turbostat.c @@ -3283,8 +3283,6 @@ int dump_counters(PER_THREAD_PARAMS) double rapl_counter_get_value(const struct rapl_counter *c, enum rapl_unit desired_unit, double interval) { - assert(desired_unit != RAPL_UNIT_INVALID); - /* * For now we don't expect anything other than joules, * so just simplify the logic. -- cgit v1.2.3 From 0529764225d5729f6964d50437b2c2f942ebcb51 Mon Sep 17 00:00:00 2001 From: Len Brown Date: Thu, 30 Apr 2026 21:47:43 -0400 Subject: tools/power turbostat: Cleanup: Unify comparisons to max_cpu_num Unify the syntax of loops over all CPUs for consistency, since we just fixed two off-by-one bugs. No functional change. Signed-off-by: Len Brown --- tools/power/x86/turbostat/turbostat.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c index 57d342464fdf..32802e2553f0 100644 --- a/tools/power/x86/turbostat/turbostat.c +++ b/tools/power/x86/turbostat/turbostat.c @@ -5834,7 +5834,7 @@ void free_fd_percpu(void) if (!fd_percpu) return; - for (i = 0; i < topo.max_cpu_num + 1; ++i) { + for (i = 0; i <= topo.max_cpu_num; ++i) { if (fd_percpu[i] != 0) close(fd_percpu[i]); } @@ -5848,7 +5848,7 @@ void free_fd_instr_count_percpu(void) if (!fd_instr_count_percpu) return; - for (int i = 0; i < topo.max_cpu_num + 1; ++i) { + for (int i = 0; i <= topo.max_cpu_num; ++i) { if (fd_instr_count_percpu[i] != 0) close(fd_instr_count_percpu[i]); } @@ -5862,7 +5862,7 @@ void free_fd_llc_percpu(void) if (!fd_llc_percpu) return; - for (int i = 0; i < topo.max_cpu_num + 1; ++i) { + for (int i = 0; i <= topo.max_cpu_num; ++i) { if (fd_llc_percpu[i] != 0) close(fd_llc_percpu[i]); } @@ -5879,7 +5879,7 @@ void free_fd_l2_percpu(void) if (!fd_l2_percpu) return; - for (int i = 0; i < topo.max_cpu_num + 1; ++i) { + for (int i = 0; i <= topo.max_cpu_num; ++i) { if (fd_l2_percpu[i] != 0) close(fd_l2_percpu[i]); } @@ -8668,7 +8668,7 @@ void rapl_perf_init(void) memset(domain_visited, 0, num_domains * sizeof(*domain_visited)); - for (int cpu = 0; cpu < topo.max_cpu_num + 1; ++cpu) { + for (int cpu = 0; cpu <= topo.max_cpu_num; ++cpu) { if (cpu_is_not_allowed(cpu)) continue; @@ -9782,7 +9782,7 @@ void allocate_counters(struct counters *counters) if (counters->threads == NULL) goto error; - for (i = 0; i < topo.max_cpu_num + 1; i++) + for (i = 0; i <= topo.max_cpu_num; i++) (counters->threads)[i].cpu_id = -1; counters->cores = calloc(num_cores, sizeof(struct core_data)); @@ -9911,7 +9911,7 @@ void set_master_cpu(void) { int i; - for (i = 0; i < topo.max_cpu_num + 1; ++i) { + for (i = 0; i <= topo.max_cpu_num; ++i) { if (cpu_is_not_allowed(i)) continue; master_cpu = i; @@ -10028,7 +10028,7 @@ int added_perf_counters_init_(struct perf_counter_info *pinfo) memset(domain_visited, 0, max_num_domains * sizeof(*domain_visited)); - for (int cpu = 0; cpu < topo.max_cpu_num + 1; ++cpu) { + for (int cpu = 0; cpu <= topo.max_cpu_num; ++cpu) { next_domain = cpu_to_domain(pinfo, cpu); -- cgit v1.2.3 From 5ab1fb5b2562eba10eda8af69356f3aaaa042b21 Mon Sep 17 00:00:00 2001 From: Len Brown Date: Sun, 3 May 2026 13:13:50 -0400 Subject: tools/power turbostat: Cleanup counter sets debug code When the debug "--Dump" option is used to dump counter values, also dump the counter sets. Update that counter counter-set debug code to pass code review. Assisted-by: Claude: Claude Sonnet 4.6 Signed-off-by: Len Brown --- tools/power/x86/turbostat/turbostat.c | 33 +++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c index 32802e2553f0..8f35ea3466c3 100644 --- a/tools/power/x86/turbostat/turbostat.c +++ b/tools/power/x86/turbostat/turbostat.c @@ -98,6 +98,8 @@ #define ROUND_UP_TO_PAGE_SIZE(n) (((n) + 0x1000UL-1UL) & ~(0x1000UL-1UL)) +FILE *outf; + enum counter_scope { SCOPE_CPU, SCOPE_CORE, SCOPE_PACKAGE }; enum counter_type { COUNTER_ITEMS, COUNTER_CYCLES, COUNTER_SECONDS, COUNTER_USEC, COUNTER_K2M }; enum counter_format { FORMAT_RAW, FORMAT_DELTA, FORMAT_PERCENT, FORMAT_AVERAGE }; @@ -292,20 +294,23 @@ enum bic_names { #define bic_set_t cpu_set_t /* implement bic_set_t using cpu_set_t */ -void print_bic_set(char *s, bic_set_t *set) +/* + * debug helper + */ +static void print_bic_set(const char *s, const bic_set_t *set) { int i; assert(MAX_BIC < CPU_SETSIZE); - printf("%s:", s); + fprintf(outf, "%s:", s); for (i = 0; i < MAX_BIC; ++i) { if (CPU_ISSET(i, set)) - printf(" %s", bic[i].name); + fprintf(outf, " %s", bic[i].name); } - putchar('\n'); + fputc('\n', outf); } static bic_set_t bic_group_topology; @@ -320,6 +325,21 @@ static bic_set_t bic_group_disabled_by_default; static bic_set_t bic_enabled; static bic_set_t bic_present; +static void dump_bic_sets(void) +{ + print_bic_set("bic_enabled", &bic_enabled); + print_bic_set("bic_present", &bic_present); + print_bic_set("bic_group_topology", &bic_group_topology); + print_bic_set("bic_group_thermal_pwr", &bic_group_thermal_pwr); + print_bic_set("bic_group_frequency", &bic_group_frequency); + print_bic_set("bic_group_hw_idle", &bic_group_hw_idle); + print_bic_set("bic_group_sw_idle", &bic_group_sw_idle); + print_bic_set("bic_group_idle", &bic_group_idle); + print_bic_set("bic_group_cache", &bic_group_cache); + print_bic_set("bic_group_other", &bic_group_other); + print_bic_set("bic_group_disabled_by_default", &bic_group_disabled_by_default); +} + /* modify */ #define BIC_INIT(set) CPU_ZERO(set) @@ -485,7 +505,6 @@ static void bic_groups_init(void) #define PCLUNL 15 /* Unlimited */ char *proc_stat = "/proc/stat"; -FILE *outf; int *fd_percpu; int *fd_instr_count_percpu; int *fd_llc_percpu; @@ -11714,8 +11733,10 @@ skip_cgroup_setting: msr_sum_record(); /* dump counters and exit */ - if (dump_only) + if (dump_only) { + dump_bic_sets(); return get_and_dump_counters(); + } /* list header and exit */ if (list_header_only) { -- cgit v1.2.3 From c53b2bf518f3d6279261099d168c9a85bc54e5d5 Mon Sep 17 00:00:00 2001 From: Len Brown Date: Sun, 3 May 2026 13:27:29 -0400 Subject: tools/power turbostat: Cleanup add_counter declaration Declaration and definition parameter name mis-match was confusing. Some params can be 'const'. No Functional Change Assisted-by: Claude: Claude Sonnet 4.6 Signed-off-by: Len Brown --- tools/power/x86/turbostat/turbostat.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c index 8f35ea3466c3..d7e38087b023 100644 --- a/tools/power/x86/turbostat/turbostat.c +++ b/tools/power/x86/turbostat/turbostat.c @@ -590,8 +590,7 @@ struct gfx_sysfs_info { static struct gfx_sysfs_info gfx_info[GFX_MAX]; int get_msr(int cpu, off_t offset, unsigned long long *msr); -int add_counter(unsigned int msr_num, char *path, char *name, - unsigned int width, enum counter_scope scope, enum counter_type type, enum counter_format format, int flags, int package_num); +int add_counter(unsigned int, const char *, const char *, unsigned int, enum counter_scope, enum counter_type, enum counter_format, int, int); /* Model specific support Start */ @@ -10653,7 +10652,7 @@ void print_bootcmd(void) fclose(fp); } -struct msr_counter *find_msrp_by_name(struct msr_counter *head, char *name) +struct msr_counter *find_msrp_by_name(struct msr_counter *head, const char *name) { struct msr_counter *mp; @@ -10666,7 +10665,7 @@ struct msr_counter *find_msrp_by_name(struct msr_counter *head, char *name) return NULL; } -int add_counter(unsigned int msr_num, char *path, char *name, +int add_counter(unsigned int msr_num, const char *path, const char *name, unsigned int width, enum counter_scope scope, enum counter_type type, enum counter_format format, int flags, int id) { struct msr_counter *msrp; -- cgit v1.2.3 From 2538ae8afb14062e307ed514d744c78444235f8d Mon Sep 17 00:00:00 2001 From: Len Brown Date: Sun, 3 May 2026 14:29:40 -0400 Subject: tools/power turbostat: Cleanup: consistently use warn/err, not perror No functional change, unless you count the uniform return value of -1 on calloc failure. Assisted-by: Claude: Claude Sonnet 4.6 Signed-off-by: Len Brown --- tools/power/x86/turbostat/turbostat.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c index d7e38087b023..9b14855fc499 100644 --- a/tools/power/x86/turbostat/turbostat.c +++ b/tools/power/x86/turbostat/turbostat.c @@ -7971,7 +7971,7 @@ static int print_rapl_sysfs(void) snprintf(path, PATH_MAX, "%s/%s", PATH_RAPL_SYSFS, entry->d_name); if ((cdir = opendir(path)) == NULL) { - perror("opendir() error"); + warn("%s", path); return 1; } @@ -10755,10 +10755,8 @@ int add_counter(unsigned int msr_num, const char *path, const char *name, struct sysfs_path *sp; sp = calloc(1, sizeof(struct sysfs_path)); - if (sp == NULL) { - perror("calloc"); - exit(1); - } + if (sp == NULL) + err(-1, "%s", path); strncpy(sp->path, path, PATH_BYTES - 1); sp->id = id; sp->next = msrp->sp; @@ -11700,7 +11698,7 @@ int main(int argc, char **argv) ret = write(fd, "0\n", 2); if (ret == -1) - perror("Can't update cgroup\n"); + warn("/sys/fs/cgroup/cgroup.procs"); close(fd); -- cgit v1.2.3 From b66204a067e2117d5736bd3ea01c0a673235ba39 Mon Sep 17 00:00:00 2001 From: Len Brown Date: Mon, 4 May 2026 15:10:44 -0400 Subject: tools/power turbostat: Cleanup: get_msr() return value get_msr() exits on failure. Delete its return value. Assisted-by: Claude: Claude Sonnet 4.6 Signed-off-by: Len Brown --- tools/power/x86/turbostat/turbostat.c | 245 ++++++++++++---------------------- 1 file changed, 84 insertions(+), 161 deletions(-) diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c index 9b14855fc499..80436e8cb674 100644 --- a/tools/power/x86/turbostat/turbostat.c +++ b/tools/power/x86/turbostat/turbostat.c @@ -589,7 +589,7 @@ struct gfx_sysfs_info { static struct gfx_sysfs_info gfx_info[GFX_MAX]; -int get_msr(int cpu, off_t offset, unsigned long long *msr); +void get_msr(int cpu, off_t offset, unsigned long long *msr); int add_counter(unsigned int, const char *, const char *, unsigned int, enum counter_scope, enum counter_type, enum counter_format, int, int); /* Model specific support Start */ @@ -647,8 +647,8 @@ double slm_bclk(void) unsigned int i; double freq; - if (get_msr(master_cpu, MSR_FSB_FREQ, &msr)) - fprintf(outf, "SLM BCLK: unknown\n"); + get_msr(master_cpu, MSR_FSB_FREQ, &msr); + fprintf(outf, "SLM BCLK: unknown\n"); i = msr & 0xf; if (i >= SLM_BCLK_FREQS) { @@ -2646,18 +2646,12 @@ int get_instr_count_fd(int cpu) return fd_instr_count_percpu[cpu]; } -int get_msr(int cpu, off_t offset, unsigned long long *msr) +void get_msr(int cpu, off_t offset, unsigned long long *msr) { - ssize_t retval; - assert(!no_msr); - retval = pread(get_msr_fd(cpu), msr, sizeof(*msr), offset); - - if (retval != sizeof *msr) + if (pread(get_msr_fd(cpu), msr, sizeof(*msr), offset) != sizeof *msr) err(-1, "cpu%d: msr offset 0x%llx read failed", cpu, (unsigned long long)offset); - - return 0; } int add_msr_counter(int cpu, off_t offset) @@ -4551,8 +4545,7 @@ int get_mp(int cpu, struct msr_counter *mp, unsigned long long *counterp, char * { if (mp->msr_num != 0) { assert(!no_msr); - if (get_msr(cpu, mp->msr_num, counterp)) - return -1; + get_msr(cpu, mp->msr_num, counterp); } else { char path[128 + PATH_BYTES]; @@ -4916,13 +4909,10 @@ int get_rapl_counters(int cpu, unsigned int domain, struct core_data *c, struct fprintf(stderr, "Reading rapl counter via msr at %u\n", i); assert(!no_msr); - if (rci->flags[i] & RAPL_COUNTER_FLAG_USE_MSR_SUM) { - if (get_msr_sum(cpu, rci->msr[i], &rci->data[i])) - return -13 - i; - } else { - if (get_msr(cpu, rci->msr[i], &rci->data[i])) - return -13 - i; - } + if (rci->flags[i] & RAPL_COUNTER_FLAG_USE_MSR_SUM) + get_msr_sum(cpu, rci->msr[i], &rci->data[i]); + else + get_msr(cpu, rci->msr[i], &rci->data[i]); rci->data[i] &= rci->msr_mask[i]; if (rci->msr_shift[i] >= 0) @@ -5051,8 +5041,7 @@ int get_cstate_counters(unsigned int cpu, PER_THREAD_PARAMS) case COUNTER_SOURCE_MSR: assert(!no_msr); - if (get_msr(cpu, cci->msr[i], &cci->data[i])) - return -13 - i; + get_msr(cpu, cci->msr[i], &cci->data[i]); if (debug >= 2) fprintf(stderr, "cstate via %s0x%llx %u: %llu\n", "msr", cci->msr[i], i, cci->data[i]); @@ -5150,8 +5139,7 @@ int get_smi_aperf_mperf(unsigned int cpu, struct thread_data *t) case COUNTER_SOURCE_MSR: assert(!no_msr); - if (get_msr(cpu, mci->msr[i], &mci->data[i])) - return -2 - i; + get_msr(cpu, mci->msr[i], &mci->data[i]); mci->data[i] &= mci->msr_mask[i]; @@ -5316,12 +5304,10 @@ int get_counters(PER_THREAD_PARAMS) } if (DO_BIC(BIC_Mod_c6)) - if (get_msr(cpu, MSR_MODULE_C6_RES_MS, &c->mc6_us)) - return -8; + get_msr(cpu, MSR_MODULE_C6_RES_MS, &c->mc6_us); if (DO_BIC(BIC_CoreTmp)) { - if (get_msr(cpu, MSR_IA32_THERM_STATUS, &msr)) - return -9; + get_msr(cpu, MSR_IA32_THERM_STATUS, &msr); c->core_temp_c = tj_max - ((msr >> 16) & 0x7F); } @@ -5343,22 +5329,14 @@ int get_counters(PER_THREAD_PARAMS) if (!is_cpu_first_core_in_package(t, p)) goto done; - if (DO_BIC(BIC_Totl_c0)) { - if (get_msr(cpu, MSR_PKG_WEIGHTED_CORE_C0_RES, &p->pkg_wtd_core_c0)) - return -10; - } - if (DO_BIC(BIC_Any_c0)) { - if (get_msr(cpu, MSR_PKG_ANY_CORE_C0_RES, &p->pkg_any_core_c0)) - return -11; - } - if (DO_BIC(BIC_GFX_c0)) { - if (get_msr(cpu, MSR_PKG_ANY_GFXE_C0_RES, &p->pkg_any_gfxe_c0)) - return -12; - } - if (DO_BIC(BIC_CPUGFX)) { - if (get_msr(cpu, MSR_PKG_BOTH_CORE_GFXE_C0_RES, &p->pkg_both_core_gfxe_c0)) - return -13; - } + if (DO_BIC(BIC_Totl_c0)) + get_msr(cpu, MSR_PKG_WEIGHTED_CORE_C0_RES, &p->pkg_wtd_core_c0); + if (DO_BIC(BIC_Any_c0)) + get_msr(cpu, MSR_PKG_ANY_CORE_C0_RES, &p->pkg_any_core_c0); + if (DO_BIC(BIC_GFX_c0)) + get_msr(cpu, MSR_PKG_ANY_GFXE_C0_RES, &p->pkg_any_gfxe_c0); + if (DO_BIC(BIC_CPUGFX)) + get_msr(cpu, MSR_PKG_BOTH_CORE_GFXE_C0_RES, &p->pkg_both_core_gfxe_c0); if (DO_BIC(BIC_CPU_LPI)) p->cpu_lpi = cpuidle_cur_cpu_lpi_us; @@ -5372,8 +5350,7 @@ int get_counters(PER_THREAD_PARAMS) } if (DO_BIC(BIC_PkgTmp)) { - if (get_msr(cpu, MSR_IA32_PACKAGE_THERM_STATUS, &msr)) - return -17; + get_msr(cpu, MSR_IA32_PACKAGE_THERM_STATUS, &msr); p->pkg_temp_c = tj_max - ((msr >> 16) & 0x7F); } @@ -6705,7 +6682,7 @@ void do_sleep(void) int get_msr_sum(int cpu, off_t offset, unsigned long long *msr) { - int ret, idx; + int idx; unsigned long long msr_cur, msr_last; assert(!no_msr); @@ -6717,9 +6694,7 @@ int get_msr_sum(int cpu, off_t offset, unsigned long long *msr) if (idx < 0) return idx; /* get_msr_sum() = sum + (get_msr() - last) */ - ret = get_msr(cpu, offset, &msr_cur); - if (ret) - return ret; + get_msr(cpu, offset, &msr_cur); msr_last = per_cpu_msr_sum[cpu].entries[idx].last; DELTA_WRAP32(msr_cur, msr_last); *msr = msr_last + per_cpu_msr_sum[cpu].entries[idx].sum; @@ -6732,7 +6707,7 @@ timer_t timerid; /* Timer callback, update the sum of MSRs periodically. */ static int update_msr_sum(PER_THREAD_PARAMS) { - int i, ret; + int i; int cpu = t->cpu_id; UNUSED(c); @@ -6749,11 +6724,7 @@ static int update_msr_sum(PER_THREAD_PARAMS) offset = idx_to_offset(i); if (offset < 0) continue; - ret = get_msr(cpu, offset, &msr_cur); - if (ret) { - fprintf(outf, "Can not update msr(0x%llx)\n", (unsigned long long)offset); - continue; - } + get_msr(cpu, offset, &msr_cur); msr_last = per_cpu_msr_sum[cpu].entries[i].last; per_cpu_msr_sum[cpu].entries[i].last = msr_cur & 0xffffffff; @@ -7563,27 +7534,21 @@ int print_hwp(PER_THREAD_PARAMS) return -1; } - if (get_msr(cpu, MSR_PM_ENABLE, &msr)) - return 0; - + get_msr(cpu, MSR_PM_ENABLE, &msr); fprintf(outf, "cpu%d: MSR_PM_ENABLE: 0x%08llx (%sHWP)\n", cpu, msr, (msr & (1 << 0)) ? "" : "No-"); /* MSR_PM_ENABLE[1] == 1 if HWP is enabled and MSRs visible */ if ((msr & (1 << 0)) == 0) return 0; - if (get_msr(cpu, MSR_HWP_CAPABILITIES, &msr)) - return 0; - + get_msr(cpu, MSR_HWP_CAPABILITIES, &msr); fprintf(outf, "cpu%d: MSR_HWP_CAPABILITIES: 0x%08llx " "(high %d guar %d eff %d low %d)\n", cpu, msr, (unsigned int)HWP_HIGHEST_PERF(msr), (unsigned int)HWP_GUARANTEED_PERF(msr), (unsigned int)HWP_MOSTEFFICIENT_PERF(msr), (unsigned int)HWP_LOWEST_PERF(msr)); - if (get_msr(cpu, MSR_HWP_REQUEST, &msr)) - return 0; - + get_msr(cpu, MSR_HWP_REQUEST, &msr); fprintf(outf, "cpu%d: MSR_HWP_REQUEST: 0x%08llx " "(min %d max %d des %d epp 0x%x window 0x%x pkg 0x%x)\n", cpu, msr, @@ -7593,9 +7558,7 @@ int print_hwp(PER_THREAD_PARAMS) (unsigned int)(((msr) >> 24) & 0xff), (unsigned int)(((msr) >> 32) & 0xff3), (unsigned int)(((msr) >> 42) & 0x1)); if (has_hwp_pkg) { - if (get_msr(cpu, MSR_HWP_REQUEST_PKG, &msr)) - return 0; - + get_msr(cpu, MSR_HWP_REQUEST_PKG, &msr); fprintf(outf, "cpu%d: MSR_HWP_REQUEST_PKG: 0x%08llx " "(min %d max %d des %d epp 0x%x window 0x%x)\n", cpu, msr, @@ -7604,15 +7567,11 @@ int print_hwp(PER_THREAD_PARAMS) (unsigned int)(((msr) >> 16) & 0xff), (unsigned int)(((msr) >> 24) & 0xff), (unsigned int)(((msr) >> 32) & 0xff3)); } if (has_hwp_notify) { - if (get_msr(cpu, MSR_HWP_INTERRUPT, &msr)) - return 0; - + get_msr(cpu, MSR_HWP_INTERRUPT, &msr); fprintf(outf, "cpu%d: MSR_HWP_INTERRUPT: 0x%08llx " "(%s_Guaranteed_Perf_Change, %s_Excursion_Min)\n", cpu, msr, ((msr) & 0x1) ? "EN" : "Dis", ((msr) & 0x2) ? "EN" : "Dis"); } - if (get_msr(cpu, MSR_HWP_STATUS, &msr)) - return 0; - + get_msr(cpu, MSR_HWP_STATUS, &msr); fprintf(outf, "cpu%d: MSR_HWP_STATUS: 0x%08llx " "(%sGuaranteed_Perf_Change, %sExcursion_Min)\n", cpu, msr, ((msr) & 0x1) ? "" : "No-", ((msr) & 0x4) ? "" : "No-"); @@ -7725,9 +7684,10 @@ double get_tdp_intel(void) { unsigned long long msr; - if (valid_rapl_msrs & RAPL_PKG_POWER_INFO) - if (!get_msr(master_cpu, MSR_PKG_POWER_INFO, &msr)) - return ((msr >> 0) & RAPL_POWER_GRANULARITY) * rapl_power_units; + if (valid_rapl_msrs & RAPL_PKG_POWER_INFO) { + get_msr(master_cpu, MSR_PKG_POWER_INFO, &msr); + return ((msr >> 0) & RAPL_POWER_GRANULARITY) * rapl_power_units; + } return get_quirk_tdp(); } @@ -7765,8 +7725,7 @@ void rapl_probe_intel(void) CLR_BIC(BIC_RAM__, &bic_enabled); /* units on package 0, verify later other packages match */ - if (get_msr(master_cpu, MSR_RAPL_POWER_UNIT, &msr)) - return; + get_msr(master_cpu, MSR_RAPL_POWER_UNIT, &msr); rapl_power_units = 1.0 / (1 << (msr & 0xF)); if (platform->has_rapl_divisor) @@ -7813,9 +7772,7 @@ void rapl_probe_amd(void) if (!valid_rapl_msrs || no_msr) return; - if (get_msr(master_cpu, MSR_RAPL_PWR_UNIT, &msr)) - return; - + get_msr(master_cpu, MSR_RAPL_PWR_UNIT, &msr); rapl_time_units = ldexp(1.0, -(msr >> 16 & 0xf)); rapl_energy_units = ldexp(1.0, -(msr >> 8 & 0x1f)); rapl_power_units = ldexp(1.0, -(msr & 0xf)); @@ -8014,21 +7971,17 @@ int print_rapl(PER_THREAD_PARAMS) if (valid_rapl_msrs & RAPL_AMD_F17H) { msr_name = "MSR_RAPL_PWR_UNIT"; - if (get_msr(cpu, MSR_RAPL_PWR_UNIT, &msr)) - return -1; + get_msr(cpu, MSR_RAPL_PWR_UNIT, &msr); } else { msr_name = "MSR_RAPL_POWER_UNIT"; - if (get_msr(cpu, MSR_RAPL_POWER_UNIT, &msr)) - return -1; + get_msr(cpu, MSR_RAPL_POWER_UNIT, &msr); } fprintf(outf, "cpu%d: %s: 0x%08llx (%f Watts, %f Joules, %f sec.)\n", cpu, msr_name, msr, rapl_power_units, rapl_energy_units, rapl_time_units); if (valid_rapl_msrs & RAPL_PKG_POWER_INFO) { - if (get_msr(cpu, MSR_PKG_POWER_INFO, &msr)) - return -5; - + get_msr(cpu, MSR_PKG_POWER_INFO, &msr); fprintf(outf, "cpu%d: MSR_PKG_POWER_INFO: 0x%08llx (%.0f W TDP, RAPL %.0f - %.0f W, %f sec.)\n", cpu, msr, ((msr >> 0) & RAPL_POWER_GRANULARITY) * rapl_power_units, @@ -8038,9 +7991,7 @@ int print_rapl(PER_THREAD_PARAMS) } if (valid_rapl_msrs & RAPL_PKG) { - if (get_msr(cpu, MSR_PKG_POWER_LIMIT, &msr)) - return -9; - + get_msr(cpu, MSR_PKG_POWER_LIMIT, &msr); fprintf(outf, "cpu%d: MSR_PKG_POWER_LIMIT: 0x%08llx (%slocked)\n", cpu, msr, (msr >> 63) & 1 ? "" : "UN"); print_power_limit_msr(cpu, msr, "PKG Limit #1"); @@ -8050,17 +8001,13 @@ int print_rapl(PER_THREAD_PARAMS) ((msr >> 32) & 0x7FFF) * rapl_power_units, (1.0 + (((msr >> 54) & 0x3) / 4.0)) * (1 << ((msr >> 49) & 0x1F)) * rapl_time_units, ((msr >> 48) & 1) ? "EN" : "DIS"); - if (get_msr(cpu, MSR_VR_CURRENT_CONFIG, &msr)) - return -9; - + get_msr(cpu, MSR_VR_CURRENT_CONFIG, &msr); fprintf(outf, "cpu%d: MSR_VR_CURRENT_CONFIG: 0x%08llx\n", cpu, msr); fprintf(outf, "cpu%d: PKG Limit #4: %f Watts (%slocked)\n", cpu, ((msr >> 0) & 0x1FFF) * rapl_power_units, (msr >> 31) & 1 ? "" : "UN"); } if (valid_rapl_msrs & RAPL_DRAM_POWER_INFO) { - if (get_msr(cpu, MSR_DRAM_POWER_INFO, &msr)) - return -6; - + get_msr(cpu, MSR_DRAM_POWER_INFO, &msr); fprintf(outf, "cpu%d: MSR_DRAM_POWER_INFO,: 0x%08llx (%.0f W TDP, RAPL %.0f - %.0f W, %f sec.)\n", cpu, msr, ((msr >> 0) & RAPL_POWER_GRANULARITY) * rapl_power_units, @@ -8068,32 +8015,25 @@ int print_rapl(PER_THREAD_PARAMS) ((msr >> 32) & RAPL_POWER_GRANULARITY) * rapl_power_units, ((msr >> 48) & RAPL_TIME_GRANULARITY) * rapl_time_units); } if (valid_rapl_msrs & RAPL_DRAM) { - if (get_msr(cpu, MSR_DRAM_POWER_LIMIT, &msr)) - return -9; + get_msr(cpu, MSR_DRAM_POWER_LIMIT, &msr); fprintf(outf, "cpu%d: MSR_DRAM_POWER_LIMIT: 0x%08llx (%slocked)\n", cpu, msr, (msr >> 31) & 1 ? "" : "UN"); print_power_limit_msr(cpu, msr, "DRAM Limit"); } if (valid_rapl_msrs & RAPL_CORE_POLICY) { - if (get_msr(cpu, MSR_PP0_POLICY, &msr)) - return -7; - + get_msr(cpu, MSR_PP0_POLICY, &msr); fprintf(outf, "cpu%d: MSR_PP0_POLICY: %lld\n", cpu, msr & 0xF); } if (valid_rapl_msrs & RAPL_CORE_POWER_LIMIT) { - if (get_msr(cpu, MSR_PP0_POWER_LIMIT, &msr)) - return -9; + get_msr(cpu, MSR_PP0_POWER_LIMIT, &msr); fprintf(outf, "cpu%d: MSR_PP0_POWER_LIMIT: 0x%08llx (%slocked)\n", cpu, msr, (msr >> 31) & 1 ? "" : "UN"); print_power_limit_msr(cpu, msr, "Cores Limit"); } if (valid_rapl_msrs & RAPL_GFX) { - if (get_msr(cpu, MSR_PP1_POLICY, &msr)) - return -8; - + get_msr(cpu, MSR_PP1_POLICY, &msr); fprintf(outf, "cpu%d: MSR_PP1_POLICY: %lld\n", cpu, msr & 0xF); - if (get_msr(cpu, MSR_PP1_POWER_LIMIT, &msr)) - return -9; + get_msr(cpu, MSR_PP1_POWER_LIMIT, &msr); fprintf(outf, "cpu%d: MSR_PP1_POWER_LIMIT: 0x%08llx (%slocked)\n", cpu, msr, (msr >> 31) & 1 ? "" : "UN"); print_power_limit_msr(cpu, msr, "GFX Limit"); } @@ -8108,7 +8048,6 @@ int print_rapl(PER_THREAD_PARAMS) */ void probe_rapl_msrs(void) { - int ret; off_t offset; unsigned long long msr_value; @@ -8122,12 +8061,7 @@ void probe_rapl_msrs(void) if (offset < 0) return; - ret = get_msr(master_cpu, offset, &msr_value); - if (ret) { - if (debug) - fprintf(outf, "Can not read RAPL_PKG_ENERGY MSR(0x%llx)\n", (unsigned long long)offset); - return; - } + get_msr(master_cpu, offset, &msr_value); if (msr_value == 0) { if (debug) fprintf(outf, "RAPL_PKG_ENERGY MSR(0x%llx) == ZERO: disabling all RAPL MSRs\n", (unsigned long long)offset); @@ -8207,17 +8141,17 @@ int set_temperature_target(PER_THREAD_PARAMS) if (!platform->has_nhm_msrs || no_msr) goto guess; - if (get_msr(master_cpu, MSR_IA32_TEMPERATURE_TARGET, &msr)) - goto guess; - + get_msr(master_cpu, MSR_IA32_TEMPERATURE_TARGET, &msr); tcc_default = (msr >> 16) & 0xFF; if (!quiet) { int bits = platform->tcc_offset_bits; unsigned long long enabled = 0; - if (bits && !get_msr(master_cpu, MSR_PLATFORM_INFO, &enabled)) + if (bits) { + get_msr(master_cpu, MSR_PLATFORM_INFO, &enabled); enabled = (enabled >> 30) & 1; + } if (bits && enabled) { tcc_offset = (msr >> 24) & GENMASK(bits - 1, 0); @@ -8269,15 +8203,11 @@ int print_thermal(PER_THREAD_PARAMS) } if (do_ptm && is_cpu_first_core_in_package(t, p)) { - if (get_msr(cpu, MSR_IA32_PACKAGE_THERM_STATUS, &msr)) - return 0; - + get_msr(cpu, MSR_IA32_PACKAGE_THERM_STATUS, &msr); dts = (msr >> 16) & 0x7F; fprintf(outf, "cpu%d: MSR_IA32_PACKAGE_THERM_STATUS: 0x%08llx (%d C)\n", cpu, msr, tj_max - dts); - if (get_msr(cpu, MSR_IA32_PACKAGE_THERM_INTERRUPT, &msr)) - return 0; - + get_msr(cpu, MSR_IA32_PACKAGE_THERM_INTERRUPT, &msr); dts = (msr >> 16) & 0x7F; dts2 = (msr >> 8) & 0x7F; fprintf(outf, "cpu%d: MSR_IA32_PACKAGE_THERM_INTERRUPT: 0x%08llx (%d C, %d C)\n", cpu, msr, tj_max - dts, tj_max - dts2); @@ -8286,16 +8216,12 @@ int print_thermal(PER_THREAD_PARAMS) if (do_dts && debug) { unsigned int resolution; - if (get_msr(cpu, MSR_IA32_THERM_STATUS, &msr)) - return 0; - + get_msr(cpu, MSR_IA32_THERM_STATUS, &msr); dts = (msr >> 16) & 0x7F; resolution = (msr >> 27) & 0xF; fprintf(outf, "cpu%d: MSR_IA32_THERM_STATUS: 0x%08llx (%d C +/- %d)\n", cpu, msr, tj_max - dts, resolution); - if (get_msr(cpu, MSR_IA32_THERM_INTERRUPT, &msr)) - return 0; - + get_msr(cpu, MSR_IA32_THERM_INTERRUPT, &msr); dts = (msr >> 16) & 0x7F; dts2 = (msr >> 8) & 0x7F; fprintf(outf, "cpu%d: MSR_IA32_THERM_INTERRUPT: 0x%08llx (%d C, %d C)\n", cpu, msr, tj_max - dts, tj_max - dts2); @@ -8354,9 +8280,9 @@ void decode_feature_control_msr(void) if (quiet) return; - if (!get_msr(master_cpu, MSR_IA32_FEAT_CTL, &msr)) - fprintf(outf, "cpu%d: MSR_IA32_FEATURE_CONTROL: 0x%08llx (%sLocked %s)\n", - master_cpu, msr, msr & FEAT_CTL_LOCKED ? "" : "UN-", msr & (1 << 18) ? "SGX" : ""); + get_msr(master_cpu, MSR_IA32_FEAT_CTL, &msr); + fprintf(outf, "cpu%d: MSR_IA32_FEATURE_CONTROL: 0x%08llx (%sLocked %s)\n", + master_cpu, msr, msr & FEAT_CTL_LOCKED ? "" : "UN-", msr & (1 << 18) ? "SGX" : ""); } void decode_misc_enable_msr(void) @@ -8369,13 +8295,13 @@ void decode_misc_enable_msr(void) if (!genuine_intel) return; - if (!get_msr(master_cpu, MSR_IA32_MISC_ENABLE, &msr)) - fprintf(outf, "cpu%d: MSR_IA32_MISC_ENABLE: 0x%08llx (%sTCC %sEIST %sMWAIT %sPREFETCH %sTURBO)\n", - master_cpu, msr, - msr & MSR_IA32_MISC_ENABLE_TM1 ? "" : "No-", - msr & MSR_IA32_MISC_ENABLE_ENHANCED_SPEEDSTEP ? "" : "No-", - msr & MSR_IA32_MISC_ENABLE_MWAIT ? "" : "No-", - msr & MSR_IA32_MISC_ENABLE_PREFETCH_DISABLE ? "No-" : "", msr & MSR_IA32_MISC_ENABLE_TURBO_DISABLE ? "No-" : ""); + get_msr(master_cpu, MSR_IA32_MISC_ENABLE, &msr); + fprintf(outf, "cpu%d: MSR_IA32_MISC_ENABLE: 0x%08llx (%sTCC %sEIST %sMWAIT %sPREFETCH %sTURBO)\n", + master_cpu, msr, + msr & MSR_IA32_MISC_ENABLE_TM1 ? "" : "No-", + msr & MSR_IA32_MISC_ENABLE_ENHANCED_SPEEDSTEP ? "" : "No-", + msr & MSR_IA32_MISC_ENABLE_MWAIT ? "" : "No-", + msr & MSR_IA32_MISC_ENABLE_PREFETCH_DISABLE ? "No-" : "", msr & MSR_IA32_MISC_ENABLE_TURBO_DISABLE ? "No-" : ""); } void decode_misc_feature_control(void) @@ -8388,10 +8314,10 @@ void decode_misc_feature_control(void) if (!platform->has_msr_misc_feature_control) return; - if (!get_msr(master_cpu, MSR_MISC_FEATURE_CONTROL, &msr)) - fprintf(outf, - "cpu%d: MSR_MISC_FEATURE_CONTROL: 0x%08llx (%sL2-Prefetch %sL2-Prefetch-pair %sL1-Prefetch %sL1-IP-Prefetch)\n", - master_cpu, msr, msr & (0 << 0) ? "No-" : "", msr & (1 << 0) ? "No-" : "", msr & (2 << 0) ? "No-" : "", msr & (3 << 0) ? "No-" : ""); + get_msr(master_cpu, MSR_MISC_FEATURE_CONTROL, &msr); + fprintf(outf, + "cpu%d: MSR_MISC_FEATURE_CONTROL: 0x%08llx (%sL2-Prefetch %sL2-Prefetch-pair %sL1-Prefetch %sL1-IP-Prefetch)\n", + master_cpu, msr, msr & (0 << 0) ? "No-" : "", msr & (1 << 0) ? "No-" : "", msr & (2 << 0) ? "No-" : "", msr & (3 << 0) ? "No-" : ""); } /* @@ -8411,9 +8337,9 @@ void decode_misc_pwr_mgmt_msr(void) if (!platform->has_msr_misc_pwr_mgmt) return; - if (!get_msr(master_cpu, MSR_MISC_PWR_MGMT, &msr)) - fprintf(outf, "cpu%d: MSR_MISC_PWR_MGMT: 0x%08llx (%sable-EIST_Coordination %sable-EPB %sable-OOB)\n", - master_cpu, msr, msr & (1 << 0) ? "DIS" : "EN", msr & (1 << 1) ? "EN" : "DIS", msr & (1 << 8) ? "EN" : "DIS"); + get_msr(master_cpu, MSR_MISC_PWR_MGMT, &msr); + fprintf(outf, "cpu%d: MSR_MISC_PWR_MGMT: 0x%08llx (%sable-EIST_Coordination %sable-EPB %sable-OOB)\n", + master_cpu, msr, msr & (1 << 0) ? "DIS" : "EN", msr & (1 << 1) ? "EN" : "DIS", msr & (1 << 8) ? "EN" : "DIS"); } /* @@ -8432,11 +8358,11 @@ void decode_c6_demotion_policy_msr(void) if (!platform->has_msr_c6_demotion_policy_config) return; - if (!get_msr(master_cpu, MSR_CC6_DEMOTION_POLICY_CONFIG, &msr)) - fprintf(outf, "cpu%d: MSR_CC6_DEMOTION_POLICY_CONFIG: 0x%08llx (%sable-CC6-Demotion)\n", master_cpu, msr, msr & (1 << 0) ? "EN" : "DIS"); + get_msr(master_cpu, MSR_CC6_DEMOTION_POLICY_CONFIG, &msr); + fprintf(outf, "cpu%d: MSR_CC6_DEMOTION_POLICY_CONFIG: 0x%08llx (%sable-CC6-Demotion)\n", master_cpu, msr, msr & (1 << 0) ? "EN" : "DIS"); - if (!get_msr(master_cpu, MSR_MC6_DEMOTION_POLICY_CONFIG, &msr)) - fprintf(outf, "cpu%d: MSR_MC6_DEMOTION_POLICY_CONFIG: 0x%08llx (%sable-MC6-Demotion)\n", master_cpu, msr, msr & (1 << 0) ? "EN" : "DIS"); + get_msr(master_cpu, MSR_MC6_DEMOTION_POLICY_CONFIG, &msr); + fprintf(outf, "cpu%d: MSR_MC6_DEMOTION_POLICY_CONFIG: 0x%08llx (%sable-MC6-Demotion)\n", master_cpu, msr, msr & (1 << 0) ? "EN" : "DIS"); } void print_dev_latency(void) @@ -9131,13 +9057,10 @@ void process_cpuid() cpuid_has_hv = ecx_flags & (1 << 31); if (!no_msr) { - if (get_msr(sched_getcpu(), MSR_IA32_UCODE_REV, &ucode_patch)) { - warnx("get_msr(UCODE)"); - } else { - ucode_patch_valid = true; - if (!authentic_amd && !hygon_genuine) - ucode_patch >>= 32; - } + get_msr(sched_getcpu(), MSR_IA32_UCODE_REV, &ucode_patch); + ucode_patch_valid = true; + if (!authentic_amd && !hygon_genuine) + ucode_patch >>= 32; } /* -- cgit v1.2.3 From 56c811d5a583269015e72d7a821b93f3f44316df Mon Sep 17 00:00:00 2001 From: Len Brown Date: Mon, 4 May 2026 15:27:12 -0400 Subject: tools/power turbostat: Fix typo: MHz, not Mhz The SI symbol for mega-hertz capitalises both the SI prefix ('M') and the unit symbol ('Hz') Assisted-by: Claude: Claude Sonnet 4.6 Signed-off-by: Len Brown --- tools/power/x86/turbostat/turbostat.8 | 4 ++-- tools/power/x86/turbostat/turbostat.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/power/x86/turbostat/turbostat.8 b/tools/power/x86/turbostat/turbostat.8 index 9b81dca7032f..c905198d0501 100644 --- a/tools/power/x86/turbostat/turbostat.8 +++ b/tools/power/x86/turbostat/turbostat.8 @@ -269,7 +269,7 @@ Periodic output goes to stdout, by default, unless --out is used to specify an o The 5-second interval can be changed with the "-i sec" option. .nf sudo turbostat --quiet --show CPU,frequency - Core CPU Avg_MHz Busy% Bzy_MHz TSC_MHz CPU%c7 UncMhz + Core CPU Avg_MHz Busy% Bzy_MHz TSC_MHz CPU%c7 UncMHz - - 524 12.48 4198 3096 74.53 3800 0 0 4 0.09 4081 3096 98.88 3800 0 4 1 0.02 4063 3096 @@ -279,7 +279,7 @@ sudo turbostat --quiet --show CPU,frequency 2 6 3 0.08 4159 3096 3 3 1 0.04 4046 3096 99.66 3 7 0 0.01 3989 3096 - Core CPU Avg_MHz Busy% Bzy_MHz TSC_MHz CPU%c7 UncMhz + Core CPU Avg_MHz Busy% Bzy_MHz TSC_MHz CPU%c7 UncMHz - - 525 12.52 4198 3096 74.54 3800 0 0 4 0.10 4051 3096 99.49 3800 0 4 2 0.04 3993 3096 diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c index 80436e8cb674..ee0edabf5ac1 100644 --- a/tools/power/x86/turbostat/turbostat.c +++ b/tools/power/x86/turbostat/turbostat.c @@ -658,7 +658,7 @@ double slm_bclk(void) freq = slm_freq_table[i]; if (!quiet) - fprintf(outf, "SLM BCLK: %.1f Mhz\n", freq); + fprintf(outf, "SLM BCLK: %.1f MHz\n", freq); return freq; } -- cgit v1.2.3 From f88e0542da4268bf6f7a5fa250aa6d61b2dce128 Mon Sep 17 00:00:00 2001 From: Len Brown Date: Mon, 4 May 2026 15:32:12 -0400 Subject: tools/power turbostat: Cleanup: bool force_load It is confusing to increment what is effectively a boolean value. Define force_load as boolean, and access it accordingly. Assisted-by: Claude: Claude Sonnet 4.6 Signed-off-by: Len Brown --- tools/power/x86/turbostat/turbostat.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c index ee0edabf5ac1..6819b70e0d56 100644 --- a/tools/power/x86/turbostat/turbostat.c +++ b/tools/power/x86/turbostat/turbostat.c @@ -523,7 +523,7 @@ unsigned int valid_rapl_msrs; unsigned int summary_only; unsigned int list_header_only; unsigned int dump_only; -unsigned int force_load; +bool force_load; unsigned int cpuid_has_aperf_mperf; unsigned int cpuid_has_hv; unsigned int has_aperf_access; @@ -1360,7 +1360,7 @@ void probe_platform_features(unsigned int family, unsigned int model) if (authentic_amd || hygon_genuine) { /* fallback to default features on unsupported models */ - force_load++; + force_load = true; if (max_extended_level >= 0x80000007) { unsigned int eax, ebx, ecx, edx; @@ -1383,7 +1383,7 @@ void probe_platform_features(unsigned int family, unsigned int model) } end: - if (force_load && !platform) { + if ((force_load == true) && !platform) { fprintf(outf, "Forced to run on unsupported platform!\n"); platform = &default_features; } @@ -11496,7 +11496,7 @@ void cmdline(int argc, char **argv) bic_lookup(&bic_enabled, optarg, SHOW_LIST); break; case 'f': - force_load++; + force_load = true; break; case 'd': debug++; -- cgit v1.2.3 From 1c996a37fd244d19e5dbb715328c1676e28ef607 Mon Sep 17 00:00:00 2001 From: Len Brown Date: Mon, 4 May 2026 15:38:34 -0400 Subject: tools/power turbostat: pmt: Improve sscanf() hygiene Explicitly check sscanf() return values for number of matches, and handle EOF (-1). Defensive programming -- not an issue seen by users. Fixes: 4265a86582ea ("tools/power turbostat: Add PMT directory iterator helper") Assisted-by: Claude: Claude Sonnet 4.6 Signed-off-by: Len Brown --- tools/power/x86/turbostat/turbostat.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c index 6819b70e0d56..4ad7cb1df5c5 100644 --- a/tools/power/x86/turbostat/turbostat.c +++ b/tools/power/x86/turbostat/turbostat.c @@ -2011,15 +2011,17 @@ int pmt_telemdir_filter(const struct dirent *e) { unsigned int dummy; - return sscanf(e->d_name, "telem%u", &dummy); + return (sscanf(e->d_name, "telem%u", &dummy) == 1); } int pmt_telemdir_sort(const struct dirent **a, const struct dirent **b) { unsigned int aidx = 0, bidx = 0; - sscanf((*a)->d_name, "telem%u", &aidx); - sscanf((*b)->d_name, "telem%u", &bidx); + if (sscanf((*a)->d_name, "telem%u", &aidx) != 1) + aidx = 0; + if (sscanf((*b)->d_name, "telem%u", &bidx) != 1) + bidx = 0; return (aidx > bidx) ? 1 : (aidx < bidx) ? -1 : 0; } -- cgit v1.2.3