Fix failure to resume from initrds.

PATCH In review Nigel Cunningham 5 participants 11 messages Sep 11, 2007 VFS

lore

Share & tools

lore

Nigel Cunningham

author

2007-09-11 03:54 UTC #0 lore raw

PATCH 1/1 Fix failure to resume from initrds.

Hi all.

Commit 831441862956fffa17b9801db37e6ea1650b0f69 (Freezer: make kernel threads nonfreezable by default) breaks freezing when attempting to resume from an initrd, because the init (which is freezeable) spins while waiting for another thread to run /linuxrc, but doesn't check whether it has been told to enter the refrigerator. The original patch replaced a call to try_to_freeze() with a call to yield(). I believe a simple reversion is wrong because
if !CONFIG_PM_SLEEP, try_to_freeze() is a noop. It should still yield.

Signed-off-by Nigel Cunningham <[email protected]>

 do_mounts_initrd.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

init/do_mounts_initrd.c +3 -1

diff -ruNp 952-fix-initrd-resume.patch-old/init/do_mounts_initrd.c 952-fix-initrd-resume.patch-new/init/do_mounts_initrd.c
@@ -58,8 +58,10 @@ static void __init handle_initrd(void)
 
 	pid = kernel_thread(do_linuxrc, "/linuxrc", SIGCHLD);
 	if (pid > 0)
-		while (pid != sys_wait4(-1, NULL, 0, NULL))
+		while (pid != sys_wait4(-1, NULL, 0, NULL)) {
 			yield();
+			try_to_freeze();
+		}
 
 	if (!resume_attempted)
 		printk(KERN_ERR "Suspend2: No attempt was made to resume from "

Signature · 2 lines

See http://www.tuxonice.net for Howtos, FAQs, mailing
lists, wiki and bugzilla info.

Rafael J. Wysocki

2007-09-11 10:53 UTC #1 lore raw

Quoted 7 lines · Nigel Cunningham

Hi all.
Commit 831441862956fffa17b9801db37e6ea1650b0f69 (Freezer: make kernel threads nonfreezable by default) breaks freezing when attempting to resume from an initrd, because the init (which is freezeable) spins while waiting for another thread to run /linuxrc, but doesn't check whether it has been told to enter the refrigerator.

Hm.

I use a resume from an initrd on a regular basis and it works without the patch below.

I think we need to investigate what happens in your test case a bit.

Quoted 14 lines

The original patch replaced a call to try_to_freeze() with a call to yield(). I believe a simple reversion is wrong because if !CONFIG_PM_SLEEP, try_to_freeze() is a noop. It should still yield.
Signed-off-by Nigel Cunningham <[email protected]>
 do_mounts_initrd.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
diff -ruNp 952-fix-initrd-resume.patch-old/init/do_mounts_initrd.c 952-fix-initrd-resume.patch-new/init/do_mounts_initrd.c
init/do_mounts_initrd.c +0 -0
@@ -58,8 +58,10 @@ static void __init handle_initrd(void)
 

init/do_mounts_initrd.c static void __init handle_initrd(void) · line 67

Quoted 10 lines

init/do_mounts_initrd.c +3 -1

 	pid = kernel_thread(do_linuxrc, "/linuxrc", SIGCHLD);
 	if (pid > 0)
-		while (pid != sys_wait4(-1, NULL, 0, NULL))
+		while (pid != sys_wait4(-1, NULL, 0, NULL)) {
 			yield();
+			try_to_freeze();
+		}
 
 	if (!resume_attempted)
 		printk(KERN_ERR "Suspend2: No attempt was made to resume from "

Greetings,
Rafael

Nigel Cunningham

2007-09-11 11:28 UTC #2 lore raw

Hi.

Quoted 4 lines · On Tuesday 11 September 2007 21:04:22 Rafael J. Wysocki wrote:

Quoted 3 lines · Nigel Cunningham
Hi all.
Commit 831441862956fffa17b9801db37e6ea1650b0f69 (Freezer: make kernel

threads

Quoted 2 lines

Quoted 2 lines
nonfreezable by default) breaks freezing when attempting to resume from an initrd, because the init (which is freezeable) spins while waiting for

another

Quoted 1 line

Quoted 1 line
thread to run /linuxrc, but doesn't check whether it has been told to

enter

Quoted 5 lines

Quoted 1 line
the refrigerator.
Hm.
I use a resume from an initrd on a regular basis and it works without the

patch

Quoted 3 lines

below.
I think we need to investigate what happens in your test case a bit.

Ah. That makes me realise that I see that too - my AMD64 uniprocessor laptop didn't need the patch (guess that's why I didn't notice the need and ack'd the patch). But my x86 SMP machine... it needs this. I'll see if they're running on different processors.

Regards,

Nigel

Signature · 5 lines

Nigel Cunningham
Christian Reformed Church of Cobden
103 Curdie Street, Cobden 3266, Victoria, Australia
Ph. +61 3 5595 1185 / +61 417 100 574
Communal Worship: 11 am Sunday.

Rafael J. Wysocki

2007-09-11 11:43 UTC #3 lore raw

Quoted 19 lines · Nigel Cunningham

Hi.
Quoted 4 lines · On Tuesday 11 September 2007 21:04:22 Rafael J. Wysocki wrote:
Quoted 3 lines · Nigel Cunningham
Hi all.
Commit 831441862956fffa17b9801db37e6ea1650b0f69 (Freezer: make kernel
threads
Quoted 2 lines
Quoted 2 lines
nonfreezable by default) breaks freezing when attempting to resume from an initrd, because the init (which is freezeable) spins while waiting for
another
Quoted 1 line
Quoted 1 line
thread to run /linuxrc, but doesn't check whether it has been told to
enter
Quoted 5 lines
Quoted 1 line
the refrigerator.
Hm.
I use a resume from an initrd on a regular basis and it works without the
patch

Quoted 8 lines

Quoted 3 lines
below.
I think we need to investigate what happens in your test case a bit.
Ah. That makes me realise that I see that too - my AMD64 uniprocessor laptop didn't need the patch (guess that's why I didn't notice the need and ack'd the patch). But my x86 SMP machine... it needs this. I'll see if they're running on different processors.

Well, strange. My x86_64 SMP machines don't need the patch too.

Greetings,
Rafael

Rafael J. Wysocki

2007-09-11 13:01 UTC #4 lore raw

Quoted 22 lines · Rafael J. Wysocki

Quoted 21 lines · Nigel Cunningham
Hi.
Quoted 4 lines · On Tuesday 11 September 2007 21:04:22 Rafael J. Wysocki wrote:
Quoted 3 lines · Nigel Cunningham
Hi all.
Commit 831441862956fffa17b9801db37e6ea1650b0f69 (Freezer: make kernel
threads
Quoted 2 lines
Quoted 2 lines
nonfreezable by default) breaks freezing when attempting to resume from an initrd, because the init (which is freezeable) spins while waiting for
another
Quoted 1 line
Quoted 1 line
thread to run /linuxrc, but doesn't check whether it has been told to
enter
Quoted 5 lines
Quoted 1 line
the refrigerator.
Hm.
I use a resume from an initrd on a regular basis and it works without the
patch
Quoted 2 lines
below.

Quoted 8 lines

Quoted 6 lines
Quoted 1 line
I think we need to investigate what happens in your test case a bit.
Ah. That makes me realise that I see that too - my AMD64 uniprocessor laptop didn't need the patch (guess that's why I didn't notice the need and ack'd the patch). But my x86 SMP machine... it needs this. I'll see if they're running on different processors.
Well, strange. My x86_64 SMP machines don't need the patch too.

Anyway, yes, init is freezable, but should it be?

I mean, shouldn't we rather add PF_NOFREEZE to kernel_init()?

Greetings,
Rafael

Rafael J. Wysocki

2007-09-11 13:12 UTC #5 lore raw

Quoted 30 lines · Rafael J. Wysocki

Quoted 29 lines · Rafael J. Wysocki
Quoted 27 lines · Nigel Cunningham
Hi.
Quoted 4 lines · On Tuesday 11 September 2007 21:04:22 Rafael J. Wysocki wrote:
Quoted 3 lines · Nigel Cunningham
Hi all.
Commit 831441862956fffa17b9801db37e6ea1650b0f69 (Freezer: make kernel
threads
Quoted 2 lines
Quoted 2 lines
nonfreezable by default) breaks freezing when attempting to resume from an initrd, because the init (which is freezeable) spins while waiting for
another
Quoted 1 line
Quoted 1 line
thread to run /linuxrc, but doesn't check whether it has been told to
enter
Quoted 5 lines
Quoted 1 line
the refrigerator.
Hm.
I use a resume from an initrd on a regular basis and it works without the
patch
Quoted 3 lines
below.
I think we need to investigate what happens in your test case a bit.
Ah. That makes me realise that I see that too - my AMD64 uniprocessor laptop didn't need the patch (guess that's why I didn't notice the need and ack'd the patch). But my x86 SMP machine... it needs this. I'll see if they're running on different processors.

Quoted 5 lines

Quoted 1 line
Well, strange. My x86_64 SMP machines don't need the patch too.
Anyway, yes, init is freezable, but should it be?
I mean, shouldn't we rather add PF_NOFREEZE to kernel_init()?

Argh, no. PF_NOFREEZE is inherited by the children.

So, I think that your patch is correct, but there's some suspend2-specific stuff in it. I've rediffed it against 2.6.23-rc6 and moved try_to_freeze() before yield().

---
From: Nigel Cunningham <[email protected]>

Signed-off-by Nigel Cunningham <[email protected]>

Signed-off-by Rafael J. Wysocki <[email protected]>

---
 init/do_mounts_initrd.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

init/do_mounts_initrd.c +3 -1

Index: linux-2.6.23-rc6/init/do_mounts_initrd.c
===================================================================
@@ -57,8 +57,10 @@ static void __init handle_initrd(void)
 
 	pid = kernel_thread(do_linuxrc, "/linuxrc", SIGCHLD);
 	if (pid > 0)
-		while (pid != sys_wait4(-1, NULL, 0, NULL))
+		while (pid != sys_wait4(-1, NULL, 0, NULL)) {
+			try_to_freeze();
 			yield();
+		}
 
 	/* move initrd to rootfs' /old */
 	sys_fchdir(old_fd);

Nigel Cunningham

author

2007-09-11 13:41 UTC #6 lore raw

Hi.

Quoted 10 lines · On Tuesday 11 September 2007 23:23:32 Rafael J. Wysocki wrote:

Quoted 9 lines · Rafael J. Wysocki
Quoted 8 lines · Rafael J. Wysocki
Quoted 7 lines · Nigel Cunningham
Hi.
Quoted 4 lines · On Tuesday 11 September 2007 21:04:22 Rafael J. Wysocki wrote:
Quoted 3 lines · Nigel Cunningham
Hi all.
Commit 831441862956fffa17b9801db37e6ea1650b0f69 (Freezer: make

kernel

Quoted 2 lines

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
threads
Quoted 1 line
Quoted 1 line
nonfreezable by default) breaks freezing when attempting to resume

from an

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
initrd, because the init (which is freezeable) spins while waiting

for

Quoted 2 lines

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
another
Quoted 1 line
Quoted 1 line
thread to run /linuxrc, but doesn't check whether it has been told

Quoted 6 lines

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
enter
Quoted 5 lines
Quoted 1 line
the refrigerator.
Hm.
I use a resume from an initrd on a regular basis and it works

without the

Quoted 6 lines

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
patch
Quoted 3 lines
below.
I think we need to investigate what happens in your test case a bit.
Ah. That makes me realise that I see that too - my AMD64 uniprocessor

laptop

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
didn't need the patch (guess that's why I didn't notice the need and

ack'd

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
the patch). But my x86 SMP machine... it needs this. I'll see if

they're

Quoted 6 lines

Quoted 6 lines
Quoted 3 lines
Quoted 1 line
running on different processors.
Well, strange. My x86_64 SMP machines don't need the patch too.
Anyway, yes, init is freezable, but should it be?

Quoted 7 lines

Quoted 1 line
I mean, shouldn't we rather add PF_NOFREEZE to kernel_init()?
Argh, no. PF_NOFREEZE is inherited by the children.
So, I think that your patch is correct, but there's some suspend2-specific stuff in it. I've rediffed it against 2.6.23-rc6 and moved try_to_freeze() before yield().

Ah yeah. Sorry about that. Is there some reason I've forgotten that makes the order of try_to_freeze & yield in a loop like this matter?

By the way, I had a go at getting fuse processes frozen today. Seems to be doable if you take a freeze filesystems prior to processes approach. I've got a lot more testing and a bit of cleaning up to do before I'd want to show it to anyone, but did successfully do cycles with sshfs, fuseiso and curlftpfs. Of course I don't seriously expect it to get merged - everyone's too much in love with kexec at the moment :)

Regards,

Nigel

Signature · 2 lines

See http://www.tuxonice.net for Howtos, FAQs, mailing
lists, wiki and bugzilla info.

Rafael J. Wysocki

2007-09-11 19:14 UTC #7 lore raw

Quoted 48 lines · Nigel Cunningham

Hi.
Quoted 10 lines · On Tuesday 11 September 2007 23:23:32 Rafael J. Wysocki wrote:
Quoted 9 lines · Rafael J. Wysocki
Quoted 8 lines · Rafael J. Wysocki
Quoted 7 lines · Nigel Cunningham
Hi.
Quoted 4 lines · On Tuesday 11 September 2007 21:04:22 Rafael J. Wysocki wrote:
Quoted 3 lines · Nigel Cunningham
Hi all.
Commit 831441862956fffa17b9801db37e6ea1650b0f69 (Freezer: make
kernel
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
threads
Quoted 1 line
Quoted 1 line
nonfreezable by default) breaks freezing when attempting to resume
from an
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
initrd, because the init (which is freezeable) spins while waiting
for
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
another
Quoted 1 line
Quoted 1 line
thread to run /linuxrc, but doesn't check whether it has been told
to
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
enter
Quoted 5 lines
Quoted 1 line
the refrigerator.
Hm.
I use a resume from an initrd on a regular basis and it works
without the
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
patch
Quoted 3 lines
below.
I think we need to investigate what happens in your test case a bit.
Ah. That makes me realise that I see that too - my AMD64 uniprocessor
laptop
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
didn't need the patch (guess that's why I didn't notice the need and
ack'd
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
the patch). But my x86 SMP machine... it needs this. I'll see if
they're
Quoted 8 lines
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
running on different processors.
Well, strange. My x86_64 SMP machines don't need the patch too.
Anyway, yes, init is freezable, but should it be?
I mean, shouldn't we rather add PF_NOFREEZE to kernel_init()?

Quoted 8 lines

Quoted 5 lines
Argh, no. PF_NOFREEZE is inherited by the children.
So, I think that your patch is correct, but there's some suspend2-specific stuff in it. I've rediffed it against 2.6.23-rc6 and moved try_to_freeze() before yield().
Ah yeah. Sorry about that. Is there some reason I've forgotten that makes the order of try_to_freeze & yield in a loop like this matter?

Technically it's not that important, but conceptually try_to_freeze() should happen right after we've been woken up.

Quoted 4 lines

By the way, I had a go at getting fuse processes frozen today. Seems to be doable if you take a freeze filesystems prior to processes approach. I've got a lot more testing and a bit of cleaning up to do before I'd want to show it to anyone, but did successfully do cycles with sshfs, fuseiso and curlftpfs.

Sounds promising. :-)

Quoted 2 lines

Of course I don't seriously expect it to get merged - everyone's too much in love with kexec at the moment :)

Well, I guess it'll take some time to get that work reliable and it would be nice to have a fix before it's ready.

Greetings,
Rafael

Linus Torvalds

2007-09-11 14:41 UTC #8 lore raw

In reply to Rafael J. Wysocki

Quoted 5 lines · Rafael J. Wysocki

Quoted 3 lines
Anyway, yes, init is freezable, but should it be?
I mean, shouldn't we rather add PF_NOFREEZE to kernel_init()?
Argh, no. PF_NOFREEZE is inherited by the children.

Umm. All of this is __init code - why is freezability even an issue? We shouldn't be suspending at this point anyway afaik..

Is suspend2 perhaps doing something different here?

Linus

Pavel Machek

2007-09-11 16:26 UTC #9 lore raw

Hi!

Quoted 8 lines

Quoted 5 lines
Quoted 3 lines
Anyway, yes, init is freezable, but should it be?
I mean, shouldn't we rather add PF_NOFREEZE to kernel_init()?
Argh, no. PF_NOFREEZE is inherited by the children.
Umm. All of this is __init code - why is freezability even an issue? We shouldn't be suspending at this point anyway afaik..

We want to do resume sometime around here, and resume wants to run with userspace frozen, as does suspend.
Pavel

Signature · 2 lines

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

Nigel Cunningham

author

2007-09-11 13:02 UTC #10 lore raw

In reply to Rafael J. Wysocki

Hi again.

Quoted 10 lines · On Tuesday 11 September 2007 21:55:06 Rafael J. Wysocki wrote:

Quoted 9 lines · Nigel Cunningham
Hi.
Quoted 4 lines · On Tuesday 11 September 2007 21:04:22 Rafael J. Wysocki wrote:
Quoted 3 lines · Nigel Cunningham
Hi all.
Commit 831441862956fffa17b9801db37e6ea1650b0f69 (Freezer: make kernel
threads
Quoted 1 line
Quoted 1 line
nonfreezable by default) breaks freezing when attempting to resume

from an

Quoted 9 lines

Quoted 9 lines
Quoted 1 line
Quoted 1 line
initrd, because the init (which is freezeable) spins while waiting for
another
Quoted 1 line
Quoted 1 line
thread to run /linuxrc, but doesn't check whether it has been told to
enter
Quoted 5 lines
Quoted 1 line
the refrigerator.
Hm.
I use a resume from an initrd on a regular basis and it works without

the

Quoted 6 lines

Quoted 6 lines
patch
Quoted 3 lines
below.
I think we need to investigate what happens in your test case a bit.
Ah. That makes me realise that I see that too - my AMD64 uniprocessor

laptop

Quoted 5 lines

Quoted 3 lines
didn't need the patch (guess that's why I didn't notice the need and ack'd the patch). But my x86 SMP machine... it needs this. I'll see if they're running on different processors.
Well, strange. My x86_64 SMP machines don't need the patch too.

Oh well. I'm probably doing something wrong then and haven't clicked yet.

Regards,

Nigel

Signature · 2 lines

See http://www.tuxonice.net for Howtos, FAQs, mailing
lists, wiki and bugzilla info.