Maverick [PATCH 1/2] PM / Runtime: Add runtime PM statistics (v3)
Tim Gardner
tim.gardner at canonical.com
Tue Sep 14 16:23:27 UTC 2010
On 09/13/2010 12:31 AM, yong.shen at linaro.org wrote:
> From: Arjan van de Ven<arjan at 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 Ven<arjan at linux.intel.com>
> Signed-off-by: Rafael J. Wysocki<rjw at sisk.pl>
> Signed-off-by: Shen Yong<yong.shen at 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;
> -}
> -
> 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.
rtg
--
Tim Gardner tim.gardner at canonical.com
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0001-PM-Runtime-Add-runtime-PM-statistics-v3.patch.rtg
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20100914/af19d8d7/attachment.ksh>
More information about the kernel-team
mailing list