A series of patches towards limiting memory corruption and foot print
by Kalowsky, Daniel
Hi Powertop,
I've been working with the interactive mode and have run into cases where powertop will crash due to a series of memory corruptions. The following series of patches have helped to reduce the frequency of the issue, although not completely solved it. The issues arise much faster on platforms where there are constrained amounts of RAM to work within.
Patch 1 - When shutting down the interactive display, the display bits of memory is not correctly released. This patch provides a method for correctly doing so.
Patch 2 - Solving a documented memory leak with a non-elegant solution. The path either adds the bundle to the stack, or it forgets about it. If it is forgotten about, make sure to clear that memory before moving on. This is done with a simple flag variable being set.
Patch 3 - When the tuning window is updated, the current pointer is just set adrift and not properly free'd. This patch catches that issue and removes the dangling pointer by holding a reference to the pointer until it is reset or specifically free'd.
Patch 4 - Someone actually added in the code to create a onetime pretty-print array, this patch just puts it to use by setting the variable.
Patch 5 - Limiting the buffer copy to the size of the allocated buffer with snprintf.
Patch 6 - There exist some processes and entries that can and do extend beyond the length of these buffers. This limits those entries so as not to corrupt other memory on the system when in interactive mode.
Patch 7 - This is an untested patch, but follows along the same lines of Patch 6. It applies the same principals only for the report method.
Patch 8 - Creates a clean_shutdown function that can be used to cleanup the memory space at shutdown time. Calls upon parts of Patch 1 to make this happen.
There will more than likely be some more patches in the future as time permits.
7 years, 6 months
[PATCH v3 00/15] pull: third time lucky
by Sami Kerola
Hello,
The following 15 patches are re-submission. Reply indicators are related
to feedback from:
https://lists.01.org/pipermail/powertop/2014-August/001495.html
> 0- When creating your patches modify the patch set subject using: git
> format-patch --cover-letter --subject-prefix="PATCH Vx"... Where x is
> the next version for your patches. For your next version use v3.
> This very useful for sorting purposes on my inbox.
That makes sense. Done.
> 1- Every time you make changes to your patches you would need to send
> the entire set of patches with an updated version. This link
> explains patch best practices that we follow on PowerTOP and across
> the Linux kernel community:
> http://kernelnewbies.org/OPWfirstpatch#head-067c0e555973ae364180b2e3fbe90...
Are you sure kernel newbies page is telling to resubmit long patch sets
when there is only modest change is some in middle? IMHO resubmitting
individual patches that has changed when combined with a git remote
repository containing the full patch set is good way to do keep noise
level low.
But then again all projects has some habits that I don't understand/agree
with which shouldn't really be a problem. I will keep on resubmitting
patch sets when requested to do so (and protest a bit by having the git
remote available ;-)
> 3- It is a good practice to add the names of the people who reviewed
> your patches, at the commit paragraph Reviewed_by:[the name of the
> person].
Done.
> Here is my review to the 14 patches you sent on v2 of your code:
>
> P1, P2, P4, P5, P6, P10, P11, P12, P13, and P14:
> -Look good.
Great.
> P3:
> - Change the subject line to something more descriptive. Add a
> description of why are you making this change, and how it benefits
> PowerTOP. The subject lines and descriptions are very useful when
> bisecting the tree for debugging purposes.
Done: add src/cpu/rapl/.deps to .gitignore
> P7:
> - Please add a description to this patch. (Same reasoning as on P3)
See the patch 0007.
> P8:
> - I have a problem naming -h --html and -u --help. Please change -h to
> --help, and use another letter for --html for instance: r as of
> report would be more fitting. New users will want to do powertop -h
> and see the help not the html report and get totally lost. It is
> just matter of reusing the already established usability metaphors.
I did change -h to --help, which is ABI change to undocumented interface,
which may or may not better than having unconventional short option
assignment. BTW I think it would be good to add this change to release
notes, just that no-one can later tell breaking changes happen without
noise.
> P9:
> - Please abbreviate the subject and add a description to this patch.
Done.
> - To make this a feature-complete Sergey's feedback should be
> implemented: "...to extend ui_notify_user() so it can cover both
> cases: initialised ncurses -- calling mvprintw(), and uninitialised
> ncurses -- printf (to stdout). this will change a bit 'powertop
> --auto-tune' behaviour, since by default user will now see a 'list'
> of commands executed by powertop, but it's really easy to silent
> powertop via stdout redirection".
As Sergey mentioned the mvprintw() avoidance is not needed. But since I
figured out how to make both cases, ncurses and console, work without
pain a patch 0014 was added to the change set. At the moment one can say
usefulness of the change is close to zero, as the ui_notify_user() is not
reached in case of --auto-tune, but perhaps that changes in future. And
then it's nice to have a facility in place that just works.
The following changes since commit cdff23d6e40e31a68286a51e3c94c8a901a74091:
powertop: remove temporary file (2014-08-22 09:18:34 -0700)
are available in the git repository at:
git://github.com/kerolasa/powertop.git sami
for you to fetch changes up to 295c86c4f5fa22024792e8e850f303eec7f4b6a3:
allow none-ncurses messaging with ui_notify_user() (2014-09-07 21:52:48 +0100)
----------------------------------------------------------------
Sami Kerola (15):
configure: use vertical lists
configure: prefer AS_IF macro rather than shell if statement
add src/cpu/rapl/.deps to .gitignore
clean up Makefile.am files
fix clang++ compilation errors
remove unnecessary assignments
remove unnecessary code reported by cppcheck
add short options to usage, and reorder options in code
do not use ncurses with --auto-tune
improve manual groff syntax
fix couple typos
wrap lines in README.traceevent
clean up .gitignore file
add --auto-tune and --workload to manual page
allow none-ncurses messaging with ui_notify_user()
.gitignore | 110 ++++++++++-------------
Makefile.am | 14 ++-
README.traceevent | 21 +++--
configure.ac | 102 ++++++++++++++++-----
doc/powertop.8 | 149 ++++++++++++++++---------------
src/Makefile.am | 193 +++++++++++++++++++++++++++++++---------
src/cpu/cpu.cpp | 18 ++--
src/cpu/rapl/rapl_interface.cpp | 4 +-
src/devices/ahci.cpp | 3 +-
src/devices/alsa.cpp | 1 -
src/devices/device.cpp | 6 +-
src/devlist.cpp | 3 +-
src/display.cpp | 9 +-
src/lib.cpp | 19 ++--
src/lib.h | 4 +-
src/main.cpp | 181 +++++++++++++++++++------------------
src/process/do_process.cpp | 11 ++-
src/report/report.cpp | 3 +-
src/tuning/nl80211.h | 4 +-
src/tuning/runtime.cpp | 4 -
src/tuning/tuning.cpp | 11 +--
21 files changed, 533 insertions(+), 337 deletions(-)
--
2.1.0
7 years, 7 months
[PATCH] devfreq: add devfreq devices stats support
by Sanjay Singh Rawat
add window to show frequency stats for devfreq devices
Signed-off-by: Rajagopal Venkat <rajagopal.venkat(a)gmail.com>
Signed-off-by: Sanjay Singh Rawat <sanjay.rawat(a)linaro.org>
---
v2 - Show devfreq window on support basis. Check for empty devfreq
directory.
- Free the open dirp while exiting.
---
src/Makefile.am | 1 +
src/devices/devfreq.cpp | 367 ++++++++++++++++++++++++++++++++++++++++++++++
src/devices/devfreq.h | 75 ++++++++++
src/main.cpp | 9 ++
src/report/report-maker.h | 1 +
5 files changed, 453 insertions(+)
create mode 100644 src/devices/devfreq.cpp
create mode 100644 src/devices/devfreq.h
diff --git a/src/Makefile.am b/src/Makefile.am
index 311b75e..d2f1da7 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -39,6 +39,7 @@ powertop_SOURCES = \
devices/alsa.h \
devices/backlight.cpp \
devices/backlight.h \
+ devices/devfreq.cpp \
devices/device.cpp \
devices/device.h \
devices/gpu_rapl_device.cpp \
diff --git a/src/devices/devfreq.cpp b/src/devices/devfreq.cpp
new file mode 100644
index 0000000..3eb63e5
--- /dev/null
+++ b/src/devices/devfreq.cpp
@@ -0,0 +1,367 @@
+/*
+ * Copyright 2012, Linaro
+ *
+ * This file is part of PowerTOP
+ *
+ * This program file is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program in a file named COPYING; if not, write to the
+ * Free Software Foundation, Inc,
+ * 51 Franklin Street, Fifth Floor,
+ * Boston, MA 02110-1301 USA
+ * or just google for it.
+ *
+ * Authors:
+ * Rajagopal Venkat <rajagopal.venkat(a)linaro.org>
+ */
+
+#include <iostream>
+#include <fstream>
+
+#include <dirent.h>
+#include <stdlib.h>
+#include <time.h>
+#include <unistd.h>
+
+#include "device.h"
+#include "devfreq.h"
+#include "../display.h"
+#include "../cpu/cpu.h"
+#include "../report/report.h"
+#include "../report/report-maker.h"
+
+static bool is_enabled = true;
+static DIR *dir = NULL;
+
+static vector<class devfreq *> all_devfreq;
+
+devfreq::devfreq(const char* dpath): device()
+{
+ strncpy(dir_name, dpath, sizeof(dir_name));
+}
+
+uint64_t devfreq::parse_freq_time(char* pchr)
+{
+ char *cptr, *pptr = pchr;
+ uint64_t ctime;
+
+ cptr = strtok(pchr, " :");
+ while (cptr != NULL) {
+ cptr = strtok(NULL, " :");
+ if (cptr )
+ pptr = cptr;
+ }
+
+ ctime = strtoull(pptr, NULL, 10);
+ return ctime;
+}
+
+void devfreq::process_time_stamps()
+{
+ unsigned int i;
+ uint64_t active_time = 0;
+
+ sample_time = (1000000.0 * (stamp_after.tv_sec - stamp_before.tv_sec))
+ + ((stamp_after.tv_usec - stamp_before.tv_usec) );
+
+ for (i=0; i < dstates.size()-1; i++) {
+ struct frequency *state = dstates[i];
+ state->time_after = 1000 * (state->time_after - state->time_before);
+ active_time += state->time_after;
+ }
+ /* Compute idle time for the device */
+ dstates[i]->time_after = sample_time - active_time;
+}
+
+void devfreq::add_devfreq_freq_state(uint64_t freq, uint64_t time)
+{
+ struct frequency *state;
+
+ state = new(std::nothrow) struct frequency;
+ if (!state)
+ return;
+
+ memset(state, 0, sizeof(*state));
+ dstates.push_back(state);
+
+ state->freq = freq;
+ if (freq == 0)
+ strcpy(state->human_name, "Idle");
+ else
+ hz_to_human(freq, state->human_name);
+ state->time_before = time;
+}
+
+void devfreq::update_devfreq_freq_state(uint64_t freq, uint64_t time)
+{
+ unsigned int i;
+ struct frequency *state = NULL;
+
+ for(i=0; i < dstates.size(); i++) {
+ if (freq == dstates[i]->freq)
+ state = dstates[i];
+ }
+
+ if (state == NULL) {
+ add_devfreq_freq_state(freq, time);
+ return;
+ }
+
+ state->time_after = time;
+}
+
+void devfreq::parse_devfreq_trans_stat(char *dname)
+{
+ ifstream file;
+ char filename[256];
+
+ sprintf(filename, "/sys/class/devfreq/%s/trans_stat", dir_name);
+ file.open(filename);
+
+ if (!file)
+ return;
+
+ char line[1024];
+ char *c;
+
+ while (file) {
+ uint64_t freq;
+ uint64_t time;
+ char *pchr;
+
+ memset(line, 0, sizeof(line));
+ file.getline(line, sizeof(line));
+
+ pchr = strchr(line, '*');
+ pchr = (pchr != NULL) ? pchr+1 : line;
+
+ freq = strtoull(pchr, &c, 10);
+ if (!freq)
+ continue;
+
+ time = parse_freq_time(pchr);
+ update_devfreq_freq_state(freq, time);
+ }
+ file.close();
+}
+
+void devfreq::start_measurement(void)
+{
+ unsigned int i;
+ ifstream file;
+
+ for (i=0; i < dstates.size(); i++)
+ delete dstates[i];
+ dstates.resize(0);
+ sample_time = 0;
+
+ gettimeofday(&stamp_before, NULL);
+ parse_devfreq_trans_stat(dir_name);
+ /* add device idle state */
+ update_devfreq_freq_state(0, 0);
+}
+
+void devfreq::end_measurement(void)
+{
+ parse_devfreq_trans_stat(dir_name);
+ gettimeofday(&stamp_after, NULL);
+ process_time_stamps();
+}
+
+double devfreq::power_usage(struct result_bundle *result, struct parameter_bundle *bundle)
+{
+ return 0;
+}
+
+double devfreq::utilization(void)
+{
+ return 0;
+}
+
+void devfreq::fill_freq_utilization(unsigned int idx, char *buf)
+{
+ buf[0] = 0;
+
+ if (idx < dstates.size() && dstates[idx]) {
+ struct frequency *state = dstates[idx];
+ sprintf(buf, " %5.1f%% ", percentage(1.0 * state->time_after / sample_time));
+ }
+}
+
+void devfreq::fill_freq_name(unsigned int idx, char *buf)
+{
+ buf[0] = 0;
+
+ if (idx < dstates.size() && dstates[idx]) {
+ sprintf(buf, "%-15s", dstates[idx]->human_name);
+ }
+}
+
+void start_devfreq_measurement(void)
+{
+ unsigned int i;
+
+ for (i=0; i<all_devfreq.size(); i++)
+ all_devfreq[i]->start_measurement();
+}
+
+void end_devfreq_measurement(void)
+{
+ unsigned int i;
+
+ for (i=0; i<all_devfreq.size(); i++)
+ all_devfreq[i]->end_measurement();
+}
+
+static void devfreq_dev_callback(const char *d_name)
+{
+ devfreq *df = new(std::nothrow) class devfreq(d_name);
+ if (df)
+ all_devfreq.push_back(df);
+}
+
+void create_all_devfreq_devices(void)
+{
+ struct dirent *entry;
+ int num = 0;
+
+ std::string p = "/sys/class/devfreq/";
+ dir = opendir(p.c_str());
+ if (dir == NULL) {
+ fprintf(stderr, "Devfreq not enabled\n");
+ is_enabled = false;
+ return;
+ }
+
+ while((entry = readdir(dir)) != NULL)
+ num++;
+
+ if (num == 2) {
+ fprintf(stderr, "Devfreq not enabled\n");
+ is_enabled = false;
+ closedir(dir);
+ return;
+ }
+
+ callback fn = &devfreq_dev_callback;
+ process_directory(p.c_str(), fn);
+}
+
+void initialize_devfreq(void)
+{
+ if (is_enabled)
+ create_tab("Device Freq stats", _("Device Freq stats"));
+}
+
+void display_devfreq_devices(void)
+{
+ unsigned int i, j;
+ WINDOW *win;
+ char fline[1024];
+ char buf[128];
+
+ win = get_ncurses_win("Device Freq stats");
+ if (!win)
+ return;
+
+ wclear(win);
+ wmove(win, 2,0);
+
+ if (!is_enabled) {
+ wprintw(win, _(" Devfreq is not enabled"));
+ return;
+ }
+
+ if (!all_devfreq.size()) {
+ wprintw(win, _(" No devfreq devices available"));
+ return;
+ }
+
+ for (i=0; i<all_devfreq.size(); i++) {
+
+ class devfreq *df = all_devfreq[i];
+ wprintw(win, "\n%s\n", df->device_name());
+
+ for(j=0; j < df->dstates.size(); j++) {
+ memset(fline, 0, sizeof(fline));
+ strcpy(fline, "\t");
+ df->fill_freq_name(j, buf);
+ strcat(fline, buf);
+ df->fill_freq_utilization(j, buf);
+ strcat(fline, buf);
+ strcat(fline, "\n");
+ wprintw(win, fline);
+ }
+ wprintw(win, "\n");
+ }
+}
+
+void report_devfreq_devices(void)
+{
+ if (!is_enabled) {
+ return;
+ }
+
+/* todo: adapt to new report format */
+#if 0
+ char buffer[512];
+ unsigned int i, j;
+
+ report.begin_section(SECTION_DEVFREQ);
+ report.add_header("Device Frequency Report");
+
+ report.begin_table(TABLE_WIDE);
+ if (!all_devfreq.size()) {
+ report.begin_row();
+ report.add(" No devfreq devices available");
+ return;
+ }
+
+ for (i = 0; i < all_devfreq.size(); i++) {
+ buffer[0] = 0;
+ class devfreq *df = all_devfreq[i];
+
+ report.begin_row();
+ report.begin_cell(CELL_CPU_PSTATE_HEADER);
+ report.addf("%s", df->device_name());
+
+ for (j = 0; j < df->dstates.size(); j++) {
+ report.begin_row();
+ report.begin_cell(CELL_CPU_STATE_VALUE);
+ df->fill_freq_name(j, buffer);
+ report.add(buffer);
+ report.begin_cell(CELL_CPU_STATE_VALUE);
+ df->fill_freq_utilization(j, buffer);
+ report.add(buffer);
+ }
+ }
+#endif
+
+}
+
+void clear_all_devfreq()
+{
+ unsigned int i, j;
+
+ for (i=0; i < all_devfreq.size(); i++) {
+ class devfreq *df = all_devfreq[i];
+
+ for(j=0; j < df->dstates.size(); j++)
+ delete df->dstates[j];
+
+ df->dstates.resize(0);
+ delete df;
+ }
+ all_devfreq.clear();
+ /* close /sys/class/devfreq */
+ if (dir != NULL)
+ closedir(dir);
+}
diff --git a/src/devices/devfreq.h b/src/devices/devfreq.h
new file mode 100644
index 0000000..8ab5705
--- /dev/null
+++ b/src/devices/devfreq.h
@@ -0,0 +1,75 @@
+/*
+ * Copyright 2012, Linaro
+ *
+ * This file is part of PowerTOP
+ *
+ * This program file is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program in a file named COPYING; if not, write to the
+ * Free Software Foundation, Inc,
+ * 51 Franklin Street, Fifth Floor,
+ * Boston, MA 02110-1301 USA
+ * or just google for it.
+ *
+ * Authors:
+ * Rajagopal Venkat <rajagopal.venkat(a)linaro.org>
+ */
+#ifndef _INCLUDE_GUARD_DEVFREQ_H
+#define _INCLUDE_GUARD_DEVFREQ_H
+
+#include "device.h"
+#include "../parameters/parameters.h"
+
+struct frequency;
+
+class devfreq: public device {
+ char dir_name[4096];
+ struct timeval stamp_before, stamp_after;
+ double sample_time;
+
+ uint64_t parse_freq_time(char *ptr);
+ void add_devfreq_freq_state(uint64_t freq, uint64_t time);
+ void update_devfreq_freq_state(uint64_t freq, uint64_t time);
+ void parse_devfreq_trans_stat(char *dname);
+ void process_time_stamps();
+
+public:
+
+ vector<struct frequency *> dstates;
+
+ devfreq(const char *c);
+ void fill_freq_utilization(unsigned int idx, char *buf);
+ void fill_freq_name(unsigned int idx, char *buf);
+
+ virtual void start_measurement(void);
+ virtual void end_measurement(void);
+
+ virtual double utilization(void); /* percentage */
+
+ virtual const char * class_name(void) { return "devfreq";};
+
+ virtual const char * device_name(void) { return dir_name;};
+ virtual const char * human_name(void) { return "devfreq";};
+ virtual double power_usage(struct result_bundle *result, struct parameter_bundle *bundle);
+ virtual const char * util_units(void) { return " rpm"; };
+ virtual int power_valid(void) { return 0; /*utilization_power_valid(r_index);*/};
+ virtual int grouping_prio(void) { return 1; };
+};
+
+extern void create_all_devfreq_devices(void);
+extern void clear_all_devfreq(void);
+extern void display_devfreq_devices(void);
+extern void report_devfreq_devices(void);
+extern void initialize_devfreq(void);
+extern void start_devfreq_measurement(void);
+extern void end_devfreq_measurement(void);
+
+#endif
diff --git a/src/main.cpp b/src/main.cpp
index d3963ed..2162004 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -48,6 +48,7 @@
#include "devices/device.h"
+#include "devices/devfreq.h"
#include "devices/usb.h"
#include "devices/ahci.h"
#include "measurement/measurement.h"
@@ -201,6 +202,7 @@ void one_measurement(int seconds, char *workload)
create_all_usb_devices();
start_power_measurement();
devices_start_measurement();
+ start_devfreq_measurement();
start_process_measurement();
start_cpu_measurement();
@@ -213,6 +215,7 @@ void one_measurement(int seconds, char *workload)
end_cpu_measurement();
end_process_measurement();
collect_open_devices();
+ end_devfreq_measurement();
devices_end_measurement();
end_power_measurement();
@@ -240,6 +243,8 @@ void one_measurement(int seconds, char *workload)
report_show_open_devices();
report_devices();
+ display_devfreq_devices();
+ report_devfreq_devices();
ahci_create_device_stats_table();
store_results(measurement_time);
end_cpu_data();
@@ -353,6 +358,7 @@ static void powertop_init(void)
enumerate_cpus();
create_all_devices();
+ create_all_devfreq_devices();
detect_power_meters();
register_parameter("base power", 100, 0.5);
@@ -464,6 +470,8 @@ int main(int argc, char **argv)
}
if (!auto_tune)
init_display();
+
+ initialize_devfreq();
initialize_tuning();
/* first one is short to not let the user wait too long */
one_measurement(1, NULL);
@@ -500,6 +508,7 @@ int main(int argc, char **argv)
clean_open_devices();
clear_all_devices();
+ clear_all_devfreq();
clear_all_cpus();
return 0;
diff --git a/src/report/report-maker.h b/src/report/report-maker.h
index 423568a..bda4cef 100644
--- a/src/report/report-maker.h
+++ b/src/report/report-maker.h
@@ -89,6 +89,7 @@ enum section_type {
SECTION_SYSINFO,
SECTION_CPUIDLE,
SECTION_CPUFREQ,
+ SECTION_DEVFREQ,
SECTION_DEVPOWER,
SECTION_SOFTWARE,
SECTION_SUMMARY,
--
1.8.3.2
7 years, 7 months
[PATCH] devfreq: add devfreq devices stats support
by Sanjay Singh Rawat
add window to show frequency stats for devfreq devices
Signed-off-by: Rajagopal Venkat <rajagopal.venkat(a)gmail.com>
Signed-off-by: Sanjay Singh Rawat <sanjay.rawat(a)linaro.org>
---
src/Makefile.am | 2 +-
src/devices/devfreq.cpp | 351 ++++++++++++++++++++++++++++++++++++++++++++++
src/devices/devfreq.h | 75 ++++++++++
src/main.cpp | 9 ++
src/report/report-maker.h | 1 +
5 files changed, 437 insertions(+), 1 deletion(-)
create mode 100644 src/devices/devfreq.cpp
create mode 100644 src/devices/devfreq.h
diff --git a/src/Makefile.am b/src/Makefile.am
index 6886388..fee1ffa 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -32,7 +32,7 @@ powertop_SOURCES = parameters/persistent.cpp parameters/learn.cpp parameters/par
report/report-formatter-csv.cpp report/report-formatter-csv.h \
report/report-formatter-html.cpp report/report-formatter-html.h \
report/report-data-html.cpp report/report-data-html.h \
- main.cpp css.h powertop.css cpu/intel_gpu.cpp \
+ main.cpp css.h powertop.css cpu/intel_gpu.cpp devices/devfreq.cpp \
cpu/rapl/rapl_interface.cpp cpu/cpu_rapl_device.cpp cpu/rapl/rapl_interface.h\
cpu/dram_rapl_device.cpp devices/gpu_rapl_device.cpp cpu/cpu_rapl_device.h \
cpu/dram_rapl_device.h devices/gpu_rapl_device.h
diff --git a/src/devices/devfreq.cpp b/src/devices/devfreq.cpp
new file mode 100644
index 0000000..29d7790
--- /dev/null
+++ b/src/devices/devfreq.cpp
@@ -0,0 +1,351 @@
+/*
+ * Copyright 2012, Linaro
+ *
+ * This file is part of PowerTOP
+ *
+ * This program file is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program in a file named COPYING; if not, write to the
+ * Free Software Foundation, Inc,
+ * 51 Franklin Street, Fifth Floor,
+ * Boston, MA 02110-1301 USA
+ * or just google for it.
+ *
+ * Authors:
+ * Rajagopal Venkat <rajagopal.venkat(a)linaro.org>
+ */
+
+#include <iostream>
+#include <fstream>
+
+#include <dirent.h>
+#include <stdlib.h>
+#include <time.h>
+#include <unistd.h>
+
+#include "device.h"
+#include "devfreq.h"
+#include "../display.h"
+#include "../cpu/cpu.h"
+#include "../report/report.h"
+#include "../report/report-maker.h"
+
+static bool is_enabled = true;
+
+static vector<class devfreq *> all_devfreq;
+
+devfreq::devfreq(const char* dpath): device()
+{
+ strncpy(dir_name, dpath, sizeof(dir_name));
+}
+
+uint64_t devfreq::parse_freq_time(char* pchr)
+{
+ char *cptr, *pptr = pchr;
+ uint64_t ctime;
+
+ cptr = strtok(pchr, " :");
+ while (cptr != NULL) {
+ cptr = strtok(NULL, " :");
+ if (cptr )
+ pptr = cptr;
+ }
+
+ ctime = strtoull(pptr, NULL, 10);
+ return ctime;
+}
+
+void devfreq::process_time_stamps()
+{
+ unsigned int i;
+ uint64_t active_time = 0;
+
+ sample_time = (1000000.0 * (stamp_after.tv_sec - stamp_before.tv_sec))
+ + ((stamp_after.tv_usec - stamp_before.tv_usec) );
+
+ for (i=0; i < dstates.size()-1; i++) {
+ struct frequency *state = dstates[i];
+ state->time_after = 1000 * (state->time_after - state->time_before);
+ active_time += state->time_after;
+ }
+ /* Compute idle time for the device */
+ dstates[i]->time_after = sample_time - active_time;
+}
+
+void devfreq::add_devfreq_freq_state(uint64_t freq, uint64_t time)
+{
+ struct frequency *state;
+
+ state = new(std::nothrow) struct frequency;
+ if (!state)
+ return;
+
+ memset(state, 0, sizeof(*state));
+ dstates.push_back(state);
+
+ state->freq = freq;
+ if (freq == 0)
+ strcpy(state->human_name, "Idle");
+ else
+ hz_to_human(freq, state->human_name);
+ state->time_before = time;
+}
+
+void devfreq::update_devfreq_freq_state(uint64_t freq, uint64_t time)
+{
+ unsigned int i;
+ struct frequency *state = NULL;
+
+ for(i=0; i < dstates.size(); i++) {
+ if (freq == dstates[i]->freq)
+ state = dstates[i];
+ }
+
+ if (state == NULL) {
+ add_devfreq_freq_state(freq, time);
+ return;
+ }
+
+ state->time_after = time;
+}
+
+void devfreq::parse_devfreq_trans_stat(char *dname)
+{
+ ifstream file;
+ char filename[256];
+
+ sprintf(filename, "/sys/class/devfreq/%s/trans_stat", dir_name);
+ file.open(filename);
+
+ if (!file)
+ return;
+
+ char line[1024];
+ char *c;
+
+ while (file) {
+ uint64_t freq;
+ uint64_t time;
+ char *pchr;
+
+ memset(line, 0, sizeof(line));
+ file.getline(line, sizeof(line));
+
+ pchr = strchr(line, '*');
+ pchr = (pchr != NULL) ? pchr+1 : line;
+
+ freq = strtoull(pchr, &c, 10);
+ if (!freq)
+ continue;
+
+ time = parse_freq_time(pchr);
+ update_devfreq_freq_state(freq, time);
+ }
+ file.close();
+}
+
+void devfreq::start_measurement(void)
+{
+ unsigned int i;
+ ifstream file;
+
+ for (i=0; i < dstates.size(); i++)
+ delete dstates[i];
+ dstates.resize(0);
+ sample_time = 0;
+
+ gettimeofday(&stamp_before, NULL);
+ parse_devfreq_trans_stat(dir_name);
+ /* add device idle state */
+ update_devfreq_freq_state(0, 0);
+}
+
+void devfreq::end_measurement(void)
+{
+ parse_devfreq_trans_stat(dir_name);
+ gettimeofday(&stamp_after, NULL);
+ process_time_stamps();
+}
+
+double devfreq::power_usage(struct result_bundle *result, struct parameter_bundle *bundle)
+{
+ return 0;
+}
+
+double devfreq::utilization(void)
+{
+ return 0;
+}
+
+void devfreq::fill_freq_utilization(unsigned int idx, char *buf)
+{
+ buf[0] = 0;
+
+ if (idx < dstates.size() && dstates[idx]) {
+ struct frequency *state = dstates[idx];
+ sprintf(buf, " %5.1f%% ", percentage(1.0 * state->time_after / sample_time));
+ }
+}
+
+void devfreq::fill_freq_name(unsigned int idx, char *buf)
+{
+ buf[0] = 0;
+
+ if (idx < dstates.size() && dstates[idx]) {
+ sprintf(buf, "%-15s", dstates[idx]->human_name);
+ }
+}
+
+void start_devfreq_measurement(void)
+{
+ unsigned int i;
+
+ for (i=0; i<all_devfreq.size(); i++)
+ all_devfreq[i]->start_measurement();
+}
+
+void end_devfreq_measurement(void)
+{
+ unsigned int i;
+
+ for (i=0; i<all_devfreq.size(); i++)
+ all_devfreq[i]->end_measurement();
+}
+
+static void devfreq_dev_callback(const char *d_name)
+{
+ devfreq *df = new(std::nothrow) class devfreq(d_name);
+ if (df)
+ all_devfreq.push_back(df);
+}
+
+void create_all_devfreq_devices(void)
+{
+ DIR *dir;
+ std::string p = "/sys/class/devfreq/";
+ dir = opendir(p.c_str());
+ if (dir == NULL) {
+ fprintf(stderr, "Devfreq not enabled\n");
+ is_enabled = false;
+ return;
+ }
+
+ callback fn = &devfreq_dev_callback;
+ process_directory(p.c_str(), fn);
+}
+
+void initialize_devfreq(void)
+{
+ if (is_enabled)
+ create_tab("Device Freq stats", _("Device Freq stats"));
+}
+
+void display_devfreq_devices(void)
+{
+ unsigned int i, j;
+ WINDOW *win;
+ char fline[1024];
+ char buf[128];
+
+ win = get_ncurses_win("Device Freq stats");
+ if (!win)
+ return;
+
+ wclear(win);
+ wmove(win, 2,0);
+
+ if (!is_enabled) {
+ wprintw(win, _(" Devfreq is not enabled"));
+ return;
+ }
+
+ if (!all_devfreq.size()) {
+ wprintw(win, _(" No devfreq devices available"));
+ return;
+ }
+
+ for (i=0; i<all_devfreq.size(); i++) {
+
+ class devfreq *df = all_devfreq[i];
+ wprintw(win, "\n%s\n", df->device_name());
+
+ for(j=0; j < df->dstates.size(); j++) {
+ memset(fline, 0, sizeof(fline));
+ strcpy(fline, "\t");
+ df->fill_freq_name(j, buf);
+ strcat(fline, buf);
+ df->fill_freq_utilization(j, buf);
+ strcat(fline, buf);
+ strcat(fline, "\n");
+ wprintw(win, fline);
+ }
+ wprintw(win, "\n");
+ }
+}
+
+void report_devfreq_devices(void)
+{
+ if (!is_enabled) {
+ return;
+ }
+
+/* todo: adapt to new report format */
+#if 0
+ char buffer[512];
+ unsigned int i, j;
+
+ report.begin_section(SECTION_DEVFREQ);
+ report.add_header("Device Frequency Report");
+
+ report.begin_table(TABLE_WIDE);
+ if (!all_devfreq.size()) {
+ report.begin_row();
+ report.add(" No devfreq devices available");
+ return;
+ }
+
+ for (i = 0; i < all_devfreq.size(); i++) {
+ buffer[0] = 0;
+ class devfreq *df = all_devfreq[i];
+
+ report.begin_row();
+ report.begin_cell(CELL_CPU_PSTATE_HEADER);
+ report.addf("%s", df->device_name());
+
+ for (j = 0; j < df->dstates.size(); j++) {
+ report.begin_row();
+ report.begin_cell(CELL_CPU_STATE_VALUE);
+ df->fill_freq_name(j, buffer);
+ report.add(buffer);
+ report.begin_cell(CELL_CPU_STATE_VALUE);
+ df->fill_freq_utilization(j, buffer);
+ report.add(buffer);
+ }
+ }
+#endif
+
+}
+
+void clear_all_devfreq()
+{
+ unsigned int i, j;
+
+ for (i=0; i < all_devfreq.size(); i++) {
+ class devfreq *df = all_devfreq[i];
+
+ for(j=0; j < df->dstates.size(); j++)
+ delete df->dstates[j];
+
+ df->dstates.resize(0);
+ delete df;
+ }
+ all_devfreq.clear();
+}
diff --git a/src/devices/devfreq.h b/src/devices/devfreq.h
new file mode 100644
index 0000000..8ab5705
--- /dev/null
+++ b/src/devices/devfreq.h
@@ -0,0 +1,75 @@
+/*
+ * Copyright 2012, Linaro
+ *
+ * This file is part of PowerTOP
+ *
+ * This program file is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program in a file named COPYING; if not, write to the
+ * Free Software Foundation, Inc,
+ * 51 Franklin Street, Fifth Floor,
+ * Boston, MA 02110-1301 USA
+ * or just google for it.
+ *
+ * Authors:
+ * Rajagopal Venkat <rajagopal.venkat(a)linaro.org>
+ */
+#ifndef _INCLUDE_GUARD_DEVFREQ_H
+#define _INCLUDE_GUARD_DEVFREQ_H
+
+#include "device.h"
+#include "../parameters/parameters.h"
+
+struct frequency;
+
+class devfreq: public device {
+ char dir_name[4096];
+ struct timeval stamp_before, stamp_after;
+ double sample_time;
+
+ uint64_t parse_freq_time(char *ptr);
+ void add_devfreq_freq_state(uint64_t freq, uint64_t time);
+ void update_devfreq_freq_state(uint64_t freq, uint64_t time);
+ void parse_devfreq_trans_stat(char *dname);
+ void process_time_stamps();
+
+public:
+
+ vector<struct frequency *> dstates;
+
+ devfreq(const char *c);
+ void fill_freq_utilization(unsigned int idx, char *buf);
+ void fill_freq_name(unsigned int idx, char *buf);
+
+ virtual void start_measurement(void);
+ virtual void end_measurement(void);
+
+ virtual double utilization(void); /* percentage */
+
+ virtual const char * class_name(void) { return "devfreq";};
+
+ virtual const char * device_name(void) { return dir_name;};
+ virtual const char * human_name(void) { return "devfreq";};
+ virtual double power_usage(struct result_bundle *result, struct parameter_bundle *bundle);
+ virtual const char * util_units(void) { return " rpm"; };
+ virtual int power_valid(void) { return 0; /*utilization_power_valid(r_index);*/};
+ virtual int grouping_prio(void) { return 1; };
+};
+
+extern void create_all_devfreq_devices(void);
+extern void clear_all_devfreq(void);
+extern void display_devfreq_devices(void);
+extern void report_devfreq_devices(void);
+extern void initialize_devfreq(void);
+extern void start_devfreq_measurement(void);
+extern void end_devfreq_measurement(void);
+
+#endif
diff --git a/src/main.cpp b/src/main.cpp
index cf4e547..d33eaed 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -48,6 +48,7 @@
#include "devices/device.h"
+#include "devices/devfreq.h"
#include "devices/usb.h"
#include "devices/ahci.h"
#include "measurement/measurement.h"
@@ -194,6 +195,7 @@ void one_measurement(int seconds, char *workload)
create_all_usb_devices();
start_power_measurement();
devices_start_measurement();
+ start_devfreq_measurement();
start_process_measurement();
start_cpu_measurement();
@@ -206,6 +208,7 @@ void one_measurement(int seconds, char *workload)
end_cpu_measurement();
end_process_measurement();
collect_open_devices();
+ end_devfreq_measurement();
devices_end_measurement();
end_power_measurement();
@@ -233,6 +236,9 @@ void one_measurement(int seconds, char *workload)
report_show_open_devices();
report_devices();
+ display_devfreq_devices();
+ report_devfreq_devices();
+
ahci_create_device_stats_table();
store_results(measurement_time);
end_cpu_data();
@@ -344,6 +350,7 @@ static void powertop_init(void)
enumerate_cpus();
create_all_devices();
+ create_all_devfreq_devices();
detect_power_meters();
register_parameter("base power", 100, 0.5);
@@ -457,6 +464,7 @@ int main(int argc, char **argv)
exit(0);
}
init_display();
+ initialize_devfreq();
initialize_tuning();
/* first one is short to not let the user wait too long */
one_measurement(1, NULL);
@@ -491,6 +499,7 @@ int main(int argc, char **argv)
clean_open_devices();
clear_all_devices();
+ clear_all_devfreq();
clear_all_cpus();
return 0;
diff --git a/src/report/report-maker.h b/src/report/report-maker.h
index 423568a..bda4cef 100644
--- a/src/report/report-maker.h
+++ b/src/report/report-maker.h
@@ -89,6 +89,7 @@ enum section_type {
SECTION_SYSINFO,
SECTION_CPUIDLE,
SECTION_CPUFREQ,
+ SECTION_DEVFREQ,
SECTION_DEVPOWER,
SECTION_SOFTWARE,
SECTION_SUMMARY,
--
1.8.3.2
7 years, 7 months
[PATCH 0/1] Update target value of Power Aware CPU scheduler tunable
by Nanley Chery
Hello,
This patch fixes a problem made evident by the debian bug 655843,
'powertop report[s] incorrect "bad" value.'
The issue is that the tunable for the "Power Aware CPU scheduler" has an
outdated target value of "1". That value was correct in linux kernels
before the 2.6.29 release. However, starting with the 2.6.29 release,
the level setting for the scheduler that offered the greatest power
savings was increased to "2" (linux kernel commit:
afb8a9b70b86866a60e08b2956ae4e1406390336). Eventually, due to seemingly
poor code maintenance, the power aware scheduler feature was removed
altogther in the 3.5 linux kernel (linux kernel commit:
8e7fbcbc22c12414bcc9dfdd683637f58fb32759).
According to the README file, we currently support kernels that have the
updated power savings level (>= 2.6.38). Therefore, this patch updates
the target value to "2" and conditionally omits adding the tunable for
kernel versions that do not support the feature (>= 3.5).
Nanley Chery (1):
tunable: update sched_mc_power_savings target value
src/tuning/tuning.cpp | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
--
2.0.4
7 years, 7 months
[PATCH v2 1/1] tunables: check usb autosuspend support before add
by Nanley Chery
Some usb devices do not have drivers that support autosuspend.
These drivers should not be added to the list of tunables, because
toggling their good/bad value does nothing.
Signed-off-by: Nanley Chery <nanleychery(a)gmail.com>
---
Fixed the issue in the previous patch that allowed returning between
opendir and closedir (thanks Sergey). Also improve detection of interface
folders.
src/tuning/tuningusb.cpp | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/src/tuning/tuningusb.cpp b/src/tuning/tuningusb.cpp
index 27a6dca..d2a0c11 100644
--- a/src/tuning/tuningusb.cpp
+++ b/src/tuning/tuningusb.cpp
@@ -28,6 +28,7 @@
#include "unistd.h"
#include "tuningusb.h"
#include <string.h>
+#include <dirent.h>
#include <utility>
#include <iostream>
#include <fstream>
@@ -117,6 +118,7 @@ static void add_usb_callback(const char *d_name)
{
class usb_tunable *usb;
char filename[4096];
+ DIR *dir;
sprintf(filename, "/sys/bus/usb/devices/%s/power/control", d_name);
if (access(filename, R_OK) != 0)
@@ -126,6 +128,23 @@ static void add_usb_callback(const char *d_name)
if (access(filename, R_OK)!=0)
return;
+ /* every interface of this device should support autosuspend */
+ sprintf(filename, "/sys/bus/usb/devices/%s", d_name);
+ if ((dir = opendir(filename))) {
+ struct dirent *entry;
+ while ((entry = readdir(dir))) {
+ /* dirname: <busnum>-<devnum>...:<config num>-<interface num> */
+ if (!isdigit(entry->d_name[0]))
+ continue;
+ sprintf(filename, "/sys/bus/usb/devices/%s/%s/supports_autosuspend", d_name, entry->d_name);
+ if (access(filename, R_OK) == 0 && read_sysfs(filename) == 0)
+ break;
+ }
+ closedir(dir);
+ if (entry)
+ return;
+ }
+
sprintf(filename, "/sys/bus/usb/devices/%s", d_name);
usb = new class usb_tunable(filename, d_name);
all_tunables.push_back(usb);
--
2.0.4
7 years, 7 months
[PATCH RESEND] Powertop: remove unnecessary code
by Salman Ahmed
nr_open is already initialized by NR_OPEN_DEF. Removing unnecessary checking and
assignment.
Signed-off-by: Salman Ahmed <salman0yam(a)gmail.com>
---
src/main.cpp | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/main.cpp b/src/main.cpp
index cf4e547..990bc1f 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -293,8 +293,6 @@ static int get_nr_open(void) {
file.open("/proc/sys/fs/nr_open", ios::in);
if (file) {
file >> nr_open;
- if (!file)
- nr_open = NR_OPEN_DEF;
file.close();
}
return nr_open;
--
1.8.1.2
7 years, 7 months
