Subject: [PATCH 1/2] tools/power turbostat: Allow -e for all names.

Currently, there are a number of variables which are displayed by
default, enabled with -e all, and listed by --list, but which you can
not give to --enable/-e.

So you can enable CPU0c1 (in the bic array), but you can't enable C1 or
C1% (not in the bic array, but exists in sysfs).

This runs counter to both the documentation and user expectations, and
it's just not very user friendly.

As such, the mechanism used by --hide has been duplicated, and is now
also used by --enable, so we can handle unknown names gracefully.

Note: One impact of this is that truly unknown fields given to --enable
will no longer generate errors, they will be silently ignored, as --hide
does.

Signed-off-by: Zephaniah E. Loss-Cutler-Hull <[email protected]>
---
tools/power/x86/turbostat/turbostat.c | 49 +++++++++++++++++++--------
1 file changed, 35 insertions(+), 14 deletions(-)

diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index 47d3ba895d6d..f5d634ee5fee 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -686,7 +686,9 @@ unsigned long long bic_present = BIC_USEC | BIC_TOD | BIC_sysfs | BIC_APIC | BIC
#define BIC_IS_ENABLED(COUNTER_BIT) (bic_enabled & COUNTER_BIT)

#define MAX_DEFERRED 16
+char *deferred_add_names[MAX_DEFERRED];
char *deferred_skip_names[MAX_DEFERRED];
+int deferred_add_index;
int deferred_skip_index;

/*
@@ -775,17 +777,23 @@ unsigned long long bic_lookup(char *name_list, enum show_hide_mode mode)
}
if (i == MAX_BIC) {
if (mode == SHOW_LIST) {
- fprintf(stderr, "Invalid counter name: %s\n", name_list);
- exit(-1);
- }
- deferred_skip_names[deferred_skip_index++] = name_list;
- if (debug)
- fprintf(stderr, "deferred \"%s\"\n", name_list);
- if (deferred_skip_index >= MAX_DEFERRED) {
- fprintf(stderr, "More than max %d un-recognized --skip options '%s'\n",
- MAX_DEFERRED, name_list);
- help();
- exit(1);
+ deferred_add_names[deferred_add_index++] = name_list;
+ if (deferred_add_index >= MAX_DEFERRED) {
+ fprintf(stderr, "More than max %d un-recognized --add options '%s'\n",
+ MAX_DEFERRED, name_list);
+ help();
+ exit(1);
+ }
+ } else {
+ deferred_skip_names[deferred_skip_index++] = name_list;
+ if (debug)
+ fprintf(stderr, "deferred \"%s\"\n", name_list);
+ if (deferred_skip_index >= MAX_DEFERRED) {
+ fprintf(stderr, "More than max %d un-recognized --skip options '%s'\n",
+ MAX_DEFERRED, name_list);
+ help();
+ exit(1);
+ }
}
}

@@ -6138,6 +6146,16 @@ void parse_add_command(char *add_command)
}
}

+int is_deferred_add(char *name)
+{
+ int i;
+
+ for (i = 0; i < deferred_add_index; ++i)
+ if (!strcmp(name, deferred_add_names[i]))
+ return 1;
+ return 0;
+}
+
int is_deferred_skip(char *name)
{
int i;
@@ -6156,9 +6174,6 @@ void probe_sysfs(void)
int state;
char *sp;

- if (!DO_BIC(BIC_sysfs))
- return;
-
for (state = 10; state >= 0; --state) {

sprintf(path, "/sys/devices/system/cpu/cpu%d/cpuidle/state%d/name", base_cpu, state);
@@ -6181,6 +6196,9 @@ void probe_sysfs(void)

sprintf(path, "cpuidle/state%d/time", state);

+ if (!DO_BIC(BIC_sysfs) && !is_deferred_add(name_buf))
+ continue;
+
if (is_deferred_skip(name_buf))
continue;

@@ -6206,6 +6224,9 @@ void probe_sysfs(void)

sprintf(path, "cpuidle/state%d/usage", state);

+ if (!DO_BIC(BIC_sysfs) && !is_deferred_add(name_buf))
+ continue;
+
if (is_deferred_skip(name_buf))
continue;

--
2.33.0


Subject: [PATCH 2/2] tools/power turbostat: Allow printing header every N iterations

This gives the ability to reprint the header every N iterations, so you
can ensure that a scrolling display always has the header visible
somewhere on the screen.

Signed-off-by: Zephaniah E. Loss-Cutler-Hull <[email protected]>
---
tools/power/x86/turbostat/turbostat.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index f5d634ee5fee..b3f5a21a845a 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -48,6 +48,7 @@ struct timespec interval_ts = { 5, 0 };
unsigned int model_orig;

unsigned int num_iterations;
+unsigned int header_iterations;
unsigned int debug;
unsigned int quiet;
unsigned int shown;
@@ -722,6 +723,8 @@ void help(void)
" -l, --list list column headers only\n"
" -n, --num_iterations num\n"
" number of the measurement iterations\n"
+ " -N, --header_iterations num\n"
+ " print header every num iterations\n"
" -o, --out file\n"
" create or truncate \"file\" for all output\n"
" -q, --quiet skip decoding system configuration header\n"
@@ -1394,14 +1397,14 @@ void flush_output_stderr(void)

void format_all_counters(struct thread_data *t, struct core_data *c, struct pkg_data *p)
{
- static int printed;
+ static int count;

- if (!printed || !summary_only)
+ if ((!count || (header_iterations && !(count % header_iterations))) || !summary_only)
print_header("\t");

format_counters(&average.threads, &average.cores, &average.packages);

- printed = 1;
+ count++;

if (summary_only)
return;
@@ -6334,6 +6337,7 @@ void cmdline(int argc, char **argv)
{ "interval", required_argument, 0, 'i' },
{ "IPC", no_argument, 0, 'I' },
{ "num_iterations", required_argument, 0, 'n' },
+ { "header_iterations", required_argument, 0, 'N' },
{ "help", no_argument, 0, 'h' },
{ "hide", required_argument, 0, 'H' }, // meh, -h taken by --help
{ "Joules", no_argument, 0, 'J' },
@@ -6415,6 +6419,15 @@ void cmdline(int argc, char **argv)
exit(2);
}
break;
+ case 'N':
+ header_iterations = strtod(optarg, NULL);
+
+ if (header_iterations <= 0) {
+ fprintf(outf, "iterations %d should be positive number\n",
+ header_iterations);
+ exit(2);
+ }
+ break;
case 's':
/*
* --show: show only those specified
--
2.33.0

2021-10-05 20:29:32

by Doug Smythies

[permalink] [raw]
Subject: Re: [PATCH 1/2] tools/power turbostat: Allow -e for all names.

On Mon, Oct 4, 2021 at 9:54 PM Zephaniah E. Loss-Cutler-Hull
<[email protected]> wrote:
>
> Currently, there are a number of variables which are displayed by
> default, enabled with -e all, and listed by --list, but which you can
> not give to --enable/-e.
>
> So you can enable CPU0c1 (in the bic array), but you can't enable C1 or
> C1% (not in the bic array, but exists in sysfs).
>
> This runs counter to both the documentation and user expectations, and
> it's just not very user friendly.
>
> As such, the mechanism used by --hide has been duplicated, and is now
> also used by --enable, so we can handle unknown names gracefully.
>
> Note: One impact of this is that truly unknown fields given to --enable
> will no longer generate errors, they will be silently ignored, as --hide
> does.
>
> Signed-off-by: Zephaniah E. Loss-Cutler-Hull <[email protected]>

This is an incredibly useful patch. Thank you.
Tested-by and Reviewed-by: Doug Smythies <[email protected]>

> ---
> tools/power/x86/turbostat/turbostat.c | 49 +++++++++++++++++++--------
> 1 file changed, 35 insertions(+), 14 deletions(-)
>
> diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
> index 47d3ba895d6d..f5d634ee5fee 100644
> --- a/tools/power/x86/turbostat/turbostat.c
> +++ b/tools/power/x86/turbostat/turbostat.c
> @@ -686,7 +686,9 @@ unsigned long long bic_present = BIC_USEC | BIC_TOD | BIC_sysfs | BIC_APIC | BIC
> #define BIC_IS_ENABLED(COUNTER_BIT) (bic_enabled & COUNTER_BIT)
>
> #define MAX_DEFERRED 16
> +char *deferred_add_names[MAX_DEFERRED];
> char *deferred_skip_names[MAX_DEFERRED];
> +int deferred_add_index;
> int deferred_skip_index;
>
> /*
> @@ -775,17 +777,23 @@ unsigned long long bic_lookup(char *name_list, enum show_hide_mode mode)
> }
> if (i == MAX_BIC) {
> if (mode == SHOW_LIST) {
> - fprintf(stderr, "Invalid counter name: %s\n", name_list);
> - exit(-1);
> - }
> - deferred_skip_names[deferred_skip_index++] = name_list;
> - if (debug)
> - fprintf(stderr, "deferred \"%s\"\n", name_list);
> - if (deferred_skip_index >= MAX_DEFERRED) {
> - fprintf(stderr, "More than max %d un-recognized --skip options '%s'\n",
> - MAX_DEFERRED, name_list);
> - help();
> - exit(1);
> + deferred_add_names[deferred_add_index++] = name_list;
> + if (deferred_add_index >= MAX_DEFERRED) {
> + fprintf(stderr, "More than max %d un-recognized --add options '%s'\n",
> + MAX_DEFERRED, name_list);
> + help();
> + exit(1);
> + }
> + } else {
> + deferred_skip_names[deferred_skip_index++] = name_list;
> + if (debug)
> + fprintf(stderr, "deferred \"%s\"\n", name_list);
> + if (deferred_skip_index >= MAX_DEFERRED) {
> + fprintf(stderr, "More than max %d un-recognized --skip options '%s'\n",
> + MAX_DEFERRED, name_list);
> + help();
> + exit(1);
> + }
> }
> }
>
> @@ -6138,6 +6146,16 @@ void parse_add_command(char *add_command)
> }
> }
>
> +int is_deferred_add(char *name)
> +{
> + int i;
> +
> + for (i = 0; i < deferred_add_index; ++i)
> + if (!strcmp(name, deferred_add_names[i]))
> + return 1;
> + return 0;
> +}
> +
> int is_deferred_skip(char *name)
> {
> int i;
> @@ -6156,9 +6174,6 @@ void probe_sysfs(void)
> int state;
> char *sp;
>
> - if (!DO_BIC(BIC_sysfs))
> - return;
> -
> for (state = 10; state >= 0; --state) {
>
> sprintf(path, "/sys/devices/system/cpu/cpu%d/cpuidle/state%d/name", base_cpu, state);
> @@ -6181,6 +6196,9 @@ void probe_sysfs(void)
>
> sprintf(path, "cpuidle/state%d/time", state);
>
> + if (!DO_BIC(BIC_sysfs) && !is_deferred_add(name_buf))
> + continue;
> +
> if (is_deferred_skip(name_buf))
> continue;
>
> @@ -6206,6 +6224,9 @@ void probe_sysfs(void)
>
> sprintf(path, "cpuidle/state%d/usage", state);
>
> + if (!DO_BIC(BIC_sysfs) && !is_deferred_add(name_buf))
> + continue;
> +
> if (is_deferred_skip(name_buf))
> continue;
>
> --
> 2.33.0
>