[RFC] [PATCHv2 00/10] Code deduplication in cpu/*

[PATCH 1/2] parameters: fix...

Re: [Powertop] [PATCH] Fix C...

Ivan Shapovalov

Monday, 24 December 2012 Mon, 24 Dec '12

10:08 a.m.

This is a resend (unchanged but rebased) of the patch series about refactoring code in cpu/* by deduplication. From this series, only the last bugfix patch has been applied so far due to a code freeze. Ivan Shapovalov (10): 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

Show replies by date

Ivan Shapovalov

Monday, 24 December Mon, 24 Dec

10:08 a.m.

New subject: [Powertop] [RFC] [PATCHv2 01/10] 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. 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 781e33c..2304cd5 100644 --- a/src/cpu/cpu.h +++ b/src/cpu/cpu.h @@ -215,8 +215,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 19ad85c..d43e917 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.2

Ivan Shapovalov

10:08 a.m.

New subject: [Powertop] [RFC] [PATCHv2 02/10] 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 | 11 +---------- src/cpu/cpu_package.cpp | 10 ---------- src/cpu/intel_cpus.cpp | 8 -------- src/lib.cpp | 9 +++++++++ src/lib.h | 1 + 6 files changed, 11 insertions(+), 37 deletions(-) diff --git a/src/cpu/cpu_core.cpp b/src/cpu/cpu_core.cpp index 2caa58d..8aba60b 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 e7a3d37..034263b 100644 --- a/src/cpu/cpu_linux.cpp +++ b/src/cpu/cpu_linux.cpp @@ -38,16 +38,7 @@ #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::parse_cstates_start(void) +void cpu_linux::measurement_start(void) { ifstream file; DIR *dir; diff --git a/src/cpu/cpu_package.cpp b/src/cpu/cpu_package.cpp index b9d4c19..61ce050 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 d43e917..640a0d5 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 209421a..de2de5a 100644 --- a/src/lib.h +++ b/src/lib.h @@ -38,6 +38,7 @@ #define POWERTOP_VERSION "v"PACKAGE_VERSION #define POWERTOP_SHORT_VERSION PACKAGE_VERSION +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.2

Ivan Shapovalov

Tuesday, 25 December Tue, 25 Dec

12:57 a.m.

New subject: [Powertop] [RFC] [PATCHv3 02/10] 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 2caa58d..8aba60b 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 e7a3d37..2837cd0 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::parse_cstates_start(void) { ifstream file; diff --git a/src/cpu/cpu_package.cpp b/src/cpu/cpu_package.cpp index b9d4c19..61ce050 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 d43e917..640a0d5 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 209421a..de2de5a 100644 --- a/src/lib.h +++ b/src/lib.h @@ -38,6 +38,7 @@ #define POWERTOP_VERSION "v"PACKAGE_VERSION #define POWERTOP_SHORT_VERSION PACKAGE_VERSION +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.2

Ivan Shapovalov

Monday, 24 December Mon, 24 Dec

10:08 a.m.

New subject: [Powertop] [RFC] [PATCHv2 03/10] 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 | 11 ++----- 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(+), 206 deletions(-) diff --git a/src/cpu/abstract_cpu.cpp b/src/cpu/abstract_cpu.cpp index 537f3cb..5618761 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 2304cd5..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,13 +160,6 @@ extern vector<class abstract_cpu *> all_cpus; class cpu_linux: public abstract_cpu { - - void account_freq(uint64_t frequency, uint64_t duration); - void parse_pstates_start(void); - void parse_cstates_start(void); - void parse_pstates_end(void); - void parse_cstates_end(void); - public: virtual void measurement_start(void); virtual void measurement_end(void); @@ -186,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); @@ -201,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 8aba60b..8249d83 100644 --- a/src/cpu/cpu_core.cpp +++ b/src/cpu/cpu_core.cpp @@ -77,45 +77,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 034263b..41a2238 100644 --- a/src/cpu/cpu_linux.cpp +++ b/src/cpu/cpu_linux.cpp @@ -350,45 +350,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 61ce050..6794a38 100644 --- a/src/cpu/cpu_package.cpp +++ b/src/cpu/cpu_package.cpp @@ -102,46 +102,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 640a0d5..4d52206 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; @@ -418,42 +380,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.2

Sergey Senozhatsky

9:30 p.m.

New subject: [Powertop] [RFC] [PATCHv2 03/10] Deduplication: move all instances of account_freq() to abstract_cpu::account_freq()

Hello, On (12/24/12 03:08), Ivan Shapovalov wrote:

...

I see that you've dropped states start/end parsing, but I can't see where did you land them -ss

...

public: virtual void measurement_start(void); virtual void measurement_end(void); @@ -186,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); @@ -201,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 8aba60b..8249d83 100644 --- a/src/cpu/cpu_core.cpp +++ b/src/cpu/cpu_core.cpp @@ -77,45 +77,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 034263b..41a2238 100644 --- a/src/cpu/cpu_linux.cpp +++ b/src/cpu/cpu_linux.cpp @@ -350,45 +350,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 61ce050..6794a38 100644 --- a/src/cpu/cpu_package.cpp +++ b/src/cpu/cpu_package.cpp @@ -102,46 +102,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 640a0d5..4d52206 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; @@ -418,42 +380,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.2 _______________________________________________ PowerTop mailing list PowerTop(a)lists.01.org https://lists.01.org/mailman/listinfo/powertop

Ivan Shapovalov

Tuesday, 25 December Tue, 25 Dec

12:50 a.m.

New subject: [Powertop] [RFC] [PATCHv2 03/10] Deduplication: move all instances of account_freq() to abstract_cpu::account_freq()

On 24 December 2012 13:30:52 Sergey Senozhatsky wrote:

...

Hello, On (12/24/12 03:08), Ivan Shapovalov wrote: > 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 | 11 ++----- > 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(+), 206 deletions(-) > > diff --git a/src/cpu/abstract_cpu.cpp b/src/cpu/abstract_cpu.cpp > index 537f3cb..5618761 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 2304cd5..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,13 +160,6 @@ extern vector<class abstract_cpu *> all_cpus; > > class cpu_linux: public abstract_cpu > { > > - > - void account_freq(uint64_t frequency, uint64_t duration); > - void parse_pstates_start(void); > - void parse_cstates_start(void); > - void parse_pstates_end(void); > - void parse_cstates_end(void); > - I see that you've dropped states start/end parsing, but I can't see where did you land them -ss

Argh. I've misrebased the patches; wrong are 01/10 and 03/10. Will resend them as v3.

...

> public: > virtual void measurement_start(void); > virtual void measurement_end(void); > > @@ -186,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); > > @@ -201,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 8aba60b..8249d83 100644 > --- a/src/cpu/cpu_core.cpp > +++ b/src/cpu/cpu_core.cpp > @@ -77,45 +77,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 034263b..41a2238 100644 > --- a/src/cpu/cpu_linux.cpp > +++ b/src/cpu/cpu_linux.cpp > @@ -350,45 +350,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 61ce050..6794a38 100644 > --- a/src/cpu/cpu_package.cpp > +++ b/src/cpu/cpu_package.cpp > @@ -102,46 +102,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 640a0d5..4d52206 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; > > @@ -418,42 +380,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);

Ivan Shapovalov

12:55 a.m.

New subject: [Powertop] [RFC] [PATCHv2 03/10] Deduplication: move all instances of account_freq() to abstract_cpu::account_freq()

On 24 December 2012 17:50:06 Ivan Shapovalov wrote:

...

<...> Argh. I've misrebased the patches; wrong are 01/10 and 03/10. Will resend them as v3.

Mistake on mistake... I meant 02/10 and 03/10.

...

<...>

Sergey Senozhatsky

1:39 a.m.

New subject: [Powertop] [RFC] [PATCHv2 03/10] Deduplication: move all instances of account_freq() to abstract_cpu::account_freq()

On (12/24/12 17:55), Ivan Shapovalov wrote:

...

On 24 December 2012 17:50:06 Ivan Shapovalov wrote: > <...> > > Argh. I've misrebased the patches; wrong are 01/10 and 03/10. > Will resend them as v3. Mistake on mistake... I meant 02/10 and 03/10.

no problem :-) -ss

...

> > <...>

Ivan Shapovalov

12:58 a.m.

New subject: [Powertop] [RFC] [PATCHv3 03/10] 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 537f3cb..5618761 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 2304cd5..6f0203f 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); void parse_pstates_start(void); void parse_cstates_start(void); void parse_pstates_end(void); @@ -186,7 +186,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); @@ -201,7 +200,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 8aba60b..8249d83 100644 --- a/src/cpu/cpu_core.cpp +++ b/src/cpu/cpu_core.cpp @@ -77,45 +77,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 2837cd0..1a0eb4d 100644 --- a/src/cpu/cpu_linux.cpp +++ b/src/cpu/cpu_linux.cpp @@ -350,45 +350,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 61ce050..6794a38 100644 --- a/src/cpu/cpu_package.cpp +++ b/src/cpu/cpu_package.cpp @@ -102,46 +102,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 640a0d5..4d52206 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; @@ -418,42 +380,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.2

Ivan Shapovalov

Monday, 24 December Mon, 24 Dec

10:08 a.m.

New subject: [Powertop] [RFC] [PATCHv2 04/10] 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 4d52206..57cb43b 100644 --- a/src/cpu/intel_cpus.cpp +++ b/src/cpu/intel_cpus.cpp @@ -544,42 +544,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.2

Ivan Shapovalov

10:08 a.m.

New subject: [Powertop] [RFC] [PATCHv2 05/10] 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 57cb43b..ae31112 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; @@ -407,28 +384,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; @@ -553,25 +508,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.2

Ivan Shapovalov

10:08 a.m.

New subject: [Powertop] [RFC] [PATCHv2 06/10] 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 5618761..ec52f55 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 8249d83..7ec498d 100644 --- a/src/cpu/cpu_core.cpp +++ b/src/cpu/cpu_core.cpp @@ -102,29 +102,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 41a2238..a2e6cb9 100644 --- a/src/cpu/cpu_linux.cpp +++ b/src/cpu/cpu_linux.cpp @@ -359,25 +359,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 6794a38..ef54524 100644 --- a/src/cpu/cpu_package.cpp +++ b/src/cpu/cpu_package.cpp @@ -128,23 +128,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.2

Ivan Shapovalov

10:08 a.m.

New subject: [Powertop] [RFC] [PATCHv2 07/10] 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 | 25 ------------------------- src/cpu/intel_cpus.cpp | 26 -------------------------- src/cpu/intel_cpus.h | 2 -- 4 files changed, 54 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 7ec498d..65c2718 100644 --- a/src/cpu/cpu_core.cpp +++ b/src/cpu/cpu_core.cpp @@ -77,31 +77,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] && children[i]->has_pstates()) { - 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 ae31112..031c7ac 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] && children[i]->has_pstates()) { - 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.2

Ivan Shapovalov

10:08 a.m.

New subject: [Powertop] [RFC] [PATCHv2 08/10] 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 031c7ac..a898c06 100644 --- a/src/cpu/intel_cpus.cpp +++ b/src/cpu/intel_cpus.cpp @@ -331,33 +331,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] && children[i]->has_pstates()) { - 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.2

Ivan Shapovalov

10:08 a.m.

New subject: [Powertop] [RFC] [PATCHv2 09/10] 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 ec52f55..fea377b 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 ef54524..c264e54 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; @@ -101,30 +112,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] && children[i]->has_pstates()) { - 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.2

Ivan Shapovalov

10:08 a.m.

New subject: [Powertop] [RFC] [PATCHv2 10/10] 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 a2e6cb9..196bfdb 100644 --- a/src/cpu/cpu_linux.cpp +++ b/src/cpu/cpu_linux.cpp @@ -346,34 +346,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 a898c06..2b78d31 100644 --- a/src/cpu/intel_cpus.cpp +++ b/src/cpu/intel_cpus.cpp @@ -445,31 +445,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.2

Sergey Senozhatsky

Tuesday, 25 December Tue, 25 Dec

3:25 a.m.

New subject: [Powertop] [RFC] [PATCHv2 00/10] Code deduplication in cpu/*

On (12/24/12 03:08), Ivan Shapovalov wrote:

...

Hello Chris, patch series looks good to me. thanks to Ivan. please note V3 of 02 and 03 patches. -ss

Ferron, Chris E

7:17 a.m.

New subject: [Powertop] [RFC] [PATCHv2 00/10] Code deduplication in cpu/*

Thanks, I will attend to outstanding patches on Wednesday (GMT+8) when I return to the office. -Chris -----Original Message----- From: powertop-bounces(a)lists.01.org [mailto:powertop-bounces@lists.01.org] On Behalf Of Sergey Senozhatsky Sent: Monday, December 24, 2012 8:25 AM To: Chris Ferron Cc: powertop(a)lists.01.org Subject: Re: [Powertop] [RFC] [PATCHv2 00/10] Code deduplication in cpu/* On (12/24/12 03:08), Ivan Shapovalov wrote:

...

Hello Chris, patch series looks good to me. thanks to Ivan. please note V3 of 02 and 03 patches. -ss _______________________________________________ PowerTop mailing list PowerTop(a)lists.01.org https://lists.01.org/mailman/listinfo/powertop

Sergey Senozhatsky

7:41 a.m.

New subject: [Powertop] [RFC] [PATCHv2 00/10] Code deduplication in cpu/*

On (12/24/12 20:17), Ferron, Chris E wrote:

...

Thanks, I will attend to outstanding patches on Wednesday (GMT+8) when I return to the office. -Chris

Thanks, Merry Christmas! -ss

...

-----Original Message----- From: powertop-bounces(a)lists.01.org [mailto:powertop-bounces@lists.01.org] On Behalf Of Sergey Senozhatsky Sent: Monday, December 24, 2012 8:25 AM To: Chris Ferron Cc: powertop(a)lists.01.org Subject: Re: [Powertop] [RFC] [PATCHv2 00/10] Code deduplication in cpu/* On (12/24/12 03:08), Ivan Shapovalov wrote: > This is a resend (unchanged but rebased) of the patch series about > refactoring code in cpu/* by deduplication. From this series, only the > last bugfix patch has been applied so far due to a code freeze. > > Ivan Shapovalov (10): > 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 > Hello Chris, patch series looks good to me. thanks to Ivan. please note V3 of 02 and 03 patches. -ss _______________________________________________ PowerTop mailing list PowerTop(a)lists.01.org https://lists.01.org/mailman/listinfo/powertop

Chris Ferron

Thursday, 27 December Thu, 27 Dec

7:56 a.m.

New subject: [Powertop] Fwd: RE: [RFC] [PATCHv2 00/10] Code deduplication in cpu/*

Arjan, Can you take a look at this patch set. It looks good to me, but I wanted to make sure I am not missing something. There is duplication there, but not all. I have built and run some tests. Seems OK at a glance. -Chris -------- Original Message -------- Subject: RE: [Powertop] [RFC] [PATCHv2 00/10] Code deduplication in cpu/* Date: Mon, 24 Dec 2012 20:17:54 +0000 From: Ferron, Chris E <chris.e.ferron(a)intel.com> To: Sergey Senozhatsky <sergey.senozhatsky(a)gmail.com>, Chris Ferron <chris.e.ferron(a)linux.intel.com> CC: powertop(a)lists.01.org <powertop(a)lists.01.org> Thanks, I will attend to outstanding patches on Wednesday (GMT+8) when I return to the office. -Chris -----Original Message----- From: powertop-bounces(a)lists.01.org [mailto:powertop-bounces@lists.01.org] On Behalf Of Sergey Senozhatsky Sent: Monday, December 24, 2012 8:25 AM To: Chris Ferron Cc: powertop(a)lists.01.org Subject: Re: [Powertop] [RFC] [PATCHv2 00/10] Code deduplication in cpu/* On (12/24/12 03:08), Ivan Shapovalov wrote:

...