[PATCH 0/2] Add new features to powertop

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/2] Add new features to powertop

Amit Kucheria-6
From: Amit Kucheria <[hidden email]>

This pull request adds support for two new powertop features:

1. Add counter to HDA sound driver to expose ON and OFF accounting
2. New TRACE_EVENT in the mark_inode_dirty function to allow powertop to flag
programs that cause disk wakeups.

Please note, that these are only useful with the powertop version from git.
I'll work with our package maintainers to see how we can get an updated
version into Lucid.

Regards,
Amit

The following changes since commit 2908bd6316fcdb6b0edbdf3a0d43e32dc10f9eeb:
  Leann Ogasawara (1):
        UBUNTU: [SCSI] megaraid_sas: remove sysfs poll_mode_io world writeable
permissions

are available in the git repository at:

  git://zinc.ubuntu.com/amitk/lucid.git powertop

Arjan van de Ven (1):
  vfs: Add a trace point in the mark_inode_dirty function

Takashi Iwai (1):
  ALSA: hda - Add power on/off counter

 fs/fs-writeback.c          |    3 ++
 fs/inode.c                 |    4 +++
 include/trace/events/vfs.h |   53 ++++++++++++++++++++++++++++++++++++++++++++
 sound/pci/hda/hda_codec.c  |   16 +++++++++++++
 sound/pci/hda/hda_codec.h  |    4 +++
 sound/pci/hda/hda_hwdep.c  |   38 +++++++++++++++++++++++++++++++
 sound/pci/hda/hda_local.h  |    9 +++++++
 7 files changed, 127 insertions(+), 0 deletions(-)
 create mode 100644 include/trace/events/vfs.h


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/2] ALSA: hda - Add power on/off counter

Amit Kucheria-6
From: Takashi Iwai <[hidden email]>

Added the power on/off counter and expose via sysfs files.
The sysfs files, power_on_acct and power_off_acct, are created under
each codec hwdep sysfs directory (e.g. /sys/class/sound/hwC0D0).
The files show the msec length of the codec power-on and power-off,
respectively.

Note: This patch is useful to powertop to tell the utilisation of the sound
device. It is cherry-picked from upstream
git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-2.6.git
SHA ID: a2f6309e8

Signed-off-by: Takashi Iwai <[hidden email]>
Signed-off-by: Amit Kucheria <[hidden email]>
---
 sound/pci/hda/hda_codec.c |   16 ++++++++++++++++
 sound/pci/hda/hda_codec.h |    4 ++++
 sound/pci/hda/hda_hwdep.c |   38 ++++++++++++++++++++++++++++++++++++++
 sound/pci/hda/hda_local.h |    9 +++++++++
 4 files changed, 67 insertions(+), 0 deletions(-)

diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index af989f6..25522dc 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -515,6 +515,7 @@ static int snd_hda_bus_dev_register(struct snd_device *device)
  struct hda_codec *codec;
  list_for_each_entry(codec, &bus->codec_list, list) {
  snd_hda_hwdep_add_sysfs(codec);
+ snd_hda_hwdep_add_power_sysfs(codec);
  }
  return 0;
 }
@@ -2452,9 +2453,11 @@ static void hda_call_codec_suspend(struct hda_codec *codec)
     codec->afg ? codec->afg : codec->mfg,
     AC_PWRST_D3);
 #ifdef CONFIG_SND_HDA_POWER_SAVE
+ snd_hda_update_power_acct(codec);
  cancel_delayed_work(&codec->power_work);
  codec->power_on = 0;
  codec->power_transition = 0;
+ codec->power_jiffies = jiffies;
 #endif
 }
 
@@ -3207,6 +3210,17 @@ static void hda_keep_power_on(struct hda_codec *codec)
 {
  codec->power_count++;
  codec->power_on = 1;
+ codec->power_jiffies = jiffies;
+}
+
+void snd_hda_update_power_acct(struct hda_codec *codec)
+{
+ unsigned long delta = jiffies - codec->power_jiffies;
+ if (codec->power_on)
+ codec->power_on_acct += delta;
+ else
+ codec->power_off_acct += delta;
+ codec->power_jiffies += delta;
 }
 
 void snd_hda_power_up(struct hda_codec *codec)
@@ -3217,7 +3231,9 @@ void snd_hda_power_up(struct hda_codec *codec)
  if (codec->power_on || codec->power_transition)
  return;
 
+ snd_hda_update_power_acct(codec);
  codec->power_on = 1;
+ codec->power_jiffies = jiffies;
  if (bus->ops.pm_notify)
  bus->ops.pm_notify(bus);
  hda_call_codec_resume(codec);
diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h
index 99552fb..b47664e 100644
--- a/sound/pci/hda/hda_codec.h
+++ b/sound/pci/hda/hda_codec.h
@@ -811,6 +811,9 @@ struct hda_codec {
  unsigned int power_transition :1; /* power-state in transition */
  int power_count; /* current (global) power refcount */
  struct delayed_work power_work; /* delayed task for powerdown */
+ unsigned long power_on_acct;
+ unsigned long power_off_acct;
+ unsigned long power_jiffies;
 #endif
 
  /* codec-specific additional proc output */
@@ -933,6 +936,7 @@ const char *snd_hda_get_jack_location(u32 cfg);
 void snd_hda_power_up(struct hda_codec *codec);
 void snd_hda_power_down(struct hda_codec *codec);
 #define snd_hda_codec_needs_resume(codec) codec->power_count
+void snd_hda_update_power_acct(struct hda_codec *codec);
 #else
 static inline void snd_hda_power_up(struct hda_codec *codec) {}
 static inline void snd_hda_power_down(struct hda_codec *codec) {}
diff --git a/sound/pci/hda/hda_hwdep.c b/sound/pci/hda/hda_hwdep.c
index cc24e67..d243286 100644
--- a/sound/pci/hda/hda_hwdep.c
+++ b/sound/pci/hda/hda_hwdep.c
@@ -154,6 +154,44 @@ int /*__devinit*/ snd_hda_create_hwdep(struct hda_codec *codec)
  return 0;
 }
 
+#ifdef CONFIG_SND_HDA_POWER_SAVE
+static ssize_t power_on_acct_show(struct device *dev,
+  struct device_attribute *attr,
+  char *buf)
+{
+ struct snd_hwdep *hwdep = dev_get_drvdata(dev);
+ struct hda_codec *codec = hwdep->private_data;
+ snd_hda_update_power_acct(codec);
+ return sprintf(buf, "%u\n", jiffies_to_msecs(codec->power_on_acct));
+}
+
+static ssize_t power_off_acct_show(struct device *dev,
+   struct device_attribute *attr,
+   char *buf)
+{
+ struct snd_hwdep *hwdep = dev_get_drvdata(dev);
+ struct hda_codec *codec = hwdep->private_data;
+ snd_hda_update_power_acct(codec);
+ return sprintf(buf, "%u\n", jiffies_to_msecs(codec->power_off_acct));
+}
+
+static struct device_attribute power_attrs[] = {
+ __ATTR_RO(power_on_acct),
+ __ATTR_RO(power_off_acct),
+};
+
+int snd_hda_hwdep_add_power_sysfs(struct hda_codec *codec)
+{
+ struct snd_hwdep *hwdep = codec->hwdep;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(power_attrs); i++)
+ snd_add_device_sysfs_file(SNDRV_DEVICE_TYPE_HWDEP, hwdep->card,
+  hwdep->device, &power_attrs[i]);
+ return 0;
+}
+#endif /* CONFIG_SND_HDA_POWER_SAVE */
+
 #ifdef CONFIG_SND_HDA_RECONFIG
 
 /*
diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h
index 5f1dcc5..ab99bf4 100644
--- a/sound/pci/hda/hda_local.h
+++ b/sound/pci/hda/hda_local.h
@@ -437,6 +437,15 @@ int snd_hda_create_hwdep(struct hda_codec *codec);
 static inline int snd_hda_create_hwdep(struct hda_codec *codec) { return 0; }
 #endif
 
+#ifdef CONFIG_SND_HDA_POWER_SAVE
+int snd_hda_hwdep_add_power_sysfs(struct hda_codec *codec);
+#else
+static inline int snd_hda_hwdep_add_power_sysfs(struct hda_codec *codec)
+{
+ return 0;
+}
+#endif
+
 #ifdef CONFIG_SND_HDA_RECONFIG
 int snd_hda_hwdep_add_sysfs(struct hda_codec *codec);
 #else
--
1.6.3.3


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 2/2] vfs: Add a trace point in the mark_inode_dirty function

Amit Kucheria-6
In reply to this post by Amit Kucheria-6
From: Arjan van de Ven <[hidden email]>

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.

Note: This patch was posted to lkml and might potentially go into 2.6.33 but I
have not seen which maintainer will take it.

Signed-of-by: Arjan van de Ven <[hidden email]>
Signed-off-by: Amit Kucheria <[hidden email]>
---
 fs/fs-writeback.c          |    3 ++
 fs/inode.c                 |    4 +++
 include/trace/events/vfs.h |   53 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 60 insertions(+), 0 deletions(-)
 create mode 100644 include/trace/events/vfs.h

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index 9d5360c..4102f20 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -25,6 +25,7 @@
 #include <linux/blkdev.h>
 #include <linux/backing-dev.h>
 #include <linux/buffer_head.h>
+#include <trace/events/vfs.h>
 #include "internal.h"
 
 #define inode_to_bdi(inode) ((inode)->i_mapping->backing_dev_info)
@@ -1071,6 +1072,8 @@ void __mark_inode_dirty(struct inode *inode, int flags)
  if ((inode->i_state & flags) == flags)
  return;
 
+ trace_dirty_inode(inode, current);
+
  if (unlikely(block_dump))
  block_dump___mark_inode_dirty(inode);
 
diff --git a/fs/inode.c b/fs/inode.c
index 4d8e3be..b89af38 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1624,3 +1624,7 @@ void init_special_inode(struct inode *inode, umode_t mode, dev_t rdev)
   inode->i_ino);
 }
 EXPORT_SYMBOL(init_special_inode);
+
+#define CREATE_TRACE_POINTS
+#include <trace/events/vfs.h>
+
diff --git a/include/trace/events/vfs.h b/include/trace/events/vfs.h
new file mode 100644
index 0000000..3c170f8
--- /dev/null
+++ b/include/trace/events/vfs.h
@@ -0,0 +1,53 @@
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM vfs
+
+#if !defined(_TRACE_VFS_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_VFS_H
+
+/*
+ * Tracepoint for dirtying an inode:
+ */
+TRACE_EVENT(dirty_inode,
+
+ TP_PROTO(struct inode *inode, struct task_struct *task),
+
+ TP_ARGS(inode, task),
+
+ TP_STRUCT__entry(
+ __array( char, comm, TASK_COMM_LEN )
+ __field( pid_t, pid )
+ __array( char,  dev,    16 )
+ __array( char,  file,   32 )
+ ),
+
+ TP_fast_assign(
+ if (inode->i_ino || strcmp(inode->i_sb->s_id, "bdev")) {
+ struct dentry *dentry;
+ const char *name = "?";
+
+ dentry = d_find_alias(inode);
+ if (dentry) {
+ spin_lock(&dentry->d_lock);
+ name = (const char *) dentry->d_name.name;
+ }
+
+ memcpy(__entry->comm, task->comm, TASK_COMM_LEN);
+ __entry->pid = task->pid;
+ strlcpy(__entry->file, name, 32);
+ strlcpy(__entry->dev, inode->i_sb->s_id, 16);
+
+ if (dentry) {
+ spin_unlock(&dentry->d_lock);
+ dput(dentry);
+ }
+ }
+ ),
+
+ TP_printk("task=%i (%s) file=%s dev=%s",
+ __entry->pid, __entry->comm, __entry->file, __entry->dev)
+);
+
+#endif /* _TRACE_VFS_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
--
1.6.3.3


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[APPLIED] [PATCH 0/2] Add new features to powertop

Andy Whitcroft-3
In reply to this post by Amit Kucheria-6
Applied to Lucid.

-apw

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team