[PATCH v7 0/3] Move pm_runtime accounted time to raw nsec

[PATCH v7 0/3] Move pm_runtime accounted time to raw nsec

[Vincent Guittot]

Move pm_runtime accounted time to raw nsec. The subject of the patchset
has changed as 1st patch o the previous versions has been queued by
Rafael.

Patch 1 set accounting_timestamp to 0 in pm_runtime_init and update it when enable.
So we remove ordering constraint between timekeeping_init and pm_runtime_init

Patch 2 moves time accounting on raw ns. This patch initially used
ktime instead of raw ns but it was easier to move i915 driver on raw ns
than on ktime.

Change since v6:
- move code that set accounting_timestamp in pm_runtime_enable

Changes since v5:
- removed patches already queued.
- set accounting_timestamp to 0 in pm_runtime_init and update it when enable

Changes since v4:
-Update commit message

Changes since v3:
- Rebase on v4.20-rc7 without patch that has been queued by Rafael
- Simplify the new interface pm_runtime_suspended_time()

Changes since v2:
- remove patch1 that has been queued by rafael
- add new interface in pm_runtime to get accounted time
- reorder patchset to prevent compilation error

Changes since v1:
- updated commit message of patch 1
- Added patches 2 & 3 to move runtime_pm accounting on raw ns
  
Thara Gopinath (1):
  PM-runtime: Replace jiffies based accounting with ktime-based
    accounting

Vincent Guittot (1):
  PM-runtime: update accounting_timestamp only when enable

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

-- 
2.7.4

[PATCH v7 1/2] PM-runtime: update accounting_timestamp only when enable

[Vincent Guittot]

Initializing accounting_timestamp to something different from 0 during
pm_runtime_init() doesn't make sense and put useless ordering constraint between
timekeeping_init() and pm_runtime_init().
PM runtime should start accounting time only when it is enable and discard
the period when disabled.
Set accounting_timestamp to now when enabling PM runtime.

Suggested-by: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
---
 drivers/base/power/runtime.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index fb5e2b6..eccd37f1 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -1306,10 +1306,15 @@ void pm_runtime_enable(struct device *dev)
 
 	spin_lock_irqsave(&dev->power.lock, flags);
 
-	if (dev->power.disable_depth > 0)
+	if (dev->power.disable_depth > 0) {
 		dev->power.disable_depth--;
-	else
+
+		/* About to enable runtime pm, set accounting_timestamp to now */
+		if (!dev->power.disable_depth)
+			dev->power.accounting_timestamp = jiffies;
+	} else {
 		dev_warn(dev, "Unbalanced %s!\n", __func__);
+	}
 
 	WARN(!dev->power.disable_depth &&
 	     dev->power.runtime_status == RPM_SUSPENDED &&
@@ -1506,7 +1511,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;
+	dev->power.accounting_timestamp = 0;
 	INIT_WORK(&dev->power.work, pm_runtime_work);
 
 	dev->power.timer_expires = 0;
-- 
2.7.4

[PATCH v7 2/2] PM-runtime: Replace jiffies based accounting with ktime-based accounting

[Vincent Guittot]

From: Thara Gopinath <thara.gopinath@linaro.org>

This patch replaces jiffies based accounting for runtime_active_time
and runtime_suspended_time with ktime-based accounting. This makes the
runtime debug counters inline with genpd and other PM subsytems which
use ktime-based accounting.

Timekeeping is initialized before driver_init(). It's only at that time
that PM runtime can be enabled.

Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
[switch from ktime to raw nsec]
Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
---
 drivers/base/power/runtime.c | 17 +++++++++--------
 drivers/base/power/sysfs.c   | 11 ++++++++---
 include/linux/pm.h           |  6 +++---
 3 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index eccd37f1..e9dae6d7 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -66,8 +66,8 @@ static int rpm_suspend(struct device *dev, int rpmflags);
  */
 void update_pm_runtime_accounting(struct device *dev)
 {
-	unsigned long now = jiffies;
-	unsigned long delta;
+	u64 now = ktime_to_ns(ktime_get());
+	u64 delta;
 
 	delta = now - dev->power.accounting_timestamp;
 
@@ -77,9 +77,9 @@ void update_pm_runtime_accounting(struct device *dev)
 		return;
 
 	if (dev->power.runtime_status == RPM_SUSPENDED)
-		dev->power.suspended_jiffies += delta;
+		dev->power.suspended_time += delta;
 	else
-		dev->power.active_jiffies += delta;
+		dev->power.active_time += delta;
 }
 
 static void __update_runtime_status(struct device *dev, enum rpm_status status)
@@ -90,16 +90,17 @@ static void __update_runtime_status(struct device *dev, enum rpm_status status)
 
 u64 pm_runtime_suspended_time(struct device *dev)
 {
-	unsigned long flags, time;
+	u64 time;
+	unsigned long flags;
 
 	spin_lock_irqsave(&dev->power.lock, flags);
 
 	update_pm_runtime_accounting(dev);
-	time = dev->power.suspended_jiffies;
+	time = dev->power.suspended_time;
 
 	spin_unlock_irqrestore(&dev->power.lock, flags);
 
-	return jiffies_to_nsecs(time);
+	return time;
 }
 EXPORT_SYMBOL_GPL(pm_runtime_suspended_time);
 
@@ -1311,7 +1312,7 @@ void pm_runtime_enable(struct device *dev)
 
 		/* About to enable runtime pm, set accounting_timestamp to now */
 		if (!dev->power.disable_depth)
-			dev->power.accounting_timestamp = jiffies;
+			dev->power.accounting_timestamp = ktime_to_ns(ktime_get());
 	} else {
 		dev_warn(dev, "Unbalanced %s!\n", __func__);
 	}
diff --git a/drivers/base/power/sysfs.c b/drivers/base/power/sysfs.c
index d713738..96c8a22 100644
--- a/drivers/base/power/sysfs.c
+++ b/drivers/base/power/sysfs.c
@@ -125,9 +125,12 @@ static ssize_t runtime_active_time_show(struct device *dev,
 				struct device_attribute *attr, char *buf)
 {
 	int ret;
+	u64 tmp;
 	spin_lock_irq(&dev->power.lock);
 	update_pm_runtime_accounting(dev);
-	ret = sprintf(buf, "%i\n", jiffies_to_msecs(dev->power.active_jiffies));
+	tmp = dev->power.active_time;
+	do_div(tmp, NSEC_PER_MSEC);
+	ret = sprintf(buf, "%llu\n", tmp);
 	spin_unlock_irq(&dev->power.lock);
 	return ret;
 }
@@ -138,10 +141,12 @@ static ssize_t runtime_suspended_time_show(struct device *dev,
 				struct device_attribute *attr, char *buf)
 {
 	int ret;
+	u64 tmp;
 	spin_lock_irq(&dev->power.lock);
 	update_pm_runtime_accounting(dev);
-	ret = sprintf(buf, "%i\n",
-		jiffies_to_msecs(dev->power.suspended_jiffies));
+	tmp = dev->power.suspended_time;
+	do_div(tmp, NSEC_PER_MSEC);
+	ret = sprintf(buf, "%llu\n", tmp);
 	spin_unlock_irq(&dev->power.lock);
 	return ret;
 }
diff --git a/include/linux/pm.h b/include/linux/pm.h
index 0bd9de1..3d2cbf9 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -633,9 +633,9 @@ struct dev_pm_info {
 	int			runtime_error;
 	int			autosuspend_delay;
 	u64			last_busy;
-	unsigned long		active_jiffies;
-	unsigned long		suspended_jiffies;
-	unsigned long		accounting_timestamp;
+	u64			active_time;
+	u64			suspended_time;
+	u64			accounting_timestamp;
 #endif
 	struct pm_subsys_data	*subsys_data;  /* Owned by the subsystem. */
 	void (*set_latency_tolerance)(struct device *, s32);
-- 
2.7.4

Re: [PATCH v7 0/3] Move pm_runtime accounted time to raw nsec

[Rafael J. Wysocki]

On Wednesday, January 23, 2019 8:50:12 AM CET Vincent Guittot wrote:
> Move pm_runtime accounted time to raw nsec. The subject of the patchset
> has changed as 1st patch o the previous versions has been queued by
> Rafael.
> 
> Patch 1 set accounting_timestamp to 0 in pm_runtime_init and update it when enable.
> So we remove ordering constraint between timekeeping_init and pm_runtime_init
> 
> Patch 2 moves time accounting on raw ns. This patch initially used
> ktime instead of raw ns but it was easier to move i915 driver on raw ns
> than on ktime.
> 
> Change since v6:
> - move code that set accounting_timestamp in pm_runtime_enable
> 
> Changes since v5:
> - removed patches already queued.
> - set accounting_timestamp to 0 in pm_runtime_init and update it when enable
> 
> Changes since v4:
> -Update commit message
> 
> Changes since v3:
> - Rebase on v4.20-rc7 without patch that has been queued by Rafael
> - Simplify the new interface pm_runtime_suspended_time()
> 
> Changes since v2:
> - remove patch1 that has been queued by rafael
> - add new interface in pm_runtime to get accounted time
> - reorder patchset to prevent compilation error
> 
> Changes since v1:
> - updated commit message of patch 1
> - Added patches 2 & 3 to move runtime_pm accounting on raw ns
>   
> Thara Gopinath (1):
>   PM-runtime: Replace jiffies based accounting with ktime-based
>     accounting
> 
> Vincent Guittot (1):
>   PM-runtime: update accounting_timestamp only when enable
> 
>  drivers/base/power/runtime.c | 26 ++++++++++++++++----------
>  drivers/base/power/sysfs.c   | 11 ++++++++---
>  include/linux/pm.h           |  6 +++---
>  3 files changed, 27 insertions(+), 16 deletions(-)

Both patches applied, thanks!

Re: [PATCH v7 0/3] Move pm_runtime accounted time to raw nsec

[Ulf Hansson]

On Thu, 31 Jan 2019 at 00:51, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>
> On Wednesday, January 23, 2019 8:50:12 AM CET Vincent Guittot wrote:
> > Move pm_runtime accounted time to raw nsec. The subject of the patchset
> > has changed as 1st patch o the previous versions has been queued by
> > Rafael.
> >
> > Patch 1 set accounting_timestamp to 0 in pm_runtime_init and update it when enable.
> > So we remove ordering constraint between timekeeping_init and pm_runtime_init
> >
> > Patch 2 moves time accounting on raw ns. This patch initially used
> > ktime instead of raw ns but it was easier to move i915 driver on raw ns
> > than on ktime.
> >
> > Change since v6:
> > - move code that set accounting_timestamp in pm_runtime_enable
> >
> > Changes since v5:
> > - removed patches already queued.
> > - set accounting_timestamp to 0 in pm_runtime_init and update it when enable
> >
> > Changes since v4:
> > -Update commit message
> >
> > Changes since v3:
> > - Rebase on v4.20-rc7 without patch that has been queued by Rafael
> > - Simplify the new interface pm_runtime_suspended_time()
> >
> > Changes since v2:
> > - remove patch1 that has been queued by rafael
> > - add new interface in pm_runtime to get accounted time
> > - reorder patchset to prevent compilation error
> >
> > Changes since v1:
> > - updated commit message of patch 1
> > - Added patches 2 & 3 to move runtime_pm accounting on raw ns
> >
> > Thara Gopinath (1):
> >   PM-runtime: Replace jiffies based accounting with ktime-based
> >     accounting
> >
> > Vincent Guittot (1):
> >   PM-runtime: update accounting_timestamp only when enable
> >
> >  drivers/base/power/runtime.c | 26 ++++++++++++++++----------
> >  drivers/base/power/sysfs.c   | 11 ++++++++---
> >  include/linux/pm.h           |  6 +++---
> >  3 files changed, 27 insertions(+), 16 deletions(-)
>
> Both patches applied, thanks!
>

I had some comment on v6, none that needs to prevent this from being
applied and that can't be addressed as improvements on top.

Feel free to add (if not too late) my:

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

Kind regards
Uffe

Re: [PATCH v7 0/3] Move pm_runtime accounted time to raw nsec

[Vincent Guittot]

On Thu, 31 Jan 2019 at 08:44, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Thu, 31 Jan 2019 at 00:51, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> >
> > On Wednesday, January 23, 2019 8:50:12 AM CET Vincent Guittot wrote:
> > > Move pm_runtime accounted time to raw nsec. The subject of the patchset
> > > has changed as 1st patch o the previous versions has been queued by
> > > Rafael.
> > >
> > > Patch 1 set accounting_timestamp to 0 in pm_runtime_init and update it when enable.
> > > So we remove ordering constraint between timekeeping_init and pm_runtime_init
> > >
> > > Patch 2 moves time accounting on raw ns. This patch initially used
> > > ktime instead of raw ns but it was easier to move i915 driver on raw ns
> > > than on ktime.
> > >
> > > Change since v6:
> > > - move code that set accounting_timestamp in pm_runtime_enable
> > >
> > > Changes since v5:
> > > - removed patches already queued.
> > > - set accounting_timestamp to 0 in pm_runtime_init and update it when enable
> > >
> > > Changes since v4:
> > > -Update commit message
> > >
> > > Changes since v3:
> > > - Rebase on v4.20-rc7 without patch that has been queued by Rafael
> > > - Simplify the new interface pm_runtime_suspended_time()
> > >
> > > Changes since v2:
> > > - remove patch1 that has been queued by rafael
> > > - add new interface in pm_runtime to get accounted time
> > > - reorder patchset to prevent compilation error
> > >
> > > Changes since v1:
> > > - updated commit message of patch 1
> > > - Added patches 2 & 3 to move runtime_pm accounting on raw ns
> > >
> > > Thara Gopinath (1):
> > >   PM-runtime: Replace jiffies based accounting with ktime-based
> > >     accounting
> > >
> > > Vincent Guittot (1):
> > >   PM-runtime: update accounting_timestamp only when enable
> > >
> > >  drivers/base/power/runtime.c | 26 ++++++++++++++++----------
> > >  drivers/base/power/sysfs.c   | 11 ++++++++---
> > >  include/linux/pm.h           |  6 +++---
> > >  3 files changed, 27 insertions(+), 16 deletions(-)
> >
> > Both patches applied, thanks!
> >
>
> I had some comment on v6, none that needs to prevent this from being
> applied and that can't be addressed as improvements on top.

I had put this on hold until fixing regression with hrtimer but i'm
going to restart these improvements

>
> Feel free to add (if not too late) my:
>
> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
>
> Kind regards
> Uffe