Re: Maverick [PATCH 1/2] PM / Runtime: Add runtime PM statistics (v3)

On Tue, 2010-09-14 at 10:23 -0600, Tim Gardner wrote:

On 09/13/2010 12:31 AM, yong.shen@linaro.org wrote:

...

From: Arjan van de Venarjan@linux.intel.com

In order for PowerTOP to be able to report how well the new runtime PM is working for the various drivers, the kernel needs to export some basic statistics in sysfs.

This patch adds two sysfs files in the runtime PM domain that expose the total time a device has been active, and the time a device has been suspended.

With this PowerTOP can compute the activity percentage

Active %age = 100 * (delta active) / (delta active + delta suspended)

and present the information to the user.

I've written the PowerTOP code (slated for version 1.12) already, and the output looks like this:

Runtime Device Power Management statistics Active Device name 10.0% 06:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL8101E/RTL8102E PCI Express Fast Ethernet controller

[version 2: fix stat update bugs noticed by Alan Stern] [version 3: rebase to -next and move the sysfs declaration]

Signed-off-by: Arjan van de Venarjan@linux.intel.com Signed-off-by: Rafael J. Wysockirjw@sisk.pl Signed-off-by: Shen Yongyong.shen@linaro.org

---

drivers/base/power/runtime.c | 54 ++++++++++++++++++++++++---- drivers/base/power/sysfs.c | 83 ++++++++++++++++++++++++++++++++---------- include/linux/pm.h | 6 +++ 3 files changed, 116 insertions(+), 27 deletions(-)

diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index b0ec0e9..b78c401 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -123,6 +123,45 @@ int pm_runtime_idle(struct device *dev) } EXPORT_SYMBOL_GPL(pm_runtime_idle);

+/**

• • update_pm_runtime_accounting - Update the time accounting of power states
• • @dev: Device to update the accounting for
• • In order to be able to have time accounting of the various power states
• • (as used by programs such as PowerTOP to show the effectiveness of runtime
• • PM), we need to track the time spent in each state.
• • update_pm_runtime_accounting must be called each time before the
• • runtime_status field is updated, to account the time in the old state
• • correctly.
• */

+void update_pm_runtime_accounting(struct device *dev) +{

• unsigned long now = jiffies;
• int delta;
• delta = now - dev->power.accounting_timestamp;
• if (delta< 0)

• delta = 0;
  

• dev->power.accounting_timestamp = now;
• if (dev->power.disable_depth> 0)

• return;
  

• if (dev->power.runtime_status == RPM_SUSPENDED)

• dev->power.suspended_jiffies += delta;
  

• else

• dev->power.active_jiffies += delta;
  

+}

+static void __update_runtime_status(struct device *dev, enum rpm_status status) +{

• update_pm_runtime_accounting(dev);
• dev->power.runtime_status = status;

+}

• /**
  • __pm_runtime_suspend - Carry out run-time suspend of given device.
  • @dev: Device to suspend.

@@ -197,7 +236,7 @@ int __pm_runtime_suspend(struct device *dev, bool from_wq) goto repeat; }

• dev->power.runtime_status = RPM_SUSPENDING;

• __update_runtime_status(dev, RPM_SUSPENDING); dev->power.deferred_resume = false;

  if (dev->bus&& dev->bus->pm&& dev->bus->pm->runtime_suspend) {

@@ -228,7 +267,7 @@ int __pm_runtime_suspend(struct device *dev, bool from_wq) }

if (retval) {

• dev->power.runtime_status = RPM_ACTIVE;
  

• __update_runtime_status(dev, RPM_ACTIVE);
  

  if (retval == -EAGAIN || retval == -EBUSY) { if (dev->power.timer_expires == 0) notify = true;

@@ -237,7 +276,7 @@ int __pm_runtime_suspend(struct device *dev, bool from_wq) pm_runtime_cancel_pending(dev); } } else {

• dev->power.runtime_status = RPM_SUSPENDED;
  

• __update_runtime_status(dev, RPM_SUSPENDED);
  

  pm_runtime_deactivate_timer(dev);

  if (dev->parent) {

@@ -381,7 +420,7 @@ int __pm_runtime_resume(struct device *dev, bool from_wq) goto repeat; }

• dev->power.runtime_status = RPM_RESUMING;

• __update_runtime_status(dev, RPM_RESUMING);

  if (dev->bus&& dev->bus->pm&& dev->bus->pm->runtime_resume) { spin_unlock_irq(&dev->power.lock);

@@ -411,10 +450,10 @@ int __pm_runtime_resume(struct device *dev, bool from_wq) }

if (retval) {

• dev->power.runtime_status = RPM_SUSPENDED;
  

• __update_runtime_status(dev, RPM_SUSPENDED);
  

  pm_runtime_cancel_pending(dev); } else {

• dev->power.runtime_status = RPM_ACTIVE;
  

• __update_runtime_status(dev, RPM_ACTIVE);
  

  if (parent) atomic_inc(&parent->power.child_count); }

@@ -848,7 +887,7 @@ int __pm_runtime_set_status(struct device *dev, unsigned int status) }

out_set:

• dev->power.runtime_status = status;

• __update_runtime_status(dev, status); dev->power.runtime_error = 0; out: spin_unlock_irqrestore(&dev->power.lock, flags);

@@ -1077,6 +1116,7 @@ void pm_runtime_init(struct device *dev) dev->power.request_pending = false; dev->power.request = RPM_REQ_NONE; dev->power.deferred_resume = false;

• dev->power.accounting_timestamp = jiffies; INIT_WORK(&dev->power.work, pm_runtime_work);

  dev->power.timer_expires = 0;

diff --git a/drivers/base/power/sysfs.c b/drivers/base/power/sysfs.c index a4c33bc..b831ec5 100644 --- a/drivers/base/power/sysfs.c +++ b/drivers/base/power/sysfs.c @@ -6,6 +6,7 @@ #include<linux/string.h> #include<linux/pm_runtime.h> #include<asm/atomic.h> +#include<linux/jiffies.h> #include "power.h"

/* @@ -108,6 +109,65 @@ static ssize_t control_store(struct device * dev, struct device_attribute *attr, }

static DEVICE_ATTR(control, 0644, control_show, control_store);

+static ssize_t rtpm_active_time_show(struct device *dev,

• 		struct device_attribute *attr, char *buf)
  

+{

• int ret;
• spin_lock_irq(&dev->power.lock);
• update_pm_runtime_accounting(dev);
• ret = sprintf(buf, "%i\n", jiffies_to_msecs(dev->power.active_jiffies));
• spin_unlock_irq(&dev->power.lock);
• return ret;

+}

+static DEVICE_ATTR(runtime_active_time, 0444, rtpm_active_time_show, NULL);

+static ssize_t rtpm_suspended_time_show(struct device *dev,

• 		struct device_attribute *attr, char *buf)
  

+{

• int ret;
• spin_lock_irq(&dev->power.lock);
• update_pm_runtime_accounting(dev);
• ret = sprintf(buf, "%i\n",

• jiffies_to_msecs(dev->power.suspended_jiffies));
  

• spin_unlock_irq(&dev->power.lock);
• return ret;

+}

+static DEVICE_ATTR(runtime_suspended_time, 0444, rtpm_suspended_time_show, NULL);

+static ssize_t rtpm_status_show(struct device *dev,

• 		struct device_attribute *attr, char *buf)
  

+{

• const char *p;
• if (dev->power.runtime_error) {

• p = "error\n";
  

• } else if (dev->power.disable_depth) {

• p = "unsupported\n";
  

• } else {

• switch (dev->power.runtime_status) {
  

• case RPM_SUSPENDED:
  

• 	p = "suspended\n";
  

• 	break;
  

• case RPM_SUSPENDING:
  

• 	p = "suspending\n";
  

• 	break;
  

• case RPM_RESUMING:
  

• 	p = "resuming\n";
  

• 	break;
  

• case RPM_ACTIVE:
  

• 	p = "active\n";
  

• 	break;
  

• default:
  

• 	return -EIO;
  

• }
  

• }
• return sprintf(buf, p);

+}

+static DEVICE_ATTR(runtime_status, 0444, rtpm_status_show, NULL); #endif

static ssize_t @@ -172,27 +232,8 @@ static ssize_t rtpm_enabled_show(struct device *dev, return sprintf(buf, "enabled\n"); }

-static ssize_t rtpm_status_show(struct device *dev,

• 		struct device_attribute *attr, char *buf)
  

-{

• if (dev->power.runtime_error)

• return sprintf(buf, "error\n");
  

• switch (dev->power.runtime_status) {
• case RPM_SUSPENDED:

• return sprintf(buf, "suspended\n");
  

• case RPM_SUSPENDING:

• return sprintf(buf, "suspending\n");
  

• case RPM_RESUMING:

• return sprintf(buf, "resuming\n");
  

• case RPM_ACTIVE:

• return sprintf(buf, "active\n");
  

• }
• return -EIO;

-}

Completely removing rtpm_status_show() doesn't appear correct, as this is not removed in the upstream version of the patch.

...

static DEVICE_ATTR(runtime_usage, 0444, rtpm_usagecount_show, NULL); static DEVICE_ATTR(runtime_active_kids, 0444, rtpm_children_show, NULL); -static DEVICE_ATTR(runtime_status, 0444, rtpm_status_show, NULL); static DEVICE_ATTR(runtime_enabled, 0444, rtpm_enabled_show, NULL);

#endif @@ -228,6 +269,9 @@ static DEVICE_ATTR(async, 0644, async_show, async_store); static struct attribute * power_attrs[] = { #ifdef CONFIG_PM_RUNTIME &dev_attr_control.attr,

• &dev_attr_runtime_status.attr,
• &dev_attr_runtime_suspended_time.attr,
• &dev_attr_runtime_active_time.attr, #endif &dev_attr_wakeup.attr, #ifdef CONFIG_PM_ADVANCED_DEBUG

@@ -235,7 +279,6 @@ static struct attribute * power_attrs[] = { #ifdef CONFIG_PM_RUNTIME &dev_attr_runtime_usage.attr, &dev_attr_runtime_active_kids.attr,

• &dev_attr_runtime_status.attr, &dev_attr_runtime_enabled.attr, #endif #endif

diff --git a/include/linux/pm.h b/include/linux/pm.h index 8e258c7..dca597f 100644 --- a/include/linux/pm.h +++ b/include/linux/pm.h @@ -476,9 +476,15 @@ struct dev_pm_info { enum rpm_request request; enum rpm_status runtime_status; int runtime_error;

• unsigned long active_jiffies;
• unsigned long suspended_jiffies;
• unsigned long accounting_timestamp; #endif };

+extern void update_pm_runtime_accounting(struct device *dev);

• /*
  • The PM_EVENT_ messages are also used by drivers implementing the legacy
  • suspend framework, based on the ->suspend() and ->resume() callbacks common

This isn't a faithful backport of 8d4b9d1bfef117862a2889dec4dac227068544c9. What happened to rtpm_status_show() ? Even if its not used I'd prefer to keep that function in the patch so as to cause fewer conflicts if there are upstream stable patches.

I prefer the attached version.

Tim's patch was on the right track to provide an updated rtpm_status_show() routine from what we currently have in Maverick, but this still didn't originate from upstream commit 8d4b9d1b.

I've thus taken the path of least resistance and cleanly cherry-picked the following for the Ubuntu Maverick kernel:

commit 0fcb4eef8294492c8f1de8236b1ed81f09e42922 Author: Alan Stern stern@rowland.harvard.edu Date: Thu Jul 8 00:05:37 2010 +0200

PM / Runtime: Make runtime_status attribute not debug-only (v. 2)

commit 8d4b9d1bfef117862a2889dec4dac227068544c9 Author: Arjan van de Ven arjan@linux.intel.com Date: Mon Jul 19 02:01:06 2010 +0200

PM / Runtime: Add runtime PM statistics (v3)

Thanks, Leann