| Message ID | 20211005045439.1430114-1-zephaniah@gmail.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Len Brown |
| Headers | show |
| Series | [1/2] tools/power turbostat: Allow -e for all names. | expand |
On Mon, Oct 4, 2021 at 9:54 PM Zephaniah E. Loss-Cutler-Hull <zephaniah@gmail.com> 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 <zephaniah@gmail.com> This is an incredibly useful patch. Thank you. Tested-by and Reviewed-by: Doug Smythies <dsmythies@telus.net> > --- > 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 >
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;
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 <zephaniah@gmail.com> --- tools/power/x86/turbostat/turbostat.c | 49 +++++++++++++++++++-------- 1 file changed, 35 insertions(+), 14 deletions(-)