Discussion:
Various powertop patches
Henry Gebhardt
2012-04-10 16:37:37 UTC
Permalink
Dear lesswatts discussers,

Here are a few mostly small patches to get powertop to work on my x86_64
laptop running Gentoo.

Please consider applying them.

Sincerely,

Henry

[PATCH 01/10] Makefile: Fix linking with libnl-genl-3.0
[PATCH 02/10] report: Get "OS Information" from /etc/os-release
[PATCH 03/10] .gitignore for po/powertop.pot
[PATCH 04/10] minor code cleanup
[PATCH 05/10] do_process: optimize handle_trace_point()
[PATCH 06/10] main: use -t option to set update interval in
[PATCH 07/10] patches: ported to linux-3.3.0
[PATCH 08/10] README: add needed kernel options
[PATCH 09/10] Makefile: Remove unused variables
[PATCH 10/10] main: Add 'i' key event to update display
Henry Gebhardt
2012-04-10 16:37:38 UTC
Permalink
---
Makefile | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 59d827e..7315a2f 100644
--- a/Makefile
+++ b/Makefile
@@ -38,8 +38,7 @@ endif

ifeq ($(NL3FOUND),Y)
CFLAGS += -DCONFIG_LIBNL20
-LIBS += -lnl-genl
-NLLIBNAME = libnl-3.0
+NLLIBNAME = libnl-3.0 libnl-genl-3.0
endif

ifeq ($(NLLIBNAME),)
--
1.7.8.5
Henry Gebhardt
2012-04-10 16:37:39 UTC
Permalink
/etc/os-release is used by systemd.
---

I also have a more general patch that reads some more variables from
/etc/os-release. However, I think PRETTY_NAME is enough.

report.cpp | 31 +++++++++++++++++++++++++++++++
1 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/report.cpp b/report.cpp
index d203d52..47f2f37 100644
--- a/report.cpp
+++ b/report.cpp
@@ -84,6 +84,35 @@ string cpu_model(void)
return "";
}

+static string read_os_release(const string &filename)
+{
+ ifstream file;
+ char content[4096];
+ char *c;
+ const char *pname = "PRETTY_NAME=";
+ string os("");
+
+ file.open(filename.c_str(), ios::in);
+ if (!file)
+ return "";
+ while (file.getline(content, 4096)) {
+ if (strncasecmp(pname, content, strlen(pname)) == 0) {
+ c = strchr(content, '=');
+ if (!c)
+ break;
+ c += 1;
+ if (*c == '"' || *c == '\'')
+ c += 1;
+ *strchrnul(c, '"') = 0;
+ *strchrnul(c, '\'') = 0;
+ os = c;
+ break;
+ }
+ }
+ file.close();
+ return os;
+}
+
static void system_info(void)
{
string str, str2, str3;
@@ -133,6 +162,8 @@ static void system_info(void)
str = read_sysfs_string("/etc/system-release");
if (str.length() < 1)
str = read_sysfs_string("/etc/redhat-release");
+ if (str.length() < 1)
+ str = read_os_release("/etc/os-release");

if (reporttype) {
fprintf(reportout.http_report, "<tr class=\"system_even\"><td>OS Information</td><td>%s</td></tr>\n",
--
1.7.8.5
Henry Gebhardt
2012-04-10 16:37:40 UTC
Permalink
---
.gitignore | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/.gitignore b/.gitignore
index 48ac7ee..761bcc5 100644
--- a/.gitignore
+++ b/.gitignore
@@ -4,4 +4,4 @@ powertop
powertop.html
css.h
csstoh
-
+po/powertop.pot
--
1.7.8.5
Henry Gebhardt
2012-04-10 16:37:41 UTC
Permalink
---
main.cpp | 10 +++++-----
process/do_process.cpp | 1 -
2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/main.cpp b/main.cpp
index cc1c7d3..4ae70d1 100644
--- a/main.cpp
+++ b/main.cpp
@@ -68,7 +68,7 @@ static const struct option long_options[] =
{"help",no_argument, NULL, 'u'}, /* u for usage */
{"calibrate",no_argument, NULL, 'c'},
{"html", optional_argument, NULL, 'h'},
- {"csv", optional_argument, NULL, 'C'},
+ {"csv", optional_argument, NULL, 'C'},
{"extech", optional_argument, NULL, 'e'},
{"time", optional_argument, NULL, 't'},
{"iteration", optional_argument, NULL, 'i'},
@@ -340,9 +340,9 @@ int main(int argc, char **argv)
iterations = (optarg ? atoi(optarg) : 1);
break;

- case 'C': /* csv report*/
- wantreport = TRUE;
- reporttype = 0;
+ case 'C': /* csv report*/
+ wantreport = TRUE;
+ reporttype = 0;
sprintf(filename, "%s", optarg ? optarg : "powertop.csv");
break;
case '?': /* Unknown option */
@@ -359,7 +359,7 @@ int main(int argc, char **argv)



- learn_parameters(250, 0);
+ learn_parameters(250, 0);
save_parameters("saved_parameters.powertop");


diff --git a/process/do_process.cpp b/process/do_process.cpp
index f97b7e7..d98d55a 100644
--- a/process/do_process.cpp
+++ b/process/do_process.cpp
@@ -990,7 +990,6 @@ void process_process_data(void)
all_devices_to_all_power();

sort(all_power.begin(), all_power.end(), power_cpu_sort);
-
}
--
1.7.8.5
Henry Gebhardt
2012-04-10 16:37:43 UTC
Permalink
---
main.cpp | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/main.cpp b/main.cpp
index 4ae70d1..1b3ec90 100644
--- a/main.cpp
+++ b/main.cpp
@@ -381,7 +381,7 @@ int main(int argc, char **argv)


while (!leave_powertop) {
- one_measurement(20);
+ one_measurement(timetotest);
show_cur_tab();
learn_parameters(15, 0);
}
--
1.7.8.5
Henry Gebhardt
2012-04-10 16:37:42 UTC
Permalink
---
process/do_process.cpp | 30 +++++++++++++++---------------
1 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/process/do_process.cpp b/process/do_process.cpp
index d98d55a..e247f66 100644
--- a/process/do_process.cpp
+++ b/process/do_process.cpp
@@ -256,7 +256,7 @@ void perf_process_bundle::handle_trace_point(int type, void *trace, int cpu, uin
}
new_proc->waker = NULL;
}
- if (strcmp(event_name, "sched:sched_wakeup") == 0) {
+ else if (strcmp(event_name, "sched:sched_wakeup") == 0) {
struct wakeup_entry *we;
class power_consumer *from = NULL;
class process *dest_proc, *from_proc;
@@ -298,7 +298,7 @@ void perf_process_bundle::handle_trace_point(int type, void *trace, int cpu, uin
from->xwakes ++ ;

}
- if (strcmp(event_name, "irq:irq_handler_entry") == 0) {
+ else if (strcmp(event_name, "irq:irq_handler_entry") == 0) {
struct irq_entry *irqe;
class interrupt *irq;
irqe = (struct irq_entry *)trace;
@@ -314,7 +314,7 @@ void perf_process_bundle::handle_trace_point(int type, void *trace, int cpu, uin

}

- if (strcmp(event_name, "irq:irq_handler_exit") == 0) {
+ else if (strcmp(event_name, "irq:irq_handler_exit") == 0) {
class interrupt *irq;
uint64_t t;

@@ -328,7 +328,7 @@ void perf_process_bundle::handle_trace_point(int type, void *trace, int cpu, uin
consumer_child_time(cpu, t);
}

- if (strcmp(event_name, "irq:softirq_entry") == 0) {
+ else if (strcmp(event_name, "irq:softirq_entry") == 0) {
struct softirq_entry *irqe;
class interrupt *irq;

@@ -349,7 +349,7 @@ void perf_process_bundle::handle_trace_point(int type, void *trace, int cpu, uin
irq->start_interrupt(time);
change_blame(cpu, irq, LEVEL_SOFTIRQ);
}
- if (strcmp(event_name, "irq:softirq_exit") == 0) {
+ else if (strcmp(event_name, "irq:softirq_exit") == 0) {
class interrupt *irq;
uint64_t t;

@@ -361,7 +361,7 @@ void perf_process_bundle::handle_trace_point(int type, void *trace, int cpu, uin
t = irq->end_interrupt(time);
consumer_child_time(cpu, t);
}
- if (strcmp(event_name, "timer:timer_expire_entry") == 0) {
+ else if (strcmp(event_name, "timer:timer_expire_entry") == 0) {
struct timer_expire *tmr;
class timer *timer;
tmr = (struct timer_expire *)trace;
@@ -377,7 +377,7 @@ void perf_process_bundle::handle_trace_point(int type, void *trace, int cpu, uin
if (strcmp(timer->handler, "delayed_work_timer_fn"))
change_blame(cpu, timer, LEVEL_TIMER);
}
- if (strcmp(event_name, "timer:timer_expire_exit") == 0) {
+ else if (strcmp(event_name, "timer:timer_expire_exit") == 0) {
class timer *timer;
struct timer_cancel *tmr;
uint64_t t;
@@ -391,7 +391,7 @@ void perf_process_bundle::handle_trace_point(int type, void *trace, int cpu, uin
t = timer->done(time, (uint64_t)tmr->timer);
consumer_child_time(cpu, t);
}
- if (strcmp(event_name, "timer:hrtimer_expire_entry") == 0) {
+ else if (strcmp(event_name, "timer:hrtimer_expire_entry") == 0) {
struct hrtimer_expire *tmr;
class timer *timer;
tmr = (struct hrtimer_expire *)trace;
@@ -405,7 +405,7 @@ void perf_process_bundle::handle_trace_point(int type, void *trace, int cpu, uin
if (strcmp(timer->handler, "delayed_work_timer_fn"))
change_blame(cpu, timer, LEVEL_TIMER);
}
- if (strcmp(event_name, "timer:hrtimer_expire_exit") == 0) {
+ else if (strcmp(event_name, "timer:hrtimer_expire_exit") == 0) {
class timer *timer;
struct timer_cancel *tmr;
uint64_t t;
@@ -419,7 +419,7 @@ void perf_process_bundle::handle_trace_point(int type, void *trace, int cpu, uin
t = timer->done(time, (uint64_t)tmr->timer);
consumer_child_time(cpu, t);
}
- if (strcmp(event_name, "workqueue:workqueue_execute_start") == 0) {
+ else if (strcmp(event_name, "workqueue:workqueue_execute_start") == 0) {
struct workqueue_start *wq;
class work *work;
wq = (struct workqueue_start *)trace;
@@ -433,7 +433,7 @@ void perf_process_bundle::handle_trace_point(int type, void *trace, int cpu, uin
if (strcmp(work->handler, "do_dbs_timer") != 0 && strcmp(work->handler, "vmstat_update") != 0)
change_blame(cpu, work, LEVEL_WORK);
}
- if (strcmp(event_name, "workqueue:workqueue_execute_end") == 0) {
+ else if (strcmp(event_name, "workqueue:workqueue_execute_end") == 0) {
class work *work;
struct workqueue_end *wq;
uint64_t t;
@@ -447,13 +447,13 @@ void perf_process_bundle::handle_trace_point(int type, void *trace, int cpu, uin
t = work->done(time, (uint64_t)wq->work);
consumer_child_time(cpu, t);
}
- if (strcmp(event_name, "power:power_start") == 0) {
+ else if (strcmp(event_name, "power:power_start") == 0) {
set_wakeup_pending(cpu);
}
- if (strcmp(event_name, "power:power_end") == 0) {
+ else if (strcmp(event_name, "power:power_end") == 0) {
consume_blame(cpu);
}
- if (strcmp(event_name, "i915:i915_gem_ring_dispatch") == 0
+ else if (strcmp(event_name, "i915:i915_gem_ring_dispatch") == 0
|| strcmp(event_name, "i915:i915_gem_request_submit") == 0) {
/* any kernel contains only one of the these tracepoints,
* the latter one got replaced by the former one */
@@ -480,7 +480,7 @@ void perf_process_bundle::handle_trace_point(int type, void *trace, int cpu, uin
consumer->gpu_ops++;
}
}
- if (strcmp(event_name, "writeback:writeback_inode_dirty") == 0) {
+ else if (strcmp(event_name, "writeback:writeback_inode_dirty") == 0) {
static uint64_t prev_time;
class power_consumer *consumer;
struct dirty_inode *drty;
--
1.7.8.5
Henry Gebhardt
2012-04-10 16:37:45 UTC
Permalink
---
README | 24 ++++++++++++++++++++++++
1 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/README b/README
index 6dc6f40..1eacfa7 100644
--- a/README
+++ b/README
@@ -28,6 +28,30 @@ and a functional glibc/pthreads development environment



+Kernel Parameters:
+------------------
+
+PowerTOP needs some kernel config options enabled in order function properly.
+As of linux-3.3.0 these are (list probably incomplete):
+
+CONFIG_NO_HZ
+CONFIG_HIGH_RES_TIMERS
+CONFIG_HPET_TIMER
+CONFIG_CPU_FREQ_GOV_ONDEMAND
+CONFIG_USB_SUSPEND
+CONFIG_SND_AC97_POWER_SAVE
+CONFIG_TIMER_STATS
+CONFIG_PERF_EVENTS
+CONFIG_PERF_COUNTERS
+CONFIG_TRACEPOINTS
+CONFIG_TRACING
+CONFIG_EVENT_POWER_TRACING_DEPRECATED
+
+The patches in the patches/ subdirectory are required for PowerTOP to function
+fully.
+
+
+
Outputting a report
-------------------
When invoking PowerTOP without arguments, it goes into interactive mode.
--
1.7.8.5
Henry Gebhardt
2012-04-10 16:37:44 UTC
Permalink
---
patches/linux-3.3.0-ahci-alpm-accounting.patch | 307 ++++++++++++++++++++++++
patches/linux-3.3.0-vfs-dirty-inode.patch | 103 ++++++++
2 files changed, 410 insertions(+), 0 deletions(-)
create mode 100644 patches/linux-3.3.0-ahci-alpm-accounting.patch
create mode 100644 patches/linux-3.3.0-vfs-dirty-inode.patch

diff --git a/patches/linux-3.3.0-ahci-alpm-accounting.patch b/patches/linux-3.3.0-ahci-alpm-accounting.patch
new file mode 100644
index 0000000..57faa98
--- /dev/null
+++ b/patches/linux-3.3.0-ahci-alpm-accounting.patch
@@ -0,0 +1,307 @@
+From 3d1f00627a8db9d71091db32cd8109962f69ec43 Mon Sep 17 00:00:00 2001
+From: Arjan van de Ven <arjan-VuQAYsv1563Yd54FQh9/***@public.gmane.org>
+Date: Sat, 31 Mar 2012 20:35:12 +0200
+Subject: [PATCH 2/2] libata: Add ALPM power state accounting to the AHCI
+ driver
+
+PowerTOP wants to be able to show the user how effective the ALPM link
+power management is for the user. ALPM is worth around 0.5W on a quiet
+link; PowerTOP wants to be able to find cases where the "quiet link" isn't
+actually quiet.
+
+This patch adds state accounting functionality to the AHCI driver for
+PowerTOP to use.
+The parts of the patch are
+1) the sysfs logic of exposing the stats for each state in sysfs
+2) the basic accounting logic that gets update on link change interrupts
+ (or when the user accesses the info from sysfs)
+3) a "accounting enable" flag; in order to get the accounting to work,
+ the driver needs to get phyrdy interrupts on link status changes.
+ Normally and currently this is disabled by the driver when ALPM is
+ on (to reduce overhead); when PowerTOP is running this will need
+ to be on to get usable statistics... hence the sysfs tunable.
+
+The PowerTOP output currently looks like this:
+
+Recent SATA AHCI link activity statistics
+Active Partial Slumber Device name
+ 0.5% 99.5% 0.0% host0
+
+(work to resolve "host0" to a more human readable name is in progress)
+
+Signed-off-by: Arjan van de Ven <arjan-VuQAYsv1563Yd54FQh9/***@public.gmane.org>
+---
+ drivers/ata/ahci.h | 15 ++++
+ drivers/ata/libahci.c | 187 ++++++++++++++++++++++++++++++++++++++++++++++++-
+ 2 files changed, 200 insertions(+), 2 deletions(-)
+
+diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
+index b175000..38297f9 100644
+--- a/drivers/ata/ahci.h
++++ b/drivers/ata/ahci.h
+@@ -264,6 +264,13 @@ struct ahci_em_priv {
+ unsigned long led_state;
+ };
+
++enum ahci_port_states {
++ AHCI_PORT_NOLINK = 0,
++ AHCI_PORT_ACTIVE = 1,
++ AHCI_PORT_PARTIAL = 2,
++ AHCI_PORT_SLUMBER = 3
++};
++
+ struct ahci_port_priv {
+ struct ata_link *active_link;
+ struct ahci_cmd_hdr *cmd_slot;
+@@ -282,6 +289,14 @@ struct ahci_port_priv {
+ int fbs_last_dev; /* save FBS.DEV of last FIS */
+ /* enclosure management info per PM slot */
+ struct ahci_em_priv em_priv[EM_MAX_SLOTS];
++
++ /* ALPM accounting state and stats */
++ unsigned int accounting_active:1;
++ u64 active_jiffies;
++ u64 partial_jiffies;
++ u64 slumber_jiffies;
++ int previous_state;
++ int previous_jiffies;
+ };
+
+ struct ahci_host_priv {
+diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
+index a72bfd0..5859215 100644
+--- a/drivers/ata/libahci.c
++++ b/drivers/ata/libahci.c
+@@ -58,6 +58,17 @@ MODULE_PARM_DESC(ignore_sss, "Ignore staggered spinup flag (0=don't ignore, 1=ig
+
+ static int ahci_set_lpm(struct ata_link *link, enum ata_lpm_policy policy,
+ unsigned hints);
++static ssize_t ahci_alpm_show_active(struct device *dev,
++ struct device_attribute *attr, char *buf);
++static ssize_t ahci_alpm_show_slumber(struct device *dev,
++ struct device_attribute *attr, char *buf);
++static ssize_t ahci_alpm_show_partial(struct device *dev,
++ struct device_attribute *attr, char *buf);
++static ssize_t ahci_alpm_show_accounting(struct device *dev,
++ struct device_attribute *attr, char *buf);
++static ssize_t ahci_alpm_set_accounting(struct device *dev,
++ struct device_attribute *attr,
++ const char *buf, size_t count);
+ static ssize_t ahci_led_show(struct ata_port *ap, char *buf);
+ static ssize_t ahci_led_store(struct ata_port *ap, const char *buf,
+ size_t size);
+@@ -118,6 +129,12 @@ static DEVICE_ATTR(ahci_host_caps, S_IRUGO, ahci_show_host_caps, NULL);
+ static DEVICE_ATTR(ahci_host_cap2, S_IRUGO, ahci_show_host_cap2, NULL);
+ static DEVICE_ATTR(ahci_host_version, S_IRUGO, ahci_show_host_version, NULL);
+ static DEVICE_ATTR(ahci_port_cmd, S_IRUGO, ahci_show_port_cmd, NULL);
++static DEVICE_ATTR(ahci_alpm_active, S_IRUGO, ahci_alpm_show_active, NULL);
++static DEVICE_ATTR(ahci_alpm_partial, S_IRUGO, ahci_alpm_show_partial, NULL);
++static DEVICE_ATTR(ahci_alpm_slumber, S_IRUGO, ahci_alpm_show_slumber, NULL);
++static DEVICE_ATTR(ahci_alpm_accounting, S_IRUGO | S_IWUSR,
++ ahci_alpm_show_accounting, ahci_alpm_set_accounting);
++
+ static DEVICE_ATTR(em_buffer, S_IWUSR | S_IRUGO,
+ ahci_read_em_buffer, ahci_store_em_buffer);
+ static DEVICE_ATTR(em_message_supported, S_IRUGO, ahci_show_em_supported, NULL);
+@@ -130,6 +147,10 @@ struct device_attribute *ahci_shost_attrs[] = {
+ &dev_attr_ahci_host_cap2,
+ &dev_attr_ahci_host_version,
+ &dev_attr_ahci_port_cmd,
++ &dev_attr_ahci_alpm_active,
++ &dev_attr_ahci_alpm_partial,
++ &dev_attr_ahci_alpm_slumber,
++ &dev_attr_ahci_alpm_accounting,
+ &dev_attr_em_buffer,
+ &dev_attr_em_message_supported,
+ NULL
+@@ -673,9 +694,14 @@ static int ahci_set_lpm(struct ata_link *link, enum ata_lpm_policy policy,
+ * Disable interrupts on Phy Ready. This keeps us from
+ * getting woken up due to spurious phy ready
+ * interrupts.
++ *
++ * However, when accounting_active is set, we do want
++ * the interrupts for accounting purposes.
+ */
+- pp->intr_mask &= ~PORT_IRQ_PHYRDY;
+- writel(pp->intr_mask, port_mmio + PORT_IRQ_MASK);
++ if (!pp->accounting_active) {
++ pp->intr_mask &= ~PORT_IRQ_PHYRDY;
++ writel(pp->intr_mask, port_mmio + PORT_IRQ_MASK);
++ }
+
+ sata_link_scr_lpm(link, policy, false);
+ }
+@@ -1633,6 +1659,162 @@ static void ahci_error_intr(struct ata_port *ap, u32 irq_stat)
+ ata_port_abort(ap);
+ }
+
++static int get_current_alpm_state(struct ata_port *ap)
++{
++ u32 status = 0;
++
++ ahci_scr_read(&ap->link, SCR_STATUS, &status);
++
++ /* link status is in bits 11-8 */
++ status = status >> 8;
++ status = status & 0x7;
++
++ if (status == 6)
++ return AHCI_PORT_SLUMBER;
++ if (status == 2)
++ return AHCI_PORT_PARTIAL;
++ if (status == 1)
++ return AHCI_PORT_ACTIVE;
++ return AHCI_PORT_NOLINK;
++}
++
++static void account_alpm_stats(struct ata_port *ap)
++{
++ struct ahci_port_priv *pp;
++
++ int new_state;
++ u64 new_jiffies, jiffies_delta;
++
++ if (ap == NULL)
++ return;
++ pp = ap->private_data;
++
++ if (!pp) return;
++
++ new_state = get_current_alpm_state(ap);
++ new_jiffies = jiffies;
++
++ jiffies_delta = new_jiffies - pp->previous_jiffies;
++
++ switch (pp->previous_state) {
++ case AHCI_PORT_NOLINK:
++ pp->active_jiffies = 0;
++ pp->partial_jiffies = 0;
++ pp->slumber_jiffies = 0;
++ break;
++ case AHCI_PORT_ACTIVE:
++ pp->active_jiffies += jiffies_delta;
++ break;
++ case AHCI_PORT_PARTIAL:
++ pp->partial_jiffies += jiffies_delta;
++ break;
++ case AHCI_PORT_SLUMBER:
++ pp->slumber_jiffies += jiffies_delta;
++ break;
++ default:
++ break;
++ }
++ pp->previous_state = new_state;
++ pp->previous_jiffies = new_jiffies;
++}
++
++static ssize_t ahci_alpm_show_active(struct device *dev,
++ struct device_attribute *attr, char *buf)
++{
++ struct Scsi_Host *shost = class_to_shost(dev);
++ struct ata_port *ap = ata_shost_to_port(shost);
++ struct ahci_port_priv *pp;
++
++ if (!ap || ata_port_is_dummy(ap))
++ return -EINVAL;
++ pp = ap->private_data;
++ account_alpm_stats(ap);
++
++ return sprintf(buf, "%u\n", jiffies_to_msecs(pp->active_jiffies));
++}
++
++static ssize_t ahci_alpm_show_partial(struct device *dev,
++ struct device_attribute *attr, char *buf)
++{
++ struct Scsi_Host *shost = class_to_shost(dev);
++ struct ata_port *ap = ata_shost_to_port(shost);
++ struct ahci_port_priv *pp;
++
++ if (!ap || ata_port_is_dummy(ap))
++ return -EINVAL;
++
++ pp = ap->private_data;
++ account_alpm_stats(ap);
++
++ return sprintf(buf, "%u\n", jiffies_to_msecs(pp->partial_jiffies));
++}
++
++static ssize_t ahci_alpm_show_slumber(struct device *dev,
++ struct device_attribute *attr, char *buf)
++{
++ struct Scsi_Host *shost = class_to_shost(dev);
++ struct ata_port *ap = ata_shost_to_port(shost);
++ struct ahci_port_priv *pp;
++
++ if (!ap || ata_port_is_dummy(ap))
++ return -EINVAL;
++
++ pp = ap->private_data;
++
++ account_alpm_stats(ap);
++
++ return sprintf(buf, "%u\n", jiffies_to_msecs(pp->slumber_jiffies));
++}
++
++static ssize_t ahci_alpm_show_accounting(struct device *dev,
++ struct device_attribute *attr, char *buf)
++{
++ struct Scsi_Host *shost = class_to_shost(dev);
++ struct ata_port *ap = ata_shost_to_port(shost);
++ struct ahci_port_priv *pp;
++
++ if (!ap || ata_port_is_dummy(ap))
++ return -EINVAL;
++
++ pp = ap->private_data;
++
++ return sprintf(buf, "%u\n", pp->accounting_active);
++}
++
++static ssize_t ahci_alpm_set_accounting(struct device *dev,
++ struct device_attribute *attr,
++ const char *buf, size_t count)
++{
++ unsigned long flags;
++ struct Scsi_Host *shost = class_to_shost(dev);
++ struct ata_port *ap = ata_shost_to_port(shost);
++ struct ahci_port_priv *pp;
++ void __iomem *port_mmio;
++
++ if (!ap || ata_port_is_dummy(ap))
++ return 1;
++
++ pp = ap->private_data;
++ port_mmio = ahci_port_base(ap);
++
++ if (!pp)
++ return 1;
++ if (buf[0] == '0')
++ pp->accounting_active = 0;
++ if (buf[0] == '1')
++ pp->accounting_active = 1;
++
++ /* we need to enable the PHYRDY interrupt when we want accounting */
++ if (pp->accounting_active) {
++ spin_lock_irqsave(ap->lock, flags);
++ pp->intr_mask |= PORT_IRQ_PHYRDY;
++ writel(pp->intr_mask, port_mmio + PORT_IRQ_MASK);
++ spin_unlock_irqrestore(ap->lock, flags);
++ }
++ return count;
++}
++
++
+ static void ahci_port_intr(struct ata_port *ap)
+ {
+ void __iomem *port_mmio = ahci_port_base(ap);
+@@ -1653,6 +1835,7 @@ static void ahci_port_intr(struct ata_port *ap)
+ /* if LPM is enabled, PHYRDY doesn't mean anything */
+ if (ap->link.lpm_policy > ATA_LPM_MAX_POWER) {
+ status &= ~PORT_IRQ_PHYRDY;
++ account_alpm_stats(ap);
+ ahci_scr_write(&ap->link, SCR_ERROR, SERR_PHYRDY_CHG);
+ }
+
+--
+1.7.8.5
+
diff --git a/patches/linux-3.3.0-vfs-dirty-inode.patch b/patches/linux-3.3.0-vfs-dirty-inode.patch
new file mode 100644
index 0000000..837286e
--- /dev/null
+++ b/patches/linux-3.3.0-vfs-dirty-inode.patch
@@ -0,0 +1,103 @@
+From 9f7c5d04fdee46dbe715f2758152bb1664d4259c Mon Sep 17 00:00:00 2001
+From: Arjan van de Ven <arjan-VuQAYsv1563Yd54FQh9/***@public.gmane.org>
+Date: Fri, 26 Nov 2010 12:18:03 -0800
+Subject: [PATCH 1/2] vfs: Add a trace point in the mark_inode_dirty function
+
+PowerTOP would like to be able to show who is keeping the disk
+busy by dirtying data. The most logical spot for this is in the vfs
+in the mark_inode_dirty() function, doing this on the block level
+is not possible because by the time the IO hits the block layer the
+guilty party can no longer be found ("kjournald" and "pdflush" are not
+useful answers to "who caused this file to be dirty).
+
+The trace point follows the same logic/style as the block_dump code
+and pretty much dumps the same data, just not to dmesg (and thus to
+/var/log/messages) but via the trace events streams.
+
+Eventually we should be able to phase out the block dump code, but that's
+for later on after a transition time.
+---
+ fs/fs-writeback.c | 3 +++
+ include/linux/fs.h | 11 +++++++++++
+ include/trace/events/writeback.h | 28 ++++++++++++++++++++++++++++
+ 3 files changed, 42 insertions(+), 0 deletions(-)
+
+diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
+index 5b4a936..5ef5bb0 100644
+--- a/fs/fs-writeback.c
++++ b/fs/fs-writeback.c
+@@ -1081,6 +1081,9 @@ void __mark_inode_dirty(struct inode *inode, int flags)
+ if ((inode->i_state & flags) == flags)
+ return;
+
++ if (flags & (I_DIRTY_SYNC | I_DIRTY_DATASYNC | I_DIRTY_PAGES))
++ trace_writeback_inode_dirty(inode, flags);
++
+ if (unlikely(block_dump))
+ block_dump___mark_inode_dirty(inode);
+
+diff --git a/include/linux/fs.h b/include/linux/fs.h
+index 69cd5bb..e0ac37c 100644
+--- a/include/linux/fs.h
++++ b/include/linux/fs.h
+@@ -1768,6 +1768,18 @@ struct super_operations {
+
+ #define I_DIRTY (I_DIRTY_SYNC | I_DIRTY_DATASYNC | I_DIRTY_PAGES)
+
++#define INODE_DIRTY_FLAGS \
++ { I_DIRTY_SYNC, "DIRTY-SYNC" }, \
++ { I_DIRTY_DATASYNC, "DIRTY-DATASYNC" }, \
++ { I_DIRTY_PAGES, "DIRTY-PAGES" }, \
++ { I_NEW, "NEW" }, \
++ { I_WILL_FREE, "WILL-FREE" }, \
++ { I_FREEING, "FREEING" }, \
++ { I_CLEAR, "CLEAR" }, \
++ { I_SYNC, "SYNC" }, \
++ { I_REFERENCED, "REFERENCED" }
++
++
+ extern void __mark_inode_dirty(struct inode *, int);
+ static inline void mark_inode_dirty(struct inode *inode)
+ {
+diff --git a/include/trace/events/writeback.h b/include/trace/events/writeback.h
+index 5973410..5f1e2a3 100644
+--- a/include/trace/events/writeback.h
++++ b/include/trace/events/writeback.h
+@@ -408,6 +408,34 @@ DEFINE_EVENT(writeback_congest_waited_template, writeback_wait_iff_congested,
+ TP_ARGS(usec_timeout, usec_delayed)
+ );
+
++/*
++ * Tracepoint for dirtying an inode; used by PowerTOP
++ */
++TRACE_EVENT(writeback_inode_dirty,
++
++ TP_PROTO(struct inode *inode, int flags),
++
++ TP_ARGS(inode, flags),
++
++ TP_STRUCT__entry(
++ __field( __kernel_dev_t, dev )
++ __field( ino_t, ino )
++ __field( u32, flags )
++ ),
++
++ TP_fast_assign(
++ __entry->dev = inode->i_sb->s_dev;
++ __entry->ino = inode->i_ino;
++ __entry->flags = flags;
++ ),
++
++ TP_printk("dev %d:%d ino %lu flags %d %s", MAJOR(__entry->dev), MINOR(__entry->dev),
++ (unsigned long) __entry->ino,
++ __entry->flags,
++ __print_flags(__entry->flags, "|", INODE_DIRTY_FLAGS)
++ )
++);
++
+ DECLARE_EVENT_CLASS(writeback_single_inode_template,
+
+ TP_PROTO(struct inode *inode,
+--
+1.7.8.5
+
--
1.7.8.5
Henry Gebhardt
2012-04-10 16:37:46 UTC
Permalink
---
Makefile | 3 ---
1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 7315a2f..ac70767 100644
--- a/Makefile
+++ b/Makefile
@@ -3,8 +3,6 @@ all: powertop po/powertop.pot
VERSION := 1.98

CFLAGS += -Wall -O2 -g -fno-omit-frame-pointer -fstack-protector -Wshadow -Wformat -D_FORTIFY_SOURCE=2
-CPPFLAGS += -Wall -O2 -g -fno-omit-frame-pointer
-CXXFLAGS += -Wall -O2 -g -fno-omit-frame-pointer -fstack-protector -Wshadow -Wformat -D_FORTIFY_SOURCE=2
PKG_CONFIG ?= pkg-config

OBJS := lib.o main.o display.o report.o devlist.o
@@ -32,7 +30,6 @@ endif

ifeq ($(NL2FOUND),Y)
CFLAGS += -DCONFIG_LIBNL20
-LIBS += -lnl-genl
NLLIBNAME = libnl-2.0
endif
--
1.7.8.5
Henry Gebhardt
2012-04-10 16:37:47 UTC
Permalink
---
main.cpp | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/main.cpp b/main.cpp
index 1b3ec90..59be1fe 100644
--- a/main.cpp
+++ b/main.cpp
@@ -133,6 +133,8 @@ static void do_sleep(int seconds)
case 10:
cursor_enter();
break;
+ case 'i':
+ return;
case KEY_EXIT:
case 'q':
case 27:
--
1.7.8.5
Paul Menzel
2012-04-10 19:43:40 UTC
Permalink
Dear Henry,


thank you for the patches.
Post by Henry Gebhardt
---
main.cpp | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/main.cpp b/main.cpp
index 1b3ec90..59be1fe 100644
--- a/main.cpp
+++ b/main.cpp
@@ -133,6 +133,8 @@ static void do_sleep(int seconds)
cursor_enter();
break;
+ return;
Could you elaborate (in the commit message) why you chose »i« and not
_u_pdate or _r_efresh(?)?

Are these options documented in some manual or help text?
Thanks,

Paul
Arjan van de Ven
2012-04-10 19:45:10 UTC
Permalink
Post by Paul Menzel
Dear Henry,
thank you for the patches.
yeah I like the patches as well...

return;
Post by Paul Menzel
Could you elaborate (in the commit message) why you chose »i« and
not _u_pdate or _r_efresh(?)?
Are these options documented in some manual or help text?
ideally we show them at the bottom of the screen in the status bar.
Henry Gebhardt
2012-04-10 21:23:35 UTC
Permalink
Post by Arjan van de Ven
Post by Paul Menzel
Dear Henry,
thank you for the patches.
yeah I like the patches as well...
Thank you for the reviews.
Post by Arjan van de Ven
return;
Post by Paul Menzel
Could you elaborate (in the commit message) why you chose »i« and
not _u_pdate or _r_efresh(?)?
That is what I thought powertop-1.13 used... Just tried again, and
almost any keypress updates the display. I suppose 'i' stands for
_i_nterval?

I wouldn't mind changing it. Favourites?
Post by Arjan van de Ven
Post by Paul Menzel
Are these options documented in some manual or help text?
ideally we show them at the bottom of the screen in the status bar.
I will have to look into how to do that. Unless Sergey's patch already
does that?


Henry
Sergey Senozhatsky
2012-04-12 09:13:36 UTC
Permalink
Post by Henry Gebhardt
Post by Arjan van de Ven
Post by Paul Menzel
Dear Henry,
thank you for the patches.
yeah I like the patches as well...
Thank you for the reviews.
Post by Arjan van de Ven
return;
Post by Paul Menzel
Could you elaborate (in the commit message) why you chose »i« and
not _u_pdate or _r_efresh(?)?
That is what I thought powertop-1.13 used... Just tried again, and
almost any keypress updates the display. I suppose 'i' stands for
_i_nterval?
I wouldn't mind changing it. Favourites?
Post by Arjan van de Ven
Post by Paul Menzel
Are these options documented in some manual or help text?
ideally we show them at the bottom of the screen in the status bar.
I will have to look into how to do that. Unless Sergey's patch already
does that?
Sorry for the dealy.

Well, the patch does not do exactly the same. However, it provides a bit more --
it gives ability to introduce `refresh on-demand` to every tab_window in the
way each tab_window should be refreshed. E.g. tuning tab_window requires
[..]
add_usb_tunables();
add_runtime_tunables("pci");
add_ethernet_tunable();
add_bt_tunable();
add_wifi_tunables();
add_cpufreq_tunable();
[..]

and friedns to be executed (following the proper cleanup).


The patch has evolved since the time it was posted to the mailing list and
requires a bit of work to be published again.
It didn't get any NACK or ACK I'm aware of, and landed in my tree:
https://github.com/sergey-senozhatsky/powertop2-ss

There are also some additional functionality as well, which has been posted to the
list but still is missing in upstream:

-- set main tab refresh time via -s switch (as in top)
-- show user processes via -u switch (as in top) (implementation a bit sucks)
-- window refresh (for tunables only at this time)
-- some code cleanups/refactorings
-- etc.

In general it should be safe to merge with that tree, I'm trying to keep it in sync
with the upstream.

The only part that I'm really not proud of (probably should be reverted) is attempt
to support
power:cpu_frequency
power:cpu_idle
tracing API.


I'm sorry, but I really don't have much time right now to re-send all of the
patches, I probably would do that during upcoming weekend, unless they are
totally wrong and just a waste of bits.

So comments would be highly appreciated.


-ss
Sergey Senozhatsky
2012-04-10 20:10:58 UTC
Permalink
Post by Paul Menzel
Dear Henry,
thank you for the patches.
Post by Henry Gebhardt
---
main.cpp | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/main.cpp b/main.cpp
index 1b3ec90..59be1fe 100644
--- a/main.cpp
+++ b/main.cpp
@@ -133,6 +133,8 @@ static void do_sleep(int seconds)
cursor_enter();
break;
+ return;
Could you elaborate (in the commit message) why you chose »i« and not
_u_pdate or _r_efresh(?)?
I believe I've posted something similar more than a year ago
http://lists.lesswatts.org/pipermail/discuss/2011-February/000540.html
Post by Paul Menzel
Currently list of (un)tunables filled once - during tuning window creation and
remains untouched during it life time. For example, adding/removing USB
device while powertop is active will not give us the whole picture.
Patch introduces window_refresh and adds on-demand list refresh ability with
<r> key for tuning window.
But it didn't make its way to upstream.

The basic idea was to extend class tab_window with `virtual void window_refresh() {}'
function and override its implementation per window, e.g. tuning.


-ss
Post by Paul Menzel
Are these options documented in some manual or help text?
Thanks,
Paul
_______________________________________________
Discuss mailing list
http://lists.lesswatts.org/listinfo/discuss
Chris Ferron
2012-04-13 17:47:42 UTC
Permalink
Post by Henry Gebhardt
Dear lesswatts discussers,
Here are a few mostly small patches to get powertop to work on my x86_64
laptop running Gentoo.
Please consider applying them.
Sincerely,
Henry
[PATCH 01/10] Makefile: Fix linking with libnl-genl-3.0
[PATCH 02/10] report: Get "OS Information" from /etc/os-release
[PATCH 03/10] .gitignore for po/powertop.pot
[PATCH 04/10] minor code cleanup
[PATCH 05/10] do_process: optimize handle_trace_point()
[PATCH 06/10] main: use -t option to set update interval in
[PATCH 07/10] patches: ported to linux-3.3.0
[PATCH 08/10] README: add needed kernel options
[PATCH 09/10] Makefile: Remove unused variables
[PATCH 10/10] main: Add 'i' key event to update display
Thank you for your patches. They have been committed to <powertop>
git-9UaJU3cA/F/QT0dZR+***@public.gmane.org:fenrus75/powertop.git.

Thanks,
Chris Ferron

Loading...