[RESEND PATCH] powertop: fix erroneous C state detection on ARM
by Lorenzo Pieralisi
When the kernel is configured with CONFIG_CPU_IDLE_MULTIPLE_DRIVERS,
the cpuidle directory contains a "driver" subdirectory that confuses
the current code parsing the C states on ARM platforms, ending up
with a C state entry corresponding to the actual driver name instead
of a proper C state entry.
This patch fixes the code by stopping the parsing if the files that
characterise the C states (ie usage) are not found in the respective:
/sys/devices/system/cpu/cpuX/cpuidle
subdirectory containing C states information.
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi(a)arm.com>
Tested-by: Kevin Hilman <khilman(a)linaro.org>
---
Alexandra, Arjan,
previous posting here:
https://lists.01.org/pipermail/powertop/2014-December/001724.html
Please consider pulling this patch.
Thanks,
Lorenzo
src/cpu/cpu_linux.cpp | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/cpu/cpu_linux.cpp b/src/cpu/cpu_linux.cpp
index e1ff165..78fd2d3 100644
--- a/src/cpu/cpu_linux.cpp
+++ b/src/cpu/cpu_linux.cpp
@@ -83,7 +83,8 @@ void cpu_linux::parse_cstates_start(void)
if (file) {
file >> usage;
file.close();
- }
+ } else
+ continue;
sprintf(filename + len, "/%s/time", entry->d_name);
@@ -172,7 +173,8 @@ void cpu_linux::parse_cstates_end(void)
if (file) {
file >> usage;
file.close();
- }
+ } else
+ continue;
sprintf(filename + len, "/%s/time", entry->d_name);
--
1.9.1
6 years, 9 months
[Patches] snprintf, PATH_MAX + improvement of handling reporting filenames
by Jaroslav Skarvada
Hi,
I have noticed there are some sprintf that may lead to buffer
overflows, e.g. the following can crash powertop:
# powertop --csv=`printf 'a%.0s' {1..5000}`
I attempted to fix hopefully most of them by converting
the code to snprintf. For data returned by kernel
it's probably unlikely to cause overflows, but why
not cover it all. I also tried to unify buffer
sizes for paths/filenames to PATH_MAX.
The second patch tries to improve handling of reporting
filenames. It works the following way:
powertop --html
generates 'powertop.html' file
powertop --html=myfile.suffix
generates 'myfile.suffix' file
powertop -i 2 --html
generates 'powertop-TIMESTAMPS.html' files
powertop -i 2 --html=myfile.suffix
generates 'myfile-TIMESTAMPS.suffix' files
powertop -i 2 --html=myfile
generates 'myfile-TIMESTAMPS' files
Similarly for CSV.
I think this is more logical behavior
regards
Jaroslav
6 years, 9 months
[PATCH 1/2] prevent segment fault for android built 2
by Zhaoyang Huang
the buffer will be turn into NULL after invoking the mbsrtowcs function
of bionic libc. Add a condition judgement for that
Signed-off-by: Zhaoyang Huang <zhaoyang.huang(a)linaro.org>
---
src/lib.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/lib.cpp b/src/lib.cpp
index 88fe5f3..8d1ca9f 100644
--- a/src/lib.cpp
+++ b/src/lib.cpp
@@ -285,7 +285,7 @@ void align_string(char *buffer, size_t min_sz, size_t max_sz)
/* start with mbsrtowcs() local mbstate_t * and
* NULL dst pointer*/
sz = mbsrtowcs(NULL, (const char **)&buffer, max_sz, NULL);
- if (sz == (size_t)-1) {
+ if ((sz == (size_t)-1) && (NULL != buffer)) {
buffer[min_sz] = 0x00;
return;
}
--
1.7.9.5
6 years, 9 months
[PATCH] Do not exit on MSR read errors
by Jaroslav Škarvada
There are virtual environments not correctly implementing the MSR (e.g.
VirtualBox bug: https://www.virtualbox.org/ticket/14099) or not implementing
it at all (e.g. KVM). PowerTOP just exits in such environments and
cannot run.
The proposed patch makes PowerTOP not to exit on MSR read errors, but still
report them.
Signed-off-by: Jaroslav Škarvada <jskarvad(a)redhat.com>
---
src/cpu/intel_cpus.cpp | 103 +++++++++++++++++++++++++++++--------------------
src/cpu/intel_cpus.h | 1 +
2 files changed, 62 insertions(+), 42 deletions(-)
diff --git a/src/cpu/intel_cpus.cpp b/src/cpu/intel_cpus.cpp
index 1f3647a..69fdc84 100644
--- a/src/cpu/intel_cpus.cpp
+++ b/src/cpu/intel_cpus.cpp
@@ -79,20 +79,20 @@ int is_supported_intel_cpu(int model)
return 0;
}
-static uint64_t get_msr(int cpu, uint64_t offset)
+static int get_msr(int cpu, uint64_t offset, uint64_t *msr)
{
ssize_t retval;
- uint64_t msr;
- retval = read_msr(cpu, offset, &msr);
+ retval = read_msr(cpu, offset, msr);
if (retval < 0) {
reset_display();
fprintf(stderr, _("read_msr cpu%d 0x%llx : "), cpu, (unsigned long long)offset);
fprintf(stderr, "%s\n", strerror(errno));
- exit(-2);
+ *msr = 0;
+ return 0;
}
- return msr;
+ return 1;
}
intel_util::intel_util()
@@ -152,13 +152,18 @@ void nhm_core::measurement_start(void)
last_stamp = 0;
if (this->has_c1_res)
- c1_before = get_msr(first_cpu, MSR_CORE_C1_RESIDENCY);
+ if (!get_msr(first_cpu, MSR_CORE_C1_RESIDENCY, &c1_before))
+ this->has_c1_res = 0;
if (this->has_c3_res)
- c3_before = get_msr(first_cpu, MSR_CORE_C3_RESIDENCY);
- c6_before = get_msr(first_cpu, MSR_CORE_C6_RESIDENCY);
+ if (!get_msr(first_cpu, MSR_CORE_C3_RESIDENCY, &c3_before))
+ this->has_c3_res = 0;
+ if (this->has_c6_res)
+ if (!get_msr(first_cpu, MSR_CORE_C6_RESIDENCY, &c6_before))
+ this->has_c6_res = 0;
if (this->has_c7_res)
- c7_before = get_msr(first_cpu, MSR_CORE_C7_RESIDENCY);
- tsc_before = get_msr(first_cpu, MSR_TSC);
+ if (!get_msr(first_cpu, MSR_CORE_C7_RESIDENCY, &c7_before))
+ this->has_c7_res = 0;
+ get_msr(first_cpu, MSR_TSC, &tsc_before);
if (this->has_c1_res)
insert_cstate("core c1", "C1 (cc1)", 0, c1_before, 1);
@@ -196,13 +201,18 @@ void nhm_core::measurement_end(void)
double ratio;
if (this->has_c1_res)
- c1_after = get_msr(first_cpu, MSR_CORE_C1_RESIDENCY);
+ if (!get_msr(first_cpu, MSR_CORE_C1_RESIDENCY, &c1_after))
+ this->has_c1_res = 0;
if (this->has_c3_res)
- c3_after = get_msr(first_cpu, MSR_CORE_C3_RESIDENCY);
- c6_after = get_msr(first_cpu, MSR_CORE_C6_RESIDENCY);
+ if (!get_msr(first_cpu, MSR_CORE_C3_RESIDENCY, &c3_after))
+ this->has_c3_res = 0;
+ if (this->has_c6_res)
+ if (!get_msr(first_cpu, MSR_CORE_C6_RESIDENCY, &c6_after))
+ this->has_c6_res = 0;
if (this->has_c7_res)
- c7_after = get_msr(first_cpu, MSR_CORE_C7_RESIDENCY);
- tsc_after = get_msr(first_cpu, MSR_TSC);
+ if (!get_msr(first_cpu, MSR_CORE_C7_RESIDENCY, &c7_after))
+ this->has_c7_res = 0;
+ get_msr(first_cpu, MSR_TSC, &tsc_after);
if (this->has_c1_res)
finalize_cstate("core c1", 0, c1_after, 1);
@@ -360,27 +370,31 @@ void nhm_package::measurement_start(void)
last_stamp = 0;
if (this->has_c2c6_res)
- c2_before = get_msr(number, MSR_PKG_C2_RESIDENCY);
+ if (!get_msr(number, MSR_PKG_C2_RESIDENCY, &c2_before))
+ this->has_c2c6_res = 0;
if (this->has_c3_res)
- c3_before = get_msr(number, MSR_PKG_C3_RESIDENCY);
+ if (!get_msr(number, MSR_PKG_C3_RESIDENCY, &c3_before))
+ this->has_c3_res = 0;
/*
* Hack for Braswell where C7 MSR is actually BSW C6
*/
if (this->has_c6c_res)
- c6_before = get_msr(number, MSR_PKG_C7_RESIDENCY);
+ get_msr(number, MSR_PKG_C7_RESIDENCY, &c6_before);
else
- c6_before = get_msr(number, MSR_PKG_C6_RESIDENCY);
+ get_msr(number, MSR_PKG_C6_RESIDENCY, &c6_before);
if (this->has_c7_res)
- c7_before = get_msr(number, MSR_PKG_C7_RESIDENCY);
+ if (!get_msr(number, MSR_PKG_C7_RESIDENCY, &c7_before))
+ this->has_c7_res = 0;
if (this->has_c8c9c10_res) {
- c8_before = get_msr(number, MSR_PKG_C8_RESIDENCY);
- c9_before = get_msr(number, MSR_PKG_C9_RESIDENCY);
- c10_before = get_msr(number, MSR_PKG_C10_RESIDENCY);
+ if (!get_msr(number, MSR_PKG_C8_RESIDENCY, &c8_before) ||
+ !get_msr(number, MSR_PKG_C9_RESIDENCY, &c9_before) ||
+ !get_msr(number, MSR_PKG_C10_RESIDENCY, &c10_before))
+ this->has_c8c9c10_res = 0;
}
- tsc_before = get_msr(first_cpu, MSR_TSC);
+ get_msr(first_cpu, MSR_TSC, &tsc_before);
if (this->has_c2c6_res)
insert_cstate("pkg c2", "C2 (pc2)", 0, c2_before, 1);
@@ -409,24 +423,28 @@ void nhm_package::measurement_end(void)
if (this->has_c2c6_res)
- c2_after = get_msr(number, MSR_PKG_C2_RESIDENCY);
+ if (!get_msr(number, MSR_PKG_C2_RESIDENCY, &c2_after))
+ this->has_c2c6_res = 0;
if (this->has_c3_res)
- c3_after = get_msr(number, MSR_PKG_C3_RESIDENCY);
+ if (!get_msr(number, MSR_PKG_C3_RESIDENCY, &c3_after))
+ this->has_c3_res = 0;
if (this->has_c6c_res)
- c6_after = get_msr(number, MSR_PKG_C7_RESIDENCY);
+ get_msr(number, MSR_PKG_C7_RESIDENCY, &c6_after);
else
- c6_after = get_msr(number, MSR_PKG_C6_RESIDENCY);
+ get_msr(number, MSR_PKG_C6_RESIDENCY, &c6_after);
if (this->has_c7_res)
- c7_after = get_msr(number, MSR_PKG_C7_RESIDENCY);
- if (has_c8c9c10_res) {
- c8_after = get_msr(number, MSR_PKG_C8_RESIDENCY);
- c9_after = get_msr(number, MSR_PKG_C9_RESIDENCY);
- c10_after = get_msr(number, MSR_PKG_C10_RESIDENCY);
+ if (!get_msr(number, MSR_PKG_C7_RESIDENCY, &c7_after))
+ this->has_c7_res = 0;
+ if (this->has_c8c9c10_res) {
+ if (!get_msr(number, MSR_PKG_C8_RESIDENCY, &c8_after) ||
+ !get_msr(number, MSR_PKG_C9_RESIDENCY, &c9_after) ||
+ !get_msr(number, MSR_PKG_C10_RESIDENCY, &c10_after))
+ this->has_c8c9c10_res = 0;
}
- tsc_after = get_msr(first_cpu, MSR_TSC);
+ get_msr(first_cpu, MSR_TSC, &tsc_after);
gettimeofday(&stamp_after, NULL);
@@ -441,7 +459,7 @@ void nhm_package::measurement_end(void)
finalize_cstate("pkg c6", 0, c6_after, 1);
if (this->has_c7_res)
finalize_cstate("pkg c7", 0, c7_after, 1);
- if (has_c8c9c10_res) {
+ if (this->has_c8c9c10_res) {
finalize_cstate("pkg c8", 0, c8_after, 1);
finalize_cstate("pkg c9", 0, c9_after, 1);
finalize_cstate("pkg c10", 0, c10_after, 1);
@@ -497,9 +515,9 @@ void nhm_cpu::measurement_start(void)
last_stamp = 0;
- aperf_before = get_msr(number, MSR_APERF);
- mperf_before = get_msr(number, MSR_MPERF);
- tsc_before = get_msr(number, MSR_TSC);
+ get_msr(number, MSR_APERF, &aperf_before);
+ get_msr(number, MSR_MPERF, &mperf_before);
+ get_msr(number, MSR_TSC, &tsc_before);
insert_cstate("active", _("C0 active"), 0, aperf_before, 1);
@@ -527,9 +545,9 @@ void nhm_cpu::measurement_end(void)
double ratio;
unsigned int i;
- aperf_after = get_msr(number, MSR_APERF);
- mperf_after = get_msr(number, MSR_MPERF);
- tsc_after = get_msr(number, MSR_TSC);
+ get_msr(number, MSR_APERF, &aperf_after);
+ get_msr(number, MSR_MPERF, &mperf_after);
+ get_msr(number, MSR_TSC, &tsc_after);
@@ -582,7 +600,8 @@ char * nhm_cpu::fill_pstate_line(int line_nr, char *buffer)
if (line_nr == LEVEL_C0) {
double F;
- F = 1.0 * (tsc_after - tsc_before) * (aperf_after - aperf_before) / (mperf_after - mperf_before) / time_factor * 1000;
+ F = mperf_after - mperf_before;
+ F = 1.0 * (tsc_after - tsc_before) * (aperf_after - aperf_before) / (F ? F : 1) / time_factor * 1000;
hz_to_human(F, buffer, 1);
return buffer;
}
diff --git a/src/cpu/intel_cpus.h b/src/cpu/intel_cpus.h
index 0331069..e909018 100644
--- a/src/cpu/intel_cpus.h
+++ b/src/cpu/intel_cpus.h
@@ -100,6 +100,7 @@ private:
uint64_t total_stamp;
public:
int has_c1_res;
+ int has_c6_res = 1;
int has_c7_res;
int has_c3_res;
nhm_core(int model);
--
2.1.0
6 years, 9 months
PowerTop On ARM
by Chetan Patil
Hi,
I successfully compiled PowerTop 2.7 for CyanogenMod 11.0 (Android KitKat)
on Nexus 4 which has ARM running kernel version is 3.4. Perf support is
enabled with perf tool accessible via adb.
If I am correct, powertop now supports ARM, and following error tells me
that it's trying to run profiling as per x86. Also, the perf error doesn't
make sense, as I do have proper support enabled. I would appreciate any
help with this.
Also, there is a small error in patch file *update_androidmk.patch*, after
line number 417, there should be *#include <stdlib.h>, *without this the
patch doesn't work correctly.
127|root@mako:/ # powertop
powertop
modprobe cpufreq_stats failedmodprobe msr failedCannot load from file
/data/local/powertop/saved_results.powertop
RAPL device for cpu 0
Model-specific registers (MSR) not found (try enabling
CONFIG_X86_MSR).
Model-specific registers (MSR) not found (try enabling
CONFIG_X86_MSR).
Model-specific registers (MSR) not found (try enabling
CONFIG_X86_MSR).
Model-specific registers (MSR) not found (try enabling
CONFIG_X86_MSR).
Model-specific registers (MSR) not found (try enabling
CONFIG_X86_MSR).
Model-specific registers (MSR) not found (try enabling
CONFIG_X86_MSR).
Model-specific registers (MSR) not found (try enabling
CONFIG_X86_MSR).
RAPL device for cpu 0
Model-specific registers (MSR) not found (try enabling
CONFIG_X86_MSR).
Model-specific registers (MSR) not found (try enabling
CONFIG_X86_MSR).
Model-specific registers (MSR) not found (try enabling
CONFIG_X86_MSR).
Model-specific registers (MSR) not found (try enabling
CONFIG_X86_MSR).
Model-specific registers (MSR) not found (try enabling
CONFIG_X86_MSR).
Model-specific registers (MSR) not found (try enabling
CONFIG_X86_MSR).
Model-specific registers (MSR) not found (try enabling
CONFIG_X86_MSR).
Devfreq not enabled
trace-cmd: Not a directory
unknown op '{'
PowerTOP v2.7 needs the kernel to support the 'perf' subsystem
as well as support for trace points in the kernel:
CONFIG_PERF_EVENTS=y
CONFIG_PERF_COUNTERS=y failedmodprobe msr failedCannot load from file
/data/local/powertop/sCONFIG_TRACEPOINTS=yp
CONFIG_TRACING=ycpu 0
Thanks,
Chetan Patil
6 years, 11 months
Unstable Values in Powertop
by Chandra Satriana
Hello,
I am currently using Powertop to estimate the power consumption of several
processes in Ubuntu,
I plot the values from the csv report but I found that the values are
rather unstable (in the graph it has many spikes). I wonder if you could
tell me why this is happening ?
I guess it is related with how powertop estimates the power ?
Thanks,
Chandra
6 years, 11 months
[PATCH 2/2] comment process_cpu_data in the one_measurement() for it clears all pstat data
by Zhaoyang Huang
all children cpu's pstat data will be cleared in process_cpu_data,which
cause the pstat shows abnormal, we have to comment this line of code. However,
it looks too rough.
Signed-off-by: Zhaoyang Huang <zhaoyang.huang(a)linaro.org>
---
src/main.cpp | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/src/main.cpp b/src/main.cpp
index 1e56af1..9526084 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -221,7 +221,14 @@ void one_measurement(int seconds, char *workload)
devices_end_measurement();
end_power_measurement();
+ /*
+ FIX ME:all children cpu's pstat data will be cleared in process_cpu_data,which
+ cause the pstat shows abnormal, we have to comment this line of code. However,
+ it looks to rough
+ */
+ /*
process_cpu_data();
+ */
process_process_data();
/* output stats */
--
1.7.9.5
6 years, 11 months
(no subject)
by Zhaoyang Huang
>From 761b5c604568ef9b5db4eb3d73ba3053ef3f6d17 Mon Sep 17 00:00:00 2001
From: Zhaoyang Huang <zhaoyang.huang(a)linaro.org>
Date: Wed, 1 Jul 2015 15:22:01 +0800
Subject: [PATCH 1/2] prevent segment fault for android built 2
Hi powertop guys,
The bellowing patch is generated as the buffer within the function will
turn to be null in Android enviroment. I think maybe the reason should be
related to bionic libc.
the buffer will be turn into NULL after invoking the mbsrtowcs function
of bionic libc. Add a condition judgement for that
Signed-off-by: Zhaoyang Huang <zhaoyang.huang(a)linaro.org>
---
src/lib.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/lib.cpp b/src/lib.cpp
index 88fe5f3..8d1ca9f 100644
--- a/src/lib.cpp
+++ b/src/lib.cpp
@@ -285,7 +285,7 @@ void align_string(char *buffer, size_t min_sz, size_t max_sz)
/* start with mbsrtowcs() local mbstate_t * and
* NULL dst pointer*/
sz = mbsrtowcs(NULL, (const char **)&buffer, max_sz, NULL);
- if (sz == (size_t)-1) {
+ if ((sz == (size_t)-1) && (NULL != buffer)) {
buffer[min_sz] = 0x00;
return;
}
--
1.7.9.5
6 years, 12 months
