All of lore.kernel.org
 help / color / mirror / Atom feed
* [patch] x86: avoid unnecessary smp alternatives switch during suspend/resume
@ 2010-11-20  0:09 Suresh Siddha
  2010-11-20 16:31 ` Ben Gamari
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Suresh Siddha @ 2010-11-20  0:09 UTC (permalink / raw)
  To: H. Peter Anvin, Ingo Molnar, Thomas Gleixner; +Cc: LKML, Arjan van de Ven

During suspend, we disable all the non boot cpus. And during resume we bring
them all back again. So no need to do alternatives_smp_switch() in between.

This speeds up both suspend and resume paths.

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
---

 arch/x86/kernel/smpboot.c |    7 ++++++-
 include/linux/cpu.h       |    2 ++
 kernel/cpu.c              |    3 +++
 3 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index f0a0624..0b04ca3 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1349,7 +1349,12 @@ void native_cpu_die(unsigned int cpu)
 			if (system_state == SYSTEM_RUNNING)
 				pr_info("CPU %u is now offline\n", cpu);
 
-			if (1 == num_online_cpus())
+			/*
+			 * Don't do the smp alternatives switch during
+			 * suspend. We will be back in the SMP mode after
+			 * resume.
+			 */
+			if (1 == num_online_cpus() && !pm_sleep_smp)
 				alternatives_smp_switch(0);
 			return;
 		}
diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index 4823af6..8cab04c 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -169,11 +169,13 @@ static inline void cpu_hotplug_driver_unlock(void)
 
 #ifdef CONFIG_PM_SLEEP_SMP
 extern int suspend_cpu_hotplug;
+extern int pm_sleep_smp;
 
 extern int disable_nonboot_cpus(void);
 extern void enable_nonboot_cpus(void);
 #else /* !CONFIG_PM_SLEEP_SMP */
 #define suspend_cpu_hotplug	0
+#define pm_sleep_smp		0
 
 static inline int disable_nonboot_cpus(void) { return 0; }
 static inline void enable_nonboot_cpus(void) {}
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 8615aa6..2eed810 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -381,6 +381,7 @@ out:
 
 #ifdef CONFIG_PM_SLEEP_SMP
 static cpumask_var_t frozen_cpus;
+int pm_sleep_smp;
 
 int disable_nonboot_cpus(void)
 {
@@ -393,6 +394,7 @@ int disable_nonboot_cpus(void)
 	 * with the userspace trying to use the CPU hotplug at the same time
 	 */
 	cpumask_clear(frozen_cpus);
+	pm_sleep_smp = 1;
 
 	printk("Disabling non-boot CPUs ...\n");
 	for_each_online_cpu(cpu) {
@@ -454,6 +456,7 @@ void __ref enable_nonboot_cpus(void)
 
 	cpumask_clear(frozen_cpus);
 out:
+	pm_sleep_smp = 0;
 	cpu_maps_update_done();
 }
 



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [patch] x86: avoid unnecessary smp alternatives switch during suspend/resume
  2010-11-20  0:09 [patch] x86: avoid unnecessary smp alternatives switch during suspend/resume Suresh Siddha
@ 2010-11-20 16:31 ` Ben Gamari
  2010-11-21  6:02 ` Américo Wang
  2010-11-21  9:27 ` Nigel Cunningham
  2 siblings, 0 replies; 11+ messages in thread
From: Ben Gamari @ 2010-11-20 16:31 UTC (permalink / raw)
  To: Suresh Siddha, H. Peter Anvin, Ingo Molnar, Thomas Gleixner
  Cc: LKML, Arjan van de Ven

On Fri, 19 Nov 2010 16:09:24 -0800, Suresh Siddha <suresh.b.siddha@intel.com> wrote:
> During suspend, we disable all the non boot cpus. And during resume we bring
> them all back again. So no need to do alternatives_smp_switch() in between.
> 
> This speeds up both suspend and resume paths.
> 
Do you have typical before/after values? Is the change on the order of
microseconds, hundreds of milliseconds, or somewhere in between? It
might be good to note this in the changelog.

- Ben

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [patch] x86: avoid unnecessary smp alternatives switch during suspend/resume
  2010-11-20  0:09 [patch] x86: avoid unnecessary smp alternatives switch during suspend/resume Suresh Siddha
  2010-11-20 16:31 ` Ben Gamari
@ 2010-11-21  6:02 ` Américo Wang
  2010-11-21  9:27 ` Nigel Cunningham
  2 siblings, 0 replies; 11+ messages in thread
From: Américo Wang @ 2010-11-21  6:02 UTC (permalink / raw)
  To: Suresh Siddha
  Cc: H. Peter Anvin, Ingo Molnar, Thomas Gleixner, LKML, Arjan van de Ven

On Fri, Nov 19, 2010 at 04:09:24PM -0800, Suresh Siddha wrote:
>During suspend, we disable all the non boot cpus. And during resume we bring
>them all back again. So no need to do alternatives_smp_switch() in between.
>
>This speeds up both suspend and resume paths.
>
>Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>

This is a cool idea!

The patch looks good for me, just note that kexec
also calls disable_nonboot_cpus(), so the name 'pm_sleep_smp'
is a bit confused. ;)

Reviewed-by: WANG Cong <xiyou.wangcong@gmail.com>

Thanks!

-- 
Live like a child, think like the god.
 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [patch] x86: avoid unnecessary smp alternatives switch during suspend/resume
  2010-11-20  0:09 [patch] x86: avoid unnecessary smp alternatives switch during suspend/resume Suresh Siddha
  2010-11-20 16:31 ` Ben Gamari
  2010-11-21  6:02 ` Américo Wang
@ 2010-11-21  9:27 ` Nigel Cunningham
  2010-11-21 10:03   ` Borislav Petkov
                     ` (2 more replies)
  2 siblings, 3 replies; 11+ messages in thread
From: Nigel Cunningham @ 2010-11-21  9:27 UTC (permalink / raw)
  To: Suresh Siddha
  Cc: H. Peter Anvin, Ingo Molnar, Thomas Gleixner, LKML, Arjan van de Ven

Hi.

On 20/11/10 11:09, Suresh Siddha wrote:
> During suspend, we disable all the non boot cpus. And during resume we bring
> them all back again. So no need to do alternatives_smp_switch() in between.
>
> This speeds up both suspend and resume paths.
>
> Signed-off-by: Suresh Siddha<suresh.b.siddha@intel.com>
> ---
>
>   arch/x86/kernel/smpboot.c |    7 ++++++-
>   include/linux/cpu.h       |    2 ++
>   kernel/cpu.c              |    3 +++
>   3 files changed, 11 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
> index f0a0624..0b04ca3 100644
> --- a/arch/x86/kernel/smpboot.c
> +++ b/arch/x86/kernel/smpboot.c
> @@ -1349,7 +1349,12 @@ void native_cpu_die(unsigned int cpu)
>   			if (system_state == SYSTEM_RUNNING)
>   				pr_info("CPU %u is now offline\n", cpu);
>
> -			if (1 == num_online_cpus())
> +			/*
> +			 * Don't do the smp alternatives switch during
> +			 * suspend. We will be back in the SMP mode after
> +			 * resume.
> +			 */
> +			if (1 == num_online_cpus()&&  !pm_sleep_smp)
>   				alternatives_smp_switch(0);
>   			return;
>   		}
> diff --git a/include/linux/cpu.h b/include/linux/cpu.h
> index 4823af6..8cab04c 100644
> --- a/include/linux/cpu.h
> +++ b/include/linux/cpu.h
> @@ -169,11 +169,13 @@ static inline void cpu_hotplug_driver_unlock(void)
>
>   #ifdef CONFIG_PM_SLEEP_SMP
>   extern int suspend_cpu_hotplug;
> +extern int pm_sleep_smp;
>
>   extern int disable_nonboot_cpus(void);
>   extern void enable_nonboot_cpus(void);
>   #else /* !CONFIG_PM_SLEEP_SMP */
>   #define suspend_cpu_hotplug	0
> +#define pm_sleep_smp		0
>
>   static inline int disable_nonboot_cpus(void) { return 0; }
>   static inline void enable_nonboot_cpus(void) {}
> diff --git a/kernel/cpu.c b/kernel/cpu.c
> index 8615aa6..2eed810 100644
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -381,6 +381,7 @@ out:
>
>   #ifdef CONFIG_PM_SLEEP_SMP
>   static cpumask_var_t frozen_cpus;
> +int pm_sleep_smp;
>
>   int disable_nonboot_cpus(void)
>   {
> @@ -393,6 +394,7 @@ int disable_nonboot_cpus(void)
>   	 * with the userspace trying to use the CPU hotplug at the same time
>   	 */
>   	cpumask_clear(frozen_cpus);
> +	pm_sleep_smp = 1;
>
>   	printk("Disabling non-boot CPUs ...\n");
>   	for_each_online_cpu(cpu) {
> @@ -454,6 +456,7 @@ void __ref enable_nonboot_cpus(void)
>
>   	cpumask_clear(frozen_cpus);
>   out:
> +	pm_sleep_smp = 0;
>   	cpu_maps_update_done();
>   }
We have a few others things that want to modify their behaviour 
according to whether we're doing the atomic copy/restore. Perhaps it 
would be an idea to just use a single flag, perhaps a value for 
system_state?

Nigel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [patch] x86: avoid unnecessary smp alternatives switch during suspend/resume
  2010-11-21  9:27 ` Nigel Cunningham
@ 2010-11-21 10:03   ` Borislav Petkov
  2011-01-02  9:11     ` Pavel Machek
  2010-11-22  2:30   ` H. Peter Anvin
  2010-11-24  0:11   ` Suresh Siddha
  2 siblings, 1 reply; 11+ messages in thread
From: Borislav Petkov @ 2010-11-21 10:03 UTC (permalink / raw)
  To: Nigel Cunningham
  Cc: Suresh Siddha, H. Peter Anvin, Ingo Molnar, Thomas Gleixner,
	LKML, Arjan van de Ven

[-- Attachment #1: Type: text/plain, Size: 735 bytes --]

On Sun, Nov 21, 2010 at 08:27:41PM +1100, Nigel Cunningham wrote:
> Hi.
> 
> We have a few others things that want to modify their behaviour
> according to whether we're doing the atomic copy/restore. Perhaps it
> would be an idea to just use a single flag, perhaps a value for
> system_state?

I agree, it would be nicer if we don't introduce a special flag
just for that. Other than that, I like all sane code that speeds up
suspend/resume :). I've attached before/after dmesg excerpts on my
system with the patch ontop of v2.6.37-rc2-181-gb86db47. We end up
saving 11601 µsecs according to CONFIG_PRINTK_TIME but hey, the code is
simple enough :).

Tested-by: Borislav Petkov <bp@alien8.de>

Thanks.

-- 
Regards/Gruss,
    Boris.

[-- Attachment #2: suspend.before --]
[-- Type: text/plain, Size: 1598 bytes --]

[  122.004099] [drm] Resetting GPU
[  122.014614] PM: Syncing filesystems ... done.
[  122.027334] Freezing user space processes ... (elapsed 0.01 seconds) done.
[  122.039644] Freezing remaining freezable tasks ... (elapsed 0.01 seconds) done.
[  122.050629] PM: Preallocating image memory... done (allocated 147655 pages)
[  122.324258] PM: Allocated 590620 kbytes in 0.27 seconds (2187.48 MB/s)
[  122.324298] Suspending console(s) (use no_console_suspend to debug)
[  122.325991] sd 3:0:0:0: [sdb] Synchronizing SCSI cache
[  122.326104] sd 1:0:0:0: [sda] Synchronizing SCSI cache
[  122.330976] serial 00:0b: disabled
[  122.331077] serial 00:0b: wake-up capability disabled by ACPI
[  122.331161] i8042 kbd 00:0a: wake-up capability enabled by ACPI
[  122.331560] pci 0000:01:00.0: PCI INT A disabled
[  122.331967] ATIIXP_IDE 0000:00:14.1: PCI INT A disabled
[  122.332736] ahci 0000:00:11.0: PCI INT A disabled
[  122.432770] HDA Intel 0000:01:00.1: PCI INT B disabled
[  122.432932] ACPI handle has no context!
[  122.433187] HDA Intel 0000:00:14.2: PCI INT A disabled
[  122.444799] PM: freeze of devices complete after 119.816 msecs
[  122.448594] PM: late freeze of devices complete after 3.785 msecs
[  122.448628] Disabling non-boot CPUs ...
[  122.471872] CPU 1 is now offline
[  122.489261] CPU 2 is now offline
[  122.498177] CPU 3 is now offline
[  122.498183] lockdep: fixing up alternatives.
[  122.498228] SMP alternatives: switching to UP code
[  122.509784] Extended CMOS year: 2000
[  122.510008] PM: Creating hibernation image:
[  122.510008] PM: Need to copy 146817 pages

[-- Attachment #3: suspend.after --]
[-- Type: text/plain, Size: 1497 bytes --]

[   72.337143] [drm] Resetting GPU
[   72.347054] PM: Syncing filesystems ... done.
[   72.377683] Freezing user space processes ... (elapsed 0.01 seconds) done.
[   72.390714] Freezing remaining freezable tasks ... (elapsed 0.01 seconds) done.
[   72.401705] PM: Preallocating image memory... done (allocated 147611 pages)
[   72.690885] PM: Allocated 590444 kbytes in 0.28 seconds (2108.72 MB/s)
[   72.690925] Suspending console(s) (use no_console_suspend to debug)
[   72.692422] sd 3:0:0:0: [sdb] Synchronizing SCSI cache
[   72.692432] sd 1:0:0:0: [sda] Synchronizing SCSI cache
[   72.704185] serial 00:0b: disabled
[   72.704275] serial 00:0b: wake-up capability disabled by ACPI
[   72.704356] i8042 kbd 00:0a: wake-up capability enabled by ACPI
[   72.704742] pci 0000:01:00.0: PCI INT A disabled
[   72.705258] ATIIXP_IDE 0000:00:14.1: PCI INT A disabled
[   72.705915] ahci 0000:00:11.0: PCI INT A disabled
[   72.806818] HDA Intel 0000:01:00.1: PCI INT B disabled
[   72.806831] HDA Intel 0000:00:14.2: PCI INT A disabled
[   72.807141] ACPI handle has no context!
[   72.819836] PM: freeze of devices complete after 128.198 msecs
[   72.823408] PM: late freeze of devices complete after 3.565 msecs
[   72.823443] Disabling non-boot CPUs ...
[   72.840884] CPU 1 is now offline
[   72.866229] CPU 2 is now offline
[   72.884239] CPU 3 is now offline
[   72.886941] Extended CMOS year: 2000
[   72.887016] PM: Creating hibernation image:
[   72.887016] PM: Need to copy 146787 pages


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [patch] x86: avoid unnecessary smp alternatives switch during suspend/resume
  2010-11-21  9:27 ` Nigel Cunningham
  2010-11-21 10:03   ` Borislav Petkov
@ 2010-11-22  2:30   ` H. Peter Anvin
  2010-11-24  0:11   ` Suresh Siddha
  2 siblings, 0 replies; 11+ messages in thread
From: H. Peter Anvin @ 2010-11-22  2:30 UTC (permalink / raw)
  To: Nigel Cunningham
  Cc: Suresh Siddha, Ingo Molnar, Thomas Gleixner, LKML, Arjan van de Ven

On 11/21/2010 01:27 AM, Nigel Cunningham wrote:
> We have a few others things that want to modify their behaviour
> according to whether we're doing the atomic copy/restore. Perhaps it
> would be an idea to just use a single flag, perhaps a value for
> system_state?

system_state is pretty much considered harmful...

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [patch] x86: avoid unnecessary smp alternatives switch during suspend/resume
  2010-11-21  9:27 ` Nigel Cunningham
  2010-11-21 10:03   ` Borislav Petkov
  2010-11-22  2:30   ` H. Peter Anvin
@ 2010-11-24  0:11   ` Suresh Siddha
  2010-12-14 21:21     ` [tip:x86/alternatives] x86, suspend: Avoid " tip-bot for Suresh Siddha
  2 siblings, 1 reply; 11+ messages in thread
From: Suresh Siddha @ 2010-11-24  0:11 UTC (permalink / raw)
  To: Nigel Cunningham
  Cc: H. Peter Anvin, Ingo Molnar, Thomas Gleixner, LKML,
	Arjan van de Ven, Ben Gamari, xiyou.wangcong, Borislav Petkov

On Sun, 2010-11-21 at 01:27 -0800, Nigel Cunningham wrote:
> We have a few others things that want to modify their behaviour 
> according to whether we're doing the atomic copy/restore.

What are these other things and shouldn't they be using the smp_mode in
arch/x86/kernel/alternative.c ? Perhaps we need an API to export this
for others to use?

>  Perhaps it 
> would be an idea to just use a single flag, perhaps a value for 
> system_state?

I initially thought of system_state but as Peter mentioned, I was
worried that the system_state modification in this path might break some
suspend/resume code flow.

Anyways appended a slightly modified patch that uses similar flow like
the existing arch_enable_nonboot_cpus_{begin, end}

thanks,
suresh
---

From: Suresh Siddha <suresh.b.siddha@intel.com>
Subject: x86: avoid unnecessary smp alternatives switch during suspend/resume

During suspend, we disable all the non boot cpus. And during resume we bring
them all back again. So no need to do alternatives_smp_switch() in between.

On my core 2 based laptop, this speeds up the suspend path by 15msec and the
resume path by 5 msec (suspend/resume speed up differences can be attributed
to the different P-states that the cpu is in during suspend/resume).

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
---
 arch/x86/include/asm/alternative.h |    1 +
 arch/x86/kernel/alternative.c      |    3 ++-
 arch/x86/kernel/smpboot.c          |   14 ++++++++++++++
 kernel/cpu.c                       |   11 +++++++++++
 4 files changed, 28 insertions(+), 1 deletions(-)

diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h
index 76561d2..01171f6 100644
--- a/arch/x86/include/asm/alternative.h
+++ b/arch/x86/include/asm/alternative.h
@@ -66,6 +66,7 @@ extern void alternatives_smp_module_add(struct module *mod, char *name,
 extern void alternatives_smp_module_del(struct module *mod);
 extern void alternatives_smp_switch(int smp);
 extern int alternatives_text_reserved(void *start, void *end);
+extern bool skip_smp_alternatives;
 #else
 static inline void alternatives_smp_module_add(struct module *mod, char *name,
 					       void *locks, void *locks_end,
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 5079f24..9f98eb4 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -353,6 +353,7 @@ void __init_or_module alternatives_smp_module_del(struct module *mod)
 	mutex_unlock(&smp_alt);
 }
 
+bool skip_smp_alternatives;
 void alternatives_smp_switch(int smp)
 {
 	struct smp_alt_module *mod;
@@ -368,7 +369,7 @@ void alternatives_smp_switch(int smp)
 	printk("lockdep: fixing up alternatives.\n");
 #endif
 
-	if (noreplace_smp || smp_alt_once)
+	if (noreplace_smp || smp_alt_once || skip_smp_alternatives)
 		return;
 	BUG_ON(!smp && (num_online_cpus() > 1));
 
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index f0a0624..589db1d 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1158,6 +1158,20 @@ out:
 	preempt_enable();
 }
 
+void arch_disable_nonboot_cpus_begin(void)
+{
+	/*
+	 * Avoid the smp alternatives switch during the disable_nonboot_cpus().
+	 * In the suspend path, we will be back in the SMP mode shortly anyways.
+	 */
+	skip_smp_alternatives = true;
+}
+
+void arch_disable_nonboot_cpus_end(void)
+{
+	skip_smp_alternatives = false;
+}
+
 void arch_enable_nonboot_cpus_begin(void)
 {
 	set_mtrr_aps_delayed_init();
diff --git a/kernel/cpu.c b/kernel/cpu.c
index cb7a1ef..156cc55 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -384,6 +384,14 @@ out:
 #ifdef CONFIG_PM_SLEEP_SMP
 static cpumask_var_t frozen_cpus;
 
+void __weak arch_disable_nonboot_cpus_begin(void)
+{
+}
+
+void __weak arch_disable_nonboot_cpus_end(void)
+{
+}
+
 int disable_nonboot_cpus(void)
 {
 	int cpu, first_cpu, error = 0;
@@ -395,6 +403,7 @@ int disable_nonboot_cpus(void)
 	 * with the userspace trying to use the CPU hotplug at the same time
 	 */
 	cpumask_clear(frozen_cpus);
+	arch_disable_nonboot_cpus_begin();
 
 	printk("Disabling non-boot CPUs ...\n");
 	for_each_online_cpu(cpu) {
@@ -410,6 +419,8 @@ int disable_nonboot_cpus(void)
 		}
 	}
 
+	arch_disable_nonboot_cpus_end();
+
 	if (!error) {
 		BUG_ON(num_online_cpus() > 1);
 		/* Make sure the CPUs won't be enabled by someone else */



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [tip:x86/alternatives] x86, suspend: Avoid unnecessary smp alternatives switch during suspend/resume
  2010-11-24  0:11   ` Suresh Siddha
@ 2010-12-14 21:21     ` tip-bot for Suresh Siddha
  2010-12-14 22:31       ` Rafael J. Wysocki
  0 siblings, 1 reply; 11+ messages in thread
From: tip-bot for Suresh Siddha @ 2010-12-14 21:21 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, suresh.b.siddha, tglx, hpa, rjw

Commit-ID:  3fb82d56ad003e804923185316236f26b30dfdd5
Gitweb:     http://git.kernel.org/tip/3fb82d56ad003e804923185316236f26b30dfdd5
Author:     Suresh Siddha <suresh.b.siddha@intel.com>
AuthorDate: Tue, 23 Nov 2010 16:11:40 -0800
Committer:  H. Peter Anvin <hpa@linux.intel.com>
CommitDate: Mon, 13 Dec 2010 16:23:56 -0800

x86, suspend: Avoid unnecessary smp alternatives switch during suspend/resume

During suspend, we disable all the non boot cpus. And during resume we bring
them all back again. So no need to do alternatives_smp_switch() in between.

On my core 2 based laptop, this speeds up the suspend path by 15msec and the
resume path by 5 msec (suspend/resume speed up differences can be attributed
to the different P-states that the cpu is in during suspend/resume).

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
LKML-Reference: <1290557500.4946.8.camel@sbsiddha-MOBL3.sc.intel.com>
Cc: Rafael J. Wysocki <rjw@sisk.pl>
Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
---
 arch/x86/include/asm/alternative.h |    1 +
 arch/x86/kernel/alternative.c      |    3 ++-
 arch/x86/kernel/smpboot.c          |   14 ++++++++++++++
 kernel/cpu.c                       |   11 +++++++++++
 4 files changed, 28 insertions(+), 1 deletions(-)

diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h
index 76561d2..01171f6 100644
--- a/arch/x86/include/asm/alternative.h
+++ b/arch/x86/include/asm/alternative.h
@@ -66,6 +66,7 @@ extern void alternatives_smp_module_add(struct module *mod, char *name,
 extern void alternatives_smp_module_del(struct module *mod);
 extern void alternatives_smp_switch(int smp);
 extern int alternatives_text_reserved(void *start, void *end);
+extern bool skip_smp_alternatives;
 #else
 static inline void alternatives_smp_module_add(struct module *mod, char *name,
 					       void *locks, void *locks_end,
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 5079f24..9f98eb4 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -353,6 +353,7 @@ void __init_or_module alternatives_smp_module_del(struct module *mod)
 	mutex_unlock(&smp_alt);
 }
 
+bool skip_smp_alternatives;
 void alternatives_smp_switch(int smp)
 {
 	struct smp_alt_module *mod;
@@ -368,7 +369,7 @@ void alternatives_smp_switch(int smp)
 	printk("lockdep: fixing up alternatives.\n");
 #endif
 
-	if (noreplace_smp || smp_alt_once)
+	if (noreplace_smp || smp_alt_once || skip_smp_alternatives)
 		return;
 	BUG_ON(!smp && (num_online_cpus() > 1));
 
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 083e99d..837c81e 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1166,6 +1166,20 @@ out:
 	preempt_enable();
 }
 
+void arch_disable_nonboot_cpus_begin(void)
+{
+	/*
+	 * Avoid the smp alternatives switch during the disable_nonboot_cpus().
+	 * In the suspend path, we will be back in the SMP mode shortly anyways.
+	 */
+	skip_smp_alternatives = true;
+}
+
+void arch_disable_nonboot_cpus_end(void)
+{
+	skip_smp_alternatives = false;
+}
+
 void arch_enable_nonboot_cpus_begin(void)
 {
 	set_mtrr_aps_delayed_init();
diff --git a/kernel/cpu.c b/kernel/cpu.c
index f6e726f..8ccc182 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -386,6 +386,14 @@ out:
 #ifdef CONFIG_PM_SLEEP_SMP
 static cpumask_var_t frozen_cpus;
 
+void __weak arch_disable_nonboot_cpus_begin(void)
+{
+}
+
+void __weak arch_disable_nonboot_cpus_end(void)
+{
+}
+
 int disable_nonboot_cpus(void)
 {
 	int cpu, first_cpu, error = 0;
@@ -397,6 +405,7 @@ int disable_nonboot_cpus(void)
 	 * with the userspace trying to use the CPU hotplug at the same time
 	 */
 	cpumask_clear(frozen_cpus);
+	arch_disable_nonboot_cpus_begin();
 
 	printk("Disabling non-boot CPUs ...\n");
 	for_each_online_cpu(cpu) {
@@ -412,6 +421,8 @@ int disable_nonboot_cpus(void)
 		}
 	}
 
+	arch_disable_nonboot_cpus_end();
+
 	if (!error) {
 		BUG_ON(num_online_cpus() > 1);
 		/* Make sure the CPUs won't be enabled by someone else */

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [tip:x86/alternatives] x86, suspend: Avoid unnecessary smp alternatives switch during suspend/resume
  2010-12-14 21:21     ` [tip:x86/alternatives] x86, suspend: Avoid " tip-bot for Suresh Siddha
@ 2010-12-14 22:31       ` Rafael J. Wysocki
  0 siblings, 0 replies; 11+ messages in thread
From: Rafael J. Wysocki @ 2010-12-14 22:31 UTC (permalink / raw)
  To: mingo, hpa, linux-kernel, suresh.b.siddha, tglx, hpa; +Cc: linux-tip-commits

On Tuesday, December 14, 2010, tip-bot for Suresh Siddha wrote:
> Commit-ID:  3fb82d56ad003e804923185316236f26b30dfdd5
> Gitweb:     http://git.kernel.org/tip/3fb82d56ad003e804923185316236f26b30dfdd5
> Author:     Suresh Siddha <suresh.b.siddha@intel.com>
> AuthorDate: Tue, 23 Nov 2010 16:11:40 -0800
> Committer:  H. Peter Anvin <hpa@linux.intel.com>
> CommitDate: Mon, 13 Dec 2010 16:23:56 -0800
> 
> x86, suspend: Avoid unnecessary smp alternatives switch during suspend/resume
> 
> During suspend, we disable all the non boot cpus. And during resume we bring
> them all back again. So no need to do alternatives_smp_switch() in between.
> 
> On my core 2 based laptop, this speeds up the suspend path by 15msec and the
> resume path by 5 msec (suspend/resume speed up differences can be attributed
> to the different P-states that the cpu is in during suspend/resume).
> 
> Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
> LKML-Reference: <1290557500.4946.8.camel@sbsiddha-MOBL3.sc.intel.com>
> Cc: Rafael J. Wysocki <rjw@sisk.pl>
> Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>

OK, I guess there's no nicer way to implement that.

Acked-by: Rafael J. Wysocki <rjw@sisk.pl>

> ---
>  arch/x86/include/asm/alternative.h |    1 +
>  arch/x86/kernel/alternative.c      |    3 ++-
>  arch/x86/kernel/smpboot.c          |   14 ++++++++++++++
>  kernel/cpu.c                       |   11 +++++++++++
>  4 files changed, 28 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h
> index 76561d2..01171f6 100644
> --- a/arch/x86/include/asm/alternative.h
> +++ b/arch/x86/include/asm/alternative.h
> @@ -66,6 +66,7 @@ extern void alternatives_smp_module_add(struct module *mod, char *name,
>  extern void alternatives_smp_module_del(struct module *mod);
>  extern void alternatives_smp_switch(int smp);
>  extern int alternatives_text_reserved(void *start, void *end);
> +extern bool skip_smp_alternatives;
>  #else
>  static inline void alternatives_smp_module_add(struct module *mod, char *name,
>  					       void *locks, void *locks_end,
> diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
> index 5079f24..9f98eb4 100644
> --- a/arch/x86/kernel/alternative.c
> +++ b/arch/x86/kernel/alternative.c
> @@ -353,6 +353,7 @@ void __init_or_module alternatives_smp_module_del(struct module *mod)
>  	mutex_unlock(&smp_alt);
>  }
>  
> +bool skip_smp_alternatives;
>  void alternatives_smp_switch(int smp)
>  {
>  	struct smp_alt_module *mod;
> @@ -368,7 +369,7 @@ void alternatives_smp_switch(int smp)
>  	printk("lockdep: fixing up alternatives.\n");
>  #endif
>  
> -	if (noreplace_smp || smp_alt_once)
> +	if (noreplace_smp || smp_alt_once || skip_smp_alternatives)
>  		return;
>  	BUG_ON(!smp && (num_online_cpus() > 1));
>  
> diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
> index 083e99d..837c81e 100644
> --- a/arch/x86/kernel/smpboot.c
> +++ b/arch/x86/kernel/smpboot.c
> @@ -1166,6 +1166,20 @@ out:
>  	preempt_enable();
>  }
>  
> +void arch_disable_nonboot_cpus_begin(void)
> +{
> +	/*
> +	 * Avoid the smp alternatives switch during the disable_nonboot_cpus().
> +	 * In the suspend path, we will be back in the SMP mode shortly anyways.
> +	 */
> +	skip_smp_alternatives = true;
> +}
> +
> +void arch_disable_nonboot_cpus_end(void)
> +{
> +	skip_smp_alternatives = false;
> +}
> +
>  void arch_enable_nonboot_cpus_begin(void)
>  {
>  	set_mtrr_aps_delayed_init();
> diff --git a/kernel/cpu.c b/kernel/cpu.c
> index f6e726f..8ccc182 100644
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -386,6 +386,14 @@ out:
>  #ifdef CONFIG_PM_SLEEP_SMP
>  static cpumask_var_t frozen_cpus;
>  
> +void __weak arch_disable_nonboot_cpus_begin(void)
> +{
> +}
> +
> +void __weak arch_disable_nonboot_cpus_end(void)
> +{
> +}
> +
>  int disable_nonboot_cpus(void)
>  {
>  	int cpu, first_cpu, error = 0;
> @@ -397,6 +405,7 @@ int disable_nonboot_cpus(void)
>  	 * with the userspace trying to use the CPU hotplug at the same time
>  	 */
>  	cpumask_clear(frozen_cpus);
> +	arch_disable_nonboot_cpus_begin();
>  
>  	printk("Disabling non-boot CPUs ...\n");
>  	for_each_online_cpu(cpu) {
> @@ -412,6 +421,8 @@ int disable_nonboot_cpus(void)
>  		}
>  	}
>  
> +	arch_disable_nonboot_cpus_end();
> +
>  	if (!error) {
>  		BUG_ON(num_online_cpus() > 1);
>  		/* Make sure the CPUs won't be enabled by someone else */
> 
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [patch] x86: avoid unnecessary smp alternatives switch during suspend/resume
  2010-11-21 10:03   ` Borislav Petkov
@ 2011-01-02  9:11     ` Pavel Machek
  2011-01-03 23:44       ` Suresh Siddha
  0 siblings, 1 reply; 11+ messages in thread
From: Pavel Machek @ 2011-01-02  9:11 UTC (permalink / raw)
  To: Borislav Petkov, Nigel Cunningham, Suresh Siddha, H. Peter Anvin,
	Ingo Molnar, Thomas Gleixner, LKML, Arjan van de Ven

Hi!

> > We have a few others things that want to modify their behaviour
> > according to whether we're doing the atomic copy/restore. Perhaps it
> > would be an idea to just use a single flag, perhaps a value for
> > system_state?
> 
> I agree, it would be nicer if we don't introduce a special flag
> just for that. Other than that, I like all sane code that speeds up
> suspend/resume :). I've attached before/after dmesg excerpts on my
> system with the patch ontop of v2.6.37-rc2-181-gb86db47. We end up
> saving 11601 ??secs according to CONFIG_PRINTK_TIME but hey, the code is
> simple enough :).

11msec is not worth the uglyness of global variable like this. Should
we get a parameter, or something?

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [patch] x86: avoid unnecessary smp alternatives switch during suspend/resume
  2011-01-02  9:11     ` Pavel Machek
@ 2011-01-03 23:44       ` Suresh Siddha
  0 siblings, 0 replies; 11+ messages in thread
From: Suresh Siddha @ 2011-01-03 23:44 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Borislav Petkov, Nigel Cunningham, H. Peter Anvin, Ingo Molnar,
	Thomas Gleixner, LKML, Arjan van de Ven

On Sun, 2011-01-02 at 01:11 -0800, Pavel Machek wrote:
> Hi!
> 
> > > We have a few others things that want to modify their behaviour
> > > according to whether we're doing the atomic copy/restore. Perhaps it
> > > would be an idea to just use a single flag, perhaps a value for
> > > system_state?
> > 
> > I agree, it would be nicer if we don't introduce a special flag
> > just for that. Other than that, I like all sane code that speeds up
> > suspend/resume :). I've attached before/after dmesg excerpts on my
> > system with the patch ontop of v2.6.37-rc2-181-gb86db47. We end up
> > saving 11601 ??secs according to CONFIG_PRINTK_TIME but hey, the code is
> > simple enough :).
> 
> 11msec is not worth the uglyness of global variable like this. Should
> we get a parameter, or something?

Pavel, Latest patch (that is now in -tip) moved the global variable to
the x86 code.

http://marc.info/?l=linux-kernel&m=129236179019935&w=2

thanks,
suresh


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2011-01-03 23:44 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-11-20  0:09 [patch] x86: avoid unnecessary smp alternatives switch during suspend/resume Suresh Siddha
2010-11-20 16:31 ` Ben Gamari
2010-11-21  6:02 ` Américo Wang
2010-11-21  9:27 ` Nigel Cunningham
2010-11-21 10:03   ` Borislav Petkov
2011-01-02  9:11     ` Pavel Machek
2011-01-03 23:44       ` Suresh Siddha
2010-11-22  2:30   ` H. Peter Anvin
2010-11-24  0:11   ` Suresh Siddha
2010-12-14 21:21     ` [tip:x86/alternatives] x86, suspend: Avoid " tip-bot for Suresh Siddha
2010-12-14 22:31       ` Rafael J. Wysocki

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.