Skip to contentSkip to conversation

tools/power turbostat: if --max_loop, print for specific time of loops

PATCH Superseded Yu Chen 2 participants 3 messages Apr 11, 2018 Turbostat utility

v3 of 6 Superseded Latest is v6 →

lore
Share & tools
lore

Yu Chen

author
#0 lore raw

PATCH v3 1/1 tools/power turbostat: if --max_loop, print for specific time of loops

From: Chen Yu <[email protected]>

There's a use case during test to only print specific round of loops if --iterations is specified, for example, with this patch applied:

turbostat -i 5 -t 4
will capture 4 samples with 5 seconds interval.

Cc Len Brown <[email protected]>
Cc Rafael J Wysocki <[email protected]>
Cc Artem Bityutskiy <[email protected]>
---
 tools/power/x86/turbostat/turbostat.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)
tools/power/x86/turbostat/turbostat.c +22 -1
index bd9c6b31a504..29419b1535bb 100644
@@ -48,6 +48,7 @@ char *proc_stat = "/proc/stat";
 FILE *outf;
 int *fd_percpu;
 struct timespec interval_ts = {5, 0};
+int iterations;
 unsigned int debug;
 unsigned int quiet;
 unsigned int sums_need_wide_columns;
@@ -470,6 +471,7 @@ void help(void)
 	"		{core | package | j,k,l..m,n-p }\n"
 	"--quiet	skip decoding system configuration header\n"
 	"--interval sec	Override default 5-second measurement interval\n"
+	"--iterations loops	The number of loops if interval is specified\n"
 	"--help		print this help message\n"
 	"--list		list column headers only\n"
 	"--out file	create or truncate \"file\" for all output\n"
@@ -2565,6 +2567,7 @@ void turbostat_loop()
 {
 	int retval;
 	int restarted = 0;
+	int loops = 0;
 
 restart:
 	restarted++;
@@ -2583,6 +2586,7 @@ void turbostat_loop()
 	restarted = 0;
 	gettimeofday(&tv_even, (struct timezone *)NULL);
 
+	loops = 0;
 	while (1) {
 		if (for_all_proc_cpus(cpu_is_not_present)) {
 			re_initialize();
@@ -2607,6 +2611,10 @@ void turbostat_loop()
 		compute_average(EVEN_COUNTERS);
 		format_all_counters(EVEN_COUNTERS);
 		flush_output_stdout();
+
+		if (iterations && (++loops >= iterations))
+			break;
+
 		nanosleep(&interval_ts, NULL);
 		if (snapshot_proc_sysfs_files())
 			goto restart;
@@ -2626,6 +2634,9 @@ void turbostat_loop()
 		compute_average(ODD_COUNTERS);
 		format_all_counters(ODD_COUNTERS);
 		flush_output_stdout();
+
+		if (iterations && (++loops >= iterations))
+			break;
 	}
 }
 
@@ -4999,6 +5010,7 @@ void cmdline(int argc, char **argv)
 		{"Dump",	no_argument,		0, 'D'},
 		{"debug",	no_argument,		0, 'd'},	/* internal, not documented */
 		{"interval",	required_argument,	0, 'i'},
+		{"iterations",	required_argument,	0, 't'},
 		{"help",	no_argument,		0, 'h'},
 		{"hide",	required_argument,	0, 'H'},	// meh, -h taken by --help
 		{"Joules",	no_argument,		0, 'J'},
@@ -5014,7 +5026,7 @@ void cmdline(int argc, char **argv)
 
 	progname = argv[0];
 
-	while ((opt = getopt_long_only(argc, argv, "+C:c:Ddhi:JM:m:o:qST:v",
+	while ((opt = getopt_long_only(argc, argv, "+C:c:Ddhi:JM:m:o:qST:vt:",
 				long_options, &option_index)) != -1) {
 		switch (opt) {
 		case 'a':
@@ -5069,6 +5081,15 @@ void cmdline(int argc, char **argv)
 		case 'S':
 			summary_only++;
 			break;
+		case 't':
+			iterations = strtod(optarg, NULL);
+
+			if (iterations <= 0) {
+				fprintf(outf, "loops %d should be positive number\n",
+					iterations);
+				exit(2);
+			}
+			break;
 		case 'T':
 			tcc_activation_temp_override = atoi(optarg);
 			break;
Signature · 1 line
2.13.6
#1 lore raw

A couple of nitpicks.

tools/power/x86/turbostat/turbostat.c char *proc_stat = "/proc/stat"; · line 51

Quoted 5 lines · Yu Chen
tools/power/x86/turbostat/turbostat.c +1 -0
@@ -48,6 +48,7 @@ char *proc_stat = "/proc/stat";
 FILE *outf;
 int *fd_percpu;
 struct timespec interval_ts = {5, 0};
+int iterations;

OK, out of several choices, you selected "iterations".

tools/power/x86/turbostat/turbostat.c void help(void) · line 474

Quoted 8 lines
tools/power/x86/turbostat/turbostat.c +1 -0
 unsigned int debug;
 unsigned int quiet;
 unsigned int sums_need_wide_columns;
@@ -470,6 +471,7 @@ void help(void)
 	"		{core | package | j,k,l..m,n-p }\n"
 	"--quiet	skip decoding system configuration header\n"
 	"--interval sec	Override default 5-second measurement interval\n"
+	"--iterations loops	The number of loops if interval is specified\n"

Since "iterations" is the term, be consistent and do not mix it with "loops". Who knows may be the "loops" term will be used for something else in the future. Use something like this:

"--iterations count Number of measurement iterations (requires '
--interval')"

tools/power/x86/turbostat/turbostat.c void turbostat_loop() · line 2570

Quoted 8 lines
tools/power/x86/turbostat/turbostat.c +1 -0
 	print this help mkk
 	"--list		list column headers only\n"
 	"--out file	create or truncate \"file\" for all output\n"
@@ -2565,6 +2567,7 @@ void turbostat_loop()
 {
 	int retval;
 	int restarted = 0;
+	int loops = 0;

Please, name variables in a consistent manner, this should really be something like 'int iters = 0'. Or may be 'done_iters', or something. But not "loops".

tools/power/x86/turbostat/turbostat.c void cmdline(int argc, char **argv) · line 5013

Quoted 5 lines
tools/power/x86/turbostat/turbostat.c +1 -0
@@ -4999,6 +5010,7 @@ void cmdline(int argc, char **argv)
 		{"Dump",	no_argument,		0, 'D'},
 		{"debug",	no_argument,		0, 'd'},	/* internal, not documented */
 		{"interval",	required_argument,	0, 'i'},
+		{"iterations",	required_argument,	0, 't'},

If you used term "count", you could have consistent long and short option names, like '--count / -c'. I find '--iterations / -t' to be inconsistent, and harder to remember the short option, because I think about time, not "iterations" when I see -t.

author
#2 lore raw

Hi,

Quoted 24 lines · Artem Bityutskiy

A couple of nitpicks.

tools/power/x86/turbostat/turbostat.c char *proc_stat = "/proc/stat"; · line 51

Quoted 5 lines · Yu Chen
tools/power/x86/turbostat/turbostat.c +1 -0
@@ -48,6 +48,7 @@ char *proc_stat = "/proc/stat";
 FILE *outf;
 int *fd_percpu;
 struct timespec interval_ts = {5, 0};
+int iterations;

OK, out of several choices, you selected "iterations".

tools/power/x86/turbostat/turbostat.c void help(void) · line 474

Quoted 8 lines
tools/power/x86/turbostat/turbostat.c +1 -0
 unsigned int debug;
 unsigned int quiet;
 unsigned int sums_need_wide_columns;
@@ -470,6 +471,7 @@ void help(void)
 	"		{core | package | j,k,l..m,n-p }\n"
 	"--quiet	skip decoding system configuration header\n"
 	"--interval sec	Override default 5-second measurement interval\n"
+	"--iterations loops	The number of loops if interval is specified\n"

Since "iterations" is the term, be consistent and do not mix it with "loops". Who knows may be the "loops" term will be used for something else in the future. Use something like this:

Quoted 3 lines

"--iterations count Number of measurement iterations (requires '
--interval')"

OK, this looks more consistent.

Quoted 4 lines
Quoted 4 lines
tools/power/x86/turbostat/turbostat.c +0 -0
 	print this help mkk
 	"--list		list column headers only\n"
 	"--out file	create or truncate \"file\" for all output\n"
@@ -2565,6 +2567,7 @@ void turbostat_loop()
Quoted 9 lines

tools/power/x86/turbostat/turbostat.c void turbostat_loop() · line 2570

Quoted 4 lines
tools/power/x86/turbostat/turbostat.c +1 -0
 {
 	int retval;
 	int restarted = 0;
+	int loops = 0;

Please, name variables in a consistent manner, this should really be something like 'int iters = 0'. Or may be 'done_iters', or something. But not "loops".

OK.

Quoted 10 lines

tools/power/x86/turbostat/turbostat.c void cmdline(int argc, char **argv) · line 5013

Quoted 5 lines
tools/power/x86/turbostat/turbostat.c +1 -0
@@ -4999,6 +5010,7 @@ void cmdline(int argc, char **argv)
 		{"Dump",	no_argument,		0, 'D'},
 		{"debug",	no_argument,		0, 'd'},	/* internal, not documented */
 		{"interval",	required_argument,	0, 'i'},
+		{"iterations",	required_argument,	0, 't'},

If you used term "count", you could have consistent long and short option names, like '--count / -c'. I find '--iterations / -t' to be inconsistent, and harder to remember the short option, because I think about time, not "iterations" when I see -t.

However the '-c' is already used as a short form for '--cpu', so I chose --iterations previously.

Thanks,
Yu