ACPI sleep: check error values and disable it on SMP

ACPI sleep: check error values and disable it on SMP

[Pavel Machek]

Hi!

Wakeup is hard to do on SMP, and freezer_processes is just not ready
to work on SMP machine. As I have no SMP (hint hint ;-), it is better
to disable it.

ACPI should really really check if freezing processes worked, and
handle failure. Please apply,
								Pavel

--- linux-cvs.clean/drivers/acpi/sleep/main.c	2003-02-28 10:23:47.000000000 -0800
+++ linux-cvs/drivers/acpi/sleep/main.c	2003-06-08 14:39:03.000000000 -0700
@@ -214,6 +214,11 @@
 {
 	acpi_status status;
 
+#ifdef CONFIG_SMP
+	/* Suspend is hard to get right on SMP. */
+	return AE_ERROR;
+#endif
+
 	/* get out if state is invalid */
 	if (state < ACPI_STATE_S1 || state > ACPI_STATE_S5)
 		return AE_ERROR;
@@ -226,7 +231,10 @@
 	 * TBD: S1 can be done without device_suspend.  Make a CONFIG_XX
 	 * to handle however when S1 failed without device_suspend.
 	 */
-	freeze_processes();		/* device_suspend needs processes to be stopped */
+	if (freeze_processes()) {
+		thaw_processes();
+		return AE_ERROR;		/* device_suspend needs processes to be stopped */
+	}
 
 	/* do we have a wakeup address for S2 and S3? */
 	/* Here, we support only S4BIOS, those we set the wakeup address */

-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]


-------------------------------------------------------
This SF.net email is sponsored by:  Etnus, makers of TotalView, The best
thread debugger on the planet. Designed with thread debugging features
you've never dreamed of, try TotalView 6 free at www.etnus.com.

Re: ACPI sleep: check error values and disable it on SMP

[Rusty Russell]

In message <20030608203823.GA9415-I/5MKhXcvmPrBKCeMvbIDA@public.gmane.org> you write:
> Hi!
> 
> Wakeup is hard to do on SMP, and freezer_processes is just not ready
> to work on SMP machine. As I have no SMP (hint hint ;-), it is better
> to disable it.
> 
> ACPI should really really check if freezing processes worked, and
> handle failure. Please apply,

Gcc doesn't give any warnings about unreachable code?

> --- linux-cvs.clean/drivers/acpi/sleep/main.c	2003-02-28 10:23:47.000000000 -0800
> +++ linux-cvs/drivers/acpi/sleep/main.c	2003-06-08 14:39:03.000000000 -0700
> @@ -214,6 +214,11 @@
>  {
>  	acpi_status status;
>  
> +#ifdef CONFIG_SMP
> +	/* Suspend is hard to get right on SMP. */
> +	return AE_ERROR;
> +#endif
> +
>  	/* get out if state is invalid */
>  	if (state < ACPI_STATE_S1 || state > ACPI_STATE_S5)
>  		return AE_ERROR;
> @@ -226,7 +231,10 @@
>  	 * TBD: S1 can be done without device_suspend.  Make a CONFIG_XX
>  	 * to handle however when S1 failed without device_suspend.
>  	 */
> -	freeze_processes();		/* device_suspend needs processes to be stopped */
> +	if (freeze_processes()) {
> +		thaw_processes();
> +		return AE_ERROR;		/* device_suspend needs processes to be stopped */
> +	}
>  
>  	/* do we have a wakeup address for S2 and S3? */
>  	/* Here, we support only S4BIOS, those we set the wakeup address */
> 

Cheers,
Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.


-------------------------------------------------------
This SF.net email is sponsored by:  Etnus, makers of TotalView, The best
thread debugger on the planet. Designed with thread debugging features
you've never dreamed of, try TotalView 6 free at www.etnus.com.

Re: ACPI sleep: check error values and disable it on SMP

[Pavel Machek]

Hi!

> > Wakeup is hard to do on SMP, and freezer_processes is just not ready
> > to work on SMP machine. As I have no SMP (hint hint ;-), it is better
> > to disable it.
> > 
> > ACPI should really really check if freezing processes worked, and
> > handle failure. Please apply,
> 
> Gcc doesn't give any warnings about unreachable code?

I've just tested it, and no warnings are given.
								Pavel

> > --- linux-cvs.clean/drivers/acpi/sleep/main.c	2003-02-28 10:23:47.000000000 -0800
> > +++ linux-cvs/drivers/acpi/sleep/main.c	2003-06-08 14:39:03.000000000 -0700
> > @@ -214,6 +214,11 @@
> >  {
> >  	acpi_status status;
> >  
> > +#ifdef CONFIG_SMP
> > +	/* Suspend is hard to get right on SMP. */
> > +	return AE_ERROR;
> > +#endif
> > +
> >  	/* get out if state is invalid */
> >  	if (state < ACPI_STATE_S1 || state > ACPI_STATE_S5)
> >  		return AE_ERROR;
> > @@ -226,7 +231,10 @@
> >  	 * TBD: S1 can be done without device_suspend.  Make a CONFIG_XX
> >  	 * to handle however when S1 failed without device_suspend.
> >  	 */
> > -	freeze_processes();		/* device_suspend needs processes to be stopped */
> > +	if (freeze_processes()) {
> > +		thaw_processes();
> > +		return AE_ERROR;		/* device_suspend needs processes to be stopped */
> > +	}
> >  
> >  	/* do we have a wakeup address for S2 and S3? */
> >  	/* Here, we support only S4BIOS, those we set the wakeup address */
> > 

-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]


-------------------------------------------------------
This SF.net email is sponsored by:  Etnus, makers of TotalView, The best
thread debugger on the planet. Designed with thread debugging features
you've never dreamed of, try TotalView 6 free at www.etnus.com.

RE: ACPI sleep: check error values and disable it on SMP

[Grover, Andrew]

> From: Pavel Machek [mailto:pavel-+ZI9xUNit7I@public.gmane.org] 
> > > +#ifdef CONFIG_SMP
> > > +	/* Suspend is hard to get right on SMP. */
> > > +	return AE_ERROR;
> > > +#endif

This part shouldn't be an #ifdef it should be a runtime check of
num_online_cpus().

Regards -- Andy


-------------------------------------------------------
This SF.net email is sponsored by:  Etnus, makers of TotalView, The best
thread debugger on the planet. Designed with thread debugging features
you've never dreamed of, try TotalView 6 free at www.etnus.com.

Re: ACPI sleep: check error values and disable it on SMP

[Pavel Machek]

Hi!

> > > > +#ifdef CONFIG_SMP
> > > > +	/* Suspend is hard to get right on SMP. */
> > > > +	return AE_ERROR;
> > > > +#endif
> 
> This part shouldn't be an #ifdef it should be a runtime check of
> num_online_cpus().

Okay, that makes sense.

Here's updated patch, please apply.
								Pavel

--- /usr/src/tmp/linux/drivers/acpi/sleep/main.c	2003-06-10 21:10:18.000000000 +0200
+++ /usr/src/linux/drivers/acpi/sleep/main.c	2003-06-10 21:08:32.000000000 +0200
@@ -220,6 +220,10 @@
 {
 	acpi_status status;
 
+	/* Suspend is hard to get right on SMP. */
+	if (num_online_cpus() != 1)
+		return AE_ERROR;
+
 	/* get out if state is invalid */
 	if (state < ACPI_STATE_S1 || state > ACPI_STATE_S5)
 		return AE_ERROR;
@@ -232,7 +236,10 @@
 	 * TBD: S1 can be done without device_suspend.  Make a CONFIG_XX
 	 * to handle however when S1 failed without device_suspend.
 	 */
-	freeze_processes();		/* device_suspend needs processes to be stopped */
+	if (freeze_processes()) {
+		thaw_processes();
+		return AE_ERROR;		/* device_suspend needs processes to be stopped */
+	}
 
 	/* do we have a wakeup address for S2 and S3? */
 	/* Here, we support only S4BIOS, those we set the wakeup address */

-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]


-------------------------------------------------------
This SF.net email is sponsored by:  Etnus, makers of TotalView, The best
thread debugger on the planet. Designed with thread debugging features
you've never dreamed of, try TotalView 6 free at www.etnus.com.