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

2018-04-10 Thread 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


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

2018-04-10 Thread 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  wrote:
> > From: Chen Yu 
> >
> > 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

2018-04-10 Thread 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

2018-04-10 Thread 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 
> wrote:
> > From: Chen Yu 
> > 
> > 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

2018-04-10 Thread Rafael J. Wysocki
On Tue, Apr 10, 2018 at 12:18 PM, Yu Chen  wrote:
> From: Chen Yu 
>
> 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 
> ---
>  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"
> "--quietskip 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
>


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

2018-04-10 Thread Yu Chen
From: Chen Yu 

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 
---
 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"
"--quietskip 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