linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] tools/power turbostat: Allow -e for all names.
@ 2021-10-05  4:54 Zephaniah E. Loss-Cutler-Hull
  2021-10-05  4:54 ` [PATCH 2/2] tools/power turbostat: Allow printing header every N iterations Zephaniah E. Loss-Cutler-Hull
  2021-10-05 20:27 ` [PATCH 1/2] tools/power turbostat: Allow -e for all names Doug Smythies
  0 siblings, 2 replies; 3+ messages in thread
From: Zephaniah E. Loss-Cutler-Hull @ 2021-10-05  4:54 UTC (permalink / raw)
  To: linux-pm; +Cc: linux-kernel, Zephaniah E. Loss-Cutler-Hull

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(-)

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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH 2/2] tools/power turbostat: Allow printing header every N iterations
  2021-10-05  4:54 [PATCH 1/2] tools/power turbostat: Allow -e for all names Zephaniah E. Loss-Cutler-Hull
@ 2021-10-05  4:54 ` Zephaniah E. Loss-Cutler-Hull
  2021-10-05 20:27 ` [PATCH 1/2] tools/power turbostat: Allow -e for all names Doug Smythies
  1 sibling, 0 replies; 3+ messages in thread
From: Zephaniah E. Loss-Cutler-Hull @ 2021-10-05  4:54 UTC (permalink / raw)
  To: linux-pm; +Cc: linux-kernel, Zephaniah E. Loss-Cutler-Hull

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 <zephaniah@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/2] tools/power turbostat: Allow -e for all names.
  2021-10-05  4:54 [PATCH 1/2] tools/power turbostat: Allow -e for all names Zephaniah E. Loss-Cutler-Hull
  2021-10-05  4:54 ` [PATCH 2/2] tools/power turbostat: Allow printing header every N iterations Zephaniah E. Loss-Cutler-Hull
@ 2021-10-05 20:27 ` Doug Smythies
  1 sibling, 0 replies; 3+ messages in thread
From: Doug Smythies @ 2021-10-05 20:27 UTC (permalink / raw)
  To: Zephaniah E. Loss-Cutler-Hull
  Cc: Linux PM list, Linux Kernel Mailing List, dsmythies

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
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-10-05 20:27 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-05  4:54 [PATCH 1/2] tools/power turbostat: Allow -e for all names Zephaniah E. Loss-Cutler-Hull
2021-10-05  4:54 ` [PATCH 2/2] tools/power turbostat: Allow printing header every N iterations Zephaniah E. Loss-Cutler-Hull
2021-10-05 20:27 ` [PATCH 1/2] tools/power turbostat: Allow -e for all names Doug Smythies

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).