Messages in this thread | | | From | "Rafael J. Wysocki" <> | Subject | Re: [PATCH] cpupower: mperf monitor: fix output in MAX_FREQ_SYSFS mode | Date | Wed, 20 May 2015 02:26:47 +0200 |
| |
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 <herton@redhat.com> > > --- > > 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" <herton@redhat.com> > 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 <herton@redhat.com> > --- > 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.
| |