Re: better oopsing when frozen

Re: better oopsing when frozen

[Borislav Petkov]

On Mon, Jul 25, 2011 at 10:43:19AM +0200, Oliver Neukum wrote:
> Hi Rafael,
> 
> I had a problem with the kernel stopping the machine forever because I got an
> oops while tasks were frozen. It seems to me that we should thaw when this
> happens. How about this approach?
> 
> 	Regards
> 		Oliver
> 
> From 6f3b5e7a5c7ccf3564bdd2e703eba7eee753ecdc Mon Sep 17 00:00:00 2001
> From: Oliver Neukum <oliver@neukum.org>
> Date: Fri, 22 Jul 2011 11:20:19 +0200
> Subject: [PATCH] unfreeze tasks if an oops happens while tasks are frozen
> 
> If an oops kills the task suspending or snapshotting
> is system, the system is dead because the action is
> never completed and the tasks never thawed.
> 
> Signed-off-by: Oliver Neukum <oneukum@suse.de>
> ---
>  include/linux/freezer.h |    1 +
>  kernel/panic.c          |    2 ++
>  kernel/power/process.c  |   11 +++++++++++
>  3 files changed, 14 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/freezer.h b/include/linux/freezer.h
> index 1effc8b..9907cf6 100644
> --- a/include/linux/freezer.h
> +++ b/include/linux/freezer.h
> @@ -50,6 +50,7 @@ extern int thaw_process(struct task_struct *p);
>  extern void refrigerator(void);
>  extern int freeze_processes(void);
>  extern void thaw_processes(void);
> +extern void thaw_in_oops(void);
>  
>  static inline int try_to_freeze(void)
>  {
> diff --git a/kernel/panic.c b/kernel/panic.c
> index 6923167..255e662 100644
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -23,6 +23,7 @@
>  #include <linux/init.h>
>  #include <linux/nmi.h>
>  #include <linux/dmi.h>
> +#include <linux/freezer.h>
>  
>  #define PANIC_TIMER_STEP 100
>  #define PANIC_BLINK_SPD 18
> @@ -355,6 +356,7 @@ void oops_exit(void)
>  	do_oops_enter_exit();
>  	print_oops_end_marker();
>  	kmsg_dump(KMSG_DUMP_OOPS);
> +	thaw_in_oops();
>  }
>  
>  #ifdef WANT_WARN_ON_SLOWPATH
> diff --git a/kernel/power/process.c b/kernel/power/process.c
> index 0cf3a27..20994cd 100644
> --- a/kernel/power/process.c
> +++ b/kernel/power/process.c
> @@ -22,6 +22,9 @@
>   */
>  #define TIMEOUT	(20 * HZ)
>  
> +/* in case we oops while processes are frozen */
> +static bool tasks_fozen = false;

I think you mean 'tasks_frozen' here :).

-- 
Regards/Gruss,
    Boris.

Re: better oopsing when frozen

[Rafael J. Wysocki]

Hi,

On Monday, July 25, 2011, Oliver Neukum wrote:
> Hi Rafael,
> 
> I had a problem with the kernel stopping the machine forever because I got an
> oops while tasks were frozen. It seems to me that we should thaw when this
> happens. How about this approach?

Well, we do something like this already for the OOM killer (see
oom_killer_disable() and friends), so I think it would be better to
simply extend/modify that mechanism instead of adding a new one
doing almost exactly the same thing.

I have no complaints about adding thaw_in_oops(), though, so long as
Andrew thinks it makes sense.

Thanks,
Rafael 


> From 6f3b5e7a5c7ccf3564bdd2e703eba7eee753ecdc Mon Sep 17 00:00:00 2001
> From: Oliver Neukum <oliver@neukum.org>
> Date: Fri, 22 Jul 2011 11:20:19 +0200
> Subject: [PATCH] unfreeze tasks if an oops happens while tasks are frozen
> 
> If an oops kills the task suspending or snapshotting
> is system, the system is dead because the action is
> never completed and the tasks never thawed.
> 
> Signed-off-by: Oliver Neukum <oneukum@suse.de>
> ---
>  include/linux/freezer.h |    1 +
>  kernel/panic.c          |    2 ++
>  kernel/power/process.c  |   11 +++++++++++
>  3 files changed, 14 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/freezer.h b/include/linux/freezer.h
> index 1effc8b..9907cf6 100644
> --- a/include/linux/freezer.h
> +++ b/include/linux/freezer.h
> @@ -50,6 +50,7 @@ extern int thaw_process(struct task_struct *p);
>  extern void refrigerator(void);
>  extern int freeze_processes(void);
>  extern void thaw_processes(void);
> +extern void thaw_in_oops(void);
>  
>  static inline int try_to_freeze(void)
>  {
> diff --git a/kernel/panic.c b/kernel/panic.c
> index 6923167..255e662 100644
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -23,6 +23,7 @@
>  #include <linux/init.h>
>  #include <linux/nmi.h>
>  #include <linux/dmi.h>
> +#include <linux/freezer.h>
>  
>  #define PANIC_TIMER_STEP 100
>  #define PANIC_BLINK_SPD 18
> @@ -355,6 +356,7 @@ void oops_exit(void)
>  	do_oops_enter_exit();
>  	print_oops_end_marker();
>  	kmsg_dump(KMSG_DUMP_OOPS);
> +	thaw_in_oops();
>  }
>  
>  #ifdef WANT_WARN_ON_SLOWPATH
> diff --git a/kernel/power/process.c b/kernel/power/process.c
> index 0cf3a27..20994cd 100644
> --- a/kernel/power/process.c
> +++ b/kernel/power/process.c
> @@ -22,6 +22,9 @@
>   */
>  #define TIMEOUT	(20 * HZ)
>  
> +/* in case we oops while processes are frozen */
> +static bool tasks_fozen = false;
> +
>  static inline int freezable(struct task_struct * p)
>  {
>  	if ((p == current) ||
> @@ -131,6 +134,7 @@ static int try_to_freeze_tasks(bool sig_only)
>  			elapsed_csecs % 100);
>  	}
>  
> +	tasks_fozen = (todo == 0);
>  	return todo ? -EBUSY : 0;
>  }
>  
> @@ -189,7 +193,14 @@ void thaw_processes(void)
>  	thaw_workqueues();
>  	thaw_tasks(true);
>  	thaw_tasks(false);
> +	tasks_fozen = false;
>  	schedule();
>  	printk("done.\n");
>  }
>  
> +void thaw_in_oops(void)
> +{
> +	if (tasks_fozen)
> +		thaw_processes();
> +}
> +
> 

Re: better oopsing when frozen

[Andrew Morton]

On Wed, 27 Jul 2011 00:24:11 +0200
"Rafael J. Wysocki" <rjw@sisk.pl> wrote:

> > Hi Rafael,
> > 
> > I had a problem with the kernel stopping the machine forever because I got an
> > oops while tasks were frozen. It seems to me that we should thaw when this
> > happens. How about this approach?
> 
> Well, we do something like this already for the OOM killer (see
> oom_killer_disable() and friends), so I think it would be better to
> simply extend/modify that mechanism instead of adding a new one
> doing almost exactly the same thing.
> 
> I have no complaints about adding thaw_in_oops(), though, so long as
> Andrew thinks it makes sense.

mm...  The patch as proposed is very simple, direct, explicit.  I
suspect that trying to embed this operation within some other one would
end up producing a less clear result.  Sometimes we do exceptional and
weird things, and leaving the code exceptional and weird-looking is
better than hiding it in some framework, if you follow what I mean.

It does need some code comments to explain to people what it's doing
and more importantly why it's doing it.  Also, something which doesn't
break the build when CONFIG_FREEZER=n would be nice.

Re: better oopsing when frozen

[Rafael J. Wysocki]

On Wednesday, July 27, 2011, Andrew Morton wrote:
> On Wed, 27 Jul 2011 00:24:11 +0200
> "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> 
> > > Hi Rafael,
> > > 
> > > I had a problem with the kernel stopping the machine forever because I got an
> > > oops while tasks were frozen. It seems to me that we should thaw when this
> > > happens. How about this approach?
> > 
> > Well, we do something like this already for the OOM killer (see
> > oom_killer_disable() and friends), so I think it would be better to
> > simply extend/modify that mechanism instead of adding a new one
> > doing almost exactly the same thing.
> > 
> > I have no complaints about adding thaw_in_oops(), though, so long as
> > Andrew thinks it makes sense.
> 
> mm...  The patch as proposed is very simple, direct, explicit.  I
> suspect that trying to embed this operation within some other one would
> end up producing a less clear result.  Sometimes we do exceptional and
> weird things, and leaving the code exceptional and weird-looking is
> better than hiding it in some framework, if you follow what I mean.

My point is that instead of using the oom_killer_disabled variable in
page_alloc.c, we could use a oom_killer_disabled() function returning
the value of the new tasks_are_frozen variable.  Then,
oom_killer_disable/enable() won't be necessary any more.

> It does need some code comments to explain to people what it's doing
> and more importantly why it's doing it.  Also, something which doesn't
> break the build when CONFIG_FREEZER=n would be nice.

Right.

Thanks,
Rafael

Re: better oopsing when frozen

[Oliver Neukum]

Am Mittwoch, 27. Juli 2011, 11:21:21 schrieb Rafael J. Wysocki:
> > mm...  The patch as proposed is very simple, direct, explicit.  I
> > suspect that trying to embed this operation within some other one would
> > end up producing a less clear result.  Sometimes we do exceptional and
> > weird things, and leaving the code exceptional and weird-looking is
> > better than hiding it in some framework, if you follow what I mean.
> 
> My point is that instead of using the oom_killer_disabled variable in
> page_alloc.c, we could use a oom_killer_disabled() function returning
> the value of the new tasks_are_frozen variable.  Then,
> oom_killer_disable/enable() won't be necessary any more.

Well, if we do this, it'll be a separate change. Not that I disagree. 

	Regards
		Oliver
-- 
- - - 
SUSE LINUX Products GmbH, GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer, HRB 16746 (AG Nürnberg) 
Maxfeldstraße 5                         
90409 Nürnberg 
Germany 
- - - 

Re: better oopsing when frozen

[Pavel Machek]

On Mon 2011-07-25 10:43:19, Oliver Neukum wrote:
> Hi Rafael,
> 
> I had a problem with the kernel stopping the machine forever because I got an
> oops while tasks were frozen. It seems to me that we should thaw when this
> happens. How about this approach?

Danger, Will Robinson. You must not write to the filesystems after
hibernation started. By thawing, you may do just that.

It should be safe to thaw as long as final suspend signature is not
on disk and will not be written there.

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

Re: better oopsing when frozen

[Oliver Neukum]

Am Freitag, 29. Juli 2011, 21:54:30 schrieb Pavel Machek:
> On Mon 2011-07-25 10:43:19, Oliver Neukum wrote:
> > Hi Rafael,
> > 
> > I had a problem with the kernel stopping the machine forever because I got an
> > oops while tasks were frozen. It seems to me that we should thaw when this
> > happens. How about this approach?
> 
> Danger, Will Robinson. You must not write to the filesystems after
> hibernation started. By thawing, you may do just that.
> 
> It should be safe to thaw as long as final suspend signature is not
> on disk and will not be written there.

Yes. But this applies only if I use kernel-space hibernation, doesn't it?
So I need a second flag for the signature being written. Any other problem?

	Regards
		Oliver

Re: better oopsing when frozen

[Rafael J. Wysocki]

On Friday, July 29, 2011, Pavel Machek wrote:
> On Mon 2011-07-25 10:43:19, Oliver Neukum wrote:
> > Hi Rafael,
> > 
> > I had a problem with the kernel stopping the machine forever because I got an
> > oops while tasks were frozen. It seems to me that we should thaw when this
> > happens. How about this approach?
> 
> Danger, Will Robinson. You must not write to the filesystems after
> hibernation started. By thawing, you may do just that.
> 
> It should be safe to thaw as long as final suspend signature is not
> on disk and will not be written there.

This only applies to hibernation and only when we're going to restore
from the image the presumably hasn't been saved yet, right?

However, there's another problem I didn't think of before.  Namely,
we cannot thaw tasks before resuming devices in case we've already
suspended them, because that will defeat the very purpose of the
freezing in the first place.

Thanks,
Rafael

Re: better oopsing when frozen

[Oliver Neukum]

Am Freitag, 29. Juli 2011, 22:27:54 schrieb Rafael J. Wysocki:
> However, there's another problem I didn't think of before.  Namely,
> we cannot thaw tasks before resuming devices in case we've already
> suspended them, because that will defeat the very purpose of the
> freezing in the first place.

That purpose is already defeated. The machine cannot be deader than dead.
We could try to go through the device tree. But the machine just oopsed
quite likely doing just that, so this doesn't seem wise to me.

IMHO if we oops while frozen, chances are the system is going to die.
It certainly dies now. However, we must certainly not thaw if a valid
image is already on disk, lest we corrupt a filesystem.

	Regards
		Oliver

Re: better oopsing when frozen

[Rafael J. Wysocki]

On Friday, July 29, 2011, Oliver Neukum wrote:
> Am Freitag, 29. Juli 2011, 22:27:54 schrieb Rafael J. Wysocki:
> > However, there's another problem I didn't think of before.  Namely,
> > we cannot thaw tasks before resuming devices in case we've already
> > suspended them, because that will defeat the very purpose of the
> > freezing in the first place.
> 
> That purpose is already defeated. The machine cannot be deader than dead.

OK, so what exactly is the purpose of the thawing, then?

Rafael

Re: better oopsing when frozen

[Oliver Neukum]

Am Freitag, 29. Juli 2011, 23:09:44 schrieb Rafael J. Wysocki:
> On Friday, July 29, 2011, Oliver Neukum wrote:
> > Am Freitag, 29. Juli 2011, 22:27:54 schrieb Rafael J. Wysocki:
> > > However, there's another problem I didn't think of before.  Namely,
> > > we cannot thaw tasks before resuming devices in case we've already
> > > suspended them, because that will defeat the very purpose of the
> > > freezing in the first place.
> > 
> > That purpose is already defeated. The machine cannot be deader than dead.
> 
> OK, so what exactly is the purpose of the thawing, then?

We may be lucky enough to get the oops written out to disk. The oops may happen
in a driver rarely used and be late in the sequence.

	Regards
		Oliver

Re: better oopsing when frozen

[Rafael J. Wysocki]

On Friday, July 29, 2011, Oliver Neukum wrote:
> Am Freitag, 29. Juli 2011, 23:09:44 schrieb Rafael J. Wysocki:
> > On Friday, July 29, 2011, Oliver Neukum wrote:
> > > Am Freitag, 29. Juli 2011, 22:27:54 schrieb Rafael J. Wysocki:
> > > > However, there's another problem I didn't think of before.  Namely,
> > > > we cannot thaw tasks before resuming devices in case we've already
> > > > suspended them, because that will defeat the very purpose of the
> > > > freezing in the first place.
> > > 
> > > That purpose is already defeated. The machine cannot be deader than dead.
> > 
> > OK, so what exactly is the purpose of the thawing, then?
> 
> We may be lucky enough to get the oops written out to disk. The oops may happen
> in a driver rarely used and be late in the sequence.

OK, so is there any guarantee that we won't corrupt things this way?

Rafael

Re: better oopsing when frozen

[Rafael J. Wysocki]

On Friday, July 29, 2011, Oliver Neukum wrote:
> Am Freitag, 29. Juli 2011, 21:54:30 schrieb Pavel Machek:
> > On Mon 2011-07-25 10:43:19, Oliver Neukum wrote:
> > > Hi Rafael,
> > > 
> > > I had a problem with the kernel stopping the machine forever because I got an
> > > oops while tasks were frozen. It seems to me that we should thaw when this
> > > happens. How about this approach?
> > 
> > Danger, Will Robinson. You must not write to the filesystems after
> > hibernation started. By thawing, you may do just that.
> > 
> > It should be safe to thaw as long as final suspend signature is not
> > on disk and will not be written there.
> 
> Yes. But this applies only if I use kernel-space hibernation, doesn't it?
> So I need a second flag for the signature being written. Any other problem?

Yes, please see my reply to Pavel.

Thanks,
Rafael

better oopsing when frozen

[Oliver Neukum]

Hi Rafael,

I had a problem with the kernel stopping the machine forever because I got an
oops while tasks were frozen. It seems to me that we should thaw when this
happens. How about this approach?

	Regards
		Oliver

>From 6f3b5e7a5c7ccf3564bdd2e703eba7eee753ecdc Mon Sep 17 00:00:00 2001
From: Oliver Neukum <oliver@neukum.org>
Date: Fri, 22 Jul 2011 11:20:19 +0200
Subject: [PATCH] unfreeze tasks if an oops happens while tasks are frozen

If an oops kills the task suspending or snapshotting
is system, the system is dead because the action is
never completed and the tasks never thawed.

Signed-off-by: Oliver Neukum <oneukum@suse.de>
---
 include/linux/freezer.h |    1 +
 kernel/panic.c          |    2 ++
 kernel/power/process.c  |   11 +++++++++++
 3 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/include/linux/freezer.h b/include/linux/freezer.h
index 1effc8b..9907cf6 100644
--- a/include/linux/freezer.h
+++ b/include/linux/freezer.h
@@ -50,6 +50,7 @@ extern int thaw_process(struct task_struct *p);
 extern void refrigerator(void);
 extern int freeze_processes(void);
 extern void thaw_processes(void);
+extern void thaw_in_oops(void);
 
 static inline int try_to_freeze(void)
 {
diff --git a/kernel/panic.c b/kernel/panic.c
index 6923167..255e662 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -23,6 +23,7 @@
 #include <linux/init.h>
 #include <linux/nmi.h>
 #include <linux/dmi.h>
+#include <linux/freezer.h>
 
 #define PANIC_TIMER_STEP 100
 #define PANIC_BLINK_SPD 18
@@ -355,6 +356,7 @@ void oops_exit(void)
 	do_oops_enter_exit();
 	print_oops_end_marker();
 	kmsg_dump(KMSG_DUMP_OOPS);
+	thaw_in_oops();
 }
 
 #ifdef WANT_WARN_ON_SLOWPATH
diff --git a/kernel/power/process.c b/kernel/power/process.c
index 0cf3a27..20994cd 100644
--- a/kernel/power/process.c
+++ b/kernel/power/process.c
@@ -22,6 +22,9 @@
  */
 #define TIMEOUT	(20 * HZ)
 
+/* in case we oops while processes are frozen */
+static bool tasks_fozen = false;
+
 static inline int freezable(struct task_struct * p)
 {
 	if ((p == current) ||
@@ -131,6 +134,7 @@ static int try_to_freeze_tasks(bool sig_only)
 			elapsed_csecs % 100);
 	}
 
+	tasks_fozen = (todo == 0);
 	return todo ? -EBUSY : 0;
 }
 
@@ -189,7 +193,14 @@ void thaw_processes(void)
 	thaw_workqueues();
 	thaw_tasks(true);
 	thaw_tasks(false);
+	tasks_fozen = false;
 	schedule();
 	printk("done.\n");
 }
 
+void thaw_in_oops(void)
+{
+	if (tasks_fozen)
+		thaw_processes();
+}
+
-- 
1.7.1


-- 
- - - 
SUSE LINUX Products GmbH, GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer, HRB 16746 (AG Nürnberg) 
Maxfeldstraße 5                         
90409 Nürnberg 
Germany 
- - -