smp/swsusp done right

public inbox for linux-acpi@vger.kernel.org
 help / color / mirror / Atom feed

* smp/swsusp done right
@ 2005-03-23 20:40 Pavel Machek
  2005-03-30 10:42 ` Rafael J. Wysocki
  2005-03-30 21:13 ` Zwane Mwaikambo
  0 siblings, 2 replies; 6+ messages in thread
From: Pavel Machek @ 2005-03-23 20:40 UTC (permalink / raw)
  To: kernel list, Rafael J. Wysocki; +Cc: seife-l3A5Bk7waGM, ACPI mailing list

Hi!

This is against -mm kernel; it is smp swsusp done right, and it
actually works for me. Unlike previous hacks, it uses cpu hotplug
infrastructure. Disable CONFIG_MTRR before you try this...

Test this if you can, and report any problems. If not enough people
scream, this is going to -mm.
								Pavel

--- clean-mm/drivers/pci/pci.c	2005-03-21 11:39:32.000000000 +0100
+++ linux-mm/drivers/pci/pci.c	2005-03-22 01:41:48.000000000 +0100
@@ -376,11 +376,13 @@
 	if (!pci_find_capability(dev, PCI_CAP_ID_PM))
 		return PCI_D0;
 
+#if 0
 	if (platform_pci_choose_state) {
 		ret = platform_pci_choose_state(dev, state);
 		if (ret >= 0)
 			state = ret;
 	}
+#endif
 	switch (state) {
 	case 0: return PCI_D0;
 	case 3: return PCI_D3hot;
--- clean-mm/kernel/power/Kconfig	2005-01-22 21:24:53.000000000 +0100
+++ linux-mm/kernel/power/Kconfig	2005-03-23 11:40:14.000000000 +0100
@@ -28,7 +28,7 @@
 
 config SOFTWARE_SUSPEND
 	bool "Software Suspend (EXPERIMENTAL)"
-	depends on EXPERIMENTAL && PM && SWAP
+	depends on EXPERIMENTAL && PM && SWAP && (HOTPLUG_CPU || !SMP)
 	---help---
 	  Enable the possibility of suspending the machine.
 	  It doesn't need APM.
--- clean-mm/kernel/power/smp.c	2005-03-19 00:32:32.000000000 +0100
+++ linux-mm/kernel/power/smp.c	2005-03-23 15:38:30.000000000 +0100
@@ -7,79 +7,53 @@
  * This file is released under the GPLv2.
  */
 
-#undef DEBUG
-
 #include <linux/smp_lock.h>
 #include <linux/interrupt.h>
 #include <linux/suspend.h>
 #include <linux/module.h>
 #include <asm/atomic.h>
 #include <asm/tlbflush.h>
+#include <asm/cpu.h>
 
-static atomic_t cpu_counter, freeze;
-
-
-static void smp_pause(void * data)
-{
-	struct saved_context ctxt;
-	__save_processor_state(&ctxt);
-	printk("Sleeping in:\n");
-	dump_stack();
-	atomic_inc(&cpu_counter);
-	while (atomic_read(&freeze)) {
-		/* FIXME: restore takes place at random piece inside this.
-		   This should probably be written in assembly, and
-		   preserve general-purpose registers, too
-
-		   What about stack? We may need to move to new stack here.
-
-		   This should better be ran with interrupts disabled.
-		 */
-		cpu_relax();
-		barrier();
-	}
-	atomic_dec(&cpu_counter);
-	__restore_processor_state(&ctxt);
-}
-
-static cpumask_t oldmask;
+cpumask_t frozen_cpus;
 
 void disable_nonboot_cpus(void)
 {
-	printk("Freezing CPUs (at %d)", smp_processor_id());
-	oldmask = current->cpus_allowed;
-	set_cpus_allowed(current, cpumask_of_cpu(0));
-	current->state = TASK_INTERRUPTIBLE;
-	schedule_timeout(HZ);
-	printk("...");
-	BUG_ON(smp_processor_id() != 0);
-
-	/* FIXME: for this to work, all the CPUs must be running
-	 * "idle" thread (or we deadlock). Is that guaranteed? */
+	int cpu, error;
 
-	atomic_set(&cpu_counter, 0);
-	atomic_set(&freeze, 1);
-	smp_call_function(smp_pause, NULL, 0, 0);
-	while (atomic_read(&cpu_counter) < (num_online_cpus() - 1)) {
-		cpu_relax();
-		barrier();
+	error = 0;
+	cpus_clear(frozen_cpus);
+	printk("Freezing cpus ...\n");
+	for_each_online_cpu(cpu) {
+		if (cpu == 0)
+			continue;
+		error = cpu_down(cpu);
+		if (!error) {
+			cpu_set(cpu, frozen_cpus);
+			printk("CPU%d is down\n", cpu);
+			continue;
+		}
+		printk("Error taking cpu %d down: %d\n", cpu, error);
 	}
-	printk("ok\n");
+	BUG_ON(smp_processor_id() != 0);
+	if (error)
+		panic("cpus not sleeping");
 }
 
 void enable_nonboot_cpus(void)
 {
-	printk("Restarting CPUs");
-	atomic_set(&freeze, 0);
-	while (atomic_read(&cpu_counter)) {
-		cpu_relax();
-		barrier();
-	}
-	printk("...");
-	set_cpus_allowed(current, oldmask);
-	schedule();
-	printk("ok\n");
+	int cpu, error;
 
+	printk("Thawing cpus ...\n");
+	for_each_cpu_mask(cpu, frozen_cpus) {
+		if (cpu == 0)
+			continue;
+		error = cpu_up(cpu);
+		if (!error) {
+			printk("CPU%d is up\n", cpu);
+			continue;
+		}
+		printk("Error taking cpu %d up: %d\n", cpu, error);
+		panic("Not enough cpus");
+	}
 }
-
-
--- clean-mm/kernel/power/swsusp.c	2005-03-21 11:39:33.000000000 +0100
+++ linux-mm/kernel/power/swsusp.c	2005-03-23 15:34:53.000000000 +0100
@@ -1194,8 +1194,11 @@
 		return "version";
 	if (strcmp(swsusp_info.uts.machine,system_utsname.machine))
 		return "machine";
+#if 0
+	/* We can't use number of CPUs when we use hotplug to remove them ;-))) */
 	if(swsusp_info.cpus != num_online_cpus())
 		return "number of cpus";
+#endif
 	return NULL;
 }
 

-- 
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!


-------------------------------------------------------
This SF.net email is sponsored by Microsoft Mobile & Embedded DevCon 2005
Attend MEDC 2005 May 9-12 in Vegas. Learn more about the latest Windows
Embedded(r) & Windows Mobile(tm) platforms, applications & content.  Register
by 3/29 & save $300 http://ads.osdn.com/?ad_id=6883&alloc_id=15149&op=click

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

* Re: smp/swsusp done right
  2005-03-23 20:40 smp/swsusp done right Pavel Machek
@ 2005-03-30 10:42 ` Rafael J. Wysocki
  2005-03-30 11:32   ` Pavel Machek
  2005-03-30 21:13 ` Zwane Mwaikambo
  1 sibling, 1 reply; 6+ messages in thread
From: Rafael J. Wysocki @ 2005-03-30 10:42 UTC (permalink / raw)
  To: Pavel Machek; +Cc: kernel list, seife, ACPI mailing list

Hi,

On Wednesday, 23 of March 2005 21:40, Pavel Machek wrote:
> Hi!
> 
> This is against -mm kernel; it is smp swsusp done right, and it
> actually works for me. Unlike previous hacks, it uses cpu hotplug
> infrastructure. Disable CONFIG_MTRR before you try this...
> 
> Test this if you can, and report any problems. If not enough people
> scream, this is going to -mm.
> 								Pavel
> 
> --- clean-mm/drivers/pci/pci.c	2005-03-21 11:39:32.000000000 +0100
> +++ linux-mm/drivers/pci/pci.c	2005-03-22 01:41:48.000000000 +0100
> @@ -376,11 +376,13 @@
>  	if (!pci_find_capability(dev, PCI_CAP_ID_PM))
>  		return PCI_D0;
>  
> +#if 0
>  	if (platform_pci_choose_state) {
>  		ret = platform_pci_choose_state(dev, state);
>  		if (ret >= 0)
>  			state = ret;
>  	}
> +#endif
>  	switch (state) {
>  	case 0: return PCI_D0;
>  	case 3: return PCI_D3hot;

You probably don't want the above change to go in the final patch?


> --- clean-mm/kernel/power/Kconfig	2005-01-22 21:24:53.000000000 +0100
> +++ linux-mm/kernel/power/Kconfig	2005-03-23 11:40:14.000000000 +0100
> @@ -28,7 +28,7 @@
>  
>  config SOFTWARE_SUSPEND
>  	bool "Software Suspend (EXPERIMENTAL)"
> -	depends on EXPERIMENTAL && PM && SWAP
> +	depends on EXPERIMENTAL && PM && SWAP && (HOTPLUG_CPU || !SMP)
>  	---help---
>  	  Enable the possibility of suspending the machine.
>  	  It doesn't need APM.
> --- clean-mm/kernel/power/smp.c	2005-03-19 00:32:32.000000000 +0100
> +++ linux-mm/kernel/power/smp.c	2005-03-23 15:38:30.000000000 +0100
> @@ -7,79 +7,53 @@
>   * This file is released under the GPLv2.
>   */
>  
> -#undef DEBUG
> -
>  #include <linux/smp_lock.h>
>  #include <linux/interrupt.h>
>  #include <linux/suspend.h>
>  #include <linux/module.h>
>  #include <asm/atomic.h>
>  #include <asm/tlbflush.h>
> +#include <asm/cpu.h>
>  
> -static atomic_t cpu_counter, freeze;
> -
> -
> -static void smp_pause(void * data)
> -{
> -	struct saved_context ctxt;
> -	__save_processor_state(&ctxt);
> -	printk("Sleeping in:\n");
> -	dump_stack();
> -	atomic_inc(&cpu_counter);
> -	while (atomic_read(&freeze)) {
> -		/* FIXME: restore takes place at random piece inside this.
> -		   This should probably be written in assembly, and
> -		   preserve general-purpose registers, too
> -
> -		   What about stack? We may need to move to new stack here.
> -
> -		   This should better be ran with interrupts disabled.
> -		 */
> -		cpu_relax();
> -		barrier();
> -	}
> -	atomic_dec(&cpu_counter);
> -	__restore_processor_state(&ctxt);
> -}
> -
> -static cpumask_t oldmask;
> +cpumask_t frozen_cpus;
>  
>  void disable_nonboot_cpus(void)
>  {
> -	printk("Freezing CPUs (at %d)", smp_processor_id());
> -	oldmask = current->cpus_allowed;
> -	set_cpus_allowed(current, cpumask_of_cpu(0));
> -	current->state = TASK_INTERRUPTIBLE;
> -	schedule_timeout(HZ);
> -	printk("...");
> -	BUG_ON(smp_processor_id() != 0);
> -
> -	/* FIXME: for this to work, all the CPUs must be running
> -	 * "idle" thread (or we deadlock). Is that guaranteed? */
> +	int cpu, error;
>  
> -	atomic_set(&cpu_counter, 0);
> -	atomic_set(&freeze, 1);
> -	smp_call_function(smp_pause, NULL, 0, 0);
> -	while (atomic_read(&cpu_counter) < (num_online_cpus() - 1)) {
> -		cpu_relax();
> -		barrier();
> +	error = 0;
> +	cpus_clear(frozen_cpus);
> +	printk("Freezing cpus ...\n");
> +	for_each_online_cpu(cpu) {
> +		if (cpu == 0)
> +			continue;
> +		error = cpu_down(cpu);
> +		if (!error) {
> +			cpu_set(cpu, frozen_cpus);
> +			printk("CPU%d is down\n", cpu);
> +			continue;
> +		}
> +		printk("Error taking cpu %d down: %d\n", cpu, error);
>  	}
> -	printk("ok\n");
> +	BUG_ON(smp_processor_id() != 0);
> +	if (error)
> +		panic("cpus not sleeping");
>  }

I'm not sure whether we should panic() here.  It may be better to make
suspend fail and print a "please reboot immediately" message for the user.
In that case, the user may be able to reboot without loosing data ...


>  void enable_nonboot_cpus(void)
>  {
> -	printk("Restarting CPUs");
> -	atomic_set(&freeze, 0);
> -	while (atomic_read(&cpu_counter)) {
> -		cpu_relax();
> -		barrier();
> -	}
> -	printk("...");
> -	set_cpus_allowed(current, oldmask);
> -	schedule();
> -	printk("ok\n");
> +	int cpu, error;
>  
> +	printk("Thawing cpus ...\n");
> +	for_each_cpu_mask(cpu, frozen_cpus) {
> +		if (cpu == 0)
> +			continue;
> +		error = cpu_up(cpu);
> +		if (!error) {
> +			printk("CPU%d is up\n", cpu);
> +			continue;
> +		}
> +		printk("Error taking cpu %d up: %d\n", cpu, error);
> +		panic("Not enough cpus");
> +	}
>  }
> -
> -
> --- clean-mm/kernel/power/swsusp.c	2005-03-21 11:39:33.000000000 +0100
> +++ linux-mm/kernel/power/swsusp.c	2005-03-23 15:34:53.000000000 +0100
> @@ -1194,8 +1194,11 @@
>  		return "version";
>  	if (strcmp(swsusp_info.uts.machine,system_utsname.machine))
>  		return "machine";
> +#if 0
> +	/* We can't use number of CPUs when we use hotplug to remove them ;-))) */
>  	if(swsusp_info.cpus != num_online_cpus())
>  		return "number of cpus";
> +#endif
>  	return NULL;
>  }
>  
> 
> -- 

I'll test it when I get the CPU hotplug on x86-64 done.

Greets,
Rafael
 

-- 
- Would you tell me, please, which way I ought to go from here?
- That depends a good deal on where you want to get to.
		-- Lewis Carroll "Alice's Adventures in Wonderland"

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

* Re: smp/swsusp done right
  2005-03-30 10:42 ` Rafael J. Wysocki
@ 2005-03-30 11:32   ` Pavel Machek
  0 siblings, 0 replies; 6+ messages in thread
From: Pavel Machek @ 2005-03-30 11:32 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: kernel list, seife, ACPI mailing list

Hi!

> > This is against -mm kernel; it is smp swsusp done right, and it
> > actually works for me. Unlike previous hacks, it uses cpu hotplug
> > infrastructure. Disable CONFIG_MTRR before you try this...
> > 
> > Test this if you can, and report any problems. If not enough people
> > scream, this is going to -mm.
> > 								Pavel
> > 
> > --- clean-mm/drivers/pci/pci.c	2005-03-21 11:39:32.000000000 +0100
> > +++ linux-mm/drivers/pci/pci.c	2005-03-22 01:41:48.000000000 +0100
> > @@ -376,11 +376,13 @@
> >  	if (!pci_find_capability(dev, PCI_CAP_ID_PM))
> >  		return PCI_D0;
> >  
> > +#if 0
> >  	if (platform_pci_choose_state) {
> >  		ret = platform_pci_choose_state(dev, state);
> >  		if (ret >= 0)
> >  			state = ret;
> >  	}
> > +#endif
> >  	switch (state) {
> >  	case 0: return PCI_D0;
> >  	case 3: return PCI_D3hot;
> 
> You probably don't want the above change to go in the final patch?

No, not in final patch.

> > -	atomic_set(&cpu_counter, 0);
> > -	atomic_set(&freeze, 1);
> > -	smp_call_function(smp_pause, NULL, 0, 0);
> > -	while (atomic_read(&cpu_counter) < (num_online_cpus() - 1)) {
> > -		cpu_relax();
> > -		barrier();
> > +	error = 0;
> > +	cpus_clear(frozen_cpus);
> > +	printk("Freezing cpus ...\n");
> > +	for_each_online_cpu(cpu) {
> > +		if (cpu == 0)
> > +			continue;
> > +		error = cpu_down(cpu);
> > +		if (!error) {
> > +			cpu_set(cpu, frozen_cpus);
> > +			printk("CPU%d is down\n", cpu);
> > +			continue;
> > +		}
> > +		printk("Error taking cpu %d down: %d\n", cpu, error);
> >  	}
> > -	printk("ok\n");
> > +	BUG_ON(smp_processor_id() != 0);
> > +	if (error)
> > +		panic("cpus not sleeping");
> >  }
> 
> I'm not sure whether we should panic() here.  It may be better to make
> suspend fail and print a "please reboot immediately" message for the user.
> In that case, the user may be able to reboot without loosing data
> -	...

I guess I could just fail the suspend, but I want to see the messages
for now. (And I do not think it will ever trigger).
								Pavel
-- 
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!

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

* Re: smp/swsusp done right
  2005-03-23 20:40 smp/swsusp done right Pavel Machek
  2005-03-30 10:42 ` Rafael J. Wysocki
@ 2005-03-30 21:13 ` Zwane Mwaikambo
       [not found]   ` <Pine.LNX.4.61.0503301413050.12965-SOP5cCwKKQRMCHjbocvOOJqQE7yCjDx5@public.gmane.org>
  1 sibling, 1 reply; 6+ messages in thread
From: Zwane Mwaikambo @ 2005-03-30 21:13 UTC (permalink / raw)
  To: Pavel Machek; +Cc: kernel list, Rafael J. Wysocki, seife, ACPI mailing list

On Wed, 23 Mar 2005, Pavel Machek wrote:

> This is against -mm kernel; it is smp swsusp done right, and it
> actually works for me. Unlike previous hacks, it uses cpu hotplug
> infrastructure. Disable CONFIG_MTRR before you try this...
> 
> Test this if you can, and report any problems. If not enough people
> scream, this is going to -mm.

Yay! Thanks for getting that done Pavel =)

	Zwane

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

[parent not found: <Pine.LNX.4.61.0503301413050.12965-SOP5cCwKKQRMCHjbocvOOJqQE7yCjDx5@public.gmane.org>]

* Re: smp/swsusp done right
       [not found]   ` <Pine.LNX.4.61.0503301413050.12965-SOP5cCwKKQRMCHjbocvOOJqQE7yCjDx5@public.gmane.org>
@ 2005-03-30 21:22     ` Pavel Machek
  2005-03-30 21:44       ` Zwane Mwaikambo
  0 siblings, 1 reply; 6+ messages in thread
From: Pavel Machek @ 2005-03-30 21:22 UTC (permalink / raw)
  To: Zwane Mwaikambo
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, seife-l3A5Bk7waGM,
	ACPI mailing list

Hi!

> > This is against -mm kernel; it is smp swsusp done right, and it
> > actually works for me. Unlike previous hacks, it uses cpu hotplug
> > infrastructure. Disable CONFIG_MTRR before you try this...
> > 
> > Test this if you can, and report any problems. If not enough people
> > scream, this is going to -mm.
> 
> Yay! Thanks for getting that done Pavel =)

Well, I guess it is thank you -- I got rid of ugly FIXME that would
involve arch-dependend assembly to be solved properly.

... hmm, can play_dead handle all the memory being overwritten? Also
it should probably save and restore registers including MTRRs.

...so I more moved ugly FIXME to better place. Oh well, at least it
uses common infrastructure now. People at Intel doing suspend-to-RAM
on smp systems will need to solve it properly, anyway, because they
need to go to real mode and back.

								Pavel
-- 
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!


-------------------------------------------------------
This SF.net email is sponsored by Demarc:
A global provider of Threat Management Solutions.
Download our HomeAdmin security software for free today!
http://www.demarc.com/Info/Sentarus/hamr30

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

* Re: smp/swsusp done right
  2005-03-30 21:22     ` Pavel Machek
@ 2005-03-30 21:44       ` Zwane Mwaikambo
  0 siblings, 0 replies; 6+ messages in thread
From: Zwane Mwaikambo @ 2005-03-30 21:44 UTC (permalink / raw)
  To: Pavel Machek; +Cc: linux-kernel, seife, ACPI mailing list

On Wed, 30 Mar 2005, Pavel Machek wrote:

> Hi!
> 
> > > This is against -mm kernel; it is smp swsusp done right, and it
> > > actually works for me. Unlike previous hacks, it uses cpu hotplug
> > > infrastructure. Disable CONFIG_MTRR before you try this...
> > > 
> > > Test this if you can, and report any problems. If not enough people
> > > scream, this is going to -mm.
> > 
> > Yay! Thanks for getting that done Pavel =)
> 
> Well, I guess it is thank you -- I got rid of ugly FIXME that would
> involve arch-dependend assembly to be solved properly.
> 
> ... hmm, can play_dead handle all the memory being overwritten? Also
> it should probably save and restore registers including MTRRs.

play_dead currently can't handle memory being overwritten, i guess you'd 
probably want to put the hook to kill the processor for real there. We 
probably only have to make sure that MTRRs are restored on resume, which 
should be taken care of by the current code since ACPI hotplug probably 
requires it too.

> ...so I more moved ugly FIXME to better place. Oh well, at least it
> uses common infrastructure now. People at Intel doing suspend-to-RAM
> on smp systems will need to solve it properly, anyway, because they
> need to go to real mode and back.

Sounds like a perfect candidate =)

Thanks,
	Zwane

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

end of thread, other threads:[~2005-03-30 21:44 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-03-23 20:40 smp/swsusp done right Pavel Machek
2005-03-30 10:42 ` Rafael J. Wysocki
2005-03-30 11:32   ` Pavel Machek
2005-03-30 21:13 ` Zwane Mwaikambo
     [not found]   ` <Pine.LNX.4.61.0503301413050.12965-SOP5cCwKKQRMCHjbocvOOJqQE7yCjDx5@public.gmane.org>
2005-03-30 21:22     ` Pavel Machek
2005-03-30 21:44       ` Zwane Mwaikambo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox