[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] 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 pread cpu0 0xe8 : Input/output error
by Stefanos Georgiou
Hello,
I am trying to execute power top on my Virtual Machine with specification
OS 6.5 Scientific Linux and I get the following message : pread cpu0 0xe8 :
input/output error. The powertop version i am using is 2.3. I have tried
running older versions like 1.11 which was running properly but as I red
1.11 measures cpu wake ups and not energy consumption as the 2.3. Any idea
what the problem may be and how can I solve it ?
*Best Regards,*
*Stefanos Georgiou*
*BSc in Network and System Programming*
*Computer Science Department*
*University of Cyprus*
*MSc in Pervasive Computing and Communications *
*for Sustainable Development (PERCCOM)*
*Joint Master Degree*
*University of Lorraine, Lappeenranta, Lulea, ITMO Saint-Petersburg*
7 years, 1 month
Fwd: Urgent.
by Dhanraj Peela
Hi Sir,
This id Dhanraj,Im working on a power optimization program and would like
to know how to only check the power consumed by a single application and
how to change its C State in Linux.
I'm using CentOs and powertop ver 2.7.
Thank You..
7 years, 1 month