Re: [PATCH V3] PM: In kernel power management domain_pm created for async schedules
From: Vikas Bansal
Date: Wed Dec 13 2017 - 04:08:29 EST
Â
Sender : Rafael J. WysockiÂ<rjw@xxxxxxxxxxxxx>
Date : 2017-12-06 19:48 (GMT+5:30)
Â
> OnÂWednesday,ÂDecemberÂ6,Â2017Â3:12:38ÂPMÂCETÂgregkh@xxxxxxxxxxxxxxxxxxxÂwrote:
> >ÂOnÂWed,ÂDecÂ06,Â2017ÂatÂ12:07:14PMÂ+0000,ÂVikasÂBansalÂwrote:
> >Â>ÂDescription:
> >Â
> >ÂWhyÂisÂthisÂhere?
> >Â
> >Â>Â
> >Â>ÂIfÂthereÂisÂaÂdriverÂinÂsystemÂwhichÂstartsÂcreatingÂasyncÂschedules
> >Â>ÂjustÂafterÂresumeÂ(SameÂasÂourÂcase,ÂinÂwhichÂweÂfacedÂissue).
> >Â>ÂThenÂasync_synchronize_fullÂAPIÂinÂPMÂcoresÂstartsÂwaitingÂforÂcompletion
> >Â>ÂofÂasyncÂschedulesÂcreatedÂbyÂthatÂdriverÂ(EvenÂthoughÂthoseÂareÂinÂaÂdomain).
> >Â>ÂBecauseÂofÂthisÂkernelÂresumeÂtimeÂisÂincreasedÂ(WeÂfacesÂtheÂsameÂissue)
> >Â>ÂandÂwholeÂsystemÂisÂdelayed.
> >Â>ÂThisÂproblemÂcanÂbeÂsolvedÂbyÂcreatingÂaÂdomainÂfor
> >Â>ÂasyncÂschedulesÂinÂPMÂcoreÂ(AsÂweÂsolvedÂinÂourÂcase).
> >Â>ÂBelowÂpatchÂisÂforÂsolvingÂthisÂproblem.
> >Â
> >ÂVeryÂoddÂformatting.
> >Â
> >Â>Â
> >Â>ÂChangelog:
> >Â>Â1.ÂCreatedÂAsyncÂdomainÂdomain_pm.
> >Â>Â2.ÂConvertedÂasync_scheduleÂtoÂasync_schedule_domain.
> >Â>Â3.ÂConvertedÂasync_synchronize_fullÂtoÂasync_synchronize_full_domain
> >Â
> >ÂI'mÂconfused.ÂÂHaveÂyouÂreadÂkernelÂpatchÂsubmissions?ÂÂLookÂatÂhowÂthey
> >ÂareÂformatted.ÂÂTheÂdocumentationÂinÂtheÂkernelÂtreeÂshouldÂhelpÂyouÂout
> >ÂaÂlotÂhere.
> >Â
> >ÂAlso,ÂthisÂisÂnotÂv1,ÂitÂhasÂchangedÂfromÂtheÂpreviousÂversion.ÂÂAlways
> >Âdescribe,ÂinÂtheÂcorrectÂway,ÂtheÂchangesÂfromÂpreviousÂsubmissions.
Setting the correct version and chaging the formatting.
> >Â
> >Â
> >Â>Â
> >Â>Â
> >Â>Â
> >Â>ÂSigned-off-by:ÂVikasÂBansalÂ<vikas.bansal@xxxxxxxxxxx>
> >Â>ÂSigned-off-by:ÂAnujÂGuptaÂ<anuj01.gupta@xxxxxxxxxxx>
> >Â>Â---
> >Â>ÂÂdrivers/base/power/main.cÂ|ÂÂÂ27Â+++++++++++++++------------
> >Â>ÂÂ1ÂfileÂchanged,Â15Âinsertions(+),Â12Âdeletions(-)
> >Â>Â
> >Â>ÂdiffÂ--gitÂa/drivers/base/power/main.cÂb/drivers/base/power/main.c
> >Â>ÂindexÂdb2f044..042b034Â100644
> >Â>Â---Âa/drivers/base/power/main.c
> >Â>Â+++Âb/drivers/base/power/main.c
> >Â>Â@@Â-39,6Â+39,7Â@@
> >Â>ÂÂ#includeÂ"power.h"
> >Â>ÂÂ
> >Â>ÂÂtypedefÂintÂ(*pm_callback_t)(structÂdeviceÂ*);
> >Â>Â+staticÂASYNC_DOMAIN(domain_pm);
> >Â>ÂÂ
> >Â>ÂÂ/*
> >Â>ÂÂÂ*ÂTheÂentriesÂinÂtheÂdpm_listÂlistÂareÂinÂaÂdepthÂfirstÂorder,Âsimply
> >Â>Â@@Â-615,7Â+616,8Â@@ÂvoidÂdpm_noirq_resume_devices(pm_message_tÂstate)
> >Â>ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂreinit_completion(&dev->power.completion);
> >Â>ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂifÂ(is_async(dev))Â{
> >Â>ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂget_device(dev);
> >Â>Â-ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂasync_schedule(async_resume_noirq,Âdev);
> >Â>Â+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂasync_schedule_domain(async_resume_noirq,Âdev,Â
> >Â
> >ÂAlwaysÂrunÂyourÂpatchesÂthroughÂscripts/checkpatch.plÂsoÂyouÂdoÂyouÂnot
> >ÂgetÂgrumpyÂmaintainersÂtellingÂyouÂtoÂuseÂscripts/checkpatch.pl
> >Â
> >ÂStop.ÂÂTakeÂsomeÂtime.ÂÂRedoÂtheÂpatchÂinÂanotherÂdayÂorÂso,ÂandÂthen
> >ÂresendÂitÂlater,Â_AFTER_ÂyouÂhaveÂaddressedÂtheÂissues.ÂÂDon'tÂrush,
> >ÂthereÂisÂnoÂraceÂhere.
>Â
> AlsoÂitÂisÂnotÂclearÂtoÂmeÂifÂthisÂfixesÂaÂmainlineÂkernelÂissue,
> becauseÂtheÂchangelogÂmentionsÂaÂdriverÂdoingÂsomethingÂodd,ÂbutÂit
> doesn'tÂsayÂwhichÂoneÂitÂisÂandÂwhetherÂorÂnotÂitÂisÂinÂtheÂtree.
No, this driver is not part of mainline yet.
Chaging the patch and changelog as suggested. Changed the name of domain from
"domain_pm" to "async_pm". But kept the name in subject as domain_pm, just to
avoid confusion.
> Â
> Thanks,
> Rafael
Â
If there is a driver in system which starts creating async schedules just after
resume (Same as our case, in which we faced issue). Then async_synchronize_full
API in PM cores starts waiting for completion of async schedules created by
that driver (Even though those are in a domain). Because of this kernel resume
time is increased (We faces the same issue) and whole system is delayed.
For solving this problem Async domain async_pm was created and "async_schedule"
API call was replaced with "async_schedule_domain"."async_synchronize_full" was
replaced with "async_synchronize_full_domain".
Signed-off-by: Vikas Bansal <vikas.bansal@xxxxxxxxxxx>
Signed-off-by: Anuj Gupta <anuj01.gupta@xxxxxxxxxxx>
---
drivers/base/power/main.c | 27 +++++++++++++++------------
1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index db2f044..03b71e3 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -39,6 +39,7 @@
#include "power.h"
typedef int (*pm_callback_t)(struct device *);
+static ASYNC_DOMAIN(async_pm);
/*
* The entries in the dpm_list list are in a depth first order, simply
@@ -615,7 +616,8 @@ void dpm_noirq_resume_devices(pm_message_t state)
reinit_completion(&dev->power.completion);
if (is_async(dev)) {
get_device(dev);
- async_schedule(async_resume_noirq, dev);
+ async_schedule_domain(async_resume_noirq, dev,
+ &async_pm);
}
}
@@ -641,7 +643,7 @@ void dpm_noirq_resume_devices(pm_message_t state)
put_device(dev);
}
mutex_unlock(&dpm_list_mtx);
- async_synchronize_full();
+ async_synchronize_full_domain(&async_pm);
dpm_show_time(starttime, state, 0, "noirq");
trace_suspend_resume(TPS("dpm_resume_noirq"), state.event, false);
}
@@ -755,7 +757,8 @@ void dpm_resume_early(pm_message_t state)
reinit_completion(&dev->power.completion);
if (is_async(dev)) {
get_device(dev);
- async_schedule(async_resume_early, dev);
+ async_schedule_domain(async_resume_early, dev,
+ &async_pm);
}
}
@@ -780,7 +783,7 @@ void dpm_resume_early(pm_message_t state)
put_device(dev);
}
mutex_unlock(&dpm_list_mtx);
- async_synchronize_full();
+ async_synchronize_full_domain(&async_pm);
dpm_show_time(starttime, state, 0, "early");
trace_suspend_resume(TPS("dpm_resume_early"), state.event, false);
}
@@ -919,7 +922,7 @@ void dpm_resume(pm_message_t state)
reinit_completion(&dev->power.completion);
if (is_async(dev)) {
get_device(dev);
- async_schedule(async_resume, dev);
+ async_schedule_domain(async_resume, dev, &async_pm);
}
}
@@ -946,7 +949,7 @@ void dpm_resume(pm_message_t state)
put_device(dev);
}
mutex_unlock(&dpm_list_mtx);
- async_synchronize_full();
+ async_synchronize_full_domain(&async_pm);
dpm_show_time(starttime, state, 0, NULL);
cpufreq_resume();
@@ -1156,7 +1159,7 @@ static int device_suspend_noirq(struct device *dev)
if (is_async(dev)) {
get_device(dev);
- async_schedule(async_suspend_noirq, dev);
+ async_schedule_domain(async_suspend_noirq, dev, &async_pm);
return 0;
}
return __device_suspend_noirq(dev, pm_transition, false);
@@ -1202,7 +1205,7 @@ int dpm_noirq_suspend_devices(pm_message_t state)
break;
}
mutex_unlock(&dpm_list_mtx);
- async_synchronize_full();
+ async_synchronize_full_domain(&async_pm);
if (!error)
error = async_error;
@@ -1316,7 +1319,7 @@ static int device_suspend_late(struct device *dev)
if (is_async(dev)) {
get_device(dev);
- async_schedule(async_suspend_late, dev);
+ async_schedule_domain(async_suspend_late, dev, &async_pm);
return 0;
}
@@ -1361,7 +1364,7 @@ int dpm_suspend_late(pm_message_t state)
break;
}
mutex_unlock(&dpm_list_mtx);
- async_synchronize_full();
+ async_synchronize_full_domain(&async_pm);
if (!error)
error = async_error;
if (error) {
@@ -1576,7 +1579,7 @@ static int device_suspend(struct device *dev)
if (is_async(dev)) {
get_device(dev);
- async_schedule(async_suspend, dev);
+ async_schedule_domain(async_suspend, dev, &async_pm);
return 0;
}
@@ -1622,7 +1625,7 @@ int dpm_suspend(pm_message_t state)
break;
}
mutex_unlock(&dpm_list_mtx);
- async_synchronize_full();
+ async_synchronize_full_domain(&async_pm);
if (!error)
error = async_error;
if (error) {
--
1.7.9.5
Â
Â