[RFC PATCH] PM/CPU: Parallel enabling nonboot cpus with resume devices

[RFC PATCH] PM/CPU: Parallel enabling nonboot cpus with resume devices

[Lan Tianyu]

In the current world, all nonboot cpus are enabled serially during system
resume. System resume sequence is that boot cpu enables nonboot cpu one by
one and then resume devices. Before resuming devices, there are few tasks
assigned to nonboot cpus after they are brought up. This waste cpu usage.

To accelerate S3, this patches adds a new kernel configure
PM_PARALLEL_CPU_UP_FOR_SUSPEND to allow boot cpu to go forward to resume
devices after bringing up one nonboot cpu. The nonboot cpu will be in charge
of bringing up other cpus. This makes enabling cpu2~x parallel with resuming
devices. From the test result on 4 logical core laptop, the time of resume
device almost wasn't affected by enabling nonboot cpus lately while the start
point is almost 30ms earlier than before.

Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
---
 kernel/cpu.c         | 82 ++++++++++++++++++++++++++++++++++++++++++++++------
 kernel/power/Kconfig | 13 +++++++++
 2 files changed, 86 insertions(+), 9 deletions(-)

diff --git a/kernel/cpu.c b/kernel/cpu.c
index a343bde..d4c1353 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -551,9 +551,27 @@ void __weak arch_enable_nonboot_cpus_end(void)
 {
 }
 
+static int _cpu_up_with_trace(int cpu)
+{
+	int error;
+
+	trace_suspend_resume(TPS("CPU_ON"), cpu, true);
+	error = _cpu_up(cpu, 1);
+	trace_suspend_resume(TPS("CPU_ON"), cpu, false);
+	if (error) {
+		pr_warn("Error taking CPU%d up: %d\n", cpu, error);
+		return error;
+	}
+
+	pr_info("CPU%d is up\n", cpu);
+	return 0;
+}
+
+#ifndef CONFIG_PM_PARALLEL_CPU_UP_FOR_SUSPEND
+
 void __ref enable_nonboot_cpus(void)
 {
-	int cpu, error;
+	int cpu;
 
 	/* Allow everyone to use the CPU hotplug again */
 	cpu_maps_update_begin();
@@ -566,14 +584,7 @@ void __ref enable_nonboot_cpus(void)
 	arch_enable_nonboot_cpus_begin();
 
 	for_each_cpu(cpu, frozen_cpus) {
-		trace_suspend_resume(TPS("CPU_ON"), cpu, true);
-		error = _cpu_up(cpu, 1);
-		trace_suspend_resume(TPS("CPU_ON"), cpu, false);
-		if (!error) {
-			pr_info("CPU%d is up\n", cpu);
-			continue;
-		}
-		pr_warn("Error taking CPU%d up: %d\n", cpu, error);
+		_cpu_up_with_trace(cpu);
 	}
 
 	arch_enable_nonboot_cpus_end();
@@ -583,6 +594,59 @@ out:
 	cpu_maps_update_done();
 }
 
+#else
+
+static int async_enable_nonboot_cpus(void *data)
+{
+	int cpu;
+
+	cpu_maps_update_begin();
+
+	for_each_cpu(cpu, frozen_cpus) {
+		_cpu_up_with_trace(cpu);
+	}
+
+	arch_enable_nonboot_cpus_end();
+	cpumask_clear(frozen_cpus);
+	cpu_maps_update_done();
+	return 0;
+}
+
+void __ref enable_nonboot_cpus(void)
+{
+	struct task_struct *tsk;
+	int cpu;
+
+	/* Allow everyone to use the CPU hotplug again */
+	cpu_maps_update_begin();
+	cpu_hotplug_disabled = 0;
+	if (cpumask_empty(frozen_cpus))
+		goto out;
+
+	arch_enable_nonboot_cpus_begin();
+
+	cpu = cpumask_first(frozen_cpus);
+	cpumask_clear_cpu(cpu, frozen_cpus);
+
+	_cpu_up_with_trace(cpu);
+
+	if (cpumask_empty(frozen_cpus)) {
+		arch_enable_nonboot_cpus_end();
+	} else {
+		tsk = kthread_create_on_cpu(async_enable_nonboot_cpus,
+				NULL, cpu, "async-enable-nonboot-cpus");
+		if (IS_ERR(tsk)) {
+			pr_err("Failed to create async enable nonboot cpus thread.\n");
+			goto out;
+		}
+
+		kthread_unpark(tsk);
+	}
+out:
+	cpu_maps_update_done();
+}
+#endif
+
 static int __init alloc_frozen_cpus(void)
 {
 	if (!alloc_cpumask_var(&frozen_cpus, GFP_KERNEL|__GFP_ZERO))
diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
index 9a83d78..e5e6671 100644
--- a/kernel/power/Kconfig
+++ b/kernel/power/Kconfig
@@ -110,6 +110,19 @@ config PM_AUTOSLEEP
 	Allow the kernel to trigger a system transition into a global sleep
 	state automatically whenever there are no active wakeup sources.
 
+config PM_PARALLEL_CPU_UP_FOR_SUSPEND
+	bool "Parallel non-boot cpus up for system suspend"
+	depends on SMP
+	depends on PM_SLEEP
+	default n
+	---help---
+	In the current world, all nonboot cpus are enabled serially during system
+	resume. System resume sequence is that boot cpu enables nonboot cpu one by
+	one and then resume devices. Before resuming devices, there are few tasks
+	assigned to nonboot cpus. To accelerate S3, this option allows boot cpu
+	to go forward to resume devices after bringing up one nonboot cpu. The
+	nonboot cpu will be in charge of bring up other cpus.
+
 config PM_WAKELOCKS
 	bool "User space wakeup sources interface"
 	depends on PM_SLEEP
-- 
1.8.3.1

Re: [RFC PATCH] PM/CPU: Parallel enabling nonboot cpus with resume devices

[Peter Zijlstra]

On Wed, Jul 23, 2014 at 05:11:34PM +0800, Lan Tianyu wrote:
> In the current world, all nonboot cpus are enabled serially during system
> resume. System resume sequence is that boot cpu enables nonboot cpu one by
> one and then resume devices. Before resuming devices, there are few tasks
> assigned to nonboot cpus after they are brought up. This waste cpu usage.
> 
> To accelerate S3, this patches adds a new kernel configure
> PM_PARALLEL_CPU_UP_FOR_SUSPEND to allow boot cpu to go forward to resume
> devices after bringing up one nonboot cpu. The nonboot cpu will be in charge
> of bringing up other cpus. This makes enabling cpu2~x parallel with resuming
> devices. From the test result on 4 logical core laptop, the time of resume
> device almost wasn't affected by enabling nonboot cpus lately while the start
> point is almost 30ms earlier than before.

Why is this a CONFIG and why do we want to add more warts to the cpu
hotplug instead of fixing it 'proper'?

Re: [RFC PATCH] PM/CPU: Parallel enabling nonboot cpus with resume devices

[Lan Tianyu]

On 2014年07月23日 17:21, Peter Zijlstra wrote:
> On Wed, Jul 23, 2014 at 05:11:34PM +0800, Lan Tianyu wrote:
>> In the current world, all nonboot cpus are enabled serially during system
>> resume. System resume sequence is that boot cpu enables nonboot cpu one by
>> one and then resume devices. Before resuming devices, there are few tasks
>> assigned to nonboot cpus after they are brought up. This waste cpu usage.
>>
>> To accelerate S3, this patches adds a new kernel configure
>> PM_PARALLEL_CPU_UP_FOR_SUSPEND to allow boot cpu to go forward to resume
>> devices after bringing up one nonboot cpu. The nonboot cpu will be in charge
>> of bringing up other cpus. This makes enabling cpu2~x parallel with resuming
>> devices. From the test result on 4 logical core laptop, the time of resume
>> device almost wasn't affected by enabling nonboot cpus lately while the start
>> point is almost 30ms earlier than before.
> 
> Why is this a CONFIG and why do we want to add more warts to the cpu
> hotplug instead of fixing it 'proper'?
> 

Hi Peter:
	Thank you for your review. Because I just tested this patch on x86
platform and didn't want to cause some regressions on the other
platforms. So I made it as a new CONFIG. In theory, this should not
cause some problems. If none objects, it can be the default behavior.

-- 
Best regards
Tianyu Lan

Re: [RFC PATCH] PM/CPU: Parallel enabling nonboot cpus with resume devices

[Pavel Machek]

Hi!

> In the current world, all nonboot cpus are enabled serially during system
> resume. System resume sequence is that boot cpu enables nonboot cpu one by
> one and then resume devices. Before resuming devices, there are few tasks
> assigned to nonboot cpus after they are brought up. This waste cpu usage.
> 
> To accelerate S3, this patches adds a new kernel configure
> PM_PARALLEL_CPU_UP_FOR_SUSPEND to allow boot cpu to go forward to resume
> devices after bringing up one nonboot cpu. The nonboot cpu will be in charge
> of bringing up other cpus. This makes enabling cpu2~x parallel with resuming
> devices. From the test result on 4 logical core laptop, the time of resume
> device almost wasn't affected by enabling nonboot cpus lately while the start
> point is almost 30ms earlier than before.

Does this mean that userspace can now run seeing the "offlined" cpus
still in offline state?

									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFC PATCH] PM/CPU: Parallel enabling nonboot cpus with resume devices

[Lan Tianyu]

On 2014年07月23日 18:53, Pavel Machek wrote:
> Hi!
> 
>> In the current world, all nonboot cpus are enabled serially during system
>> resume. System resume sequence is that boot cpu enables nonboot cpu one by
>> one and then resume devices. Before resuming devices, there are few tasks
>> assigned to nonboot cpus after they are brought up. This waste cpu usage.
>>
>> To accelerate S3, this patches adds a new kernel configure
>> PM_PARALLEL_CPU_UP_FOR_SUSPEND to allow boot cpu to go forward to resume
>> devices after bringing up one nonboot cpu. The nonboot cpu will be in charge
>> of bringing up other cpus. This makes enabling cpu2~x parallel with resuming
>> devices. From the test result on 4 logical core laptop, the time of resume
>> device almost wasn't affected by enabling nonboot cpus lately while the start
>> point is almost 30ms earlier than before.
> 
> Does this mean that userspace can now run seeing the "offlined" cpus
> still in offline state?

No, the PM event handler of cpu hotplug will reenable cpu hotplug via
cpu_hotplug_enable() at the end of system resume. The function will wait
for currently running cpu hotplug operations to complete.

> 
> 									Pavel
> 


-- 
Best regards
Tianyu Lan

Re: [RFC PATCH] PM/CPU: Parallel enabling nonboot cpus with resume devices

[Gautham R Shenoy]

Hi Lan,

On Wed, Jul 23, 2014 at 05:11:34PM +0800, Lan Tianyu wrote:
> In the current world, all nonboot cpus are enabled serially during system
> resume. System resume sequence is that boot cpu enables nonboot cpu one by
> one and then resume devices. Before resuming devices, there are few tasks
> assigned to nonboot cpus after they are brought up. This waste cpu usage.
> 
> To accelerate S3, this patches adds a new kernel configure
> PM_PARALLEL_CPU_UP_FOR_SUSPEND to allow boot cpu to go forward to resume
> devices after bringing up one nonboot cpu. The nonboot cpu will be in charge
> of bringing up other cpus. This makes enabling cpu2~x parallel with resuming
> devices. From the test result on 4 logical core laptop, the time of resume
> device almost wasn't affected by enabling nonboot cpus lately while the start
> point is almost 30ms earlier than before.
> 
> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
> ---
>  kernel/cpu.c         | 82 ++++++++++++++++++++++++++++++++++++++++++++++------
>  kernel/power/Kconfig | 13 +++++++++
>  2 files changed, 86 insertions(+), 9 deletions(-)
> 
> diff --git a/kernel/cpu.c b/kernel/cpu.c
> index a343bde..d4c1353 100644
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -551,9 +551,27 @@ void __weak arch_enable_nonboot_cpus_end(void)
>  {
>  }
> 
> +static int _cpu_up_with_trace(int cpu)
> +{
> +	int error;
> +
> +	trace_suspend_resume(TPS("CPU_ON"), cpu, true);
> +	error = _cpu_up(cpu, 1);
> +	trace_suspend_resume(TPS("CPU_ON"), cpu, false);
> +	if (error) {
> +		pr_warn("Error taking CPU%d up: %d\n", cpu, error);
> +		return error;
> +	}
> +
> +	pr_info("CPU%d is up\n", cpu);
> +	return 0;
> +}
> +

[..snip..]
> 
> +
> +void __ref enable_nonboot_cpus(void)
> +{
> +	struct task_struct *tsk;
> +	int cpu;
> +
> +	/* Allow everyone to use the CPU hotplug again */
> +	cpu_maps_update_begin();
> +	cpu_hotplug_disabled = 0;
> +	if (cpumask_empty(frozen_cpus))
> +		goto out;
> +
> +	arch_enable_nonboot_cpus_begin();
> +
> +	cpu = cpumask_first(frozen_cpus);
> +	cpumask_clear_cpu(cpu, frozen_cpus);
> +
> +	_cpu_up_with_trace(cpu);

We should be handling the error returned by _cpu_up_with_trace() 
in case 'cpu' fails to come online. Unless it is something that I am
not aware of it doesn't make much sense to create a kthread on a cpu
that we know has failed to come online.


> +
> +	if (cpumask_empty(frozen_cpus)) {
> +		arch_enable_nonboot_cpus_end();
> +	} else {
> +		tsk = kthread_create_on_cpu(async_enable_nonboot_cpus,
> +				NULL, cpu, "async-enable-nonboot-cpus");
> +		if (IS_ERR(tsk)) {
> +			pr_err("Failed to create async enable nonboot cpus thread.\n");
> +			goto out;
> +		}
> +
> +		kthread_unpark(tsk);
> +	}
> +out:
> +	cpu_maps_update_done();
> +}
> +#endif
> +

--
Thanks and Regards
gautham.

Re: [RFC PATCH] PM/CPU: Parallel enabling nonboot cpus with resume devices

[Lan Tianyu]

On 2014年08月08日 18:55, Gautham R Shenoy wrote:
> Hi Lan,
> 
> On Wed, Jul 23, 2014 at 05:11:34PM +0800, Lan Tianyu wrote:
>> In the current world, all nonboot cpus are enabled serially during system
>> resume. System resume sequence is that boot cpu enables nonboot cpu one by
>> one and then resume devices. Before resuming devices, there are few tasks
>> assigned to nonboot cpus after they are brought up. This waste cpu usage.
>>
>> To accelerate S3, this patches adds a new kernel configure
>> PM_PARALLEL_CPU_UP_FOR_SUSPEND to allow boot cpu to go forward to resume
>> devices after bringing up one nonboot cpu. The nonboot cpu will be in charge
>> of bringing up other cpus. This makes enabling cpu2~x parallel with resuming
>> devices. From the test result on 4 logical core laptop, the time of resume
>> device almost wasn't affected by enabling nonboot cpus lately while the start
>> point is almost 30ms earlier than before.
>>
>> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
>> ---
>>  kernel/cpu.c         | 82 ++++++++++++++++++++++++++++++++++++++++++++++------
>>  kernel/power/Kconfig | 13 +++++++++
>>  2 files changed, 86 insertions(+), 9 deletions(-)
>>
>> diff --git a/kernel/cpu.c b/kernel/cpu.c
>> index a343bde..d4c1353 100644
>> --- a/kernel/cpu.c
>> +++ b/kernel/cpu.c
>> @@ -551,9 +551,27 @@ void __weak arch_enable_nonboot_cpus_end(void)
>>  {
>>  }
>>
>> +static int _cpu_up_with_trace(int cpu)
>> +{
>> +	int error;
>> +
>> +	trace_suspend_resume(TPS("CPU_ON"), cpu, true);
>> +	error = _cpu_up(cpu, 1);
>> +	trace_suspend_resume(TPS("CPU_ON"), cpu, false);
>> +	if (error) {
>> +		pr_warn("Error taking CPU%d up: %d\n", cpu, error);
>> +		return error;
>> +	}
>> +
>> +	pr_info("CPU%d is up\n", cpu);
>> +	return 0;
>> +}
>> +
> 
> [..snip..]
>>
>> +
>> +void __ref enable_nonboot_cpus(void)
>> +{
>> +	struct task_struct *tsk;
>> +	int cpu;
>> +
>> +	/* Allow everyone to use the CPU hotplug again */
>> +	cpu_maps_update_begin();
>> +	cpu_hotplug_disabled = 0;
>> +	if (cpumask_empty(frozen_cpus))
>> +		goto out;
>> +
>> +	arch_enable_nonboot_cpus_begin();
>> +
>> +	cpu = cpumask_first(frozen_cpus);
>> +	cpumask_clear_cpu(cpu, frozen_cpus);
>> +
>> +	_cpu_up_with_trace(cpu);
> 
> We should be handling the error returned by _cpu_up_with_trace() 
> in case 'cpu' fails to come online. Unless it is something that I am
> not aware of it doesn't make much sense to create a kthread on a cpu
> that we know has failed to come online.
> 

Hi Gautham:

Great thanks for your review. Yes, you are right. I will take it into
account when send out V2.

> 
>> +
>> +	if (cpumask_empty(frozen_cpus)) {
>> +		arch_enable_nonboot_cpus_end();
>> +	} else {
>> +		tsk = kthread_create_on_cpu(async_enable_nonboot_cpus,
>> +				NULL, cpu, "async-enable-nonboot-cpus");
>> +		if (IS_ERR(tsk)) {
>> +			pr_err("Failed to create async enable nonboot cpus thread.\n");
>> +			goto out;
>> +		}
>> +
>> +		kthread_unpark(tsk);
>> +	}
>> +out:
>> +	cpu_maps_update_done();
>> +}
>> +#endif
>> +
> 
> --
> Thanks and Regards
> gautham.
> 


-- 
Best regards
Tianyu Lan