* [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.