2014-01-20 08:53:26

by Liu, Chuansheng

[permalink] [raw]
Subject: [PATCH 0/5] Enabling the asynchronous threads for other phases

Hello,

This patch series are for enabling the asynchronous threads for the phases
resume_noirq, resume_early, suspend_noirq and suspend_late.

Just like commit 5af84b82701a and 97df8c12995, with async threads it will
reduce the system suspending and resuming time significantly.

With these patches, in my test platform, it saved 80% time in resume_noirq
phase.

Has done the suspend-resume stress test for a long time, please help to
review.

Best Regards,

[PATCH 1/5] PM: Adding two flags for async suspend_noirq and
[PATCH 2/5] PM: Enabling the asynchronous threads for resume_noirq
[PATCH 3/5] PM: Enabling the asyncronous threads for resume_early
[PATCH 4/5] PM: Enabling the asyncronous threads for suspend_noirq
[PATCH 5/5] PM: Enabling the asyncronous threads for suspend_late

drivers/base/power/main.c | 240 ++++++++++++++++++++++++++++++++++++---------
include/linux/pm.h | 2 +
2 files changed, 197 insertions(+), 45 deletions(-)


2014-01-20 08:53:38

by Liu, Chuansheng

[permalink] [raw]
Subject: [PATCH 1/5] PM: Adding two flags for async suspend_noirq and suspend_late

From: "Liu, Chuansheng" <[email protected]>

The patch is one helper that adding two new flags for implementing
async threads for suspend_noirq and suspend_late.

Signed-off-by: Liu, Chuansheng <[email protected]>
---
drivers/base/power/main.c | 22 ++++++++++++++++++++--
include/linux/pm.h | 2 ++
2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index 6a33dd8..bf71ddb 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -72,6 +72,8 @@ void device_pm_sleep_init(struct device *dev)
{
dev->power.is_prepared = false;
dev->power.is_suspended = false;
+ dev->power.is_noirq_suspended = false;
+ dev->power.is_late_suspended = false;
init_completion(&dev->power.completion);
complete_all(&dev->power.completion);
dev->power.wakeup = NULL;
@@ -464,6 +466,9 @@ static int device_resume_noirq(struct device *dev, pm_message_t state)
if (dev->power.syscore)
goto Out;

+ if (!dev->power.is_noirq_suspended)
+ goto Out;
+
if (dev->pm_domain) {
info = "noirq power domain ";
callback = pm_noirq_op(&dev->pm_domain->ops, state);
@@ -484,6 +489,7 @@ static int device_resume_noirq(struct device *dev, pm_message_t state)
}

error = dpm_run_callback(callback, dev, state, info);
+ dev->power.is_noirq_suspended = false;

Out:
TRACE_RESUME(error);
@@ -546,6 +552,9 @@ static int device_resume_early(struct device *dev, pm_message_t state)
if (dev->power.syscore)
goto Out;

+ if (!dev->power.is_late_suspended)
+ goto Out;
+
if (dev->pm_domain) {
info = "early power domain ";
callback = pm_late_early_op(&dev->pm_domain->ops, state);
@@ -566,6 +575,7 @@ static int device_resume_early(struct device *dev, pm_message_t state)
}

error = dpm_run_callback(callback, dev, state, info);
+ dev->power.is_late_suspended = false;

Out:
TRACE_RESUME(error);
@@ -902,6 +912,7 @@ static int device_suspend_noirq(struct device *dev, pm_message_t state)
{
pm_callback_t callback = NULL;
char *info = NULL;
+ int error;

if (dev->power.syscore)
return 0;
@@ -925,7 +936,10 @@ static int device_suspend_noirq(struct device *dev, pm_message_t state)
callback = pm_noirq_op(dev->driver->pm, state);
}

- return dpm_run_callback(callback, dev, state, info);
+ error = dpm_run_callback(callback, dev, state, info);
+ if (!error)
+ dev->power.is_noirq_suspended = true;
+ return error;
}

/**
@@ -988,6 +1002,7 @@ static int device_suspend_late(struct device *dev, pm_message_t state)
{
pm_callback_t callback = NULL;
char *info = NULL;
+ int error;

__pm_runtime_disable(dev, false);

@@ -1013,7 +1028,10 @@ static int device_suspend_late(struct device *dev, pm_message_t state)
callback = pm_late_early_op(dev->driver->pm, state);
}

- return dpm_run_callback(callback, dev, state, info);
+ error = dpm_run_callback(callback, dev, state, info);
+ if (!error)
+ dev->power.is_late_suspended = true;
+ return error;
}

/**
diff --git a/include/linux/pm.h b/include/linux/pm.h
index a224c7f..fe90a8b 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -521,6 +521,8 @@ struct dev_pm_info {
unsigned int async_suspend:1;
bool is_prepared:1; /* Owned by the PM core */
bool is_suspended:1; /* Ditto */
+ bool is_noirq_suspended:1;
+ bool is_late_suspended:1;
bool ignore_children:1;
bool early_init:1; /* Owned by the PM core */
spinlock_t lock;
--
1.7.9.5

2014-01-20 08:53:45

by Liu, Chuansheng

[permalink] [raw]
Subject: [PATCH 3/5] PM: Enabling the asyncronous threads for resume_early

From: "Liu, Chuansheng" <[email protected]>

Just like commit 5af84b82701a and 97df8c12995, using the
asynchronous threads can improve the overall resume_early
time significantly.

This patch is for resume_early phase.

Signed-off-by: Liu, Chuansheng <[email protected]>
---
drivers/base/power/main.c | 48 +++++++++++++++++++++++++++++++++++----------
1 file changed, 38 insertions(+), 10 deletions(-)

diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index c8a00fc..1bad6bd 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -619,35 +619,63 @@ static int device_resume_early(struct device *dev, pm_message_t state)
return error;
}

+static void async_resume_early(void *data, async_cookie_t cookie)
+{
+ struct device *dev = (struct device *)data;
+ int error;
+
+ error = device_resume_early(dev, pm_transition);
+ if (error)
+ pm_dev_err(dev, pm_transition, " async", error);
+ put_device(dev);
+}
+
/**
* dpm_resume_early - Execute "early resume" callbacks for all devices.
* @state: PM transition of the system being carried out.
*/
static void dpm_resume_early(pm_message_t state)
{
+ struct device *dev;
ktime_t starttime = ktime_get();

mutex_lock(&dpm_list_mtx);
- while (!list_empty(&dpm_late_early_list)) {
- struct device *dev = to_device(dpm_late_early_list.next);
- int error;
+ pm_transition = state;

+ /*
+ * Advanced the async threads upfront,
+ * in case the starting of async threads is
+ * delayed by non-async resuming devices.
+ */
+ list_for_each_entry(dev, &dpm_late_early_list, power.entry) {
+ if (is_async(dev)) {
+ get_device(dev);
+ async_schedule(async_resume_early, dev);
+ }
+ }
+
+ while (!list_empty(&dpm_late_early_list)) {
+ dev = to_device(dpm_late_early_list.next);
get_device(dev);
list_move_tail(&dev->power.entry, &dpm_suspended_list);
mutex_unlock(&dpm_list_mtx);

- error = device_resume_early(dev, state);
- if (error) {
- suspend_stats.failed_resume_early++;
- dpm_save_failed_step(SUSPEND_RESUME_EARLY);
- dpm_save_failed_dev(dev_name(dev));
- pm_dev_err(dev, state, " early", error);
- }
+ if (!is_async(dev)) {
+ int error;

+ error = device_resume_early(dev, state);
+ if (error) {
+ suspend_stats.failed_resume_early++;
+ dpm_save_failed_step(SUSPEND_RESUME_EARLY);
+ dpm_save_failed_dev(dev_name(dev));
+ pm_dev_err(dev, state, " early", error);
+ }
+ }
mutex_lock(&dpm_list_mtx);
put_device(dev);
}
mutex_unlock(&dpm_list_mtx);
+ async_synchronize_full();
dpm_show_time(starttime, state, "early");
}

--
1.7.9.5

2014-01-20 08:53:50

by Liu, Chuansheng

[permalink] [raw]
Subject: [PATCH 4/5] PM: Enabling the asyncronous threads for suspend_noirq

From: "Liu, Chuansheng" <[email protected]>

Just like commit 5af84b82701a and 97df8c12995, using the
asynchronous threads can improve the overall suspend_noirq
time significantly.

This patch is for suspend_noirq phase.

Signed-off-by: Liu, Chuansheng <[email protected]>
---
drivers/base/power/main.c | 57 ++++++++++++++++++++++++++++++++++++++-------
1 file changed, 48 insertions(+), 9 deletions(-)

diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index 1bad6bd..ec946aa 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -965,12 +965,20 @@ static pm_message_t resume_event(pm_message_t sleep_state)
* The driver of @dev will not receive interrupts while this function is being
* executed.
*/
-static int device_suspend_noirq(struct device *dev, pm_message_t state)
+static int __device_suspend_noirq(struct device *dev, pm_message_t state)
{
pm_callback_t callback = NULL;
char *info = NULL;
int error;

+ if (async_error)
+ return 0;
+
+ if (pm_wakeup_pending()) {
+ async_error = -EBUSY;
+ return async_error;
+ }
+
if (dev->power.syscore)
return 0;

@@ -996,9 +1004,36 @@ static int device_suspend_noirq(struct device *dev, pm_message_t state)
error = dpm_run_callback(callback, dev, state, info);
if (!error)
dev->power.is_noirq_suspended = true;
+ else
+ async_error = error;
+
return error;
}

+static void async_suspend_noirq(void *data, async_cookie_t cookie)
+{
+ struct device *dev = (struct device *)data;
+ int error;
+
+ error = __device_suspend_noirq(dev, pm_transition);
+ if (error) {
+ dpm_save_failed_dev(dev_name(dev));
+ pm_dev_err(dev, pm_transition, " async", error);
+ }
+
+ put_device(dev);
+}
+
+static int device_suspend_noirq(struct device *dev)
+{
+ if (pm_async_enabled && dev->power.async_suspend) {
+ get_device(dev);
+ async_schedule(async_suspend_noirq, dev);
+ return 0;
+ }
+ return __device_suspend_noirq(dev, pm_transition);
+}
+
/**
* dpm_suspend_noirq - Execute "noirq suspend" callbacks for all devices.
* @state: PM transition of the system being carried out.
@@ -1014,19 +1049,20 @@ static int dpm_suspend_noirq(pm_message_t state)
cpuidle_pause();
suspend_device_irqs();
mutex_lock(&dpm_list_mtx);
+ pm_transition = state;
+ async_error = 0;
+
while (!list_empty(&dpm_late_early_list)) {
struct device *dev = to_device(dpm_late_early_list.prev);

get_device(dev);
mutex_unlock(&dpm_list_mtx);

- error = device_suspend_noirq(dev, state);
+ error = device_suspend_noirq(dev);

mutex_lock(&dpm_list_mtx);
if (error) {
pm_dev_err(dev, state, " noirq", error);
- suspend_stats.failed_suspend_noirq++;
- dpm_save_failed_step(SUSPEND_SUSPEND_NOIRQ);
dpm_save_failed_dev(dev_name(dev));
put_device(dev);
break;
@@ -1035,15 +1071,18 @@ static int dpm_suspend_noirq(pm_message_t state)
list_move(&dev->power.entry, &dpm_noirq_list);
put_device(dev);

- if (pm_wakeup_pending()) {
- error = -EBUSY;
+ if (async_error)
break;
- }
}
mutex_unlock(&dpm_list_mtx);
- if (error)
+ async_synchronize_full();
+ if (!error)
+ error = async_error;
+ if (error) {
+ suspend_stats.failed_suspend_noirq++;
+ dpm_save_failed_step(SUSPEND_SUSPEND_NOIRQ);
dpm_resume_noirq(resume_event(state));
- else
+ } else
dpm_show_time(starttime, state, "noirq");
return error;
}
--
1.7.9.5

2014-01-20 08:54:08

by Liu, Chuansheng

[permalink] [raw]
Subject: [PATCH 5/5] PM: Enabling the asyncronous threads for suspend_late

From: "Liu, Chuansheng" <[email protected]>

Just like commit 5af84b82701a and 97df8c12995, using the
asynchronous threads can improve the overall suspend_late
time significantly.

This patch is for suspend_late phase.

Signed-off-by: Liu, Chuansheng <[email protected]>
---
drivers/base/power/main.c | 54 +++++++++++++++++++++++++++++++++++++--------
1 file changed, 45 insertions(+), 9 deletions(-)

diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index ec946aa..19f5195 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -1094,7 +1094,7 @@ static int dpm_suspend_noirq(pm_message_t state)
*
* Runtime PM is disabled for @dev while this function is being executed.
*/
-static int device_suspend_late(struct device *dev, pm_message_t state)
+static int __device_suspend_late(struct device *dev, pm_message_t state)
{
pm_callback_t callback = NULL;
char *info = NULL;
@@ -1102,6 +1102,14 @@ static int device_suspend_late(struct device *dev, pm_message_t state)

__pm_runtime_disable(dev, false);

+ if (async_error)
+ return 0;
+
+ if (pm_wakeup_pending()) {
+ async_error = -EBUSY;
+ return async_error;
+ }
+
if (dev->power.syscore)
return 0;

@@ -1127,9 +1135,35 @@ static int device_suspend_late(struct device *dev, pm_message_t state)
error = dpm_run_callback(callback, dev, state, info);
if (!error)
dev->power.is_late_suspended = true;
+ else
+ async_error = error;
return error;
}

+static void async_suspend_late(void *data, async_cookie_t cookie)
+{
+ struct device *dev = (struct device *)data;
+ int error;
+
+ error = __device_suspend_late(dev, pm_transition);
+ if (error) {
+ dpm_save_failed_dev(dev_name(dev));
+ pm_dev_err(dev, pm_transition, " async", error);
+ }
+ put_device(dev);
+}
+
+static int device_suspend_late(struct device *dev)
+{
+ if (pm_async_enabled && dev->power.async_suspend) {
+ get_device(dev);
+ async_schedule(async_suspend_late, dev);
+ return 0;
+ }
+
+ return __device_suspend_late(dev, pm_transition);
+}
+
/**
* dpm_suspend_late - Execute "late suspend" callbacks for all devices.
* @state: PM transition of the system being carried out.
@@ -1140,19 +1174,20 @@ static int dpm_suspend_late(pm_message_t state)
int error = 0;

mutex_lock(&dpm_list_mtx);
+ pm_transition = state;
+ async_error = 0;
+
while (!list_empty(&dpm_suspended_list)) {
struct device *dev = to_device(dpm_suspended_list.prev);

get_device(dev);
mutex_unlock(&dpm_list_mtx);

- error = device_suspend_late(dev, state);
+ error = device_suspend_late(dev);

mutex_lock(&dpm_list_mtx);
if (error) {
pm_dev_err(dev, state, " late", error);
- suspend_stats.failed_suspend_late++;
- dpm_save_failed_step(SUSPEND_SUSPEND_LATE);
dpm_save_failed_dev(dev_name(dev));
put_device(dev);
break;
@@ -1161,15 +1196,16 @@ static int dpm_suspend_late(pm_message_t state)
list_move(&dev->power.entry, &dpm_late_early_list);
put_device(dev);

- if (pm_wakeup_pending()) {
- error = -EBUSY;
+ if (async_error)
break;
- }
}
mutex_unlock(&dpm_list_mtx);
- if (error)
+ async_synchronize_full();
+ if (error) {
+ suspend_stats.failed_suspend_late++;
+ dpm_save_failed_step(SUSPEND_SUSPEND_LATE);
dpm_resume_early(resume_event(state));
- else
+ } else
dpm_show_time(starttime, state, "late");

return error;
--
1.7.9.5

2014-01-20 08:53:42

by Liu, Chuansheng

[permalink] [raw]
Subject: [PATCH 2/5] PM: Enabling the asynchronous threads for resume_noirq

From: "Liu, Chuansheng" <[email protected]>

Just like commit 5af84b82701a and 97df8c12995, using the
asynchronous threads can improve the overall resume_noirq
time significantly.

One typical case is:
In resume_noirq phase and for the PCI devices, the function
pci_pm_resume_noirq() will be called, and there is one d3_delay
(10ms) at least.

With the way of asynchronous threads, we just need wait d3_delay
time once in parallel for each calling, which save much time to
resume quickly.

Signed-off-by: Liu, Chuansheng <[email protected]>
---
drivers/base/power/main.c | 59 +++++++++++++++++++++++++++++++++------------
1 file changed, 44 insertions(+), 15 deletions(-)

diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index bf71ddb..c8a00fc 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -496,6 +496,23 @@ static int device_resume_noirq(struct device *dev, pm_message_t state)
return error;
}

+static bool is_async(struct device *dev)
+{
+ return dev->power.async_suspend && pm_async_enabled
+ && !pm_trace_is_enabled();
+}
+
+static void async_resume_noirq(void *data, async_cookie_t cookie)
+{
+ struct device *dev = (struct device *)data;
+ int error;
+
+ error = device_resume_noirq(dev, pm_transition);
+ if (error)
+ pm_dev_err(dev, pm_transition, " async", error);
+ put_device(dev);
+}
+
/**
* dpm_resume_noirq - Execute "noirq resume" callbacks for all devices.
* @state: PM transition of the system being carried out.
@@ -505,29 +522,47 @@ static int device_resume_noirq(struct device *dev, pm_message_t state)
*/
static void dpm_resume_noirq(pm_message_t state)
{
+ struct device *dev;
ktime_t starttime = ktime_get();

mutex_lock(&dpm_list_mtx);
- while (!list_empty(&dpm_noirq_list)) {
- struct device *dev = to_device(dpm_noirq_list.next);
- int error;
+ pm_transition = state;

+ /*
+ * Advanced the async threads upfront,
+ * in case the starting of async threads is
+ * delayed by non-async resuming devices.
+ */
+ list_for_each_entry(dev, &dpm_noirq_list, power.entry) {
+ if (is_async(dev)) {
+ get_device(dev);
+ async_schedule(async_resume_noirq, dev);
+ }
+ }
+
+ while (!list_empty(&dpm_noirq_list)) {
+ dev = to_device(dpm_noirq_list.next);
get_device(dev);
list_move_tail(&dev->power.entry, &dpm_late_early_list);
mutex_unlock(&dpm_list_mtx);

- error = device_resume_noirq(dev, state);
- if (error) {
- suspend_stats.failed_resume_noirq++;
- dpm_save_failed_step(SUSPEND_RESUME_NOIRQ);
- dpm_save_failed_dev(dev_name(dev));
- pm_dev_err(dev, state, " noirq", error);
+ if (!is_async(dev)) {
+ int error;
+
+ error = device_resume_noirq(dev, state);
+ if (error) {
+ suspend_stats.failed_resume_noirq++;
+ dpm_save_failed_step(SUSPEND_RESUME_NOIRQ);
+ dpm_save_failed_dev(dev_name(dev));
+ pm_dev_err(dev, state, " noirq", error);
+ }
}

mutex_lock(&dpm_list_mtx);
put_device(dev);
}
mutex_unlock(&dpm_list_mtx);
+ async_synchronize_full();
dpm_show_time(starttime, state, "noirq");
resume_device_irqs();
cpuidle_resume();
@@ -727,12 +762,6 @@ static void async_resume(void *data, async_cookie_t cookie)
put_device(dev);
}

-static bool is_async(struct device *dev)
-{
- return dev->power.async_suspend && pm_async_enabled
- && !pm_trace_is_enabled();
-}
-
/**
* dpm_resume - Execute "resume" callbacks for non-sysdev devices.
* @state: PM transition of the system being carried out.
--
1.7.9.5

2014-01-21 08:57:33

by Li, Zhuangzhi

[permalink] [raw]
Subject: RE: [PATCH 0/5] Enabling the asynchronous threads for other phases



> -----Original Message-----
> From: Liu, Chuansheng
> Sent: Monday, January 20, 2014 4:45 PM
> To: [email protected]; [email protected]; [email protected]; Brown, Len
> Cc: [email protected]; [email protected]; Liu, Chuansheng;
> Li, Zhuangzhi
> Subject: [PATCH 0/5] Enabling the asynchronous threads for other phases
>
> Hello,
>
> This patch series are for enabling the asynchronous threads for the phases
> resume_noirq, resume_early, suspend_noirq and suspend_late.
>
> Just like commit 5af84b82701a and 97df8c12995, with async threads it will
> reduce the system suspending and resuming time significantly.
>
> With these patches, in my test platform, it saved 80% time in resume_noirq
> phase.
>
> Has done the suspend-resume stress test for a long time, please help to
> review.
>
> Best Regards,
>
> [PATCH 1/5] PM: Adding two flags for async suspend_noirq and
> [PATCH 2/5] PM: Enabling the asynchronous threads for resume_noirq
> [PATCH 3/5] PM: Enabling the asyncronous threads for resume_early
> [PATCH 4/5] PM: Enabling the asyncronous threads for suspend_noirq
> [PATCH 5/5] PM: Enabling the asyncronous threads for suspend_late

After applied the above five patches, there was no issue observed during 1000 cycles suspend/resume stress test on Baytrail platform.
Tested-by: lizhuangzhi <[email protected]>
>
> drivers/base/power/main.c | 240
> ++++++++++++++++++++++++++++++++++++---------
> include/linux/pm.h | 2 +
> 2 files changed, 197 insertions(+), 45 deletions(-)

2014-02-05 21:38:34

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH 0/5] Enabling the asynchronous threads for other phases

On Monday, January 20, 2014 04:44:34 PM Liu, Chuansheng wrote:
> Hello,
>
> This patch series are for enabling the asynchronous threads for the phases
> resume_noirq, resume_early, suspend_noirq and suspend_late.
>
> Just like commit 5af84b82701a and 97df8c12995, with async threads it will
> reduce the system suspending and resuming time significantly.
>
> With these patches, in my test platform, it saved 80% time in resume_noirq
> phase.
>
> Has done the suspend-resume stress test for a long time, please help to
> review.
>
> Best Regards,
>
> [PATCH 1/5] PM: Adding two flags for async suspend_noirq and
> [PATCH 2/5] PM: Enabling the asynchronous threads for resume_noirq
> [PATCH 3/5] PM: Enabling the asyncronous threads for resume_early
> [PATCH 4/5] PM: Enabling the asyncronous threads for suspend_noirq
> [PATCH 5/5] PM: Enabling the asyncronous threads for suspend_late

I've applied this to the bleeding-edge branch of the linux-pm.git tree, with
minor changes related to coding style, white space etc.

Can you please verify that the bleeding-edge branch works for you as expected?

Rafael

2014-02-10 08:37:13

by Liu, Chuansheng

[permalink] [raw]
Subject: RE: [PATCH 0/5] Enabling the asynchronous threads for other phases

Hello Rafael,

> -----Original Message-----
> From: Rafael J. Wysocki [mailto:[email protected]]
> Sent: Thursday, February 06, 2014 5:53 AM
> To: Liu, Chuansheng
> Cc: [email protected]; [email protected]; Brown, Len;
> [email protected]; [email protected]; Li, Zhuangzhi
> Subject: Re: [PATCH 0/5] Enabling the asynchronous threads for other phases
>
> On Monday, January 20, 2014 04:44:34 PM Liu, Chuansheng wrote:
> > Hello,
> >
> > This patch series are for enabling the asynchronous threads for the phases
> > resume_noirq, resume_early, suspend_noirq and suspend_late.
> >
> > Just like commit 5af84b82701a and 97df8c12995, with async threads it will
> > reduce the system suspending and resuming time significantly.
> >
> > With these patches, in my test platform, it saved 80% time in resume_noirq
> > phase.
> >
> > Has done the suspend-resume stress test for a long time, please help to
> > review.
> >
> > Best Regards,
> >
> > [PATCH 1/5] PM: Adding two flags for async suspend_noirq and
> > [PATCH 2/5] PM: Enabling the asynchronous threads for resume_noirq
> > [PATCH 3/5] PM: Enabling the asyncronous threads for resume_early
> > [PATCH 4/5] PM: Enabling the asyncronous threads for suspend_noirq
> > [PATCH 5/5] PM: Enabling the asyncronous threads for suspend_late
>
> I've applied this to the bleeding-edge branch of the linux-pm.git tree, with
> minor changes related to coding style, white space etc.
Thanks your help.

>
> Can you please verify that the bleeding-edge branch works for you as
> expected?
I have picked them from your bleeding-edge branch and has done the
suspend-resume stress test for about 200 times and 3 hours,
It is still working well.

Best Regards
Chuansheng Liu
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2014-02-16 23:26:14

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH 0/5] Enabling the asynchronous threads for other phases

On Monday, February 10, 2014 08:36:57 AM Liu, Chuansheng wrote:
> Hello Rafael,
>
> > -----Original Message-----
> > From: Rafael J. Wysocki [mailto:[email protected]]
> > Sent: Thursday, February 06, 2014 5:53 AM
> > To: Liu, Chuansheng
> > Cc: [email protected]; [email protected]; Brown, Len;
> > [email protected]; [email protected]; Li, Zhuangzhi
> > Subject: Re: [PATCH 0/5] Enabling the asynchronous threads for other phases
> >
> > On Monday, January 20, 2014 04:44:34 PM Liu, Chuansheng wrote:
> > > Hello,
> > >
> > > This patch series are for enabling the asynchronous threads for the phases
> > > resume_noirq, resume_early, suspend_noirq and suspend_late.
> > >
> > > Just like commit 5af84b82701a and 97df8c12995, with async threads it will
> > > reduce the system suspending and resuming time significantly.
> > >
> > > With these patches, in my test platform, it saved 80% time in resume_noirq
> > > phase.
> > >
> > > Has done the suspend-resume stress test for a long time, please help to
> > > review.
> > >
> > > Best Regards,
> > >
> > > [PATCH 1/5] PM: Adding two flags for async suspend_noirq and
> > > [PATCH 2/5] PM: Enabling the asynchronous threads for resume_noirq
> > > [PATCH 3/5] PM: Enabling the asyncronous threads for resume_early
> > > [PATCH 4/5] PM: Enabling the asyncronous threads for suspend_noirq
> > > [PATCH 5/5] PM: Enabling the asyncronous threads for suspend_late
> >
> > I've applied this to the bleeding-edge branch of the linux-pm.git tree, with
> > minor changes related to coding style, white space etc.
> Thanks your help.

Unfortunately, I've just realized that your patches don't add any mechanism
ensuring that parent devices' .suspend_noirq() will be called *after* the
.suspend_noirq() of all their children. Of course, analogously for
.suspend_late() and for the resume part (where children have to wait for
their parents).

In the original async suspend/resume code that is implemented using
power.completion and I suppose you can do the same thing here.

I have to drop the series for the time being.

Thanks!

--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

2014-02-17 01:44:35

by Liu, Chuansheng

[permalink] [raw]
Subject: RE: [PATCH 0/5] Enabling the asynchronous threads for other phases

Hello Rafael,

> -----Original Message-----
> From: Rafael J. Wysocki [mailto:[email protected]]
> Sent: Monday, February 17, 2014 7:41 AM
> To: Liu, Chuansheng
> Cc: [email protected]; [email protected]; Brown, Len;
> [email protected]; [email protected]; Li, Zhuangzhi
> Subject: Re: [PATCH 0/5] Enabling the asynchronous threads for other phases
>
> On Monday, February 10, 2014 08:36:57 AM Liu, Chuansheng wrote:
> > Hello Rafael,
> >
> > > -----Original Message-----
> > > From: Rafael J. Wysocki [mailto:[email protected]]
> > > Sent: Thursday, February 06, 2014 5:53 AM
> > > To: Liu, Chuansheng
> > > Cc: [email protected]; [email protected]; Brown, Len;
> > > [email protected]; [email protected]; Li, Zhuangzhi
> > > Subject: Re: [PATCH 0/5] Enabling the asynchronous threads for other
> phases
> > >
> > > On Monday, January 20, 2014 04:44:34 PM Liu, Chuansheng wrote:
> > > > Hello,
> > > >
> > > > This patch series are for enabling the asynchronous threads for the
> phases
> > > > resume_noirq, resume_early, suspend_noirq and suspend_late.
> > > >
> > > > Just like commit 5af84b82701a and 97df8c12995, with async threads it
> will
> > > > reduce the system suspending and resuming time significantly.
> > > >
> > > > With these patches, in my test platform, it saved 80% time in
> resume_noirq
> > > > phase.
> > > >
> > > > Has done the suspend-resume stress test for a long time, please help to
> > > > review.
> > > >
> > > > Best Regards,
> > > >
> > > > [PATCH 1/5] PM: Adding two flags for async suspend_noirq and
> > > > [PATCH 2/5] PM: Enabling the asynchronous threads for resume_noirq
> > > > [PATCH 3/5] PM: Enabling the asyncronous threads for resume_early
> > > > [PATCH 4/5] PM: Enabling the asyncronous threads for suspend_noirq
> > > > [PATCH 5/5] PM: Enabling the asyncronous threads for suspend_late
> > >
> > > I've applied this to the bleeding-edge branch of the linux-pm.git tree, with
> > > minor changes related to coding style, white space etc.
> > Thanks your help.
>
> Unfortunately, I've just realized that your patches don't add any mechanism
> ensuring that parent devices' .suspend_noirq() will be called *after* the
> .suspend_noirq() of all their children. Of course, analogously for
> .suspend_late() and for the resume part (where children have to wait for
> their parents).
>
> In the original async suspend/resume code that is implemented using
> power.completion and I suppose you can do the same thing here.

I think "parent devices suspend after their children" is not related directly
with using async or not;
In the original code, even without asyncing, the suspend/resume has the waiting
action, but there no waiting action for suspend_noirq and other phases.

If you think the parent waiting for children is necessary for suspend_noirq/suspend_late
/resume_early/resume_noirq, I can cook other series patches.

But we can separate them with current asyncing series patches. Agree?

????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2014-02-17 02:11:53

by Liu, Chuansheng

[permalink] [raw]
Subject: RE: [PATCH 0/5] Enabling the asynchronous threads for other phases

Hello Rafael,

> -----Original Message-----
> From: Liu, Chuansheng
> Sent: Monday, February 17, 2014 9:44 AM
> To: 'Rafael J. Wysocki'
> Cc: [email protected]; [email protected]; Brown, Len;
> [email protected]; [email protected]; Li, Zhuangzhi
> Subject: RE: [PATCH 0/5] Enabling the asynchronous threads for other phases
>
> Hello Rafael,
>
> > -----Original Message-----
> > From: Rafael J. Wysocki [mailto:[email protected]]
> > Sent: Monday, February 17, 2014 7:41 AM
> > To: Liu, Chuansheng
> > Cc: [email protected]; [email protected]; Brown, Len;
> > [email protected]; [email protected]; Li, Zhuangzhi
> > Subject: Re: [PATCH 0/5] Enabling the asynchronous threads for other phases
> >
> > On Monday, February 10, 2014 08:36:57 AM Liu, Chuansheng wrote:
> > > Hello Rafael,
> > >
> > > > -----Original Message-----
> > > > From: Rafael J. Wysocki [mailto:[email protected]]
> > > > Sent: Thursday, February 06, 2014 5:53 AM
> > > > To: Liu, Chuansheng
> > > > Cc: [email protected]; [email protected]; Brown, Len;
> > > > [email protected]; [email protected]; Li, Zhuangzhi
> > > > Subject: Re: [PATCH 0/5] Enabling the asynchronous threads for other
> > phases
> > > >
> > > > On Monday, January 20, 2014 04:44:34 PM Liu, Chuansheng wrote:
> > > > > Hello,
> > > > >
> > > > > This patch series are for enabling the asynchronous threads for the
> > phases
> > > > > resume_noirq, resume_early, suspend_noirq and suspend_late.
> > > > >
> > > > > Just like commit 5af84b82701a and 97df8c12995, with async threads it
> > will
> > > > > reduce the system suspending and resuming time significantly.
> > > > >
> > > > > With these patches, in my test platform, it saved 80% time in
> > resume_noirq
> > > > > phase.
> > > > >
> > > > > Has done the suspend-resume stress test for a long time, please help to
> > > > > review.
> > > > >
> > > > > Best Regards,
> > > > >
> > > > > [PATCH 1/5] PM: Adding two flags for async suspend_noirq and
> > > > > [PATCH 2/5] PM: Enabling the asynchronous threads for resume_noirq
> > > > > [PATCH 3/5] PM: Enabling the asyncronous threads for resume_early
> > > > > [PATCH 4/5] PM: Enabling the asyncronous threads for suspend_noirq
> > > > > [PATCH 5/5] PM: Enabling the asyncronous threads for suspend_late
> > > >
> > > > I've applied this to the bleeding-edge branch of the linux-pm.git tree, with
> > > > minor changes related to coding style, white space etc.
> > > Thanks your help.
> >
> > Unfortunately, I've just realized that your patches don't add any mechanism
> > ensuring that parent devices' .suspend_noirq() will be called *after* the
> > .suspend_noirq() of all their children. Of course, analogously for
> > .suspend_late() and for the resume part (where children have to wait for
> > their parents).
> >
> > In the original async suspend/resume code that is implemented using
> > power.completion and I suppose you can do the same thing here.
>
> I think "parent devices suspend after their children" is not related directly
> with using async or not;
> In the original code, even without asyncing, the suspend/resume has the
> waiting
> action, but there no waiting action for suspend_noirq and other phases.
>
Sorry, re-checked the code, the order in the list has made sure the sequence
of parent-then-children when without asyncing.
Thanks your pointing out, will re-cook this series patch.

????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?