2018-04-10 10:18:49

by Chen Yu

[permalink] [raw]
Subject: [PATCH][RFC] 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 --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 <[email protected]>
---
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



2018-04-10 10:30:03

by Rafael J. Wysocki

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

On Tue, Apr 10, 2018 at 12:18 PM, Yu Chen <[email protected]> wrote:
> From: Chen Yu <[email protected]>
>
> 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 <[email protected]>
> ---
> 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
>

2018-04-10 10:57:48

by Artem Bityutskiy

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

On Tue, 2018-04-10 at 12:22 +0200, Rafael J. Wysocki wrote:
> On Tue, Apr 10, 2018 at 12:18 PM, Yu Chen <[email protected]>
> wrote:
> > From: Chen Yu <[email protected]>
> >
> > 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.


2018-04-10 11:06:13

by Bityutskiy, Artem

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

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.

2018-04-10 12:20:19

by Chen Yu

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

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 <[email protected]> wrote:
> > From: Chen Yu <[email protected]>
> >
> > 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!

2018-04-10 13:00:34

by Chen Yu

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

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