On Monday, May 11, 2015 12:40:22 PM Herton R. Krzesinski wrote: > On Mon, May 11, 2015 at 11:35:35AM -0300, Herton R. Krzesinski wrote: > > There is clearly wrong output when mperf monitor runs in MAX_FREQ_SYSFS > > mode: > > average frequency shows in kHz unit (despite the intended output to be in > > MHz), > > and percentages for C state information are all wrong (including > > high/negative > > values shown). > > > > The problem is that the max_frequency read on initialization isn't used > > where it > > should have been used on mperf_get_count_percent (to estimate the number of > > ticks in the given time period), and the value we read from sysfs is in > > kHz, so > > we must divide it to get the MHz value to use in current calculations. > > > > While at it, also I fixed another small issues in the debug output of > > max_frequency value in mperf_get_count_freq. > > > > Signed-off-by: Herton R. Krzesinski <[email protected]> > > --- > > tools/power/cpupower/utils/idle_monitor/mperf_monitor.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > Actually please consider v2 patch below, just a minor change in the debug > output, which isn't a percentage...
Thomas, any comments? > From: "Herton R. Krzesinski" <[email protected]> > Date: Mon, 11 May 2015 11:18:14 -0300 > Subject: [PATCH v2] cpupower: mperf monitor: fix output in MAX_FREQ_SYSFS mode > > There is clearly wrong output when mperf monitor runs in MAX_FREQ_SYSFS mode: > average frequency shows in kHz unit (despite the intended output to be in > MHz), > and percentages for C state information are all wrong (including high/negative > values shown). > > The problem is that the max_frequency read on initialization isn't used where > it > should have been used on mperf_get_count_percent (to estimate the number of > ticks in the given time period), and the value we read from sysfs is in kHz, > so > we must divide it to get the MHz value to use in current calculations. > > While at it, also I fixed another small issues in the debug output of > max_frequency value in mperf_get_count_freq. > > Signed-off-by: Herton R. Krzesinski <[email protected]> > --- > tools/power/cpupower/utils/idle_monitor/mperf_monitor.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > v2: remove percent from debug output fix > > diff --git a/tools/power/cpupower/utils/idle_monitor/mperf_monitor.c > b/tools/power/cpupower/utils/idle_monitor/mperf_monitor.c > index 90a8c4f..c83f160 100644 > --- a/tools/power/cpupower/utils/idle_monitor/mperf_monitor.c > +++ b/tools/power/cpupower/utils/idle_monitor/mperf_monitor.c > @@ -135,7 +135,7 @@ static int mperf_get_count_percent(unsigned int id, > double *percent, > dprint("%s: TSC Ref - mperf_diff: %llu, tsc_diff: %llu\n", > mperf_cstates[id].name, mperf_diff, tsc_diff); > } else if (max_freq_mode == MAX_FREQ_SYSFS) { > - timediff = timespec_diff_us(time_start, time_end); > + timediff = max_frequency * timespec_diff_us(time_start, > time_end); > *percent = 100.0 * mperf_diff / timediff; > dprint("%s: MAXFREQ - mperf_diff: %llu, time_diff: %llu\n", > mperf_cstates[id].name, mperf_diff, timediff); > @@ -176,7 +176,7 @@ static int mperf_get_count_freq(unsigned int id, unsigned > long long *count, > dprint("%s: Average freq based on %s maximum frequency:\n", > mperf_cstates[id].name, > (max_freq_mode == MAX_FREQ_TSC_REF) ? "TSC calculated" : "sysfs > read"); > - dprint("%max_frequency: %lu", max_frequency); > + dprint("max_frequency: %lu\n", max_frequency); > dprint("aperf_diff: %llu\n", aperf_diff); > dprint("mperf_diff: %llu\n", mperf_diff); > dprint("avg freq: %llu\n", *count); > @@ -279,6 +279,7 @@ use_sysfs: > return -1; > } > max_freq_mode = MAX_FREQ_SYSFS; > + max_frequency /= 1000; /* Default automatically to MHz value */ > return 0; > } > > -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
