* RE: [PATCH][v3] tools/power turbostat: if --max_loop, print for specific time of loops
@ 2018-04-12 7:18 Doug Smythies
2018-04-12 7:35 ` Yu Chen
0 siblings, 1 reply; 4+ messages in thread
From: Doug Smythies @ 2018-04-12 7:18 UTC (permalink / raw)
To: 'Yu Chen', 'Len Brown'
Cc: 'Rafael J Wysocki', 'Artem Bityutskiy',
linux-pm, linux-kernel, Doug Smythies
On 2018.04.11 03:31 Yu Chen wrote:
> From: Chen Yu <yu.c.chen@intel.com>
>
> 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.
Hi Yu,
This would be a very useful addition to turbostat.
Please also update the man turbostat man page.
tools/power/x86/turbostat/turbostat.8
... Doug
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH][v3] tools/power turbostat: if --max_loop, print for specific time of loops
2018-04-12 7:18 [PATCH][v3] tools/power turbostat: if --max_loop, print for specific time of loops Doug Smythies
@ 2018-04-12 7:35 ` Yu Chen
0 siblings, 0 replies; 4+ messages in thread
From: Yu Chen @ 2018-04-12 7:35 UTC (permalink / raw)
To: Doug Smythies
Cc: 'Len Brown', 'Rafael J Wysocki',
'Artem Bityutskiy',
linux-pm, linux-kernel
Hi Doug,
On Thu, Apr 12, 2018 at 12:18:44AM -0700, Doug Smythies wrote:
> On 2018.04.11 03:31 Yu Chen wrote:
>
> > From: Chen Yu <yu.c.chen@intel.com>
> >
> > 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.
>
> Hi Yu,
>
> This would be a very useful addition to turbostat.
> Please also update the man turbostat man page.
>
> tools/power/x86/turbostat/turbostat.8
>
OK, I will do this.
Thanks,
Yu
> ... Doug
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH][v3] tools/power turbostat: if --max_loop, print for specific time of loops
2018-04-11 11:02 ` Artem Bityutskiy
@ 2018-04-11 12:51 ` Yu Chen
0 siblings, 0 replies; 4+ messages in thread
From: Yu Chen @ 2018-04-11 12:51 UTC (permalink / raw)
To: Artem Bityutskiy; +Cc: Len Brown, Rafael J Wysocki, linux-pm, linux-kernel
Hi,
On Wed, Apr 11, 2018 at 02:02:02PM +0300, Artem Bityutskiy wrote:
> A couple of nitpicks.
>
> On Wed, 2018-04-11 at 18:30 +0800, Yu Chen wrote:
> > @@ -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".
>
> > 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')"
>
OK, this looks more consistent.
> > 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".
>
OK.
> > @@ -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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH][v3] tools/power turbostat: if --max_loop, print for specific time of loops
[not found] <20180411103038.12735-1-yu.c.chen@intel.com>
@ 2018-04-11 11:02 ` Artem Bityutskiy
2018-04-11 12:51 ` Yu Chen
0 siblings, 1 reply; 4+ messages in thread
From: Artem Bityutskiy @ 2018-04-11 11:02 UTC (permalink / raw)
To: Yu Chen, Len Brown; +Cc: Rafael J Wysocki, linux-pm, linux-kernel
A couple of nitpicks.
On Wed, 2018-04-11 at 18:30 +0800, Yu Chen wrote:
> @@ -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".
> 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')"
> 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".
> @@ -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.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-04-12 7:35 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-12 7:18 [PATCH][v3] tools/power turbostat: if --max_loop, print for specific time of loops Doug Smythies
2018-04-12 7:35 ` Yu Chen
[not found] <20180411103038.12735-1-yu.c.chen@intel.com>
2018-04-11 11:02 ` Artem Bityutskiy
2018-04-11 12:51 ` Yu Chen
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).