[PATCH 1/2] panic: panic=-1 for immediate reboot

[PATCH 1/2] panic: panic=-1 for immediate reboot

[Mandeep Singh Baines]

From: Hugh Dickins <hughd@chromium.org>

When kernel BUG or oops occurs, ChromeOS intends to panic and immediately
reboot, with stacktrace and other messages preserved in RAM across reboot.
But the longer we delay, the more likely the user is to poweroff and lose
the info.

panic_timeout (seconds before rebooting) is set by panic= boot option
or sysctl or /proc/sys/kernel/panic; but 0 means wait forever, so at
present we have to delay at least 1 second.

Let a negative number mean reboot immediately (with the small cosmetic
benefit of suppressing that newline-less "Rebooting in %d seconds.."
message).

Signed-off-by: Hugh Dickins <hughd@chromium.org>
Signed-off-by: Mandeep Singh Baines <msb@chromium.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Huang Ying <ying.huang@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Hugh Dickins <hughd@chromium.org>
Cc: Olaf Hering <olaf@aepfle.de>
---
 kernel/panic.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/kernel/panic.c b/kernel/panic.c
index 6923167..d7bb697 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -119,6 +119,8 @@ NORET_TYPE void panic(const char * fmt, ...)
 			}
 			mdelay(PANIC_TIMER_STEP);
 		}
+	}
+	if (panic_timeout != 0) {
 		/*
 		 * This will not be a clean reboot, with everything
 		 * shutting down.  But if there is a chance of
-- 
1.7.3.1

[PATCH 2/2] panic: do not unblank_screen when panic_timeout < 0

[Mandeep Singh Baines]

Avoid risk (of screen_unblank) and wasted cycles unblanking if
you intend to reboot immediately.

Signed-off-by: Mandeep Singh Baines <msb@chromium.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Huang Ying <ying.huang@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Hugh Dickins <hughd@chromium.org>
Cc: Olaf Hering <olaf@aepfle.de>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Dave Airlie <airlied@gmail.com>
---
 lib/bust_spinlocks.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/lib/bust_spinlocks.c b/lib/bust_spinlocks.c
index 9681d54..6a5c7e0 100644
--- a/lib/bust_spinlocks.c
+++ b/lib/bust_spinlocks.c
@@ -21,7 +21,8 @@ void __attribute__((weak)) bust_spinlocks(int yes)
 		++oops_in_progress;
 	} else {
 #ifdef CONFIG_VT
-		unblank_screen();
+		if (panic_timeout >= 0)
+			unblank_screen();
 #endif
 		console_unblank();
 		if (--oops_in_progress == 0)
-- 
1.7.3.1

Re: [PATCH 1/2] panic: panic=-1 for immediate reboot

[Olaf Hering]

On Mon, Jun 20, Mandeep Singh Baines wrote:

> From: Hugh Dickins <hughd@chromium.org>
> 
> When kernel BUG or oops occurs, ChromeOS intends to panic and immediately
> reboot, with stacktrace and other messages preserved in RAM across reboot.
> But the longer we delay, the more likely the user is to poweroff and lose
> the info.

I have no strong opinion here, but:

Is delaying one second (instead of say 180 like on powerpc) such a big
deal if the system is dead anyway?

Olaf

Re: [PATCH 1/2] panic: panic=-1 for immediate reboot

[Mandeep Singh Baines]

On Wed, Jun 22, 2011 at 2:39 AM, Olaf Hering <olaf@aepfle.de> wrote:
> On Mon, Jun 20, Mandeep Singh Baines wrote:
>
>> From: Hugh Dickins <hughd@chromium.org>
>>
>> When kernel BUG or oops occurs, ChromeOS intends to panic and immediately
>> reboot, with stacktrace and other messages preserved in RAM across reboot.
>> But the longer we delay, the more likely the user is to poweroff and lose
>> the info.
>
> I have no strong opinion here, but:
>
> Is delaying one second (instead of say 180 like on powerpc) such a big
> deal if the system is dead anyway?
>

Our kernel is configured to preserve the panic in RAM. So we don't get
any benefit in waiting. On the reboot following the crash, the panic gets
logged. If the user hits the power button instead of waiting for the panic,
we lose the crash data. For this reason, we even set our lockup
thresholds as low as we can.

Our system is almost stateless and our boot time is 8 seconds so a panic
reboot cycle is not so bad. But the quicker we can get the system
back up, the better.

Regards,
Mandeep

> Olaf
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

[PATCH] panic, vt: do not force oops output when panic_timeout < 0

[Mandeep Singh Baines]

Mandeep Singh Baines (msb@chromium.org) wrote:
> Avoid risk (of screen_unblank) and wasted cycles unblanking if
> you intend to reboot immediately.
> 

The original patch doesn't work. It worked fine when I tested with
a serial port. When I disabled the serial out, my machine started to
get wedged on a panic. I guess screen_unblank was in bust_spinlocks
for a reason. It probably bust some spin_locks somewhere.

Below is a replacement for this patch which calls screen_unblank but
does not force output when the panic timeout is negative (no wait).

-- >8 -- (snip)

Don't force output if you intend to reboot immediately.

This patch depends on:

http://lkml.kernel.org/r/1308612129-12488-1-git-send-email-msb@chromium.org

Change-Id: I48f86746ac36d420b1b025197f50d5d56b873e9f
Signed-off-by: Mandeep Singh Baines <msb@chromium.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Huang Ying <ying.huang@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Hugh Dickins <hughd@google.com>
Cc: Olaf Hering <olaf@aepfle.de>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Dave Airlie <airlied@gmail.com>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
---
 include/linux/vt_kern.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/linux/vt_kern.h b/include/linux/vt_kern.h
index 4d05e14..c2164fa 100644
--- a/include/linux/vt_kern.h
+++ b/include/linux/vt_kern.h
@@ -137,7 +137,7 @@ int vty_init(const struct file_operations *console_fops);
 
 static inline bool vt_force_oops_output(struct vc_data *vc)
 {
-	if (oops_in_progress && vc->vc_panic_force_write)
+	if (oops_in_progress && vc->vc_panic_force_write  && panic_timeout >= 0)
 		return true;
 	return false;
 }
-- 
1.7.3.1

Re: [PATCH] panic, vt: do not force oops output when panic_timeout < 0

[Andrew Morton]

On Wed, 22 Jun 2011 15:30:39 -0700
Mandeep Singh Baines <msb@chromium.org> wrote:

> Mandeep Singh Baines (msb@chromium.org) wrote:
> > Avoid risk (of screen_unblank) and wasted cycles unblanking if
> > you intend to reboot immediately.
> > 
> 
> The original patch doesn't work. It worked fine when I tested with
> a serial port. When I disabled the serial out, my machine started to
> get wedged on a panic. I guess screen_unblank was in bust_spinlocks
> for a reason. It probably bust some spin_locks somewhere.
> 
> Below is a replacement for this patch which calls screen_unblank but
> does not force output when the panic timeout is negative (no wait).
> 
> -- >8 -- (snip)
> 
> Don't force output if you intend to reboot immediately.
> 
> This patch depends on:
> 
> http://lkml.kernel.org/r/1308612129-12488-1-git-send-email-msb@chromium.org

Please fold this into that and resend the patch series.  Please add
the needed update to Documentation/kernel-parameters.txt.

Re: [PATCH] panic, vt: do not force oops output when panic_timeout < 0

[Alan Cox]

> a serial port. When I disabled the serial out, my machine started to
> get wedged on a panic. I guess screen_unblank was in bust_spinlocks
> for a reason. It probably bust some spin_locks somewhere.

No something else is wrong here. The console panic should be reliably
breaking the locks but a quick test of taking the console lock and BUG()
on a current kernel shows something has broken this.

> Below is a replacement for this patch which calls screen_unblank but
> does not force output when the panic timeout is negative (no wait).

The on screen console is not always just a vt, and some people log remote
management console output so we really really don't want to do this.

Instead the bug in the lock busting needs fixing. To start with it will
be hiding a ton of other oopses/bugs as hangs.

Alan

Re: [PATCH] panic, vt: do not force oops output when panic_timeout < 0

[Mandeep Singh Baines]

Alan Cox (alan@lxorguk.ukuu.org.uk) wrote:
> > a serial port. When I disabled the serial out, my machine started to
> > get wedged on a panic. I guess screen_unblank was in bust_spinlocks
> > for a reason. It probably bust some spin_locks somewhere.
> 
> No something else is wrong here. The console panic should be reliably
> breaking the locks but a quick test of taking the console lock and BUG()
> on a current kernel shows something has broken this.
> 

Root caused to the issue I reported earlier with unblank_screen:

http://lkml.org/lkml/2011/6/20/394

console_unblank()
  -> c_unblank()
    -> unblank_screen()
      -> ...
        -> mutex_lock()

> > Below is a replacement for this patch which calls screen_unblank but
> > does not force output when the panic timeout is negative (no wait).
> 
> The on screen console is not always just a vt, and some people log remote
> management console output so we really really don't want to do this.
> 

In this patch, I'm disabling the functionality enabled by
vc->vc_panic_force_write if panic_timeout < 0 (i.e. no timeout).
vc_panic_force_write is only enabled for fb video consoles if the
FBINFO_CAN_FORCE_OUTPUT flag is set.

For our application, we're using ram_oops to preserved the panic in memory.
We want to reliably, and as fast as possible, machine_restart. The
vc_panic_force_write flag results in a bunch of graphics driver code to be
invoked which slows down restart and decreases reliability. Since we're
already storing the panic in RAM and are going to reboot immediately, there
is no benefit in mode switching back to the vc in order to display the panic
output. The log buffer will get flushed by the console_unblank() call so
remote management consoles should see all output.

> Instead the bug in the lock busting needs fixing. To start with it will
> be hiding a ton of other oopses/bugs as hangs.
> 
> Alan

Re: [PATCH] panic, vt: do not force oops output when panic_timeout < 0

[Mandeep Singh Baines]

Andrew Morton (akpm@linux-foundation.org) wrote:
> On Wed, 22 Jun 2011 15:30:39 -0700
> Mandeep Singh Baines <msb@chromium.org> wrote:
> 
> > Mandeep Singh Baines (msb@chromium.org) wrote:
> > > Avoid risk (of screen_unblank) and wasted cycles unblanking if
> > > you intend to reboot immediately.
> > > 
> > 
> > The original patch doesn't work. It worked fine when I tested with
> > a serial port. When I disabled the serial out, my machine started to
> > get wedged on a panic. I guess screen_unblank was in bust_spinlocks
> > for a reason. It probably bust some spin_locks somewhere.
> > 
> > Below is a replacement for this patch which calls screen_unblank but
> > does not force output when the panic timeout is negative (no wait).
> > 
> > -- >8 -- (snip)
> > 
> > Don't force output if you intend to reboot immediately.
> > 
> > This patch depends on:
> > 
> > http://lkml.kernel.org/r/1308612129-12488-1-git-send-email-msb@chromium.org
> 
> Please fold this into that and resend the patch series.  Please add
> the needed update to Documentation/kernel-parameters.txt.

Done:

http://lkml.kernel.org/r/1310770401-10739-1-git-send-email-msb@chromium.org

Re: [PATCH] panic, vt: do not force oops output when panic_timeout < 0

[Alan Cox]

On Fri, 15 Jul 2011 14:39:27 -0700
Mandeep Singh Baines <msb@chromium.org> wrote:

> Alan Cox (alan@lxorguk.ukuu.org.uk) wrote:
> > > a serial port. When I disabled the serial out, my machine started to
> > > get wedged on a panic. I guess screen_unblank was in bust_spinlocks
> > > for a reason. It probably bust some spin_locks somewhere.
> > 
> > No something else is wrong here. The console panic should be reliably
> > breaking the locks but a quick test of taking the console lock and BUG()
> > on a current kernel shows something has broken this.
> > 
> 
> Root caused to the issue I reported earlier with unblank_screen:
> 
> http://lkml.org/lkml/2011/6/20/394

Thanks - I missed that at the time and I'll take a look at it.

Re: [PATCH 1/2] panic: panic=-1 for immediate reboot

[Mandeep Singh Baines]

On Fri, Jul 15, 2011 at 3:39 PM, Mandeep Singh Baines <msb@chromium.org> wrote:
> From: Hugh Dickins <hughd@chromium.org>
>
> When kernel BUG or oops occurs, ChromeOS intends to panic and immediately
> reboot, with stacktrace and other messages preserved in RAM across reboot.
> But the longer we delay, the more likely the user is to poweroff and lose
> the info.
>
> panic_timeout (seconds before rebooting) is set by panic= boot option
> or sysctl or /proc/sys/kernel/panic; but 0 means wait forever, so at
> present we have to delay at least 1 second.
>
> Let a negative number mean reboot immediately (with the small cosmetic
> benefit of suppressing that newline-less "Rebooting in %d seconds.."
> message).
>

Ignore this patch. I re-posted the series with documentation updates:

http://lkml.kernel.org/r/1310770401-10739-1-git-send-email-msb@chromium.org

> Signed-off-by: Hugh Dickins <hughd@chromium.org>
> Signed-off-by: Mandeep Singh Baines <msb@chromium.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Huang Ying <ying.huang@intel.com>
> Cc: Andi Kleen <ak@linux.intel.com>
> Cc: Hugh Dickins <hughd@google.com>
> Cc: Olaf Hering <olaf@aepfle.de>
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Cc: Dave Airlie <airlied@gmail.com>
> Cc: Greg Kroah-Hartman <gregkh@suse.de>
> Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
> ---
>  kernel/panic.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/kernel/panic.c b/kernel/panic.c
> index 6923167..d7bb697 100644
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -119,6 +119,8 @@ NORET_TYPE void panic(const char * fmt, ...)
>                        }
>                        mdelay(PANIC_TIMER_STEP);
>                }
> +       }
> +       if (panic_timeout != 0) {
>                /*
>                 * This will not be a clean reboot, with everything
>                 * shutting down.  But if there is a chance of
> --
> 1.7.3.1
>
>

[PATCH 1/2] panic: panic=-1 for immediate reboot

[Mandeep Singh Baines]

From: Hugh Dickins <hughd@chromium.org>

When kernel BUG or oops occurs, ChromeOS intends to panic and immediately
reboot, with stacktrace and other messages preserved in RAM across reboot.
But the longer we delay, the more likely the user is to poweroff and lose
the info.

panic_timeout (seconds before rebooting) is set by panic= boot option
or sysctl or /proc/sys/kernel/panic; but 0 means wait forever, so at
present we have to delay at least 1 second.

Let a negative number mean reboot immediately (with the small cosmetic
benefit of suppressing that newline-less "Rebooting in %d seconds.."
message).

Signed-off-by: Hugh Dickins <hughd@chromium.org>
Signed-off-by: Mandeep Singh Baines <msb@chromium.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Huang Ying <ying.huang@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Hugh Dickins <hughd@google.com>
Cc: Olaf Hering <olaf@aepfle.de>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Dave Airlie <airlied@gmail.com>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
---
 Documentation/kernel-parameters.txt |    4 +++-
 kernel/panic.c                      |    2 ++
 2 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index aa47be7..4f42fc7 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1846,7 +1846,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			See Documentation/sound/oss/oss-parameters.txt
 
 	panic=		[KNL] Kernel behaviour on panic: delay <timeout>
-			seconds before rebooting
+			timeout > 0: seconds before rebooting
+			timeout = 0: wait forever
+			timeout < 0: reboot immediately
 			Format: <timeout>
 
 	parkbd.port=	[HW] Parallel port number the keyboard adapter is
diff --git a/kernel/panic.c b/kernel/panic.c
index 6923167..d7bb697 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -119,6 +119,8 @@ NORET_TYPE void panic(const char * fmt, ...)
 			}
 			mdelay(PANIC_TIMER_STEP);
 		}
+	}
+	if (panic_timeout != 0) {
 		/*
 		 * This will not be a clean reboot, with everything
 		 * shutting down.  But if there is a chance of
-- 
1.7.3.1

[PATCH 1/2] panic: panic=-1 for immediate reboot

[Mandeep Singh Baines]

From: Hugh Dickins <hughd@chromium.org>

When kernel BUG or oops occurs, ChromeOS intends to panic and immediately
reboot, with stacktrace and other messages preserved in RAM across reboot.
But the longer we delay, the more likely the user is to poweroff and lose
the info.

panic_timeout (seconds before rebooting) is set by panic= boot option
or sysctl or /proc/sys/kernel/panic; but 0 means wait forever, so at
present we have to delay at least 1 second.

Let a negative number mean reboot immediately (with the small cosmetic
benefit of suppressing that newline-less "Rebooting in %d seconds.."
message).

Signed-off-by: Hugh Dickins <hughd@chromium.org>
Signed-off-by: Mandeep Singh Baines <msb@chromium.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Huang Ying <ying.huang@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Hugh Dickins <hughd@google.com>
Cc: Olaf Hering <olaf@aepfle.de>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Dave Airlie <airlied@gmail.com>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
---
 kernel/panic.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/kernel/panic.c b/kernel/panic.c
index 6923167..d7bb697 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -119,6 +119,8 @@ NORET_TYPE void panic(const char * fmt, ...)
 			}
 			mdelay(PANIC_TIMER_STEP);
 		}
+	}
+	if (panic_timeout != 0) {
 		/*
 		 * This will not be a clean reboot, with everything
 		 * shutting down.  But if there is a chance of
-- 
1.7.3.1