[PATCH 0/6] tools/power turbostat: Fixes and cleanups

[PATCH 0/6] tools/power turbostat: Fixes and cleanups

[Artem Bityutskiy]

From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

This series fixes the following bugs:
- Using incorrect format variable for perf counters.
- --show/--hide not working with individual cpuidle counters
- Missing delimiters causing columns to merge together

There are also consistency improvements and cleanups.

Based on commit
867ea3a786e19 ("tools/power/turbostat: Fix microcode patch level output for AMD/Hygon")
in Len Brown's tree:
https://github.com/lenb/linux.git turbostat

Artem Bityutskiy (5):
  tools/power turbostat: add missing print_float_value()
  tools/power turbostat: fix incorrect format variable
  tools/power turbostat: fix --show/--hide for individual cpuidle
    counters
  tools/power turbostat: fix delimiter bug in print functions
  tools/power turbostat: cleanup print helper functions

 tools/power/x86/turbostat/turbostat.c | 64 ++++++++++++++++-----------
 1 file changed, 39 insertions(+), 25 deletions(-)

[PATCH 1/5] tools/power turbostat: add missing print_float_value()

[Artem Bityutskiy]

From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

In the core and package cases, all PMT counter prints use
print_float_value(). But the thread case uses sprintf(), which is not
a bug, but inconsistent. Most probably just an oversight.

Address the inconsistency by using print_float_value() in the thread
case too.

Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---
 tools/power/x86/turbostat/turbostat.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index b985bce69142f..9744f9caac9af 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -3489,12 +3489,12 @@ int format_counters(PER_THREAD_PARAMS)
 
 		case PMT_TYPE_XTAL_TIME:
 			value_converted = pct(value_raw / crystal_hz, interval_float);
-			outp += sprintf(outp, "%s%.2f", (printed++ ? delim : ""), value_converted);
+			outp += print_float_value(&printed, delim, value_converted);
 			break;
 
 		case PMT_TYPE_TCORE_CLOCK:
 			value_converted = pct(value_raw / tcore_clock_freq_hz, interval_float);
-			outp += sprintf(outp, "%s%.2f", (printed++ ? delim : ""), value_converted);
+			outp += print_float_value(&printed, delim, value_converted);
 		}
 	}
 
-- 
2.53.0

[PATCH 2/5] tools/power turbostat: fix incorrect format variable

[Artem Bityutskiy]

From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

In the perf thread, core, and package counter loops, an incorrect
'mp->format' variable is used instead of 'pp->format'. This looks
like a copy-paste error from the adjacent MSR counter loops, which
use 'mp'.

I did not see an actual problem related to this, but this does look
like a bug that could result in the wrong format being used.

Fixes: 696d15cbd8c2 ("tools/power turbostat: Refactor floating point printout code")
Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---
 tools/power/x86/turbostat/turbostat.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index 9744f9caac9af..4d954533c71de 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -3468,7 +3468,7 @@ int format_counters(PER_THREAD_PARAMS)
 	for (i = 0, pp = sys.perf_tp; pp; ++i, pp = pp->next) {
 		if (pp->format == FORMAT_RAW)
 			outp += print_hex_value(pp->width, &printed, delim, t->perf_counter[i]);
-		else if (pp->format == FORMAT_DELTA || mp->format == FORMAT_AVERAGE)
+		else if (pp->format == FORMAT_DELTA || pp->format == FORMAT_AVERAGE)
 			outp += print_decimal_value(pp->width, &printed, delim, t->perf_counter[i]);
 		else if (pp->format == FORMAT_PERCENT) {
 			if (pp->type == COUNTER_USEC)
@@ -3538,7 +3538,7 @@ int format_counters(PER_THREAD_PARAMS)
 	for (i = 0, pp = sys.perf_cp; pp; i++, pp = pp->next) {
 		if (pp->format == FORMAT_RAW)
 			outp += print_hex_value(pp->width, &printed, delim, c->perf_counter[i]);
-		else if (pp->format == FORMAT_DELTA || mp->format == FORMAT_AVERAGE)
+		else if (pp->format == FORMAT_DELTA || pp->format == FORMAT_AVERAGE)
 			outp += print_decimal_value(pp->width, &printed, delim, c->perf_counter[i]);
 		else if (pp->format == FORMAT_PERCENT)
 			outp += print_float_value(&printed, delim, pct(c->perf_counter[i], tsc));
@@ -3694,7 +3694,7 @@ int format_counters(PER_THREAD_PARAMS)
 			outp += print_hex_value(pp->width, &printed, delim, p->perf_counter[i]);
 		else if (pp->type == COUNTER_K2M)
 			outp += sprintf(outp, "%s%d", (printed++ ? delim : ""), (unsigned int)p->perf_counter[i] / 1000);
-		else if (pp->format == FORMAT_DELTA || mp->format == FORMAT_AVERAGE)
+		else if (pp->format == FORMAT_DELTA || pp->format == FORMAT_AVERAGE)
 			outp += print_decimal_value(pp->width, &printed, delim, p->perf_counter[i]);
 		else if (pp->format == FORMAT_PERCENT)
 			outp += print_float_value(&printed, delim, pct(p->perf_counter[i], tsc));
-- 
2.53.0

[PATCH 3/5] tools/power turbostat: fix --show/--hide for individual cpuidle counters

[Artem Bityutskiy]

From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

Problem: individual swidle counter names (C1, C1+, C1-, etc.) cannot be
selected via --show/--hide due to two bugs in probe_cpuidle_counts():
1. The function returns immediately when BIC_cpuidle is not enabled,
   without checking deferred_add_index.
2. The deferred name check runs against name_buf before the trailing
   newline is stripped, so is_deferred_add("C1\n") never matches "C1".

Fix:
1. Relax the early return to pass through when deferred names are
   queued.
2. Strip the trailing newline from name_buf before performing deferred
   name checks.
3. Check each suffixed variant (C1+, C1, C1-) individually so that
   e.g. "--show C1+" enables only the requested metric.

In addition, introduce a helper function to avoid repeating the
condition (readability cleanup).

Fixes: ec4acd3166d8 ("tools/power turbostat: disable "cpuidle" invocation counters, by default")
Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---
 tools/power/x86/turbostat/turbostat.c | 35 ++++++++++++++++-----------
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index 4d954533c71de..3487548841e1d 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -11285,6 +11285,14 @@ void probe_cpuidle_residency(void)
 	}
 }
 
+static bool cpuidle_counter_wanted(char *name)
+{
+	if (is_deferred_skip(name))
+		return false;
+
+	return DO_BIC(BIC_cpuidle) || is_deferred_add(name);
+}
+
 void probe_cpuidle_counts(void)
 {
 	char path[64];
@@ -11294,7 +11302,7 @@ void probe_cpuidle_counts(void)
 	int min_state = 1024, max_state = 0;
 	char *sp;
 
-	if (!DO_BIC(BIC_cpuidle))
+	if (!DO_BIC(BIC_cpuidle) && !deferred_add_index)
 		return;
 
 	for (state = 10; state >= 0; --state) {
@@ -11309,12 +11317,6 @@ void probe_cpuidle_counts(void)
 
 		remove_underbar(name_buf);
 
-		if (!DO_BIC(BIC_cpuidle) && !is_deferred_add(name_buf))
-			continue;
-
-		if (is_deferred_skip(name_buf))
-			continue;
-
 		/* truncate "C1-HSW\n" to "C1", or truncate "C1\n" to "C1" */
 		sp = strchr(name_buf, '-');
 		if (!sp)
@@ -11329,16 +11331,19 @@ void probe_cpuidle_counts(void)
 			 * Add 'C1+' for C1, and so on. The 'below' sysfs file always contains 0 for
 			 * the last state, so do not add it.
 			 */
-
 			*sp = '+';
 			*(sp + 1) = '\0';
-			sprintf(path, "cpuidle/state%d/below", state);
-			add_counter(0, path, name_buf, 64, SCOPE_CPU, COUNTER_ITEMS, FORMAT_DELTA, SYSFS_PERCPU, 0);
+			if (cpuidle_counter_wanted(name_buf)) {
+				sprintf(path, "cpuidle/state%d/below", state);
+				add_counter(0, path, name_buf, 64, SCOPE_CPU, COUNTER_ITEMS, FORMAT_DELTA, SYSFS_PERCPU, 0);
+			}
 		}
 
 		*sp = '\0';
-		sprintf(path, "cpuidle/state%d/usage", state);
-		add_counter(0, path, name_buf, 64, SCOPE_CPU, COUNTER_ITEMS, FORMAT_DELTA, SYSFS_PERCPU, 0);
+		if (cpuidle_counter_wanted(name_buf)) {
+			sprintf(path, "cpuidle/state%d/usage", state);
+			add_counter(0, path, name_buf, 64, SCOPE_CPU, COUNTER_ITEMS, FORMAT_DELTA, SYSFS_PERCPU, 0);
+		}
 
 		/*
 		 * The 'above' sysfs file always contains 0 for the shallowest state (smallest
@@ -11347,8 +11352,10 @@ void probe_cpuidle_counts(void)
 		if (state != min_state) {
 			*sp = '-';
 			*(sp + 1) = '\0';
-			sprintf(path, "cpuidle/state%d/above", state);
-			add_counter(0, path, name_buf, 64, SCOPE_CPU, COUNTER_ITEMS, FORMAT_DELTA, SYSFS_PERCPU, 0);
+			if (cpuidle_counter_wanted(name_buf)) {
+				sprintf(path, "cpuidle/state%d/above", state);
+				add_counter(0, path, name_buf, 64, SCOPE_CPU, COUNTER_ITEMS, FORMAT_DELTA, SYSFS_PERCPU, 0);
+			}
 		}
 	}
 }
-- 
2.53.0

[PATCH 4/5] tools/power turbostat: fix delimiter bug in print functions

[Artem Bityutskiy]

From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

Fix a bug in the following functions:
1. print_name()
2. print_hex_value()
3. print_decimal_value()
4. print_float_value()

The problem is that the 'int *printed' argument, that is intended to be
used as "has any column been printed yet" flag, never changes.

Reason: incorrect assumption about operator precedence in the expression
'(*printed++ ? delim : "")'. This expression increments the pointer,
but not the value it points to.

As a result, a command like 'turbostat --show C1,C1+' produces output
with columns merged together without any delimiter.

Fix this by using (*printed)++ instead.

Fixes: 56dbb878507b ("tools/power turbostat: Refactor added column header printing")
Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---
 tools/power/x86/turbostat/turbostat.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index 3487548841e1d..f5cf12ab84e09 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -2837,29 +2837,29 @@ static inline int print_name(int width, int *printed, char *delim, char *name, e
 	UNUSED(type);
 
 	if (format == FORMAT_RAW && width >= 64)
-		return (sprintf(outp, "%s%-8s", (*printed++ ? delim : ""), name));
+		return (sprintf(outp, "%s%-8s", ((*printed)++ ? delim : ""), name));
 	else
-		return (sprintf(outp, "%s%s", (*printed++ ? delim : ""), name));
+		return (sprintf(outp, "%s%s", ((*printed)++ ? delim : ""), name));
 }
 
 static inline int print_hex_value(int width, int *printed, char *delim, unsigned long long value)
 {
 	if (width <= 32)
-		return (sprintf(outp, "%s%08x", (*printed++ ? delim : ""), (unsigned int)value));
+		return (sprintf(outp, "%s%08x", (*(printed)++ ? delim : ""), (unsigned int)value));
 	else
-		return (sprintf(outp, "%s%016llx", (*printed++ ? delim : ""), value));
+		return (sprintf(outp, "%s%016llx", (*(printed)++ ? delim : ""), value));
 }
 
 static inline int print_decimal_value(int width, int *printed, char *delim, unsigned long long value)
 {
 	UNUSED(width);
 
-	return (sprintf(outp, "%s%lld", (*printed++ ? delim : ""), value));
+	return (sprintf(outp, "%s%lld", (*(printed)++ ? delim : ""), value));
 }
 
 static inline int print_float_value(int *printed, char *delim, double value)
 {
-	return (sprintf(outp, "%s%0.2f", (*printed++ ? delim : ""), value));
+	return (sprintf(outp, "%s%0.2f", (*(printed)++ ? delim : ""), value));
 }
 
 void print_header(char *delim)
-- 
2.53.0

[PATCH 5/5] tools/power turbostat: cleanup print helper functions

[Artem Bityutskiy]

From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

Two cleanups in printing helpers:
1. Add a local 'sep' variable to avoid repeating the ternary
   expression in each branch.
2. Remove the redundant parentheses around sprintf() calls.
3. Remove an unnecessary cast to "unsigned int" by using the '%08llx' instead
   of '%08x'.

No functional changes.

Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---
 tools/power/x86/turbostat/turbostat.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index f5cf12ab84e09..70dbd927b6521 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -2835,31 +2835,38 @@ void bic_lookup(cpu_set_t *ret_set, char *name_list, enum show_hide_mode mode)
 static inline int print_name(int width, int *printed, char *delim, char *name, enum counter_type type, enum counter_format format)
 {
 	UNUSED(type);
+	char *sep = (*printed)++ ? delim : "";
 
 	if (format == FORMAT_RAW && width >= 64)
-		return (sprintf(outp, "%s%-8s", ((*printed)++ ? delim : ""), name));
+		return sprintf(outp, "%s%-8s", sep, name);
 	else
-		return (sprintf(outp, "%s%s", ((*printed)++ ? delim : ""), name));
+		return sprintf(outp, "%s%s", sep, name);
 }
 
 static inline int print_hex_value(int width, int *printed, char *delim, unsigned long long value)
 {
+	char *sep = (*printed)++ ? delim : "";
+
 	if (width <= 32)
-		return (sprintf(outp, "%s%08x", (*(printed)++ ? delim : ""), (unsigned int)value));
+		return (sprintf(outp, "%s%08llx", sep, value));
 	else
-		return (sprintf(outp, "%s%016llx", (*(printed)++ ? delim : ""), value));
+		return (sprintf(outp, "%s%016llx", sep, value));
 }
 
 static inline int print_decimal_value(int width, int *printed, char *delim, unsigned long long value)
 {
+	char *sep = (*printed)++ ? delim : "";
+
 	UNUSED(width);
 
-	return (sprintf(outp, "%s%lld", (*(printed)++ ? delim : ""), value));
+	return (sprintf(outp, "%s%lld", sep, value));
 }
 
 static inline int print_float_value(int *printed, char *delim, double value)
 {
-	return (sprintf(outp, "%s%0.2f", (*(printed)++ ? delim : ""), value));
+	char *sep = (*printed)++ ? delim : "";
+
+	return (sprintf(outp, "%s%0.2f", sep, value));
 }
 
 void print_header(char *delim)
-- 
2.53.0

Re: [PATCH 4/5] tools/power turbostat: fix delimiter bug in print functions

[Len Brown]

typo:

-       return (sprintf(outp, "%s%0.2f", (*printed++ ? delim : ""), value));
+       return (sprintf(outp, "%s%0.2f", (*(printed)++ ? delim : ""), value));

I think you mean (*printed)++

On Wed, Mar 11, 2026 at 5:00 AM Artem Bityutskiy <dedekind1@gmail.com> wrote:
>
> From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
>
> Fix a bug in the following functions:
> 1. print_name()
> 2. print_hex_value()
> 3. print_decimal_value()
> 4. print_float_value()
>
> The problem is that the 'int *printed' argument, that is intended to be
> used as "has any column been printed yet" flag, never changes.
>
> Reason: incorrect assumption about operator precedence in the expression
> '(*printed++ ? delim : "")'. This expression increments the pointer,
> but not the value it points to.
>
> As a result, a command like 'turbostat --show C1,C1+' produces output
> with columns merged together without any delimiter.
>
> Fix this by using (*printed)++ instead.
>
> Fixes: 56dbb878507b ("tools/power turbostat: Refactor added column header printing")
> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> ---
>  tools/power/x86/turbostat/turbostat.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
> index 3487548841e1d..f5cf12ab84e09 100644
> --- a/tools/power/x86/turbostat/turbostat.c
> +++ b/tools/power/x86/turbostat/turbostat.c
> @@ -2837,29 +2837,29 @@ static inline int print_name(int width, int *printed, char *delim, char *name, e
>         UNUSED(type);
>
>         if (format == FORMAT_RAW && width >= 64)
> -               return (sprintf(outp, "%s%-8s", (*printed++ ? delim : ""), name));
> +               return (sprintf(outp, "%s%-8s", ((*printed)++ ? delim : ""), name));
>         else
> -               return (sprintf(outp, "%s%s", (*printed++ ? delim : ""), name));
> +               return (sprintf(outp, "%s%s", ((*printed)++ ? delim : ""), name));
>  }
>
>  static inline int print_hex_value(int width, int *printed, char *delim, unsigned long long value)
>  {
>         if (width <= 32)
> -               return (sprintf(outp, "%s%08x", (*printed++ ? delim : ""), (unsigned int)value));
> +               return (sprintf(outp, "%s%08x", (*(printed)++ ? delim : ""), (unsigned int)value));
>         else
> -               return (sprintf(outp, "%s%016llx", (*printed++ ? delim : ""), value));
> +               return (sprintf(outp, "%s%016llx", (*(printed)++ ? delim : ""), value));
>  }
>
>  static inline int print_decimal_value(int width, int *printed, char *delim, unsigned long long value)
>  {
>         UNUSED(width);
>
> -       return (sprintf(outp, "%s%lld", (*printed++ ? delim : ""), value));
> +       return (sprintf(outp, "%s%lld", (*(printed)++ ? delim : ""), value));
>  }
>
>  static inline int print_float_value(int *printed, char *delim, double value)
>  {
> -       return (sprintf(outp, "%s%0.2f", (*printed++ ? delim : ""), value));
> +       return (sprintf(outp, "%s%0.2f", (*(printed)++ ? delim : ""), value));
>  }
>
>  void print_header(char *delim)
> --
> 2.53.0
>


-- 
Len Brown, Intel Open Source Technology Center

Re: [PATCH 1/5] tools/power turbostat: add missing print_float_value()

[Len Brown]

Applied, thanks!

Len Brown, Intel Open Source Technology Center

Re: [PATCH 2/5] tools/power turbostat: fix incorrect format variable

[Len Brown]

Applied, thanks!

Len Brown, Intel Open Source Technology Center

Re: [PATCH 3/5] tools/power turbostat: fix --show/--hide for individual cpuidle counters

[Len Brown]

Applied, thanks!

Len Brown, Intel Open Source Technology Center