RE: -mm swsusp: do not default to platform/firmware

RE: -mm swsusp: do not default to platform/firmware

[Li, Shaohua]

Hi,

>-----Original Message-----
>From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel-
>owner@vger.kernel.org] On Behalf Of Nigel Cunningham
>Sent: Thursday, July 29, 2004 8:30 AM
>To: Pavel Machek
>Cc: Andrew Morton; Patrick Mochel; akpm@zip.com.au; Linux Kernel
Mailing
>List
>Subject: Re: -mm swsusp: do not default to platform/firmware
>
>Hi.
>
>On Thu, 2004-07-29 at 09:43, Pavel Machek wrote:
>> +I did found some kernel threads don't do it, and they don't freeze,
and
>
>"I found... threads that don't..."
>
>> +so the system can't sleep. Is this a known behavior?
>> +
>> +A: All such kernel threads need to be fixed, one by one. Select
place
>> +where it is safe to be frozen (no kernel semaphores should be held
at
>> +that point and it must be safe to sleep there), and add:
>> +
>> +            if (current->flags & PF_FREEZE)
>> +                    refrigerator(PF_FREEZE);
>> +
>
>Perhaps you should also add.
>
>If the thread is needed for writing the image to storage, you should
>instead set the PF_NOFREEZE process flag when creating the thread.
>
You know for sleep into mem (s3) we also use
'freeze_processes/refrigerator', and the threads don't write image to
storage for S3. Should the threads be set the PF_NOFREEZE? Is there any
side effect for S3 if the threads are running?

Thanks,
Shaohua

Re: -mm swsusp: do not default to platform/firmware

[Pavel Machek]

Hi!

> >If the thread is needed for writing the image to storage, you should
> >instead set the PF_NOFREEZE process flag when creating the thread.
> >
> You know for sleep into mem (s3) we also use
> 'freeze_processes/refrigerator', and the threads don't write image to
> storage for S3. Should the threads be set the PF_NOFREEZE? Is there any
> side effect for S3 if the threads are running?

Threads that are "NOFREEZE" should be carefull not to do anything bad
to drivers, and if it works as NOFREEZE for swsusp, it will work in
S3, too. No need to do additional work of freezing based on new state.

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

Re: -mm swsusp: do not default to platform/firmware

[Nigel Cunningham]

Hi.

On Thu, 2004-07-29 at 17:38, Pavel Machek wrote:
> Threads that are "NOFREEZE" should be carefull not to do anything bad
> to drivers, and if it works as NOFREEZE for swsusp, it will work in
> S3, too. No need to do additional work of freezing based on new state.

Yes. It helps to remember that even if they're NOFREEZE, they'll still
be affected by the driver suspending and resuming that's done in both S3
(I assume) and S4 support.

Regards,

Nigel

-mm swsusp: do not default to platform/firmware

[Pavel Machek]

Hi!

-mm swsusp now defaults to platform/firmware suspend... That's
certainly unexpected, changes behaviour from previous version, and
only works on one of three machines I have here. I'd like the default
to be changed back. Please apply,
								Pavel

--- clean-mm/drivers/acpi/sleep/main.c	2004-07-28 23:39:47.000000000 +0200
+++ linux-mm/drivers/acpi/sleep/main.c	2004-07-28 22:54:43.000000000 +0200
@@ -216,9 +216,7 @@
 			if (acpi_gbl_FACS->S4bios_f) {
 				sleep_states[i] = 1;
 				printk(" S4bios");
-				acpi_pm_ops.pm_disk_mode = PM_DISK_FIRMWARE;
-			} else if (sleep_states[i])
-				acpi_pm_ops.pm_disk_mode = PM_DISK_PLATFORM;
+			}
 		}
 	}
 	printk(")\n");
 
-- 
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!

Re: -mm swsusp: do not default to platform/firmware

[Andrew Morton]

Pavel Machek <pavel@ucw.cz> wrote:
>
> -mm swsusp now defaults to platform/firmware suspend... That's
> certainly unexpected, changes behaviour from previous version, and
> only works on one of three machines I have here. I'd like the default
> to be changed back.

You overestimate my knowledge of suspend stuff.  AFAICT the current -mm
default is to enter ACPI sleep state via the BIOS rather than via Linux's
ACPI driver.  Correct?

If not, then what?

If so, then why do we feel this change is needed, and why did Pat change things?

My major concern here is that Pat may have made that change to get suitable
coverage testing for new code paths, and we wouldn't want to undo that right away.

Re: -mm swsusp: do not default to platform/firmware

[Pavel Machek]

Hi!

> > -mm swsusp now defaults to platform/firmware suspend... That's
> > certainly unexpected, changes behaviour from previous version, and
> > only works on one of three machines I have here. I'd like the default
> > to be changed back.
> 
> You overestimate my knowledge of suspend stuff.  AFAICT the current -mm
> default is to enter ACPI sleep state via the BIOS rather than via Linux's
> ACPI driver.  Correct?

Its actually bit more complex. There are 3 methods:

shutdown: save state in linux, then tell bios to powerdown

platform: save state in linux, then tell bios to powerdown and blink
	  "suspended led"

firmware: tell bios to save state itself

"platform" is actually right thing to do, but "shutdown" is most
reliable.

Old code always did "shutdown". New code does "shutdown", "platform"
or "firmware" depending on what BIOS can do.

> If not, then what?
> 
> If so, then why do we feel this change is needed, and why did Pat change things?
> 
> My major concern here is that Pat may have made that change to get suitable
> coverage testing for new code paths, and we wouldn't want to undo that right away.

I believe "platform" is so unreliable (for now) that it blocks usefull
testing... I'd like it to default to "shutdown" when it is merged
upstream.
									Pavel
-- 
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!

Re: -mm swsusp: do not default to platform/firmware

[Pavel Machek]

Hi!

> > > -mm swsusp now defaults to platform/firmware suspend... That's
> > > certainly unexpected, changes behaviour from previous version, and
> > > only works on one of three machines I have here. I'd like the default
> > > to be changed back.
> > 
> > You overestimate my knowledge of suspend stuff.  AFAICT the current -mm
> > default is to enter ACPI sleep state via the BIOS rather than via Linux's
> > ACPI driver.  Correct?
> 
> Its actually bit more complex. There are 3 methods:
> 
> shutdown: save state in linux, then tell bios to powerdown
> 
> platform: save state in linux, then tell bios to powerdown and blink
> 	  "suspended led"
> 
> firmware: tell bios to save state itself
> 
> "platform" is actually right thing to do, but "shutdown" is most
> reliable.

...could you apply this? I hope it is accurate.
								Pavel

--- clean/Documentation/power/swsusp.txt	2004-06-22 12:35:44.000000000 +0200
+++ linux/Documentation/power/swsusp.txt	2004-07-29 01:42:29.000000000 +0200
@@ -199,3 +202,30 @@
 should be sent to the mailing list available through the suspend2
 website, and not to the Linux Kernel Mailing List. We are working
 toward merging suspend2 into the mainline kernel.
+
+Q: Kernel thread must voluntarily freeze itself (call 'refrigerator'). But
+I did found some kernel threads don't do it, and they don't freeze, and
+so the system can't sleep. Is this a known behavior?
+
+A: All such kernel threads need to be fixed, one by one. Select place
+where it is safe to be frozen (no kernel semaphores should be held at
+that point and it must be safe to sleep there), and add:
+
+            if (current->flags & PF_FREEZE)
+                    refrigerator(PF_FREEZE);
+
+Q: What is the difference between between "platform", "shutdown" and
+"firmware" in /sys/power/disk?
+
+A:
+
+shutdown: save state in linux, then tell bios to powerdown
+
+platform: save state in linux, then tell bios to powerdown and blink
+          "suspended led"
+
+firmware: tell bios to save state itself [needs BIOS-specific suspend
+	  partition, and has very little to do with swsusp]
+
+"platform" is actually right thing to do, but "shutdown" is most
+reliable.



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

Re: -mm swsusp: do not default to platform/firmware

[Nigel Cunningham]

Hi.

On Thu, 2004-07-29 at 09:43, Pavel Machek wrote:
> +I did found some kernel threads don't do it, and they don't freeze, and

"I found... threads that don't..."

> +so the system can't sleep. Is this a known behavior?
> +
> +A: All such kernel threads need to be fixed, one by one. Select place
> +where it is safe to be frozen (no kernel semaphores should be held at
> +that point and it must be safe to sleep there), and add:
> +
> +            if (current->flags & PF_FREEZE)
> +                    refrigerator(PF_FREEZE);
> +

Perhaps you should also add.

If the thread is needed for writing the image to storage, you should
instead set the PF_NOFREEZE process flag when creating the thread.

Regards,

Nigel

Re: -mm swsusp: do not default to platform/firmware

[Pavel Machek]

Hi!

> > +I did found some kernel threads don't do it, and they don't freeze, and
> 
> "I found... threads that don't..."
> 
> > +so the system can't sleep. Is this a known behavior?
> > +
> > +A: All such kernel threads need to be fixed, one by one. Select place
> > +where it is safe to be frozen (no kernel semaphores should be held at
> > +that point and it must be safe to sleep there), and add:
> > +
> > +            if (current->flags & PF_FREEZE)
> > +                    refrigerator(PF_FREEZE);
> > +
> 
> Perhaps you should also add.
> 
> If the thread is needed for writing the image to storage, you should
> instead set the PF_NOFREEZE process flag when creating the thread.

Thanks, text is now:

Q: Kernel thread must voluntarily freeze itself (call 'refrigerator'). But
I found some kernel threads that don't do it, and they don't freeze, and
so the system can't sleep. Is this a known behavior?

A: All such kernel threads need to be fixed, one by one. Select place
where it is safe to be frozen (no kernel semaphores should be held at
that point and it must be safe to sleep there), and add:

            if (current->flags & PF_FREEZE)
                    refrigerator(PF_FREEZE);

If the thread is needed for writing the image to storage, you should
instead set the PF_NOFREEZE process flag when creating the thread.

I'll eventually push it, too.
								Pavel

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

Re: -mm swsusp: do not default to platform/firmware

[Nigel Cunningham]

Hi.

Sorry to do this to you - there are still a couple of errors.

On Thu, 2004-07-29 at 17:39, Pavel Machek wrote:
> Thanks, text is now:
> 
> Q: Kernel thread must voluntarily freeze itself (call 'refrigerator'). But

Q: A kernel thread ... 'refrigerator') but ...

(Sentences shouldn't begin with 'But').

> I found some kernel threads that don't do it, and they don't freeze, and

the last ', and' should probably go.

> so the system can't sleep. Is this a known behavior?

American spelling. I'll resist the temptation!

> A: All such kernel threads need to be fixed, one by one. Select place

Select the place

> where it is safe to be frozen (no kernel semaphores should be held at

it -> the thread (the previous subject was plural).

> that point and it must be safe to sleep there), and add:
> 
>             if (current->flags & PF_FREEZE)
>                     refrigerator(PF_FREEZE);
> 
> If the thread is needed for writing the image to storage, you should
> instead set the PF_NOFREEZE process flag when creating the thread.
> 
> I'll eventually push it, too.

Thanks!

Nigel

Re: -mm swsusp: do not default to platform/firmware

[Patrick Mochel]

On Thu, 29 Jul 2004, Pavel Machek wrote:

> -mm swsusp now defaults to platform/firmware suspend... That's
> certainly unexpected, changes behaviour from previous version, and
> only works on one of three machines I have here. I'd like the default
> to be changed back. Please apply,

I'd rather leave it, and put pressure on the platform implementations to
be made to work. If you want to shutdown, then specify it on the command
line before you suspend (or add it to the suspend script).


	Pat

Re: -mm swsusp: do not default to platform/firmware

[Kevin Fenzi]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

>>>>> "Patrick" == Patrick Mochel <mochel@digitalimplant.org> writes:

Patrick> On Thu, 29 Jul 2004, Pavel Machek wrote:

>> -mm swsusp now defaults to platform/firmware suspend... That's
>> certainly unexpected, changes behaviour from previous version, and
>> only works on one of three machines I have here. I'd like the
>> default to be changed back. Please apply,

Patrick> I'd rather leave it, and put pressure on the platform
Patrick> implementations to be made to work. If you want to shutdown,
Patrick> then specify it on the command line before you suspend (or
Patrick> add it to the suspend script).

Does _anyone_ have a machine where platform works?

I can't recally anyone posted on the acpi/swsusp2/kernel lists that
they had a platform implementation that worked. 

Perhaps they had no reason to post? Anyone out there with a laptop
with a suspend to disk in formware/platform using ACPI that works?
I'd love to be proven wrong... 

kevin
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.4 (GNU/Linux)
Comment: Processed by Mailcrypt 3.5.8 <http://mailcrypt.sourceforge.net/>

iD8DBQFBDl4M3imCezTjY0ERAtIWAJ4osBFokXGmgWc+gUhiCkJxS1yLLgCgiLaj
LhbFqFSpmWzjWKkblsFIpqU=
=3GnI
-----END PGP SIGNATURE-----

Re: -mm swsusp: do not default to platform/firmware

[Karol Kozimor]

On Monday 02 of August 2004 17:30, Kevin Fenzi wrote:
> Patrick> I'd rather leave it, and put pressure on the platform
> Patrick> implementations to be made to work. If you want to shutdown,
> Patrick> then specify it on the command line before you suspend (or
> Patrick> add it to the suspend script).
>
> Does _anyone_ have a machine where platform works?
>
> I can't recally anyone posted on the acpi/swsusp2/kernel lists that
> they had a platform implementation that worked.
>
> Perhaps they had no reason to post? Anyone out there with a laptop
> with a suspend to disk in formware/platform using ACPI that works?
> I'd love to be proven wrong...

I guess you mean most users of the original pmdisk code, as it originally 
defaulted to platform (which in most cases should be ACPI S4). I mean, S4 
is not even remotely as obscure as S3. Then again, S4BIOS or other 
firmware methods are different beasts.

For the reference, original pmdisk code worked fine with platform on my 
laptop the last time I checked (several months ago).

Best regards,
-- 
Karol 'sziwan' Kozimor
kkozimor@aurox.org

Re: -mm swsusp: do not default to platform/firmware

[Pavel Machek]

Hi!

> > Patrick> I'd rather leave it, and put pressure on the platform
> > Patrick> implementations to be made to work. If you want to shutdown,
> > Patrick> then specify it on the command line before you suspend (or
> > Patrick> add it to the suspend script).
> >
> > Does _anyone_ have a machine where platform works?
> >
> > I can't recally anyone posted on the acpi/swsusp2/kernel lists that
> > they had a platform implementation that worked.
> >
> > Perhaps they had no reason to post? Anyone out there with a laptop
> > with a suspend to disk in formware/platform using ACPI that works?
> > I'd love to be proven wrong...
> 
> I guess you mean most users of the original pmdisk code, as it originally 
> defaulted to platform (which in most cases should be ACPI S4). I mean, S4 
> is not even remotely as obscure as S3. Then again, S4BIOS or other 
> firmware methods are different beasts.
> 
> For the reference, original pmdisk code worked fine with platform on my 
> laptop the last time I checked (several months ago).
> 

Could you try again with latest -mm kernel?

It is possible that it simply got broken while it had very
small number of users.
				Pavel
-- 
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms         

Re: -mm swsusp: do not default to platform/firmware

[Pavel Machek]

Hi!

> > -mm swsusp now defaults to platform/firmware suspend... That's
> > certainly unexpected, changes behaviour from previous version, and
> > only works on one of three machines I have here. I'd like the default
> > to be changed back. Please apply,
> 
> I'd rather leave it, and put pressure on the platform implementations to
> be made to work. If you want to shutdown, then specify it on the command
> line before you suspend (or add it to the suspend script).

I'm afraid that we'll get big storm of complains "swsusp does not
work after merge from -mm tree", "switch to shutdown method", "oh, it
works now". I assume it works in less than 30% of cases... it really
should not be default.

Oh and "firmware" suspend should never ever be autoselected -- it
needs completely different suspend partition layout which is
mainboard-specific....
								Pavel
-- 
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!