2012-02-10 15:45:31

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH V3] kmsg_dump: Don't run on non-error paths by default

Since 04c6862c055fb687c90d9652f32c11a063df15cf kmsg_dump() gets run on
normal paths including poweroff and reboot. This is less than ideal given
pstore implementations that can only represent single backtraces, since a
reboot may overwrite a stored oops before it's been picked up by userspace.
In addition, some pstore backends may have low performance and provide a
significant delay in reboot as a result.

This patch adds a printk.always_kmsg_dump kernel parameter (which can also
be changed from userspace). Without it, the code will only be run on
failure paths rather than on normal paths. The option can be enabled in
environments where there's a desire to attempt to audit whether or not
a reboot was cleanly requested or not.

Signed-off-by: Matthew Garrett <[email protected]>
Acked-by: Seiji Aguchi <[email protected]>
---
Documentation/kernel-parameters.txt | 6 ++++++
include/linux/kmsg_dump.h | 9 +++++++--
kernel/printk.c | 6 ++++++
3 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 033d4e6..d99fd9c 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -2211,6 +2211,12 @@ bytes respectively. Such letter suffixes can also be entirely omitted.

default: off.

+ printk.always_kmsg_dump=
+ Trigger kmsg_dump for cases other than kernel oops or
+ panics
+ Format: <bool> (1/Y/y=enable, 0/N/n=disable)
+ default: disabled
+
printk.time= Show timing data prefixed to each printk message line
Format: <bool> (1/Y/y=enable, 0/N/n=disable)

diff --git a/include/linux/kmsg_dump.h b/include/linux/kmsg_dump.h
index fee6631..35f7237 100644
--- a/include/linux/kmsg_dump.h
+++ b/include/linux/kmsg_dump.h
@@ -15,13 +15,18 @@
#include <linux/errno.h>
#include <linux/list.h>

+/*
+ * Keep this list arranged in rough order of priority. Anything listed after
+ * KMSG_DUMP_OOPS will not be logged by default unless printk.always_kmsg_dump
+ * is passed to the kernel.
+ */
enum kmsg_dump_reason {
- KMSG_DUMP_OOPS,
KMSG_DUMP_PANIC,
+ KMSG_DUMP_OOPS,
+ KMSG_DUMP_EMERG,
KMSG_DUMP_RESTART,
KMSG_DUMP_HALT,
KMSG_DUMP_POWEROFF,
- KMSG_DUMP_EMERG,
};

/**
diff --git a/kernel/printk.c b/kernel/printk.c
index 13c0a11..32690a0 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -702,6 +702,9 @@ static bool printk_time = 0;
#endif
module_param_named(time, printk_time, bool, S_IRUGO | S_IWUSR);

+static bool always_kmsg_dump;
+module_param_named(always_kmsg_dump, always_kmsg_dump, bool, S_IRUGO | S_IWUSR);
+
/* Check if we have any console registered that can be called early in boot. */
static int have_callable_console(void)
{
@@ -1732,6 +1735,9 @@ void kmsg_dump(enum kmsg_dump_reason reason)
unsigned long l1, l2;
unsigned long flags;

+ if ((reason > KMSG_DUMP_OOPS) && !always_kmsg_dump)
+ return;
+
/* Theoretically, the log could move on after we do this, but
there's not a lot we can do about that. The new messages
will overwrite the start of what we dump. */
--
1.7.7.6


2012-02-14 19:23:09

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH V3] kmsg_dump: Don't run on non-error paths by default

On Fri, Feb 10, 2012 at 10:45:15AM -0500, Matthew Garrett wrote:
> Since 04c6862c055fb687c90d9652f32c11a063df15cf kmsg_dump() gets run on
> normal paths including poweroff and reboot. This is less than ideal given
> pstore implementations that can only represent single backtraces, since a
> reboot may overwrite a stored oops before it's been picked up by userspace.
> In addition, some pstore backends may have low performance and provide a
> significant delay in reboot as a result.
>
> This patch adds a printk.always_kmsg_dump kernel parameter (which can also
> be changed from userspace). Without it, the code will only be run on
> failure paths rather than on normal paths. The option can be enabled in
> environments where there's a desire to attempt to audit whether or not
> a reboot was cleanly requested or not.

Anyone? Whose tree should this go through?

--
Matthew Garrett | [email protected]

2012-02-14 21:05:25

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH V3] kmsg_dump: Don't run on non-error paths by default

On Fri, 10 Feb 2012 10:45:15 -0500
Matthew Garrett <[email protected]> wrote:

> Since 04c6862c055fb687c90d9652f32c11a063df15cf kmsg_dump() gets run on
> normal paths including poweroff and reboot. This is less than ideal given
> pstore implementations that can only represent single backtraces, since a
> reboot may overwrite a stored oops before it's been picked up by userspace.
> In addition, some pstore backends may have low performance and provide a
> significant delay in reboot as a result.
>
> This patch adds a printk.always_kmsg_dump kernel parameter (which can also
> be changed from userspace). Without it, the code will only be run on
> failure paths rather than on normal paths. The option can be enabled in
> environments where there's a desire to attempt to audit whether or not
> a reboot was cleanly requested or not.

So if I'm understanding it correctly, this patch reverts the kernel
behaviour to what it was before 04c6862c055fb687c9 ("kmsg_dump: add
kmsg_dump() calls to the reboot, halt, poweroff and emergency_restart
paths"). The user must now set printk.always_kmsg_dump to get the
04c6862c055fb687c9 behaviour.

Correct? If so, we should get this into 3.3 so we don't have a oddball
behaviour in one particular kernel release.

2012-02-14 21:12:37

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH V3] kmsg_dump: Don't run on non-error paths by default

On Tue, Feb 14, 2012 at 01:05:22PM -0800, Andrew Morton wrote:

> So if I'm understanding it correctly, this patch reverts the kernel
> behaviour to what it was before 04c6862c055fb687c9 ("kmsg_dump: add
> kmsg_dump() calls to the reboot, halt, poweroff and emergency_restart
> paths"). The user must now set printk.always_kmsg_dump to get the
> 04c6862c055fb687c9 behaviour.

Correct.

> Correct? If so, we should get this into 3.3 so we don't have a oddball
> behaviour in one particular kernel release.

That would be my preference.

--
Matthew Garrett | [email protected]