[PATCH][RFC] tools/power turbostat: if --max_loop, print for specific time of loops

[PATCH][RFC] tools/power turbostat: if --max_loop, print for specific time of loops

[Yu Chen]

From: Chen Yu <yu.c.chen@intel.com>

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

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

Signed-off-by: Chen Yu <yu.c.chen@intel.com>
---
 tools/power/x86/turbostat/turbostat.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index bd9c6b31a504..a35418a59468 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -48,6 +48,7 @@ char *proc_stat = "/proc/stat";
 FILE *outf;
 int *fd_percpu;
 struct timespec interval_ts = {5, 0};
+unsigned int max_loops;
 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"
+	"--max_loops times	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();
@@ -2626,6 +2630,9 @@ void turbostat_loop()
 		compute_average(ODD_COUNTERS);
 		format_all_counters(ODD_COUNTERS);
 		flush_output_stdout();
+
+		if (++loops >= (max_loops/2))
+			break;
 	}
 }
 
@@ -5009,12 +5016,13 @@ void cmdline(int argc, char **argv)
 		{"Summary",	no_argument,		0, 'S'},
 		{"TCC",		required_argument,	0, 'T'},
 		{"version",	no_argument,		0, 'v' },
+		{"max_loops",	required_argument,	0, 'x'},
 		{0,		0,			0,  0 }
 	};
 
 	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:vx:",
 				long_options, &option_index)) != -1) {
 		switch (opt) {
 		case 'a':
@@ -5076,6 +5084,15 @@ void cmdline(int argc, char **argv)
 			print_version();
 			exit(0);
 			break;
+		case 'x':
+			{
+				unsigned int loops = strtod(optarg, NULL);
+
+				if (loops % 2)
+					loops++;
+				max_loops = loops;
+			}
+			break;
 		}
 	}
 }
-- 
2.13.6

Re: [PATCH][RFC] tools/power turbostat: if --max_loop, print for specific time of loops

[Rafael J. Wysocki]

On Tue, Apr 10, 2018 at 12:18 PM, Yu Chen <yu.c.chen@intel.com> wrote:
> From: Chen Yu <yu.c.chen@intel.com>
>
> There's a use case during test to only print specific round of loops
> if --interval is specified, for example, with this patch applied:
>
> turbostat -i 5 --max_loops 4
> will capture 4 samples with 5 seconds interval.

Why --max_loops and not just --loops or --iterations?

> Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> ---
>  tools/power/x86/turbostat/turbostat.c | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
> index bd9c6b31a504..a35418a59468 100644
> --- a/tools/power/x86/turbostat/turbostat.c
> +++ b/tools/power/x86/turbostat/turbostat.c
> @@ -48,6 +48,7 @@ char *proc_stat = "/proc/stat";
>  FILE *outf;
>  int *fd_percpu;
>  struct timespec interval_ts = {5, 0};
> +unsigned int max_loops;
>  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"
> +       "--max_loops times      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();
> @@ -2626,6 +2630,9 @@ void turbostat_loop()
>                 compute_average(ODD_COUNTERS);
>                 format_all_counters(ODD_COUNTERS);
>                 flush_output_stdout();
> +
> +               if (++loops >= (max_loops/2))
> +                       break;
>         }
>  }
>
> @@ -5009,12 +5016,13 @@ void cmdline(int argc, char **argv)
>                 {"Summary",     no_argument,            0, 'S'},
>                 {"TCC",         required_argument,      0, 'T'},
>                 {"version",     no_argument,            0, 'v' },
> +               {"max_loops",   required_argument,      0, 'x'},
>                 {0,             0,                      0,  0 }
>         };
>
>         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:vx:",
>                                 long_options, &option_index)) != -1) {
>                 switch (opt) {
>                 case 'a':
> @@ -5076,6 +5084,15 @@ void cmdline(int argc, char **argv)
>                         print_version();
>                         exit(0);
>                         break;
> +               case 'x':
> +                       {
> +                               unsigned int loops = strtod(optarg, NULL);
> +
> +                               if (loops % 2)
> +                                       loops++;
> +                               max_loops = loops;
> +                       }
> +                       break;
>                 }
>         }
>  }
> --
> 2.13.6
>

Re: [PATCH][RFC] tools/power turbostat: if --max_loop, print for specific time of loops

[Artem Bityutskiy]

On Tue, 2018-04-10 at 12:22 +0200, Rafael J. Wysocki wrote:
> On Tue, Apr 10, 2018 at 12:18 PM, Yu Chen <yu.c.chen@intel.com>
> wrote:
> > From: Chen Yu <yu.c.chen@intel.com>
> > 
> > There's a use case during test to only print specific round of
> > loops
> > if --interval is specified, for example, with this patch applied:
> > 
> > turbostat -i 5 --max_loops 4
> > will capture 4 samples with 5 seconds interval.
> 
> Why --max_loops and not just --loops or --iterations?

Or just --count.

Artem.

Re: [PATCH][RFC] tools/power turbostat: if --max_loop, print for specific time of loops

[Yu Chen]

On Tue, Apr 10, 2018 at 12:22:16PM +0200, Rafael J. Wysocki wrote:
> On Tue, Apr 10, 2018 at 12:18 PM, Yu Chen <yu.c.chen@intel.com> wrote:
> > From: Chen Yu <yu.c.chen@intel.com>
> >
> > There's a use case during test to only print specific round of loops
> > if --interval is specified, for example, with this patch applied:
> >
> > turbostat -i 5 --max_loops 4
> > will capture 4 samples with 5 seconds interval.
> 
> Why --max_loops and not just --loops or --iterations?
>
I thought -l has been used already. OK, changed to --interation and
-t respectively.

Thanks!

Re: [PATCH][RFC] tools/power turbostat: if --max_loop, print for specific time of loops

[Bityutskiy, Artem]

On Tue, 2018-04-10 at 18:18 +0800, Yu Chen wrote:
> @@ -5076,6 +5084,15 @@ void cmdline(int argc, char **argv)
>                         print_version();
>                         exit(0);
>                         break;
> +               case 'x':
> +                       {
> +                               unsigned int loops = strtod(optarg, NULL);

It would make sense to error out here if you get a value <= 0.

> +
> +                               if (loops % 2)
> +                                       loops++;

What is this for?

> +                               max_loops = loops;

Is the "loops" variable really needed? Can you strtod directly to
max_loops?

-- 
Best Regards,
Artem Bityutskiy
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

Re: [PATCH][RFC] tools/power turbostat: if --max_loop, print for specific time of loops

[Yu Chen]

Hi,
On Tue, Apr 10, 2018 at 07:02:22PM +0800, Bityutskiy, Artem wrote:
> On Tue, 2018-04-10 at 18:18 +0800, Yu Chen wrote:
> > @@ -5076,6 +5084,15 @@ void cmdline(int argc, char **argv)
> >                         print_version();
> >                         exit(0);
> >                         break;
> > +               case 'x':
> > +                       {
> > +                               unsigned int loops = strtod(optarg, NULL);
> 
> It would make sense to error out here if you get a value <= 0.
> 
OK.
> > +
> > +                               if (loops % 2)
> > +                                       loops++;
> 
> What is this for?
> 
It was intended to make the number of loops even for simplify.
But after rethink about this, it might not be necessary. I'll
change it.

> > +                               max_loops = loops;
> 
> Is the "loops" variable really needed? Can you strtod directly to
> max_loops?
If the sanity check for user input is added then I think the local
variable is needed.

Thanks,
Yu
> 
> -- 
> Best Regards,
> Artem Bityutskiy