All of lore.kernel.org
 help / color / mirror / Atom feed
* [Powertop] [PATCH] Do not exit on MSR read errors
@ 2015-05-11 14:32 
  0 siblings, 0 replies; 5+ messages in thread
From:  @ 2015-05-11 14:32 UTC (permalink / raw)
  To: powertop

[-- Attachment #1: Type: text/plain, Size: 8152 bytes --]

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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [Powertop] [PATCH] Do not exit on MSR read errors
@ 2015-05-11 16:49 Jaroslav Skarvada
  0 siblings, 0 replies; 5+ messages in thread
From: Jaroslav Skarvada @ 2015-05-11 16:49 UTC (permalink / raw)
  To: powertop

[-- Attachment #1: Type: text/plain, Size: 466 bytes --]

Hi,

it seems 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.

Resending as attachment, because the previous e-mail has been probably blocked
somewhere

thanks & regards

Jaroslav

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Do-not-exit-on-MSR-read-errors.patch --]
[-- Type: text/x-patch, Size: 8230 bytes --]

From ef45eb882274e14c60920a433a665c3695e685e7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jaroslav=20=C5=A0karvada?= <jskarvad@redhat.com>
Date: Thu, 7 May 2015 17:11:43 +0200
Subject: [PATCH] Do not exit on MSR read errors
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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@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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Powertop] [PATCH] Do not exit on MSR read errors
@ 2015-05-11 16:54 Arjan van de Ven
  0 siblings, 0 replies; 5+ messages in thread
From: Arjan van de Ven @ 2015-05-11 16:54 UTC (permalink / raw)
  To: powertop

[-- Attachment #1: Type: text/plain, Size: 1229 bytes --]

On 5/11/2015 9:49 AM, Jaroslav Skarvada wrote:
> Hi,
>
> it seems 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.
>
> Resending as attachment, because the previous e-mail has been probably blocked
> somewhere
>

Hi,


I have a slightly different patch in a local repo, but it needed a command line option
so I didn't sent it upstream yet.

the solution you have makes the app not crash, but it's really not the right thing.
the reason is that the statistics code still uses the "native intel msr" model,
so it gets all zeroes... it should use "just trust the kernel" model instead.

-       if (strcmp(vendor, "GenuineIntel") == 0)
+       if (strcmp(vendor, "GenuineIntel") == 0 && !broken_msrs)


something like that in src/cpu/cpu.cpp (3x) is needed instead....
but today the msr logic runs after this code, not before, so I have not yet found
a good way to set the broken_msrs variable for the case in question



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Powertop] [PATCH] Do not exit on MSR read errors
@ 2015-09-16 19:19 Alexandra Yates
  0 siblings, 0 replies; 5+ messages in thread
From: Alexandra Yates @ 2015-09-16 19:19 UTC (permalink / raw)
  To: powertop

[-- Attachment #1: Type: text/plain, Size: 459 bytes --]

Hi Jaroslav,

On 05/11/2015 09:54 AM, Arjan van de Ven wrote:
> but it's really not the right thing.
> the reason is that the statistics code still uses the "native intel 
> msr" model,
> so it gets all zeroes... it should use "just trust the kernel" model in
Based on Arjan's feedback do you have a change for this patch?  
Otherwise I would have to reject it.

I'm also looking into all other submitted patches.

-- 
Thank you,
<Alexandra>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Powertop] [PATCH] Do not exit on MSR read errors
@ 2015-09-16 19:31 Arjan van de Ven
  0 siblings, 0 replies; 5+ messages in thread
From: Arjan van de Ven @ 2015-09-16 19:31 UTC (permalink / raw)
  To: powertop

[-- Attachment #1: Type: text/plain, Size: 682 bytes --]

On 9/16/2015 12:19 PM, Alexandra Yates wrote:
> Hi Jaroslav,
>
> On 05/11/2015 09:54 AM, Arjan van de Ven wrote:
>> but it's really not the right thing.
>> the reason is that the statistics code still uses the "native intel msr" model,
>> so it gets all zeroes... it should use "just trust the kernel" model in
> Based on Arjan's feedback do you have a change for this patch? Otherwise I would have to reject it.

we do need to solve this problem... it's real.
just we need to really solve it, not just a workaround.

is not insane hard (we'd need to do a test read msr, if that fails because of broken virtualization, fall back to the degraded mode automatically)



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-09-16 19:31 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-16 19:19 [Powertop] [PATCH] Do not exit on MSR read errors Alexandra Yates
  -- strict thread matches above, loose matches on Subject: below --
2015-09-16 19:31 Arjan van de Ven
2015-05-11 16:54 Arjan van de Ven
2015-05-11 16:49 Jaroslav Skarvada
2015-05-11 14:32 

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.