* [PATCH 1/2][RFC v2] PM-runtime: Move all runtime usage related function to runtime.c
2020-06-08 7:41 [PATCH 0/2][RFC v2] Change trace point to cover all runtime usage count Chen Yu
@ 2020-06-08 7:41 ` Chen Yu
2020-06-08 7:42 ` [PATCH 2/2][RFC v2] PM-runtime: change the tracepoints to cover all usage_count Chen Yu
1 sibling, 0 replies; 4+ messages in thread
From: Chen Yu @ 2020-06-08 7:41 UTC (permalink / raw)
To: Rafael J. Wysocki, Len Brown, Greg Kroah-Hartman, Michal Miroslaw
Cc: linux-pm, linux-kernel, Chen Yu
In order to track all the runtime usage count change, move the code
related to runtime usage count change from pm_runtime.h to runtime.c,
so that in runtime.c we can leverage trace event to do the tracking.
Meanwhile export pm_runtime_get_noresume() and pm_runtime_put_noidle()
so the module can use them.
No functional change.
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Chen Yu <yu.c.chen@intel.com>
---
drivers/base/power/runtime.c | 12 ++++++++++++
include/linux/pm_runtime.h | 12 ++----------
2 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index 9f62790f644c..85a248e196ca 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -1738,6 +1738,18 @@ void pm_runtime_drop_link(struct device *dev)
spin_unlock_irq(&dev->power.lock);
}
+void pm_runtime_get_noresume(struct device *dev)
+{
+ atomic_inc(&dev->power.usage_count);
+}
+EXPORT_SYMBOL_GPL(pm_runtime_get_noresume);
+
+void pm_runtime_put_noidle(struct device *dev)
+{
+ atomic_add_unless(&dev->power.usage_count, -1, 0);
+}
+EXPORT_SYMBOL_GPL(pm_runtime_put_noidle);
+
static bool pm_runtime_need_not_resume(struct device *dev)
{
return atomic_read(&dev->power.usage_count) <= 1 &&
diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
index 3dbc207bff53..26510fef2acd 100644
--- a/include/linux/pm_runtime.h
+++ b/include/linux/pm_runtime.h
@@ -59,6 +59,8 @@ extern void pm_runtime_get_suppliers(struct device *dev);
extern void pm_runtime_put_suppliers(struct device *dev);
extern void pm_runtime_new_link(struct device *dev);
extern void pm_runtime_drop_link(struct device *dev);
+extern void pm_runtime_get_noresume(struct device *dev);
+extern void pm_runtime_put_noidle(struct device *dev);
static inline int pm_runtime_get_if_in_use(struct device *dev)
{
@@ -70,16 +72,6 @@ static inline void pm_suspend_ignore_children(struct device *dev, bool enable)
dev->power.ignore_children = enable;
}
-static inline void pm_runtime_get_noresume(struct device *dev)
-{
- atomic_inc(&dev->power.usage_count);
-}
-
-static inline void pm_runtime_put_noidle(struct device *dev)
-{
- atomic_add_unless(&dev->power.usage_count, -1, 0);
-}
-
static inline bool pm_runtime_suspended(struct device *dev)
{
return dev->power.runtime_status == RPM_SUSPENDED
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH 2/2][RFC v2] PM-runtime: change the tracepoints to cover all usage_count
2020-06-08 7:41 [PATCH 0/2][RFC v2] Change trace point to cover all runtime usage count Chen Yu
2020-06-08 7:41 ` [PATCH 1/2][RFC v2] PM-runtime: Move all runtime usage related function to runtime.c Chen Yu
@ 2020-06-08 7:42 ` Chen Yu
2020-06-08 7:50 ` Michal Miroslaw
1 sibling, 1 reply; 4+ messages in thread
From: Chen Yu @ 2020-06-08 7:42 UTC (permalink / raw)
To: Rafael J. Wysocki, Len Brown, Greg Kroah-Hartman, Michal Miroslaw
Cc: linux-pm, linux-kernel, Chen Yu
Commit d229290689ae ("PM-runtime: add tracepoints for usage_count changes")
has added some tracepoints to monitor the change of runtime usage, and
there is something to improve:
1. There are some places that adjust the usage count have not
been traced yet. For example, pm_runtime_get_noresume() and
pm_runtime_put_noidle()
2. The change of the usage count will not be tracked if decreased
from 1 to 0.
This patch intends to adjust the logic to be consistent with the
change of usage_counter, that is to say, only after the counter has
been possibly modified, we record it. Besides, all usage changes will
be shown using rpm_usage even if included by other trace points.
And these changes has helped track down the e1000e runtime issue.
Signed-off-by: Chen Yu <yu.c.chen@intel.com>
---
v2: According to Michal's suggestion, adjust the commit log
to better describe the meaning of this patch.
--
drivers/base/power/runtime.c | 38 +++++++++++++++++++++++-------------
1 file changed, 24 insertions(+), 14 deletions(-)
diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index 85a248e196ca..5789d2624513 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -1004,10 +1004,11 @@ int __pm_runtime_idle(struct device *dev, int rpmflags)
int retval;
if (rpmflags & RPM_GET_PUT) {
- if (!atomic_dec_and_test(&dev->power.usage_count)) {
- trace_rpm_usage_rcuidle(dev, rpmflags);
+ bool non_zero = !atomic_dec_and_test(&dev->power.usage_count);
+
+ trace_rpm_usage_rcuidle(dev, rpmflags);
+ if (non_zero)
return 0;
- }
}
might_sleep_if(!(rpmflags & RPM_ASYNC) && !dev->power.irq_safe);
@@ -1038,10 +1039,12 @@ int __pm_runtime_suspend(struct device *dev, int rpmflags)
int retval;
if (rpmflags & RPM_GET_PUT) {
- if (!atomic_dec_and_test(&dev->power.usage_count)) {
- trace_rpm_usage_rcuidle(dev, rpmflags);
+ bool non_zero = !atomic_dec_and_test(&dev->power.usage_count);
+
+ trace_rpm_usage_rcuidle(dev, rpmflags);
+ if (non_zero)
return 0;
- }
+
}
might_sleep_if(!(rpmflags & RPM_ASYNC) && !dev->power.irq_safe);
@@ -1073,8 +1076,10 @@ int __pm_runtime_resume(struct device *dev, int rpmflags)
might_sleep_if(!(rpmflags & RPM_ASYNC) && !dev->power.irq_safe &&
dev->power.runtime_status != RPM_ACTIVE);
- if (rpmflags & RPM_GET_PUT)
+ if (rpmflags & RPM_GET_PUT) {
atomic_inc(&dev->power.usage_count);
+ trace_rpm_usage_rcuidle(dev, rpmflags);
+ }
spin_lock_irqsave(&dev->power.lock, flags);
retval = rpm_resume(dev, rpmflags);
@@ -1433,6 +1438,7 @@ void pm_runtime_forbid(struct device *dev)
dev->power.runtime_auto = false;
atomic_inc(&dev->power.usage_count);
+ trace_rpm_usage_rcuidle(dev, 0);
rpm_resume(dev, 0);
out:
@@ -1448,16 +1454,17 @@ EXPORT_SYMBOL_GPL(pm_runtime_forbid);
*/
void pm_runtime_allow(struct device *dev)
{
+ bool is_zero;
+
spin_lock_irq(&dev->power.lock);
if (dev->power.runtime_auto)
goto out;
dev->power.runtime_auto = true;
- if (atomic_dec_and_test(&dev->power.usage_count))
+ is_zero = atomic_dec_and_test(&dev->power.usage_count);
+ trace_rpm_usage_rcuidle(dev, RPM_AUTO | RPM_ASYNC);
+ if (is_zero)
rpm_idle(dev, RPM_AUTO | RPM_ASYNC);
- else
- trace_rpm_usage_rcuidle(dev, RPM_AUTO | RPM_ASYNC);
-
out:
spin_unlock_irq(&dev->power.lock);
}
@@ -1523,9 +1530,8 @@ static void update_autosuspend(struct device *dev, int old_delay, int old_use)
/* If it used to be allowed then prevent it. */
if (!old_use || old_delay >= 0) {
atomic_inc(&dev->power.usage_count);
- rpm_resume(dev, 0);
- } else {
trace_rpm_usage_rcuidle(dev, 0);
+ rpm_resume(dev, 0);
}
}
@@ -1533,8 +1539,10 @@ static void update_autosuspend(struct device *dev, int old_delay, int old_use)
else {
/* If it used to be prevented then allow it. */
- if (old_use && old_delay < 0)
+ if (old_use && old_delay < 0) {
atomic_dec(&dev->power.usage_count);
+ trace_rpm_usage_rcuidle(dev, 0);
+ }
/* Maybe we can autosuspend now. */
rpm_idle(dev, RPM_AUTO);
@@ -1741,12 +1749,14 @@ void pm_runtime_drop_link(struct device *dev)
void pm_runtime_get_noresume(struct device *dev)
{
atomic_inc(&dev->power.usage_count);
+ trace_rpm_usage_rcuidle(dev, 0);
}
EXPORT_SYMBOL_GPL(pm_runtime_get_noresume);
void pm_runtime_put_noidle(struct device *dev)
{
atomic_add_unless(&dev->power.usage_count, -1, 0);
+ trace_rpm_usage_rcuidle(dev, 0);
}
EXPORT_SYMBOL_GPL(pm_runtime_put_noidle);
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread