10:12 p.m.
This is a series of refactor (well, mainly deduplication) of cpu/* code followed with a
single bugfix patch.
The bugfix itself may be applied even without applying refactoring, but then
the same change shall be applied to all copies of calculate_freq().
So the whole purpose of the series is to
1) mercilessly purge those method overrides which are identical to the methods in base
classes, and
2) prefer defining a helper function and overriding it rather than copy-pasting
the whole big function whose behavior needs to be altered and changing one-two lines
in it.
Ivan Shapovalov (11):
Fix includes: do not include intel_cpus.h inside cpu.h
Deduplication: move is_turbo() function to lib
Deduplication: move all instances of account_freq() to
abstract_cpu::account_freq()
Deduplication: remove nhm_cpu::account_freq() almost identical to
abstract_cpu::account_freq()
Deduplication: remove
nhm_{cpu,core,package}::change_effective_frequency() identical to
cpu_{linux,core,package}::change_effective_frequency()
Deduplication: merge
cpu_{core,linux,package}::change_effective_frequency() into
abstract_cpu::change_effective_frequency()
Deduplication: remove {cpu,nhm}_core::calculate_freq() identical to
abstract_cpu::calculate_freq()
Deduplication: remove nhm_package::calculate_freq() identical to
cpu_package::calculate_freq()
Deduplication: properly merge cpu_package::calculate_freq() into
abstract_cpu::calculate_freq()
Deduplication: merge go_idle()/go_unidle()/change_freq() overrides
into abstract_cpu() class
Ignore non-P-state-enabled CPUs when calculating frequency.
src/cpu/abstract_cpu.cpp | 69 ++++++++++--
src/cpu/cpu.cpp | 1 +
src/cpu/cpu.h | 29 ++----
src/cpu/cpu_core.cpp | 97 -----------------
src/cpu/cpu_linux.cpp | 98 ------------------
src/cpu/cpu_package.cpp | 102 ++----------------
src/cpu/intel_cpus.cpp | 265 +----------------------------------------------
src/cpu/intel_cpus.h | 23 +---
src/cpu/intel_gpu.cpp | 2 +-
src/lib.cpp | 9 ++
src/lib.h | 1 +
11 files changed, 94 insertions(+), 602 deletions(-)
--
1.8.0
10:12 p.m.
New subject: [Powertop] [RFC] [PATCH 01/11] Fix includes: do not include intel_cpus.h inside cpu.h
The file "cpu.h" is generic (unlike "intel_cpus.h"), so the existing
order seems
illogical. Also it broke KDevelop's code completion.
Signed-off-by: Ivan Shapovalov <intelfx100(a)gmail.com>
---
src/cpu/cpu.cpp | 1 +
src/cpu/cpu.h | 2 --
src/cpu/intel_cpus.cpp | 2 +-
src/cpu/intel_cpus.h | 2 ++
src/cpu/intel_gpu.cpp | 2 +-
5 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/src/cpu/cpu.cpp b/src/cpu/cpu.cpp
index 7f3af69..3314e82 100644
--- a/src/cpu/cpu.cpp
+++ b/src/cpu/cpu.cpp
@@ -31,6 +31,7 @@
#include <unistd.h>
#include "cpu.h"
#include "cpudevice.h"
+#include "intel_cpus.h"
#include "../parameters/parameters.h"
#include "../perf/perf_bundle.h"
diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
index 4480b11..a9f258a 100644
--- a/src/cpu/cpu.h
+++ b/src/cpu/cpu.h
@@ -210,8 +210,6 @@ public:
};
-#include "intel_cpus.h"
-
extern void enumerate_cpus(void);
extern void report_display_cpu_pstates(void);
diff --git a/src/cpu/intel_cpus.cpp b/src/cpu/intel_cpus.cpp
index 24465a0..a5684aa 100644
--- a/src/cpu/intel_cpus.cpp
+++ b/src/cpu/intel_cpus.cpp
@@ -22,7 +22,7 @@
* Authors:
* Arjan van de Ven <arjan(a)linux.intel.com>
*/
-#include "cpu.h"
+#include "intel_cpus.h"
#include <iostream>
#include <fstream>
#include <stdlib.h>
diff --git a/src/cpu/intel_cpus.h b/src/cpu/intel_cpus.h
index 1949af1..a8bb892 100644
--- a/src/cpu/intel_cpus.h
+++ b/src/cpu/intel_cpus.h
@@ -25,6 +25,8 @@
#include <stdint.h>
#include <sys/time.h>
+#include "cpu.h"
+
#define MSR_TSC 0x10
#define MSR_NEHALEM_PLATFORM_INFO 0xCE
diff --git a/src/cpu/intel_gpu.cpp b/src/cpu/intel_gpu.cpp
index 57474f1..e0f4ac2 100644
--- a/src/cpu/intel_gpu.cpp
+++ b/src/cpu/intel_gpu.cpp
@@ -22,7 +22,7 @@
* Authors:
* Arjan van de Ven <arjan(a)linux.intel.com>
*/
-#include "cpu.h"
+#include "intel_cpus.h"
#include <iostream>
#include <fstream>
#include <stdlib.h>
--
1.8.0
10:16 p.m.
New subject: [Powertop] [RFC] [PATCH 01/11] Fix includes: do not include intel_cpus.h inside cpu.h
On (11/05/12 15:12), Ivan Shapovalov wrote:
The file "cpu.h" is generic (unlike "intel_cpus.h"), so the existing
order seems
illogical.
Also it broke KDevelop's code completion.
just a side note. no offense, but are you gonna fix every product kdevelop fails to
parse instead of fixing just kdevelop?
-ss
Signed-off-by: Ivan Shapovalov <intelfx100(a)gmail.com>
---
src/cpu/cpu.cpp | 1 +
src/cpu/cpu.h | 2 --
src/cpu/intel_cpus.cpp | 2 +-
src/cpu/intel_cpus.h | 2 ++
src/cpu/intel_gpu.cpp | 2 +-
5 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/src/cpu/cpu.cpp b/src/cpu/cpu.cpp
index 7f3af69..3314e82 100644
--- a/src/cpu/cpu.cpp
+++ b/src/cpu/cpu.cpp
@@ -31,6 +31,7 @@
#include <unistd.h>
#include "cpu.h"
#include "cpudevice.h"
+#include "intel_cpus.h"
#include "../parameters/parameters.h"
#include "../perf/perf_bundle.h"
diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
index 4480b11..a9f258a 100644
--- a/src/cpu/cpu.h
+++ b/src/cpu/cpu.h
@@ -210,8 +210,6 @@ public:
};
-#include "intel_cpus.h"
-
extern void enumerate_cpus(void);
extern void report_display_cpu_pstates(void);
diff --git a/src/cpu/intel_cpus.cpp b/src/cpu/intel_cpus.cpp
index 24465a0..a5684aa 100644
--- a/src/cpu/intel_cpus.cpp
+++ b/src/cpu/intel_cpus.cpp
@@ -22,7 +22,7 @@
* Authors:
* Arjan van de Ven <arjan(a)linux.intel.com>
*/
-#include "cpu.h"
+#include "intel_cpus.h"
#include <iostream>
#include <fstream>
#include <stdlib.h>
diff --git a/src/cpu/intel_cpus.h b/src/cpu/intel_cpus.h
index 1949af1..a8bb892 100644
--- a/src/cpu/intel_cpus.h
+++ b/src/cpu/intel_cpus.h
@@ -25,6 +25,8 @@
#include <stdint.h>
#include <sys/time.h>
+#include "cpu.h"
+
#define MSR_TSC 0x10
#define MSR_NEHALEM_PLATFORM_INFO 0xCE
diff --git a/src/cpu/intel_gpu.cpp b/src/cpu/intel_gpu.cpp
index 57474f1..e0f4ac2 100644
--- a/src/cpu/intel_gpu.cpp
+++ b/src/cpu/intel_gpu.cpp
@@ -22,7 +22,7 @@
* Authors:
* Arjan van de Ven <arjan(a)linux.intel.com>
*/
-#include "cpu.h"
+#include "intel_cpus.h"
#include <iostream>
#include <fstream>
#include <stdlib.h>
--
1.8.0
_______________________________________________
PowerTop mailing list
PowerTop(a)lists.01.org
https://lists.01.org/mailman/listinfo/powertop
10:26 p.m.
New subject: [Powertop] [RFC] [PATCH 01/11] Fix includes: do not include intel_cpus.h inside cpu.h
On 05 Nov 2012 14:16:25 Sergey Senozhatsky wrote:
On (11/05/12 15:12), Ivan Shapovalov wrote:
> The file "cpu.h" is generic (unlike "intel_cpus.h"), so the
existing order
> seems illogical.
>
>Also it broke KDevelop's code completion.
just a side note. no offense, but are you gonna fix every product kdevelop
fails to parse instead of fixing just kdevelop?
It's a big architectural WIP there (on kdevelop).
- Ivan
-ss
> Signed-off-by: Ivan Shapovalov <intelfx100(a)gmail.com>
> ---
>
> src/cpu/cpu.cpp | 1 +
> src/cpu/cpu.h | 2 --
> src/cpu/intel_cpus.cpp | 2 +-
> src/cpu/intel_cpus.h | 2 ++
> src/cpu/intel_gpu.cpp | 2 +-
> 5 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/src/cpu/cpu.cpp b/src/cpu/cpu.cpp
> index 7f3af69..3314e82 100644
> --- a/src/cpu/cpu.cpp
> +++ b/src/cpu/cpu.cpp
> @@ -31,6 +31,7 @@
>
> #include <unistd.h>
> #include "cpu.h"
> #include "cpudevice.h"
>
> +#include "intel_cpus.h"
>
> #include "../parameters/parameters.h"
>
> #include "../perf/perf_bundle.h"
>
> diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
> index 4480b11..a9f258a 100644
> --- a/src/cpu/cpu.h
> +++ b/src/cpu/cpu.h
>
> @@ -210,8 +210,6 @@ public:
> };
>
> -#include "intel_cpus.h"
> -
>
> extern void enumerate_cpus(void);
>
> extern void report_display_cpu_pstates(void);
>
> diff --git a/src/cpu/intel_cpus.cpp b/src/cpu/intel_cpus.cpp
> index 24465a0..a5684aa 100644
> --- a/src/cpu/intel_cpus.cpp
> +++ b/src/cpu/intel_cpus.cpp
> @@ -22,7 +22,7 @@
>
> * Authors:
> * Arjan van de Ven <arjan(a)linux.intel.com>
> */
>
> -#include "cpu.h"
> +#include "intel_cpus.h"
>
> #include <iostream>
> #include <fstream>
> #include <stdlib.h>
>
> diff --git a/src/cpu/intel_cpus.h b/src/cpu/intel_cpus.h
> index 1949af1..a8bb892 100644
> --- a/src/cpu/intel_cpus.h
> +++ b/src/cpu/intel_cpus.h
> @@ -25,6 +25,8 @@
>
> #include <stdint.h>
> #include <sys/time.h>
>
> +#include "cpu.h"
> +
>
> #define MSR_TSC 0x10
> #define MSR_NEHALEM_PLATFORM_INFO 0xCE
>
> diff --git a/src/cpu/intel_gpu.cpp b/src/cpu/intel_gpu.cpp
> index 57474f1..e0f4ac2 100644
> --- a/src/cpu/intel_gpu.cpp
> +++ b/src/cpu/intel_gpu.cpp
> @@ -22,7 +22,7 @@
>
> * Authors:
> * Arjan van de Ven <arjan(a)linux.intel.com>
> */
>
> -#include "cpu.h"
> +#include "intel_cpus.h"
>
> #include <iostream>
> #include <fstream>
> #include <stdlib.h>
10:12 p.m.
New subject: [Powertop] [RFC] [PATCH 02/11] Deduplication: move is_turbo() function to lib
Trivial: remove 4 identical static definitions of is_turbo().
Signed-off-by: Ivan Shapovalov <intelfx100(a)gmail.com>
---
src/cpu/cpu_core.cpp | 9 ---------
src/cpu/cpu_linux.cpp | 9 ---------
src/cpu/cpu_package.cpp | 10 ----------
src/cpu/intel_cpus.cpp | 8 --------
src/lib.cpp | 9 +++++++++
src/lib.h | 1 +
6 files changed, 10 insertions(+), 36 deletions(-)
diff --git a/src/cpu/cpu_core.cpp b/src/cpu/cpu_core.cpp
index 593d5f6..f56394f 100644
--- a/src/cpu/cpu_core.cpp
+++ b/src/cpu/cpu_core.cpp
@@ -28,15 +28,6 @@
#include "../parameters/parameters.h"
-static int is_turbo(uint64_t freq, uint64_t max, uint64_t maxmo)
-{
- if (freq != max)
- return 0;
- if (maxmo + 1000 != max)
- return 0;
- return 1;
-}
-
char * cpu_core::fill_cstate_line(int line_nr, char *buffer, const char *separator)
{
unsigned int i;
diff --git a/src/cpu/cpu_linux.cpp b/src/cpu/cpu_linux.cpp
index f5e269e..56b895c 100644
--- a/src/cpu/cpu_linux.cpp
+++ b/src/cpu/cpu_linux.cpp
@@ -38,15 +38,6 @@
#include <sys/stat.h>
#include <dirent.h>
-static int is_turbo(uint64_t freq, uint64_t max, uint64_t maxmo)
-{
- if (freq != max)
- return 0;
- if (maxmo + 1000 != max)
- return 0;
- return 1;
-}
-
void cpu_linux::measurement_start(void)
{
ifstream file;
diff --git a/src/cpu/cpu_package.cpp b/src/cpu/cpu_package.cpp
index 2e2e8a3..7cdbf5d 100644
--- a/src/cpu/cpu_package.cpp
+++ b/src/cpu/cpu_package.cpp
@@ -27,16 +27,6 @@
#include "../lib.h"
#include "../parameters/parameters.h"
-static int is_turbo(uint64_t freq, uint64_t max, uint64_t maxmo)
-{
- if (freq != max)
- return 0;
- if (maxmo + 1000 != max)
- return 0;
- return 1;
-}
-
-
char * cpu_package::fill_cstate_line(int line_nr, char *buffer, const char *separator)
{
unsigned int i;
diff --git a/src/cpu/intel_cpus.cpp b/src/cpu/intel_cpus.cpp
index a5684aa..24d0398 100644
--- a/src/cpu/intel_cpus.cpp
+++ b/src/cpu/intel_cpus.cpp
@@ -40,14 +40,6 @@
#include "../parameters/parameters.h"
#include "../display.h"
-static int is_turbo(uint64_t freq, uint64_t max, uint64_t maxmo)
-{
- if (freq != max)
- return 0;
- if (maxmo + 1000 != max)
- return 0;
- return 1;
-}
int has_c2c7_res;
diff --git a/src/lib.cpp b/src/lib.cpp
index 9838c0b..723517a 100644
--- a/src/lib.cpp
+++ b/src/lib.cpp
@@ -59,6 +59,15 @@ extern "C" {
static int kallsyms_read = 0;
+int is_turbo(uint64_t freq, uint64_t max, uint64_t maxmo)
+{
+ if (freq != max)
+ return 0;
+ if (maxmo + 1000 != max)
+ return 0;
+ return 1;
+}
+
double percentage(double F)
{
F = F * 100.0;
diff --git a/src/lib.h b/src/lib.h
index 8cf4632..9b44450 100644
--- a/src/lib.h
+++ b/src/lib.h
@@ -38,6 +38,7 @@
#define POWERTOP_VERSION "v2.1"
#define POWERTOP_SHORT_VERSION "2.1"
+extern int is_turbo(uint64_t freq, uint64_t max, uint64_t maxmo);
extern int get_max_cpu(void);
extern void set_max_cpu(int cpu);
--
1.8.0
10:12 p.m.
New subject: [Powertop] [RFC] [PATCH 03/11] Deduplication: move all instances of account_freq() to abstract_cpu::account_freq()
Trivial: unify identical in-class private definitions in the common base class.
The only override left is nhm_cpu::account_freq() which is almost identical but
ignores the "Turbo Mode" state.
Seems like a hidden bug to me, but will remove it in a separate commit so one can revert
it easily in case something goes wrong.
Signed-off-by: Ivan Shapovalov <intelfx100(a)gmail.com>
---
src/cpu/abstract_cpu.cpp | 39 +++++++++++++++++++++++++
src/cpu/cpu.h | 6 ++--
src/cpu/cpu_core.cpp | 39 -------------------------
src/cpu/cpu_linux.cpp | 39 -------------------------
src/cpu/cpu_package.cpp | 40 --------------------------
src/cpu/intel_cpus.cpp | 74 ------------------------------------------------
src/cpu/intel_cpus.h | 5 ----
7 files changed, 41 insertions(+), 201 deletions(-)
diff --git a/src/cpu/abstract_cpu.cpp b/src/cpu/abstract_cpu.cpp
index c4c1e93..894a7b8 100644
--- a/src/cpu/abstract_cpu.cpp
+++ b/src/cpu/abstract_cpu.cpp
@@ -28,7 +28,46 @@
#include <stdio.h>
#include <stdlib.h>
#include "cpu.h"
+#include "../lib.h"
+void abstract_cpu::account_freq(uint64_t freq, uint64_t duration)
+{
+ struct frequency *state = NULL;
+ unsigned int i;
+
+ for (i = 0; i < pstates.size(); i++) {
+ if (freq == pstates[i]->freq) {
+ state = pstates[i];
+ break;
+ }
+ }
+
+
+ if (!state) {
+ state = new(std::nothrow) struct frequency;
+
+ if (!state)
+ return;
+
+ memset(state, 0, sizeof(*state));
+
+ pstates.push_back(state);
+
+ state->freq = freq;
+ hz_to_human(freq, state->human_name);
+ if (freq == 0)
+ strcpy(state->human_name, _("Idle"));
+ if (is_turbo(freq, max_frequency, max_minus_one_frequency))
+ sprintf(state->human_name, _("Turbo Mode"));
+
+ state->after_count = 1;
+ }
+
+
+ state->time_after += duration;
+
+
+}
void abstract_cpu::measurement_start(void)
{
diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
index a9f258a..8b9c9f9 100644
--- a/src/cpu/cpu.h
+++ b/src/cpu/cpu.h
@@ -83,6 +83,8 @@ protected:
double time_factor;
uint64_t max_frequency;
uint64_t max_minus_one_frequency;
+
+ virtual void account_freq(uint64_t frequency, uint64_t duration);
public:
uint64_t last_stamp;
uint64_t total_stamp;
@@ -158,8 +160,6 @@ extern vector<class abstract_cpu *> all_cpus;
class cpu_linux: public abstract_cpu
{
-
- void account_freq(uint64_t frequency, uint64_t duration);
public:
virtual void measurement_start(void);
virtual void measurement_end(void);
@@ -181,7 +181,6 @@ public:
class cpu_core: public abstract_cpu
{
- void account_freq(uint64_t frequency, uint64_t duration);
public:
virtual char * fill_cstate_line(int line_nr, char *buffer, const char
*separator="");
virtual char * fill_cstate_name(int line_nr, char *buffer);
@@ -196,7 +195,6 @@ public:
class cpu_package: public abstract_cpu
{
- void account_freq(uint64_t frequency, uint64_t duration);
public:
virtual char * fill_cstate_line(int line_nr, char *buffer, const char
*separator="");
virtual char * fill_cstate_name(int line_nr, char *buffer);
diff --git a/src/cpu/cpu_core.cpp b/src/cpu/cpu_core.cpp
index f56394f..93740b7 100644
--- a/src/cpu/cpu_core.cpp
+++ b/src/cpu/cpu_core.cpp
@@ -72,45 +72,6 @@ char * cpu_core::fill_pstate_name(int line_nr, char *buffer)
return buffer;
}
-
-void cpu_core::account_freq(uint64_t freq, uint64_t duration)
-{
- struct frequency *state = NULL;
- unsigned int i;
-
- for (i = 0; i < pstates.size(); i++) {
- if (freq == pstates[i]->freq) {
- state = pstates[i];
- break;
- }
- }
-
-
- if (!state) {
- state = new(std::nothrow) struct frequency;
-
- if (!state)
- return;
-
- memset(state, 0, sizeof(*state));
-
- pstates.push_back(state);
-
- state->freq = freq;
- hz_to_human(freq, state->human_name);
- if (freq == 0)
- strcpy(state->human_name, _("Idle"));
- if (is_turbo(freq, max_frequency, max_minus_one_frequency))
- sprintf(state->human_name, _("Turbo Mode"));
-
- state->after_count = 1;
- }
-
-
- state->time_after += duration;
-}
-
-
void cpu_core::calculate_freq(uint64_t time)
{
uint64_t freq = 0;
diff --git a/src/cpu/cpu_linux.cpp b/src/cpu/cpu_linux.cpp
index 56b895c..c820238 100644
--- a/src/cpu/cpu_linux.cpp
+++ b/src/cpu/cpu_linux.cpp
@@ -322,45 +322,6 @@ char * cpu_linux::fill_pstate_line(int line_nr, char *buffer)
-void cpu_linux::account_freq(uint64_t freq, uint64_t duration)
-{
- struct frequency *state = NULL;
- unsigned int i;
-
-
- for (i = 0; i < pstates.size(); i++) {
- if (freq == pstates[i]->freq) {
- state = pstates[i];
- break;
- }
- }
-
- if (!state) {
- state = new(std::nothrow) struct frequency;
-
- if (!state)
- return;
-
- memset(state, 0, sizeof(*state));
-
- pstates.push_back(state);
-
- state->freq = freq;
- hz_to_human(freq, state->human_name);
- if (freq == 0)
- strcpy(state->human_name, _("Idle"));
- if (is_turbo(freq, max_frequency, max_minus_one_frequency))
- sprintf(state->human_name, _("Turbo Mode"));
-
- state->after_count = 1;
- }
-
-
- state->time_after += duration;
-
-
-}
-
void cpu_linux::change_freq(uint64_t time, int frequency)
{
current_frequency = frequency;
diff --git a/src/cpu/cpu_package.cpp b/src/cpu/cpu_package.cpp
index 7cdbf5d..d154b90 100644
--- a/src/cpu/cpu_package.cpp
+++ b/src/cpu/cpu_package.cpp
@@ -91,46 +91,6 @@ char * cpu_package::fill_pstate_line(int line_nr, char *buffer)
return buffer;
}
-
-
-
-void cpu_package::account_freq(uint64_t freq, uint64_t duration)
-{
- struct frequency *state = NULL;
- unsigned int i;
-
- for (i = 0; i < pstates.size(); i++) {
- if (freq == pstates[i]->freq) {
- state = pstates[i];
- break;
- }
- }
-
-
- if (!state) {
- state = new(std::nothrow) struct frequency;
-
- if (!state)
- return;
-
- memset(state, 0, sizeof(*state));
-
- pstates.push_back(state);
-
- state->freq = freq;
- hz_to_human(freq, state->human_name);
- if (freq == 0)
- strcpy(state->human_name, _("Idle"));
- if (is_turbo(freq, max_frequency, max_minus_one_frequency))
- sprintf(state->human_name, _("Turbo Mode"));
- state->after_count = 1;
- }
-
- state->time_after += duration;
-
-}
-
-
void cpu_package::calculate_freq(uint64_t time)
{
uint64_t freq = 0;
diff --git a/src/cpu/intel_cpus.cpp b/src/cpu/intel_cpus.cpp
index 24d0398..2a0a874 100644
--- a/src/cpu/intel_cpus.cpp
+++ b/src/cpu/intel_cpus.cpp
@@ -185,44 +185,6 @@ void nhm_core::measurement_end(void)
total_stamp = 0;
}
-void nhm_core::account_freq(uint64_t freq, uint64_t duration)
-{
- struct frequency *state = NULL;
- unsigned int i;
-
- for (i = 0; i < pstates.size(); i++) {
- if (freq == pstates[i]->freq) {
- state = pstates[i];
- break;
- }
- }
-
-
- if (!state) {
- state = new(std::nothrow) struct frequency;
-
- if (!state)
- return;
-
- memset(state, 0, sizeof(*state));
-
- pstates.push_back(state);
-
- state->freq = freq;
- hz_to_human(freq, state->human_name);
- if (freq == 0)
- strcpy(state->human_name, _("Idle"));
- if (is_turbo(freq, max_frequency, max_minus_one_frequency))
- sprintf(state->human_name, _("Turbo Mode"));
-
- state->after_count = 1;
- }
-
-
- state->time_after += duration;
-}
-
-
void nhm_core::calculate_freq(uint64_t time)
{
uint64_t freq = 0;
@@ -407,42 +369,6 @@ void nhm_package::measurement_end(void)
}
-void nhm_package::account_freq(uint64_t freq, uint64_t duration)
-{
- struct frequency *state = NULL;
- unsigned int i;
-
- for (i = 0; i < pstates.size(); i++) {
- if (freq == pstates[i]->freq) {
- state = pstates[i];
- break;
- }
- }
-
-
- if (!state) {
- state = new(std::nothrow) struct frequency;
-
- if (!state)
- return;
-
- memset(state, 0, sizeof(*state));
-
- pstates.push_back(state);
-
- state->freq = freq;
- hz_to_human(freq, state->human_name);
- if (freq == 0)
- strcpy(state->human_name, _("Idle"));
- if (is_turbo(freq, max_frequency, max_minus_one_frequency))
- sprintf(state->human_name, _("Turbo Mode"));
- state->after_count = 1;
- }
-
- state->time_after += duration;
-
-}
-
void nhm_package::calculate_freq(uint64_t time)
{
diff --git a/src/cpu/intel_cpus.h b/src/cpu/intel_cpus.h
index a8bb892..181ffc4 100644
--- a/src/cpu/intel_cpus.h
+++ b/src/cpu/intel_cpus.h
@@ -53,9 +53,6 @@ private:
uint64_t last_stamp;
uint64_t total_stamp;
-
- void account_freq(uint64_t frequency, uint64_t duration);
-
public:
virtual void measurement_start(void);
virtual void measurement_end(void);
@@ -78,8 +75,6 @@ private:
uint64_t last_stamp;
uint64_t total_stamp;
-
- void account_freq(uint64_t frequency, uint64_t duration);
public:
virtual void measurement_start(void);
virtual void measurement_end(void);
--
1.8.0
10:12 p.m.
New subject: [Powertop] [RFC] [PATCH 04/11] Deduplication: remove nhm_cpu::account_freq() almost identical to abstract_cpu::account_freq()
See previous commit for explanation.
Signed-off-by: Ivan Shapovalov <intelfx100(a)gmail.com>
---
src/cpu/intel_cpus.cpp | 36 ------------------------------------
src/cpu/intel_cpus.h | 2 --
2 files changed, 38 deletions(-)
diff --git a/src/cpu/intel_cpus.cpp b/src/cpu/intel_cpus.cpp
index 2a0a874..eeeb8c9 100644
--- a/src/cpu/intel_cpus.cpp
+++ b/src/cpu/intel_cpus.cpp
@@ -528,42 +528,6 @@ int nhm_cpu::has_pstate_level(int level)
return cpu_linux::has_pstate_level(level);
}
-void nhm_cpu::account_freq(uint64_t freq, uint64_t duration)
-{
- struct frequency *state = NULL;
- unsigned int i;
-
-
- for (i = 0; i < pstates.size(); i++) {
- if (freq == pstates[i]->freq) {
- state = pstates[i];
- break;
- }
- }
-
- if (!state) {
- state = new(std::nothrow) struct frequency;
-
- if (!state)
- return;
-
- memset(state, 0, sizeof(*state));
-
- pstates.push_back(state);
-
- state->freq = freq;
- hz_to_human(freq, state->human_name);
- if (freq == 0)
- strcpy(state->human_name, _("Idle"));
- state->after_count = 1;
- }
-
-
- state->time_after += duration;
-
-
-}
-
void nhm_cpu::change_freq(uint64_t time, int frequency)
{
current_frequency = frequency;
diff --git a/src/cpu/intel_cpus.h b/src/cpu/intel_cpus.h
index 181ffc4..cfa46ec 100644
--- a/src/cpu/intel_cpus.h
+++ b/src/cpu/intel_cpus.h
@@ -98,8 +98,6 @@ private:
uint64_t last_stamp;
uint64_t total_stamp;
-
- void account_freq(uint64_t frequency, uint64_t duration);
public:
virtual void measurement_start(void);
virtual void measurement_end(void);
--
1.8.0
10:12 p.m.
New subject: [Powertop] [RFC] [PATCH 05/11] Deduplication: remove nhm_{cpu, core, package}::change_effective_frequency() identical to cpu_{linux, core, package}::change_effective_frequency()
Trivial: remove identical overrides wrt. direct base classes.
Signed-off-by: Ivan Shapovalov <intelfx100(a)gmail.com>
---
src/cpu/intel_cpus.cpp | 64 --------------------------------------------------
src/cpu/intel_cpus.h | 4 ----
2 files changed, 68 deletions(-)
diff --git a/src/cpu/intel_cpus.cpp b/src/cpu/intel_cpus.cpp
index eeeb8c9..30f2d38 100644
--- a/src/cpu/intel_cpus.cpp
+++ b/src/cpu/intel_cpus.cpp
@@ -211,29 +211,6 @@ void nhm_core::calculate_freq(uint64_t time)
old_idle = idle;
}
-void nhm_core::change_effective_frequency(uint64_t time, uint64_t frequency)
-{
- uint64_t freq = 0;
- uint64_t time_delta, fr;
-
-
- if (last_stamp)
- time_delta = time - last_stamp;
- else
- time_delta = 1;
-
- fr = effective_frequency;
-
- if (old_idle)
- fr = 0;
-
- account_freq(fr, time_delta);
-
- effective_frequency = freq;
- last_stamp = time;
- abstract_cpu::change_effective_frequency(time, frequency);
-}
-
char * nhm_core::fill_pstate_line(int line_nr, char *buffer)
{
buffer[0] = 0;
@@ -396,28 +373,6 @@ void nhm_package::calculate_freq(uint64_t time)
old_idle = idle;
}
-void nhm_package::change_effective_frequency(uint64_t time, uint64_t frequency)
-{
- uint64_t time_delta, fr;
-
- if (last_stamp)
- time_delta = time - last_stamp;
- else
- time_delta = 1;
-
- fr = effective_frequency;
- if (old_idle)
- fr = 0;
-
- account_freq(fr, time_delta);
-
- effective_frequency = frequency;
- last_stamp = time;
-
- abstract_cpu::change_effective_frequency(time, frequency);
-}
-
-
void nhm_cpu::measurement_start(void)
{
ifstream file;
@@ -537,25 +492,6 @@ void nhm_cpu::change_freq(uint64_t time, int frequency)
old_idle = idle;
}
-void nhm_cpu::change_effective_frequency(uint64_t time, uint64_t frequency)
-{
- uint64_t time_delta, fr;
-
- if (last_stamp)
- time_delta = time - last_stamp;
- else
- time_delta = 1;
-
- fr = effective_frequency;
- if (old_idle)
- fr = 0;
-
- account_freq(fr, time_delta);
-
- effective_frequency = frequency;
- last_stamp = time;
-}
-
void nhm_cpu::go_idle(uint64_t time)
{
diff --git a/src/cpu/intel_cpus.h b/src/cpu/intel_cpus.h
index cfa46ec..53cadab 100644
--- a/src/cpu/intel_cpus.h
+++ b/src/cpu/intel_cpus.h
@@ -61,7 +61,6 @@ public:
virtual char * fill_pstate_line(int line_nr, char *buffer);
virtual void calculate_freq(uint64_t time);
- virtual void change_effective_frequency(uint64_t time, uint64_t freq);
};
@@ -83,8 +82,6 @@ public:
virtual char * fill_pstate_line(int line_nr, char *buffer);
virtual void calculate_freq(uint64_t time);
- virtual void change_effective_frequency(uint64_t time, uint64_t freq);
-
};
class nhm_cpu: public cpu_linux
@@ -108,7 +105,6 @@ public:
virtual int has_pstate_level(int level);
virtual void change_freq(uint64_t time, int freq);
- virtual void change_effective_frequency(uint64_t time, uint64_t freq);
virtual void go_idle(uint64_t time);
virtual void go_unidle(uint64_t time);
--
1.8.0
10:12 p.m.
New subject: [Powertop] [RFC] [PATCH 06/11] Deduplication: merge cpu_{core, linux, package}::change_effective_frequency() into abstract_cpu::change_effective_frequency()
The mentioned overrides differ only in assigning 0 to effective_frequency
prior to calling abstract_cpu::change_effective_frequency(), which reassigns real
frequency
to the same field.
Moreover, the field value is never used between assignments.
So it shall be safe to merge these overrides into the base definition as shown:
* immediately assign real frequency to field effective_frequency;
* remove post-assignment to the same field.
Signed-off-by: Ivan Shapovalov <intelfx100(a)gmail.com>
---
src/cpu/abstract_cpu.cpp | 17 +++++++++++++++--
src/cpu/cpu.h | 4 ----
src/cpu/cpu_core.cpp | 23 -----------------------
src/cpu/cpu_linux.cpp | 19 -------------------
src/cpu/cpu_package.cpp | 20 --------------------
5 files changed, 15 insertions(+), 68 deletions(-)
diff --git a/src/cpu/abstract_cpu.cpp b/src/cpu/abstract_cpu.cpp
index 894a7b8..083a1e0 100644
--- a/src/cpu/abstract_cpu.cpp
+++ b/src/cpu/abstract_cpu.cpp
@@ -393,14 +393,27 @@ void abstract_cpu::calculate_freq(uint64_t time)
void abstract_cpu::change_effective_frequency(uint64_t time, uint64_t frequency)
{
unsigned int i;
+ uint64_t time_delta, fr;
+
+ if (last_stamp)
+ time_delta = time - last_stamp;
+ else
+ time_delta = 1;
+
+ fr = effective_frequency;
+ if (old_idle)
+ fr = 0;
+
+ account_freq(fr, time_delta);
+
+ effective_frequency = frequency;
+ last_stamp = time;
/* propagate to all children */
for (i = 0; i < children.size(); i++)
if (children[i]) {
children[i]->change_effective_frequency(time, frequency);
}
-
- effective_frequency = frequency;
}
diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
index 8b9c9f9..c99d4ea 100644
--- a/src/cpu/cpu.h
+++ b/src/cpu/cpu.h
@@ -173,7 +173,6 @@ public:
virtual char * fill_pstate_name(int line_nr, char *buffer);
virtual void change_freq(uint64_t time, int freq);
- virtual void change_effective_frequency(uint64_t time, uint64_t freq);
virtual void go_idle(uint64_t time);
virtual void go_unidle(uint64_t time);
@@ -190,7 +189,6 @@ public:
virtual int can_collapse(void) { return childcount == 1;};
virtual void calculate_freq(uint64_t time);
- virtual void change_effective_frequency(uint64_t time, uint64_t freq);
};
class cpu_package: public abstract_cpu
@@ -204,8 +202,6 @@ public:
virtual int can_collapse(void) { return childcount == 1;};
virtual void calculate_freq(uint64_t time);
- virtual void change_effective_frequency(uint64_t time, uint64_t freq);
-
};
extern void enumerate_cpus(void);
diff --git a/src/cpu/cpu_core.cpp b/src/cpu/cpu_core.cpp
index 93740b7..b19ca2c 100644
--- a/src/cpu/cpu_core.cpp
+++ b/src/cpu/cpu_core.cpp
@@ -98,29 +98,6 @@ void cpu_core::calculate_freq(uint64_t time)
old_idle = idle;
}
-void cpu_core::change_effective_frequency(uint64_t time, uint64_t frequency)
-{
- uint64_t freq = 0;
- uint64_t time_delta, fr;
-
-
- if (last_stamp)
- time_delta = time - last_stamp;
- else
- time_delta = 1;
-
- fr = effective_frequency;
-
- if (old_idle)
- fr = 0;
-
- account_freq(fr, time_delta);
-
- effective_frequency = freq;
- last_stamp = time;
- abstract_cpu::change_effective_frequency(time, frequency);
-}
-
char * cpu_core::fill_pstate_line(int line_nr, char *buffer)
{
buffer[0] = 0;
diff --git a/src/cpu/cpu_linux.cpp b/src/cpu/cpu_linux.cpp
index c820238..24fbcb3 100644
--- a/src/cpu/cpu_linux.cpp
+++ b/src/cpu/cpu_linux.cpp
@@ -331,25 +331,6 @@ void cpu_linux::change_freq(uint64_t time, int frequency)
old_idle = idle;
}
-void cpu_linux::change_effective_frequency(uint64_t time, uint64_t frequency)
-{
- uint64_t time_delta, fr;
-
- if (last_stamp)
- time_delta = time - last_stamp;
- else
- time_delta = 1;
-
- fr = effective_frequency;
- if (old_idle)
- fr = 0;
-
- account_freq(fr, time_delta);
-
- effective_frequency = frequency;
- last_stamp = time;
-}
-
void cpu_linux::go_idle(uint64_t time)
{
diff --git a/src/cpu/cpu_package.cpp b/src/cpu/cpu_package.cpp
index d154b90..0ad5bdd 100644
--- a/src/cpu/cpu_package.cpp
+++ b/src/cpu/cpu_package.cpp
@@ -117,23 +117,3 @@ void cpu_package::calculate_freq(uint64_t time)
old_idle = idle;
}
-void cpu_package::change_effective_frequency(uint64_t time, uint64_t frequency)
-{
- uint64_t time_delta, fr;
-
- if (last_stamp)
- time_delta = time - last_stamp;
- else
- time_delta = 1;
-
- fr = effective_frequency;
- if (old_idle)
- fr = 0;
-
- account_freq(fr, time_delta);
-
- effective_frequency = frequency;
- last_stamp = time;
-
- abstract_cpu::change_effective_frequency(time, frequency);
-}
--
1.8.0
10:12 p.m.
New subject: [Powertop] [RFC] [PATCH 07/11] Deduplication: remove {cpu, nhm}_core::calculate_freq() identical to abstract_cpu::calculate_freq()
Trivial: remove identical overrides wrt. direct base class.
Signed-off-by: Ivan Shapovalov <intelfx100(a)gmail.com>
---
src/cpu/cpu.h | 1 -
src/cpu/cpu_core.cpp | 26 --------------------------
src/cpu/intel_cpus.cpp | 26 --------------------------
src/cpu/intel_cpus.h | 2 --
4 files changed, 55 deletions(-)
diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
index c99d4ea..1e9d771 100644
--- a/src/cpu/cpu.h
+++ b/src/cpu/cpu.h
@@ -188,7 +188,6 @@ public:
virtual char * fill_pstate_name(int line_nr, char *buffer);
virtual int can_collapse(void) { return childcount == 1;};
- virtual void calculate_freq(uint64_t time);
};
class cpu_package: public abstract_cpu
diff --git a/src/cpu/cpu_core.cpp b/src/cpu/cpu_core.cpp
index b19ca2c..03acdb1 100644
--- a/src/cpu/cpu_core.cpp
+++ b/src/cpu/cpu_core.cpp
@@ -72,32 +72,6 @@ char * cpu_core::fill_pstate_name(int line_nr, char *buffer)
return buffer;
}
-void cpu_core::calculate_freq(uint64_t time)
-{
- uint64_t freq = 0;
- bool is_idle = true;
- unsigned int i;
-
-
- /* calculate the maximum frequency of all children */
- for (i = 0; i < children.size(); i++)
- if (children[i]) {
- uint64_t f = 0;
- if (!children[i]->idle) {
- f = children[i]->current_frequency;
- is_idle = false;
- }
- if (f > freq)
- freq = f;
- }
-
- current_frequency = freq;
- idle = is_idle;
- if (parent)
- parent->calculate_freq(time);
- old_idle = idle;
-}
-
char * cpu_core::fill_pstate_line(int line_nr, char *buffer)
{
buffer[0] = 0;
diff --git a/src/cpu/intel_cpus.cpp b/src/cpu/intel_cpus.cpp
index 30f2d38..688e95b 100644
--- a/src/cpu/intel_cpus.cpp
+++ b/src/cpu/intel_cpus.cpp
@@ -185,32 +185,6 @@ void nhm_core::measurement_end(void)
total_stamp = 0;
}
-void nhm_core::calculate_freq(uint64_t time)
-{
- uint64_t freq = 0;
- bool is_idle = true;
- unsigned int i;
-
-
- /* calculate the maximum frequency of all children */
- for (i = 0; i < children.size(); i++)
- if (children[i]) {
- uint64_t f = 0;
- if (!children[i]->idle) {
- f = children[i]->current_frequency;
- is_idle = false;
- }
- if (f > freq)
- freq = f;
- }
-
- current_frequency = freq;
- idle = is_idle;
- if (parent)
- parent->calculate_freq(time);
- old_idle = idle;
-}
-
char * nhm_core::fill_pstate_line(int line_nr, char *buffer)
{
buffer[0] = 0;
diff --git a/src/cpu/intel_cpus.h b/src/cpu/intel_cpus.h
index 53cadab..f41d589 100644
--- a/src/cpu/intel_cpus.h
+++ b/src/cpu/intel_cpus.h
@@ -80,8 +80,6 @@ public:
virtual int can_collapse(void) { return 0;};
virtual char * fill_pstate_line(int line_nr, char *buffer);
-
- virtual void calculate_freq(uint64_t time);
};
class nhm_cpu: public cpu_linux
--
1.8.0
10:12 p.m.
New subject: [Powertop] [RFC] [PATCH 08/11] Deduplication: remove nhm_package::calculate_freq() identical to cpu_package::calculate_freq()
Trivial - remove an identical override wrt. direct base class.
Signed-off-by: Ivan Shapovalov <intelfx100(a)gmail.com>
---
src/cpu/intel_cpus.cpp | 27 ---------------------------
src/cpu/intel_cpus.h | 3 ---
2 files changed, 30 deletions(-)
diff --git a/src/cpu/intel_cpus.cpp b/src/cpu/intel_cpus.cpp
index 688e95b..89eefee 100644
--- a/src/cpu/intel_cpus.cpp
+++ b/src/cpu/intel_cpus.cpp
@@ -320,33 +320,6 @@ void nhm_package::measurement_end(void)
}
-
-void nhm_package::calculate_freq(uint64_t time)
-{
- uint64_t freq = 0;
- bool is_idle = true;
- unsigned int i;
-
- /* calculate the maximum frequency of all children */
- for (i = 0; i < children.size(); i++)
- if (children[i]) {
- uint64_t f = 0;
- if (!children[i]->idle) {
- f = children[i]->current_frequency;
- is_idle = false;
- }
- if (f > freq)
- freq = f;
- }
-
- current_frequency = freq;
- idle = is_idle;
- if (parent)
- parent->calculate_freq(time);
- change_effective_frequency(time, current_frequency);
- old_idle = idle;
-}
-
void nhm_cpu::measurement_start(void)
{
ifstream file;
diff --git a/src/cpu/intel_cpus.h b/src/cpu/intel_cpus.h
index f41d589..a4c0852 100644
--- a/src/cpu/intel_cpus.h
+++ b/src/cpu/intel_cpus.h
@@ -59,9 +59,6 @@ public:
virtual int can_collapse(void) { return 0;};
virtual char * fill_pstate_line(int line_nr, char *buffer);
-
- virtual void calculate_freq(uint64_t time);
-
};
class nhm_core: public cpu_core
--
1.8.0
10:12 p.m.
New subject: [Powertop] [RFC] [PATCH 09/11] Deduplication: properly merge cpu_package::calculate_freq() into abstract_cpu::calculate_freq()
The overridden method "cpu_package::calculate_freq()" differs from a generic
implementation in
that it propagates the package's frequency to all in-package cores by
calling change_effective_frequency().
Instead of duplicating the code, introduce a small virtual function
freq_updated() which normally propagates frequency changes upwards and
override it for cpu_package, retaining the correct behavior. Then remove the override of
calculate_freq().
Signed-off-by: Ivan Shapovalov <intelfx100(a)gmail.com>
---
src/cpu/abstract_cpu.cpp | 11 ++++++++---
src/cpu/cpu.h | 5 +++--
src/cpu/cpu_package.cpp | 38 +++++++++++---------------------------
3 files changed, 22 insertions(+), 32 deletions(-)
diff --git a/src/cpu/abstract_cpu.cpp b/src/cpu/abstract_cpu.cpp
index 083a1e0..d64cb24 100644
--- a/src/cpu/abstract_cpu.cpp
+++ b/src/cpu/abstract_cpu.cpp
@@ -69,6 +69,13 @@ void abstract_cpu::account_freq(uint64_t freq, uint64_t duration)
}
+void abstract_cpu::freq_updated(uint64_t time)
+{
+ if(parent)
+ parent->calculate_freq(time);
+ old_idle = idle;
+}
+
void abstract_cpu::measurement_start(void)
{
unsigned int i;
@@ -385,9 +392,7 @@ void abstract_cpu::calculate_freq(uint64_t time)
current_frequency = freq;
idle = is_idle;
- if (parent)
- parent->calculate_freq(time);
- old_idle = idle;
+ freq_updated(time);
}
void abstract_cpu::change_effective_frequency(uint64_t time, uint64_t frequency)
diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
index 1e9d771..498447b 100644
--- a/src/cpu/cpu.h
+++ b/src/cpu/cpu.h
@@ -85,6 +85,7 @@ protected:
uint64_t max_minus_one_frequency;
virtual void account_freq(uint64_t frequency, uint64_t duration);
+ virtual void freq_updated(uint64_t time);
public:
uint64_t last_stamp;
uint64_t total_stamp;
@@ -192,6 +193,8 @@ public:
class cpu_package: public abstract_cpu
{
+protected:
+ virtual void freq_updated(uint64_t time);
public:
virtual char * fill_cstate_line(int line_nr, char *buffer, const char
*separator="");
virtual char * fill_cstate_name(int line_nr, char *buffer);
@@ -199,8 +202,6 @@ public:
virtual char * fill_pstate_line(int line_nr, char *buffer);
virtual char * fill_pstate_name(int line_nr, char *buffer);
virtual int can_collapse(void) { return childcount == 1;};
-
- virtual void calculate_freq(uint64_t time);
};
extern void enumerate_cpus(void);
diff --git a/src/cpu/cpu_package.cpp b/src/cpu/cpu_package.cpp
index 0ad5bdd..277b55f 100644
--- a/src/cpu/cpu_package.cpp
+++ b/src/cpu/cpu_package.cpp
@@ -27,6 +27,17 @@
#include "../lib.h"
#include "../parameters/parameters.h"
+void cpu_package::freq_updated(uint64_t time)
+{
+ if (parent)
+ parent->calculate_freq(time);
+ /*
+ * Make the frequency changes to propagate to all cores in a package.
+ */
+ change_effective_frequency(time, current_frequency);
+ old_idle = idle;
+}
+
char * cpu_package::fill_cstate_line(int line_nr, char *buffer, const char *separator)
{
unsigned int i;
@@ -90,30 +101,3 @@ char * cpu_package::fill_pstate_line(int line_nr, char *buffer)
sprintf(buffer," %5.1f%% ", percentage(1.0* (pstates[line_nr]->time_after)
/ total_stamp));
return buffer;
}
-
-void cpu_package::calculate_freq(uint64_t time)
-{
- uint64_t freq = 0;
- bool is_idle = true;
- unsigned int i;
-
- /* calculate the maximum frequency of all children */
- for (i = 0; i < children.size(); i++)
- if (children[i]) {
- uint64_t f = 0;
- if (!children[i]->idle) {
- f = children[i]->current_frequency;
- is_idle = false;
- }
- if (f > freq)
- freq = f;
- }
-
- current_frequency = freq;
- idle = is_idle;
- if (parent)
- parent->calculate_freq(time);
- change_effective_frequency(time, current_frequency);
- old_idle = idle;
-}
-
--
1.8.0
7:30 a.m.
New subject: [Powertop] [RFC] [PATCH 09/11] Deduplication: properly merge cpu_package::calculate_freq() into abstract_cpu::calculate_freq()
On (11/05/12 15:12), Ivan Shapovalov wrote:
The overridden method "cpu_package::calculate_freq()" differs from a generic
implementation in
that it propagates the package's frequency to all in-package cores by
calling change_effective_frequency().
Instead of duplicating the code, introduce a small virtual function
freq_updated() which normally propagates frequency changes upwards and
override it for cpu_package, retaining the correct behavior. Then remove the override of
calculate_freq().
Signed-off-by: Ivan Shapovalov <intelfx100(a)gmail.com>
---
src/cpu/abstract_cpu.cpp | 11 ++++++++---
src/cpu/cpu.h | 5 +++--
src/cpu/cpu_package.cpp | 38 +++++++++++---------------------------
3 files changed, 22 insertions(+), 32 deletions(-)
diff --git a/src/cpu/abstract_cpu.cpp b/src/cpu/abstract_cpu.cpp
index 083a1e0..d64cb24 100644
--- a/src/cpu/abstract_cpu.cpp
+++ b/src/cpu/abstract_cpu.cpp
@@ -69,6 +69,13 @@ void abstract_cpu::account_freq(uint64_t freq, uint64_t duration)
}
+void abstract_cpu::freq_updated(uint64_t time)
+{
+ if(parent)
+ parent->calculate_freq(time);
+ old_idle = idle;
+}
+
why is this function called something_updatED?
-ss
void abstract_cpu::measurement_start(void)
{
unsigned int i;
@@ -385,9 +392,7 @@ void abstract_cpu::calculate_freq(uint64_t time)
current_frequency = freq;
idle = is_idle;
- if (parent)
- parent->calculate_freq(time);
- old_idle = idle;
+ freq_updated(time);
}
void abstract_cpu::change_effective_frequency(uint64_t time, uint64_t frequency)
diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
index 1e9d771..498447b 100644
--- a/src/cpu/cpu.h
+++ b/src/cpu/cpu.h
@@ -85,6 +85,7 @@ protected:
uint64_t max_minus_one_frequency;
virtual void account_freq(uint64_t frequency, uint64_t duration);
+ virtual void freq_updated(uint64_t time);
public:
uint64_t last_stamp;
uint64_t total_stamp;
@@ -192,6 +193,8 @@ public:
class cpu_package: public abstract_cpu
{
+protected:
+ virtual void freq_updated(uint64_t time);
public:
virtual char * fill_cstate_line(int line_nr, char *buffer, const char
*separator="");
virtual char * fill_cstate_name(int line_nr, char *buffer);
@@ -199,8 +202,6 @@ public:
virtual char * fill_pstate_line(int line_nr, char *buffer);
virtual char * fill_pstate_name(int line_nr, char *buffer);
virtual int can_collapse(void) { return childcount == 1;};
-
- virtual void calculate_freq(uint64_t time);
};
extern void enumerate_cpus(void);
diff --git a/src/cpu/cpu_package.cpp b/src/cpu/cpu_package.cpp
index 0ad5bdd..277b55f 100644
--- a/src/cpu/cpu_package.cpp
+++ b/src/cpu/cpu_package.cpp
@@ -27,6 +27,17 @@
#include "../lib.h"
#include "../parameters/parameters.h"
+void cpu_package::freq_updated(uint64_t time)
+{
+ if (parent)
+ parent->calculate_freq(time);
+ /*
+ * Make the frequency changes to propagate to all cores in a package.
+ */
+ change_effective_frequency(time, current_frequency);
+ old_idle = idle;
+}
+
char * cpu_package::fill_cstate_line(int line_nr, char *buffer, const char *separator)
{
unsigned int i;
@@ -90,30 +101,3 @@ char * cpu_package::fill_pstate_line(int line_nr, char *buffer)
sprintf(buffer," %5.1f%% ", percentage(1.0* (pstates[line_nr]->time_after)
/ total_stamp));
return buffer;
}
-
-void cpu_package::calculate_freq(uint64_t time)
-{
- uint64_t freq = 0;
- bool is_idle = true;
- unsigned int i;
-
- /* calculate the maximum frequency of all children */
- for (i = 0; i < children.size(); i++)
- if (children[i]) {
- uint64_t f = 0;
- if (!children[i]->idle) {
- f = children[i]->current_frequency;
- is_idle = false;
- }
- if (f > freq)
- freq = f;
- }
-
- current_frequency = freq;
- idle = is_idle;
- if (parent)
- parent->calculate_freq(time);
- change_effective_frequency(time, current_frequency);
- old_idle = idle;
-}
-
--
1.8.0
_______________________________________________
PowerTop mailing list
PowerTop(a)lists.01.org
https://lists.01.org/mailman/listinfo/powertop
4:27 p.m.
New subject: [Powertop] [RFC] [PATCH 09/11] Deduplication: properly merge cpu_package::calculate_freq() into abstract_cpu::calculate_freq()
On 06 November 2012 23:30:19 Sergey Senozhatsky wrote:
On (11/05/12 15:12), Ivan Shapovalov wrote:
> The overridden method "cpu_package::calculate_freq()" differs from a
> generic implementation in that it propagates the package's frequency to
> all in-package cores by calling change_effective_frequency().
>
> Instead of duplicating the code, introduce a small virtual function
> freq_updated() which normally propagates frequency changes upwards and
> override it for cpu_package, retaining the correct behavior. Then remove
> the override of calculate_freq().
>
> Signed-off-by: Ivan Shapovalov <intelfx100(a)gmail.com>
> ---
>
> src/cpu/abstract_cpu.cpp | 11 ++++++++---
> src/cpu/cpu.h | 5 +++--
> src/cpu/cpu_package.cpp | 38 +++++++++++---------------------------
> 3 files changed, 22 insertions(+), 32 deletions(-)
>
> diff --git a/src/cpu/abstract_cpu.cpp b/src/cpu/abstract_cpu.cpp
> index 083a1e0..d64cb24 100644
> --- a/src/cpu/abstract_cpu.cpp
> +++ b/src/cpu/abstract_cpu.cpp
> @@ -69,6 +69,13 @@ void abstract_cpu::account_freq(uint64_t freq, uint64_t
> duration)>
> }
>
> +void abstract_cpu::freq_updated(uint64_t time)
> +{
> + if(parent)
> + parent->calculate_freq(time);
> + old_idle = idle;
> +}
> +
why is this function called something_updatED?
Because it is called after field "current_frequency" and/or field
"idle" are
updated by someone. The intent of the function is to propagate the local
changes to the cpu node hierarchy.
- Ivan
-ss
> void abstract_cpu::measurement_start(void)
> {
>
> unsigned int i;
>
> @@ -385,9 +392,7 @@ void abstract_cpu::calculate_freq(uint64_t time)
>
> current_frequency = freq;
> idle = is_idle;
>
> - if (parent)
> - parent->calculate_freq(time);
> - old_idle = idle;
> + freq_updated(time);
>
> }
>
> void abstract_cpu::change_effective_frequency(uint64_t time, uint64_t
> frequency)>
> diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
> index 1e9d771..498447b 100644
> --- a/src/cpu/cpu.h
> +++ b/src/cpu/cpu.h
>
> @@ -85,6 +85,7 @@ protected:
> uint64_t max_minus_one_frequency;
>
> virtual void account_freq(uint64_t frequency, uint64_t duration);
>
> + virtual void freq_updated(uint64_t time);
>
> public:
> uint64_t last_stamp;
> uint64_t total_stamp;
>
> @@ -192,6 +193,8 @@ public:
> class cpu_package: public abstract_cpu
> {
>
> +protected:
> + virtual void freq_updated(uint64_t time);
>
> public:
> virtual char * fill_cstate_line(int line_nr, char *buffer, const char
> *separator=""); virtual char * fill_cstate_name(int line_nr, char
> *buffer);
>
> @@ -199,8 +202,6 @@ public:
> virtual char * fill_pstate_line(int line_nr, char *buffer);
> virtual char * fill_pstate_name(int line_nr, char *buffer);
> virtual int can_collapse(void) { return childcount == 1;};
>
> -
> - virtual void calculate_freq(uint64_t time);
>
> };
>
> extern void enumerate_cpus(void);
>
> diff --git a/src/cpu/cpu_package.cpp b/src/cpu/cpu_package.cpp
> index 0ad5bdd..277b55f 100644
> --- a/src/cpu/cpu_package.cpp
> +++ b/src/cpu/cpu_package.cpp
> @@ -27,6 +27,17 @@
>
> #include "../lib.h"
> #include "../parameters/parameters.h"
>
> +void cpu_package::freq_updated(uint64_t time)
> +{
> + if (parent)
> + parent->calculate_freq(time);
> + /*
> + * Make the frequency changes to propagate to all cores in a package.
> + */
> + change_effective_frequency(time, current_frequency);
> + old_idle = idle;
> +}
> +
>
> char * cpu_package::fill_cstate_line(int line_nr, char *buffer, const
> char *separator) {
>
> unsigned int i;
>
> @@ -90,30 +101,3 @@ char * cpu_package::fill_pstate_line(int line_nr, char
> *buffer)>
> sprintf(buffer," %5.1f%% ", percentage(1.0*
> (pstates[line_nr]->time_after) / total_stamp)); return buffer;
>
> }
>
> -
> -void cpu_package::calculate_freq(uint64_t time)
> -{
> - uint64_t freq = 0;
> - bool is_idle = true;
> - unsigned int i;
> -
> - /* calculate the maximum frequency of all children */
> - for (i = 0; i < children.size(); i++)
> - if (children[i]) {
> - uint64_t f = 0;
> - if (!children[i]->idle) {
> - f = children[i]->current_frequency;
> - is_idle = false;
> - }
> - if (f > freq)
> - freq = f;
> - }
> -
> - current_frequency = freq;
> - idle = is_idle;
> - if (parent)
> - parent->calculate_freq(time);
> - change_effective_frequency(time, current_frequency);
> - old_idle = idle;
> -}
> -
10:12 p.m.
New subject: [Powertop] [RFC] [PATCH 10/11] Deduplication: merge go_idle()/go_unidle()/change_freq() overrides into abstract_cpu() class
When merging, also (while at it) use the new freq_updated() method in
these functions.
Signed-off-by: Ivan Shapovalov <intelfx100(a)gmail.com>
---
src/cpu/cpu.h | 11 +++--------
src/cpu/cpu_linux.cpp | 31 -------------------------------
src/cpu/intel_cpus.cpp | 28 ----------------------------
src/cpu/intel_cpus.h | 5 -----
4 files changed, 3 insertions(+), 72 deletions(-)
diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
index 498447b..9c61641 100644
--- a/src/cpu/cpu.h
+++ b/src/cpu/cpu.h
@@ -143,9 +143,9 @@ public:
/* Frequency micro accounting methods */
virtual void calculate_freq(uint64_t time);
- virtual void go_idle(uint64_t time) { idle = true; if (parent)
parent->calculate_freq(time);};
- virtual void go_unidle(uint64_t time) { idle = false; if (parent)
parent->calculate_freq(time);};;
- virtual void change_freq(uint64_t time, int freq) { current_frequency = freq; if
(parent) parent->calculate_freq(time);};
+ virtual void go_idle(uint64_t time) { idle = true; freq_updated(time); }
+ virtual void go_unidle(uint64_t time) { idle = false; freq_updated(time); }
+ virtual void change_freq(uint64_t time, int freq) { current_frequency = freq;
freq_updated(time); }
virtual void change_effective_frequency(uint64_t time, uint64_t freq);
@@ -172,11 +172,6 @@ public:
virtual char * fill_pstate_line(int line_nr, char *buffer);
virtual char * fill_pstate_name(int line_nr, char *buffer);
-
- virtual void change_freq(uint64_t time, int freq);
- virtual void go_idle(uint64_t time);
- virtual void go_unidle(uint64_t time);
-
};
class cpu_core: public abstract_cpu
diff --git a/src/cpu/cpu_linux.cpp b/src/cpu/cpu_linux.cpp
index 24fbcb3..4025880 100644
--- a/src/cpu/cpu_linux.cpp
+++ b/src/cpu/cpu_linux.cpp
@@ -318,34 +318,3 @@ char * cpu_linux::fill_pstate_line(int line_nr, char *buffer)
sprintf(buffer," %5.1f%% ", percentage(1.0* (pstates[line_nr]->time_after)
/ total_stamp));
return buffer;
}
-
-
-
-
-void cpu_linux::change_freq(uint64_t time, int frequency)
-{
- current_frequency = frequency;
-
- if (parent)
- parent->calculate_freq(time);
- old_idle = idle;
-}
-
-void cpu_linux::go_idle(uint64_t time)
-{
-
- idle = true;
-
- if (parent)
- parent->calculate_freq(time);
- old_idle = idle;
-}
-
-
-void cpu_linux::go_unidle(uint64_t time)
-{
- idle = false;
- if (parent)
- parent->calculate_freq(time);
- old_idle = idle;
-}
diff --git a/src/cpu/intel_cpus.cpp b/src/cpu/intel_cpus.cpp
index 89eefee..c49102b 100644
--- a/src/cpu/intel_cpus.cpp
+++ b/src/cpu/intel_cpus.cpp
@@ -429,31 +429,3 @@ int nhm_cpu::has_pstate_level(int level)
return 1;
return cpu_linux::has_pstate_level(level);
}
-
-void nhm_cpu::change_freq(uint64_t time, int frequency)
-{
- current_frequency = frequency;
-
- if (parent)
- parent->calculate_freq(time);
- old_idle = idle;
-}
-
-void nhm_cpu::go_idle(uint64_t time)
-{
-
- idle = true;
-
- if (parent)
- parent->calculate_freq(time);
- old_idle = idle;
-}
-
-
-void nhm_cpu::go_unidle(uint64_t time)
-{
- idle = false;
- if (parent)
- parent->calculate_freq(time);
- old_idle = idle;
-}
diff --git a/src/cpu/intel_cpus.h b/src/cpu/intel_cpus.h
index a4c0852..a125540 100644
--- a/src/cpu/intel_cpus.h
+++ b/src/cpu/intel_cpus.h
@@ -98,11 +98,6 @@ public:
virtual char * fill_pstate_name(int line_nr, char *buffer);
virtual char * fill_pstate_line(int line_nr, char *buffer);
virtual int has_pstate_level(int level);
-
- virtual void change_freq(uint64_t time, int freq);
- virtual void go_idle(uint64_t time);
- virtual void go_unidle(uint64_t time);
-
};
class atom_package: public cpu_package
--
1.8.0
10:12 p.m.
New subject: [Powertop] [RFC] [PATCH 11/11] Ignore non-P-state-enabled CPUs when calculating frequency.
This fixes bogus reported CPU frequencies (actually, uninitialized
values from i965_core) when i965 monitoring is used.
Signed-off-by: Ivan Shapovalov <intelfx100(a)gmail.com>
---
src/cpu/abstract_cpu.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/cpu/abstract_cpu.cpp b/src/cpu/abstract_cpu.cpp
index d64cb24..fea377b 100644
--- a/src/cpu/abstract_cpu.cpp
+++ b/src/cpu/abstract_cpu.cpp
@@ -380,7 +380,7 @@ void abstract_cpu::calculate_freq(uint64_t time)
/* calculate the maximum frequency of all children */
for (i = 0; i < children.size(); i++)
- if (children[i]) {
+ if (children[i] && children[i]->has_pstates()) {
uint64_t f = 0;
if (!children[i]->idle) {
f = children[i]->current_frequency;
--
1.8.0
6:14 a.m.
New subject: [Powertop] [RFC] [PATCH 11/11] Ignore non-P-state-enabled CPUs when calculating frequency.
On 11/05/2012 03:12 AM, Ivan Shapovalov wrote:
This fixes bogus reported CPU frequencies (actually, uninitialized
values from i965_core) when i965 monitoring is used.
Signed-off-by: Ivan Shapovalov <intelfx100(a)gmail.com>
---
src/cpu/abstract_cpu.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/cpu/abstract_cpu.cpp b/src/cpu/abstract_cpu.cpp
index d64cb24..fea377b 100644
--- a/src/cpu/abstract_cpu.cpp
+++ b/src/cpu/abstract_cpu.cpp
@@ -380,7 +380,7 @@ void abstract_cpu::calculate_freq(uint64_t time)
/* calculate the maximum frequency of all children */
for (i = 0; i < children.size(); i++)
- if (children[i]) {
+ if (children[i] && children[i]->has_pstates()) {
uint64_t f = 0;
if (!children[i]->idle) {
f = children[i]->current_frequency;
This patch for the bug fix has been
merged.
Thanks,
Chris
6:23 a.m.
New subject: [Powertop] [RFC] [PATCH 11/11] Ignore non-P-state-enabled CPUs when calculating frequency.
On (11/13/12 11:14), Chris Ferron wrote:
On 11/05/2012 03:12 AM, Ivan Shapovalov wrote:
>This fixes bogus reported CPU frequencies (actually, uninitialized
>values from i965_core) when i965 monitoring is used.
>
>Signed-off-by: Ivan Shapovalov <intelfx100(a)gmail.com>
>---
> src/cpu/abstract_cpu.cpp | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/src/cpu/abstract_cpu.cpp b/src/cpu/abstract_cpu.cpp
>index d64cb24..fea377b 100644
>--- a/src/cpu/abstract_cpu.cpp
>+++ b/src/cpu/abstract_cpu.cpp
>@@ -380,7 +380,7 @@ void abstract_cpu::calculate_freq(uint64_t time)
> /* calculate the maximum frequency of all children */
> for (i = 0; i < children.size(); i++)
>- if (children[i]) {
>+ if (children[i] && children[i]->has_pstates()) {
> uint64_t f = 0;
> if (!children[i]->idle) {
> f = children[i]->current_frequency;
This patch for the bug fix has been merged.
Thanks,
Chris
thanks, Chris!
sorry, my bad, I somehow forgot to ask to merge this one.
the whole patch series looks interesting and I'd like to revisit it after release.
thanks Ivan.
-ss
1:30 a.m.
New subject: [Powertop] [RFC] [PATCH 11/11] Ignore non-P-state-enabled CPUs when calculating frequency.
On 13 November 2012 11:14:52 Chris Ferron wrote:
On 11/05/2012 03:12 AM, Ivan Shapovalov wrote:
> This fixes bogus reported CPU frequencies (actually, uninitialized
> values from i965_core) when i965 monitoring is used.
>
> Signed-off-by: Ivan Shapovalov <intelfx100(a)gmail.com>
> ---
>
> src/cpu/abstract_cpu.cpp | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/cpu/abstract_cpu.cpp b/src/cpu/abstract_cpu.cpp
> index d64cb24..fea377b 100644
> --- a/src/cpu/abstract_cpu.cpp
> +++ b/src/cpu/abstract_cpu.cpp
> @@ -380,7 +380,7 @@ void abstract_cpu::calculate_freq(uint64_t time)
>
> /* calculate the maximum frequency of all children */
> for (i = 0; i < children.size(); i++)
>
> - if (children[i]) {
> + if (children[i] && children[i]->has_pstates()) {
>
> uint64_t f = 0;
> if (!children[i]->idle) {
>
> f = children[i]->current_frequency;
This patch for the bug fix has been merged.
Ugh, I forgot to notify - to make it work without previous patches,
one shall rather modify {cpu,nhm}_package::calculate_freq(),
so merging this patch alone and as-is doesn't do anything...
Please, consider this one for the release:
---
diff --git a/src/cpu/cpu_package.cpp b/src/cpu/cpu_package.cpp
index 2e2e8a3..4675167 100644
--- a/src/cpu/cpu_package.cpp
+++ b/src/cpu/cpu_package.cpp
@@ -149,7 +149,7 @@ void cpu_package::calculate_freq(uint64_t time)
/* calculate the maximum frequency of all children */
for (i = 0; i < children.size(); i++)
- if (children[i]) {
+ if (children[i] && children[i]->has_pstates()) {
uint64_t f = 0;
if (!children[i]->idle) {
f = children[i]->current_frequency;
diff --git a/src/cpu/intel_cpus.cpp b/src/cpu/intel_cpus.cpp
index 24465a0..a4ed86d 100644
--- a/src/cpu/intel_cpus.cpp
+++ b/src/cpu/intel_cpus.cpp
@@ -460,7 +460,7 @@ void nhm_package::calculate_freq(uint64_t time)
/* calculate the maximum frequency of all children */
for (i = 0; i < children.size(); i++)
- if (children[i]) {
+ if (children[i] && children[i]->has_pstates()) {
uint64_t f = 0;
if (!children[i]->idle) {
f = children[i]->current_frequency;
---
3:32 a.m.
New subject: [Powertop] [RFC] [PATCH 11/11] Ignore non-P-state-enabled CPUs when calculating frequency.
On 11/14/2012 06:30 AM, Ivan Shapovalov wrote:
On 13 November 2012 11:14:52 Chris Ferron wrote:
> On 11/05/2012 03:12 AM, Ivan Shapovalov wrote:
>> This fixes bogus reported CPU frequencies (actually, uninitialized
>> values from i965_core) when i965 monitoring is used.
>>
>> Signed-off-by: Ivan Shapovalov <intelfx100(a)gmail.com>
>> ---
>>
>> src/cpu/abstract_cpu.cpp | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/cpu/abstract_cpu.cpp b/src/cpu/abstract_cpu.cpp
>> index d64cb24..fea377b 100644
>> --- a/src/cpu/abstract_cpu.cpp
>> +++ b/src/cpu/abstract_cpu.cpp
>> @@ -380,7 +380,7 @@ void abstract_cpu::calculate_freq(uint64_t time)
>>
>> /* calculate the maximum frequency of all children */
>> for (i = 0; i < children.size(); i++)
>>
>> - if (children[i]) {
>> + if (children[i] && children[i]->has_pstates()) {
>>
>> uint64_t f = 0;
>> if (!children[i]->idle) {
>>
>> f = children[i]->current_frequency;
> This patch for the bug fix has been merged.
Ugh, I forgot to notify - to make it work without previous patches,
one shall rather modify {cpu,nhm}_package::calculate_freq(),
so merging this patch alone and as-is doesn't do anything...
Please, consider this one for the release:
---
diff --git a/src/cpu/cpu_package.cpp b/src/cpu/cpu_package.cpp
index 2e2e8a3..4675167 100644
--- a/src/cpu/cpu_package.cpp
+++ b/src/cpu/cpu_package.cpp
@@ -149,7 +149,7 @@ void cpu_package::calculate_freq(uint64_t time)
/* calculate the maximum frequency of all children */
for (i = 0; i < children.size(); i++)
- if (children[i]) {
+ if (children[i] && children[i]->has_pstates()) {
uint64_t f = 0;
if (!children[i]->idle) {
f = children[i]->current_frequency;
diff --git a/src/cpu/intel_cpus.cpp b/src/cpu/intel_cpus.cpp
index 24465a0..a4ed86d 100644
--- a/src/cpu/intel_cpus.cpp
+++ b/src/cpu/intel_cpus.cpp
@@ -460,7 +460,7 @@ void nhm_package::calculate_freq(uint64_t time)
/* calculate the maximum frequency of all children */
for (i = 0; i < children.size(); i++)
- if (children[i]) {
+ if (children[i] && children[i]->has_pstates()) {
uint64_t f = 0;
if (!children[i]->idle) {
f = children[i]->current_frequency;
---
Yep, I already caught that. Thanks for the follow up.
10:30 p.m.
New subject: [Powertop] [RFC] [PATCH 00/11] Code deduplication and a single i965 P-states accounting bugfix in cpu/*
On (11/05/12 15:12), Ivan Shapovalov wrote:
This is a series of refactor (well, mainly deduplication) of cpu/*
code followed with a
single bugfix patch.
thanks for patches, we'll review. code is frozen for a release, so it'll take some
time.
-ss
The bugfix itself may be applied even without applying refactoring,
but then
the same change shall be applied to all copies of calculate_freq().
So the whole purpose of the series is to
1) mercilessly purge those method overrides which are identical to the methods in base
classes, and
2) prefer defining a helper function and overriding it rather than copy-pasting
the whole big function whose behavior needs to be altered and changing one-two lines
in it.
Ivan Shapovalov (11):
Fix includes: do not include intel_cpus.h inside cpu.h
Deduplication: move is_turbo() function to lib
Deduplication: move all instances of account_freq() to
abstract_cpu::account_freq()
Deduplication: remove nhm_cpu::account_freq() almost identical to
abstract_cpu::account_freq()
Deduplication: remove
nhm_{cpu,core,package}::change_effective_frequency() identical to
cpu_{linux,core,package}::change_effective_frequency()
Deduplication: merge
cpu_{core,linux,package}::change_effective_frequency() into
abstract_cpu::change_effective_frequency()
Deduplication: remove {cpu,nhm}_core::calculate_freq() identical to
abstract_cpu::calculate_freq()
Deduplication: remove nhm_package::calculate_freq() identical to
cpu_package::calculate_freq()
Deduplication: properly merge cpu_package::calculate_freq() into
abstract_cpu::calculate_freq()
Deduplication: merge go_idle()/go_unidle()/change_freq() overrides
into abstract_cpu() class
Ignore non-P-state-enabled CPUs when calculating frequency.
src/cpu/abstract_cpu.cpp | 69 ++++++++++--
src/cpu/cpu.cpp | 1 +
src/cpu/cpu.h | 29 ++----
src/cpu/cpu_core.cpp | 97 -----------------
src/cpu/cpu_linux.cpp | 98 ------------------
src/cpu/cpu_package.cpp | 102 ++----------------
src/cpu/intel_cpus.cpp | 265 +----------------------------------------------
src/cpu/intel_cpus.h | 23 +---
src/cpu/intel_gpu.cpp | 2 +-
src/lib.cpp | 9 ++
src/lib.h | 1 +
11 files changed, 94 insertions(+), 602 deletions(-)
--
1.8.0
_______________________________________________
PowerTop mailing list
PowerTop(a)lists.01.org
https://lists.01.org/mailman/listinfo/powertop
11:08 p.m.
New subject: [Powertop] [RFC] [PATCH 00/11] Code deduplication and a single i965 P-states accounting bugfix in cpu/*
On 05 ноября 2012 14:30:37 Sergey Senozhatsky wrote:
On (11/05/12 15:12), Ivan Shapovalov wrote:
> This is a series of refactor (well, mainly deduplication) of cpu/* code
> followed with a single bugfix patch.
thanks for patches, we'll review. code is frozen for a release, so it'll
take some time.
Ok, thanks.
FYI: the last patch in series is a bugfix, so it probably shall go into the
upcoming release independently of refactoring..
- Ivan
-ss
> The bugfix itself may be applied even without applying refactoring, but
> then the same change shall be applied to all copies of calculate_freq().
>
> So the whole purpose of the series is to
>
> 1) mercilessly purge those method overrides which are identical to the
> methods in base>
> classes, and
>
> 2) prefer defining a helper function and overriding it rather than
> copy-pasting>
> the whole big function whose behavior needs to be altered and changing
> one-two lines in it.>
> Ivan Shapovalov (11):
> Fix includes: do not include intel_cpus.h inside cpu.h
> Deduplication: move is_turbo() function to lib
> Deduplication: move all instances of account_freq() to
>
> abstract_cpu::account_freq()
>
> Deduplication: remove nhm_cpu::account_freq() almost identical to
>
> abstract_cpu::account_freq()
>
> Deduplication: remove
>
> nhm_{cpu,core,package}::change_effective_frequency() identical to
> cpu_{linux,core,package}::change_effective_frequency()
>
> Deduplication: merge
>
> cpu_{core,linux,package}::change_effective_frequency() into
> abstract_cpu::change_effective_frequency()
>
> Deduplication: remove {cpu,nhm}_core::calculate_freq() identical to
>
> abstract_cpu::calculate_freq()
>
> Deduplication: remove nhm_package::calculate_freq() identical to
>
> cpu_package::calculate_freq()
>
> Deduplication: properly merge cpu_package::calculate_freq() into
>
> abstract_cpu::calculate_freq()
>
> Deduplication: merge go_idle()/go_unidle()/change_freq() overrides
>
> into abstract_cpu() class
>
> Ignore non-P-state-enabled CPUs when calculating frequency.
>
> src/cpu/abstract_cpu.cpp | 69 ++++++++++--
> src/cpu/cpu.cpp | 1 +
> src/cpu/cpu.h | 29 ++----
> src/cpu/cpu_core.cpp | 97 -----------------
> src/cpu/cpu_linux.cpp | 98 ------------------
> src/cpu/cpu_package.cpp | 102 ++----------------
> src/cpu/intel_cpus.cpp | 265
> +---------------------------------------------- src/cpu/intel_cpus.h
> | 23 +---
> src/cpu/intel_gpu.cpp | 2 +-
> src/lib.cpp | 9 ++
> src/lib.h | 1 +
> 11 files changed, 94 insertions(+), 602 deletions(-)
>
> --
> 1.8.0
>
> _______________________________________________
> PowerTop mailing list
> PowerTop(a)lists.01.org
> https://lists.01.org/mailman/listinfo/powertop
--
С уважением,
Шаповалов Иван.
3595
days inactive
3604
days old
21 comments
3 participants
participants (3)
-
Chris Ferron -
Ivan Shapovalov -
Sergey Senozhatsky