This patch makes panic() and die() registers available to, for example,
panic notifier functions. Panic notifier functions are quite useful
for recording crash information, but they don't get passed the register
values. This makes it hard to print register contents, do stack
backtraces, etc. The changes in this patch save the register state when
panic() is called and introduce a function for die() to call that allows
it to pass in the registers it was passed.
Following this patch are more patches, one per architecture. These include
two types of changes:
o A save_ptregs() function for the processor. I've taken a whack at
doing this for all of the processors. I have tested x86 and MIPS
versions. I was able to find cross compilers for ARM, ... and the
code compiles cleanly. Everything else, well, what you see is sheer
fantasy. You are welcome to chortle with merriment.
o When I could figure it out, I replaced the calls to panic() in
exception handling functions with calls to panic_with_regs() so
that everyone can leverage these changes without much effort. Again,
not all the code was transparent, so there are likely some places
that should have additional work done.
Note that the pointer to the struct pt_regs may be NULL. This is to
accomodate those processors which don't have a working save_ptregs(). I'd
love to eliminate this case by providing a save_ptregs() for all
architectures, but I'll need help to so.
Signed-off-by: David VomLehn <[email protected]>
---
include/linux/kernel.h | 5 ++
include/linux/ptrace.h | 9 ++++
include/linux/ptreg.h | 62 +++++++++++++++++++++++++++++++
kernel/panic.c | 96 +++++++++++++++++++++++++++++++++++++++++------
4 files changed, 159 insertions(+), 13 deletions(-)
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 328bca6..d73ae5c 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -162,6 +162,11 @@ extern struct atomic_notifier_head panic_notifier_list;
extern long (*panic_blink)(long time);
NORET_TYPE void panic(const char * fmt, ...)
__attribute__ ((NORET_AND format (printf, 1, 2))) __cold;
+NORET_TYPE void panic_with_regs(const struct pt_regs *regs,
+ const char *fmt, ...)
+ __attribute__ ((NORET_AND format (printf, 2, 3))) __cold;
+const struct pt_regs *get_panic_regs(void);
+extern const struct pt_regs *set_panic_regs(const struct pt_regs *regs);
extern void oops_enter(void);
extern void oops_exit(void);
extern int oops_may_print(void);
diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h
index 56f2d63..8f477ff 100644
--- a/include/linux/ptrace.h
+++ b/include/linux/ptrace.h
@@ -336,6 +336,15 @@ extern int task_current_syscall(struct task_struct *target, long *callno,
unsigned long args[6], unsigned int maxargs,
unsigned long *sp, unsigned long *pc);
+#ifndef arch_has_save_ptregs
+#warning save_ptregs undefined for this architecture; please define it soon
+
+static const struct pt_regs *save_ptregs(const struct pt_regs *regs)
+{
+ return NULL;
+}
+#endif
+
#endif
#endif
diff --git a/include/linux/ptreg.h b/include/linux/ptreg.h
new file mode 100644
index 0000000..f8cc2ea
--- /dev/null
+++ b/include/linux/ptreg.h
@@ -0,0 +1,62 @@
+/*
+ * Define macros for saving registers in a struct pt_regs structure
+ *
+ * Copyright (C) 2009 Cisco Systems, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#ifndef _LINUX_PTREG_H_
+#define _LINUX_PTREG_H_
+/*
+ * Define macros useful for writing save_ptregs(). Each architecture will
+ * need to define some macros, and some architectures have special cases
+ * that are most easily done as a special case. Macros that must be defined
+ * per architecture are:
+ *
+ * PTREG_SAVE - Save a register in a named location
+ * @r: Name of the register to save
+ * @name: %[name] of the location
+ *
+ * This is used in cases where the entry in the struct pt_regs is a simple
+ * string that doesn't end with a digit, e.g. pc.
+ *
+ * PTREG_INDIRECT_SAVE - move register to another register for saving
+ * @tmp_r: Name of a temporary register
+ * @r: Name of the register to be saved
+ * @name: %[name] of the location
+ *
+ * This is used for cases where the register can't be saved directly,
+ * but must first be moved to another register that can be saved
+ * directly. The second register must already have been saved
+ * by the time this macro is used.
+ */
+
+/* Register save macro where the pt_regs item is an array, e.g. r[0] */
+#define _PTREG_SAVE_IDX(r, name, i) PTREG_SAVE(r##i, name##i)
+
+/* Save register where the pt_regs item ends with a digit, e.g. r0. */
+#define _PTREG_SAVE_I(r, name, i) PTREG_SAVE(r##i, name##i)
+
+/* Save register where it must first be copied to another register and where
+ * the pt_regs element ends with a digit, e.g. a2 */
+#define _PTREG_INDIRECT_SAVE_I(tmp, r, name, i) \
+ PTREG_INDIRECT_SAVE(tmp, r##i, name##i)
+
+/* Output specification */
+#define PTREG_OUT(rp, r, name) [name] "=m" (rp->r)
+#define _PTREG_OUT_IDX(rp, r, name, i) PTREG_OUT(rp, r[i], name##i)
+#define _PTREG_OUT_I(rp, r, name, i) PTREG_OUT(rp, r##i, name##i)
+#endif
diff --git a/kernel/panic.c b/kernel/panic.c
index c787333..ef500c7 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -46,28 +46,22 @@ long (*panic_blink)(long time);
EXPORT_SYMBOL(panic_blink);
/**
- * panic - halt the system
- * @fmt: The text string to print
- *
- * Display a message, then perform cleanups.
- *
- * This function never returns.
+ * vpanic_with_regs - panic with specific register values and a argument array
+ * @regs: Pointer to a &struct pt_reg with the register values
+ * @fmt: Format for the panic message
+ * @args: Arguments for the panic message as a va_list
*/
-NORET_TYPE void panic(const char * fmt, ...)
+static NORET_TYPE void vpanic_with_regs(const struct pt_regs *regs,
+ const char *fmt, va_list args)
{
static char buf[1024];
- va_list args;
long i;
-
/*
* It's possible to come here directly from a panic-assertion and
* not have preempt disabled. Some functions called from here want
* preempt to be disabled. No point enabling it later though...
*/
- preempt_disable();
-
- bust_spinlocks(1);
- va_start(args, fmt);
+ set_panic_regs(regs);
vsnprintf(buf, sizeof(buf), fmt, args);
va_end(args);
printk(KERN_EMERG "Kernel panic - not syncing: %s\n",buf);
@@ -143,8 +137,84 @@ NORET_TYPE void panic(const char * fmt, ...)
}
}
+/* Registers stored in calls to panic() */
+static DEFINE_PER_CPU(struct pt_regs, panic_panic_regs);
+static DEFINE_PER_CPU(const struct pt_regs *, panic_regs);
+
+/**
+ * get_panic_regs - return the current pointer to panic register values
+ */
+const struct pt_regs *get_panic_regs()
+{
+ return __get_cpu_var(panic_regs);
+}
+EXPORT_SYMBOL(get_panic_regs);
+
+/**
+ * set_panic_regs - Set a pointer to the values of registers on panic()
+ * @new_regs: Pointer to register values
+ *
+ * Returns: Pointer to the previous panic registers, if any.
+ */
+const struct pt_regs *set_panic_regs(const struct pt_regs *new_regs)
+{
+ const struct pt_regs *old_regs, **pp_regs;
+
+ pp_regs = &__get_cpu_var(panic_regs);
+ old_regs = *pp_regs;
+ *pp_regs = new_regs;
+ return old_regs;
+}
+
+/**
+ * panic - halt the system
+ * @fmt: The text string to print
+ *
+ * Display a message, then perform cleanups.
+ *
+ * This function never returns.
+ */
+NORET_TYPE void panic(const char *fmt, ...)
+{
+ va_list args;
+ const struct pt_regs *regs;
+ int i;
+
+ preempt_disable();
+ bust_spinlocks(1);
+ regs = save_ptregs(&__get_cpu_var(panic_panic_regs));
+ va_start(args, fmt);
+ vpanic_with_regs(regs, fmt, args);
+ /* Since vpanic_with_regs doesn't return, we skip va_end() */
+ /* Infinite loop so compiler doesn't complain about this returning */
+ for (i = 0; ; )
+ mdelay(1);
+}
EXPORT_SYMBOL(panic);
+/**
+ * panic_with_regs - panic with specific register values
+ * @regs: Pointer to a &struct pt_reg with the register values
+ * @fmt: Format for the panic message
+ * @...: Arguments for the panic message
+ */
+NORET_TYPE void panic_with_regs(const struct pt_regs *regs, const char *fmt,
+ ...)
+{
+ va_list args;
+ int i;
+
+ preempt_disable();
+ bust_spinlocks(1);
+ va_start(args, fmt);
+ vpanic_with_regs(regs, fmt, args);
+ /* Since vpanic_with_regs doesn't return, we skip va_end() */
+ /* Infinite loop so compiler doesn't complain about this returning */
+ for (i = 0; ; )
+ mdelay(1);
+}
+EXPORT_SYMBOL(panic_with_regs);
+
struct tnt {
u8 bit;
On Mon, Apr 12, 2010 at 02:06, David VomLehn wrote:
> This patch makes panic() and die() registers available to, for example,
> panic notifier functions. Panic notifier functions are quite useful
> for recording crash information, but they don't get passed the register
> values. This makes it hard to print register contents, do stack
> backtraces, etc. The changes in this patch save the register state when
> panic() is called and introduce a function for die() to call that allows
> it to pass in the registers it was passed.
>
> Following this patch are more patches, one per architecture. These include
> two types of changes:
> o A save_ptregs() function for the processor. I've taken a whack at
> doing this for all of the processors. I have tested x86 and MIPS
> versions. I was able to find cross compilers for ARM, ... and the
> code compiles cleanly. Everything else, well, what you see is sheer
> fantasy. You are welcome to chortle with merriment.
could you post a sample module that you're using to test with here ?
presumably you have some simple code that registers a notify handler
and then calls panic_with_regs() ...
-mike
David VomLehn <[email protected]> wrote:
> +NORET_TYPE void panic(const char *fmt, ...)
> +{
> + va_list args;
> + const struct pt_regs *regs;
> + int i;
> +
> + preempt_disable();
> + bust_spinlocks(1);
> + regs = save_ptregs(&__get_cpu_var(panic_panic_regs));
> + va_start(args, fmt);
> + vpanic_with_regs(regs, fmt, args);
> + /* Since vpanic_with_regs doesn't return, we skip va_end() */
> + /* Infinite loop so compiler doesn't complain about this returning */
> + for (i = 0; ; )
> + mdelay(1);
> +}
Can the use of va_start() clobber lots of registers, thereby rendering the
exercise pointless on some arches?
Also, can the save_ptregs() function be out of line asm? The FRV constructed
inline statement is huge (and wrong).
David
On Sun, Apr 11, 2010 at 11:06:09PM -0700, David VomLehn wrote:
> This patch makes panic() and die() registers available to, for example,
> panic notifier functions. Panic notifier functions are quite useful
> for recording crash information, but they don't get passed the register
> values. This makes it hard to print register contents, do stack
> backtraces, etc. The changes in this patch save the register state when
> panic() is called and introduce a function for die() to call that allows
> it to pass in the registers it was passed.
>
> Following this patch are more patches, one per architecture. These include
> two types of changes:
> o A save_ptregs() function for the processor. I've taken a whack at
> doing this for all of the processors. I have tested x86 and MIPS
> versions. I was able to find cross compilers for ARM, ... and the
> code compiles cleanly. Everything else, well, what you see is sheer
> fantasy. You are welcome to chortle with merriment.
> o When I could figure it out, I replaced the calls to panic() in
> exception handling functions with calls to panic_with_regs() so
> that everyone can leverage these changes without much effort. Again,
> not all the code was transparent, so there are likely some places
> that should have additional work done.
>
> Note that the pointer to the struct pt_regs may be NULL. This is to
> accomodate those processors which don't have a working save_ptregs(). I'd
> love to eliminate this case by providing a save_ptregs() for all
> architectures, but I'll need help to so.
Wouldn't it be much easier to implement panic with an illegal op and let
the exception handler set up the pt regs structure instead? Just like some
architectures do that already for warnings.
Have a look at lib/bug.c and at various arch/<...>/include/asm/bug.h.
BUG_FLAG_PANIC would do the trick.
But I'm still wondering what the use case would be. You haven't posted any
code that would actually use this.
Heiko Carstens <[email protected]> wrote:
> Wouldn't it be much easier to implement panic with an illegal op and let
> the exception handler set up the pt regs structure instead? Just like some
> architectures do that already for warnings.
No, not necessarily. panic() is varargs.
David
On Mon, Apr 12, 2010 at 02:03:30PM +0200, Heiko Carstens wrote:
> Wouldn't it be much easier to implement panic with an illegal op and let
> the exception handler set up the pt regs structure instead? Just like some
> architectures do that already for warnings.
> Have a look at lib/bug.c and at various arch/<...>/include/asm/bug.h.
> BUG_FLAG_PANIC would do the trick.
You also have the problem that there are panic() statements before
exception vectors are setup.
Eg, using lmb, you might want to allocate a page for a L2 page table
so you can setup the exception vectors, but the lmb allocator has a
panic statement which will be used on failure to allocate a page.
The result is that you don't know why you didn't boot since there's no
diagnostics from the kernel.
At least with the current setup, merely (re)directing the kernel printk
output results in something you can read.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:
On Sun, Apr 11, 2010 at 11:06:09PM -0700, David VomLehn wrote:
> This patch makes panic() and die() registers available to, for example,
> panic notifier functions. Panic notifier functions are quite useful
> for recording crash information, but they don't get passed the register
> values. This makes it hard to print register contents, do stack
> backtraces, etc. The changes in this patch save the register state when
> panic() is called and introduce a function for die() to call that allows
> it to pass in the registers it was passed.
Can you explain why you want this?
I'm wondering about the value of saving the registers; normally when a panic
occurs, it's because of a well defined reason, and not because something
went wrong in some CPU register; to put it another way, a panic() is a
more controlled exception than a BUG() or a bad pointer dereference.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:
Russell King <[email protected]> wrote:
> Can you explain why you want this?
>
> I'm wondering about the value of saving the registers; normally when a panic
> occurs, it's because of a well defined reason, and not because something
> went wrong in some CPU register; to put it another way, a panic() is a
> more controlled exception than a BUG() or a bad pointer dereference.
+1.
I found in FS-Cache and CacheFiles that often the things I most wanted to know
when I had something of the form:
if (A == B)
BUG();
was a and b, so I made the following macro:
#define ASSERTCMP(X, OP, Y) \
do { \
if (unlikely(!((X) OP (Y)))) { \
printk(KERN_ERR "\n"); \
printk(KERN_ERR "AFS: Assertion failed\n"); \
printk(KERN_ERR "%lu " #OP " %lu is false\n", \
(unsigned long)(X), (unsigned long)(Y)); \
printk(KERN_ERR "0x%lx " #OP " 0x%lx is false\n", \
(unsigned long)(X), (unsigned long)(Y)); \
BUG(); \
} \
} while(0)
which I could then call like this:
ASSERTCMP(A, ==, B);
and if the assertion failed, it prints A and B explicitly. This is much
easier than trying to pick the values out of a register dump, especially as
the compiler may be free to clobber A or B immediately after testing them.
David
On Mon, 12 Apr 2010 13:27:45 +0100
Russell King <[email protected]> wrote:
> On Sun, Apr 11, 2010 at 11:06:09PM -0700, David VomLehn wrote:
> > This patch makes panic() and die() registers available to, for example,
> > panic notifier functions. Panic notifier functions are quite useful
> > for recording crash information, but they don't get passed the register
> > values. This makes it hard to print register contents, do stack
> > backtraces, etc. The changes in this patch save the register state when
> > panic() is called and introduce a function for die() to call that allows
> > it to pass in the registers it was passed.
>
> Can you explain why you want this?
>
> I'm wondering about the value of saving the registers; normally when a panic
> occurs, it's because of a well defined reason, and not because something
> went wrong in some CPU register; to put it another way, a panic() is a
> more controlled exception than a BUG() or a bad pointer dereference.
I'm curious about the potential use case as well. So far I only wanted
to know the registers if the panic has been triggered due to an
unexpected fault with panic_on_oops=1 or in_interrupt()==1. If that
happens the die() handler prints the registers. An open coded panic is
easy to analyze, imho no need for the registers.
--
blue skies,
Martin.
"Reality continues to ruin my life." - Calvin.
David Howells wrote:
> David VomLehn <[email protected]> wrote:
>
>
>> +NORET_TYPE void panic(const char *fmt, ...)
>> +{
>> + va_list args;
>> + const struct pt_regs *regs;
>> + int i;
>> +
>> + preempt_disable();
>> + bust_spinlocks(1);
>> + regs = save_ptregs(&__get_cpu_var(panic_panic_regs));
>> + va_start(args, fmt);
>> + vpanic_with_regs(regs, fmt, args);
>> + /* Since vpanic_with_regs doesn't return, we skip va_end() */
>> + /* Infinite loop so compiler doesn't complain about this returning */
>> + for (i = 0; ; )
>> + mdelay(1);
>> +}
>>
>
> Can the use of va_start() clobber lots of registers, thereby rendering the
> exercise pointless on some arches?
>
The implementations I'm familiar with only need one or two registers.
What it
*does* do is to force the contents of registers being used to pass
argument values
onto the stack. This is roughly what gcc does for asm() statements when you
tell it registers are clobbered.
> Also, can the save_ptregs() function be out of line asm? The FRV constructed
> inline statement is huge (and wrong).
>
With this implementation it has to be inline. One use of the saved
registers is to
backtrace the stack. If you call a function to save the registers, the
stack pointer
and program counter would be those of the called function, which will not be
valid after it returns. I expect that you could come up with an alternative
out-of-line function--on every processor I know, you could backtrace one
frame
to get reasonable values for those registers,. Unfortunately, you would
run the
risk of clobbering other registers by doing the function call. The more you
change register values from those in the function that calls panic(),
the less
useful this becomes. In this case, I think an inline function is worth the
effort to get working. (I'd be interested in know more details about how
tshings are broken in the FRV)
> David
>
David Howells wrote:
> David VomLehn <[email protected]> wrote:
>
>
>> +NORET_TYPE void panic(const char *fmt, ...)
>> +{
>> + va_list args;
>> + const struct pt_regs *regs;
>> + int i;
>> +
>> + preempt_disable();
>> + bust_spinlocks(1);
>> + regs = save_ptregs(&__get_cpu_var(panic_panic_regs));
>> + va_start(args, fmt);
>> + vpanic_with_regs(regs, fmt, args);
>> + /* Since vpanic_with_regs doesn't return, we skip va_end() */
>> + /* Infinite loop so compiler doesn't complain about this returning */
>> + for (i = 0; ; )
>> + mdelay(1);
>> +}
>>
>
> Can the use of va_start() clobber lots of registers, thereby rendering the
> exercise pointless on some arches?
>
The implementations I'm familiar with only need one or two registers.
What it
*does* do is to force the contents of registers being used to pass
argument values
onto the stack. This is roughly what gcc does for asm() statements when you
tell it registers are clobbered.
> Also, can the save_ptregs() function be out of line asm? The FRV constructed
> inline statement is huge (and wrong).
>
With this implementation it has to be inline. One use of the saved
registers is to
backtrace the stack. If you call a function to save the registers, the
stack pointer
and program counter would be those of the called function, which will not be
valid after it returns. I expect that you could come up with an alternative
out-of-line function--on every processor I know, you could backtrace one
frame
to get reasonable values for those registers,. Unfortunately, you would
run the
risk of clobbering other registers by doing the function call. The more you
change register values from those in the function that calls panic(),
the less
useful this becomes. In this case, I think an inline function is worth the
effort to get working. (I'd be interested in know more details about how
tshings are broken in the FRV)
> David
>
Heiko Carstens wrote:
> On Sun, Apr 11, 2010 at 11:06:09PM -0700, David VomLehn wrote:
>
>> This patch makes panic() and die() registers available to, for example,
>> panic notifier functions. Panic notifier functions are quite useful
>> for recording crash information, but they don't get passed the register
>> values. This makes it hard to print register contents, do stack
>> backtraces, etc. The changes in this patch save the register state when
>> panic() is called and introduce a function for die() to call that allows
>> it to pass in the registers it was passed.
>>
>> Following this patch are more patches, one per architecture. These include
>> two types of changes:
>> o A save_ptregs() function for the processor. I've taken a whack at
>> doing this for all of the processors. I have tested x86 and MIPS
>> versions. I was able to find cross compilers for ARM, ... and the
>> code compiles cleanly. Everything else, well, what you see is sheer
>> fantasy. You are welcome to chortle with merriment.
>> o When I could figure it out, I replaced the calls to panic() in
>> exception handling functions with calls to panic_with_regs() so
>> that everyone can leverage these changes without much effort. Again,
>> not all the code was transparent, so there are likely some places
>> that should have additional work done.
>>
>> Note that the pointer to the struct pt_regs may be NULL. This is to
>> accomodate those processors which don't have a working save_ptregs(). I'd
>> love to eliminate this case by providing a save_ptregs() for all
>> architectures, but I'll need help to so.
>>
>
> Wouldn't it be much easier to implement panic with an illegal op and let
> the exception handler set up the pt regs structure instead? Just like some
> architectures do that already for warnings.
> Have a look at lib/bug.c and at various arch/<...>/include/asm/bug.h.
> BUG_FLAG_PANIC would do the trick.
>
You could do this so long as your exception handler wasn't compromised. When
you get here, your system is already known to have failed, so the idea
is to be
as gentle as possible.
> But I'm still wondering what the use case would be. You haven't posted any
> code that would actually use this.
>
I'm working my way towards this, but the general idea is to be able to
log all system
state from within a panic handler. So, you'd want to print the
registers, the stack,
the window of memory around instructions, the stack trace, etc., all of
which need
some set of the register values.
Russell King wrote:
> On Sun, Apr 11, 2010 at 11:06:09PM -0700, David VomLehn wrote:
>
>> This patch makes panic() and die() registers available to, for example,
>> panic notifier functions. Panic notifier functions are quite useful
>> for recording crash information, but they don't get passed the register
>> values. This makes it hard to print register contents, do stack
>> backtraces, etc. The changes in this patch save the register state when
>> panic() is called and introduce a function for die() to call that allows
>> it to pass in the registers it was passed.
>>
>
> Can you explain why you want this?
>
> I'm wondering about the value of saving the registers; normally when a panic
> occurs, it's because of a well defined reason, and not because something
> went wrong in some CPU register; to put it another way, a panic() is a
> more controlled exception than a BUG() or a bad pointer dereference.
>
More context probably helps, starting with noting that the platform is
an embedded
one. (I'm at the Eembedded Linux Conference, which is why my reply is so
tardy).
In embedded systems we frequently (probably?) don't have the resources
to create or
store a crash dump. A very common approach is to record a subset of the
system
state that is likely to help diagnose the failure. The state information
is then stored\
on the system or sent upstream to some network-connected node. When I
gave a talk
about this at the ELC, I polled the audience and got at least half a
dozen other
companies using the same approach. This is the first set of patches to
allow this
common embedded community requirement to be met. My expectation that
register
values will be wanted by everyone doing this kind of targeted state
reporting.
I should also note that I regard this is more the beginning of a
conversation on how
to diagnose kernel (and, possibly, application) failures on crash
dump-less systems.
I would expect to see some number of patches, some of which will
ultimately be
dropped in favor of other approaches.
David Howells wrote:
> Russell King <[email protected]> wrote:
>
>
>> Can you explain why you want this?
>>
>> I'm wondering about the value of saving the registers; normally when a panic
>> occurs, it's because of a well defined reason, and not because something
>> went wrong in some CPU register; to put it another way, a panic() is a
>> more controlled exception than a BUG() or a bad pointer dereference.
>>
>
> +1.
>
> I found in FS-Cache and CacheFiles that often the things I most wanted to know
> when I had something of the form:
>
> if (A == B)
> BUG();
>
> was a and b, so I made the following macro:
>
> #define ASSERTCMP(X, OP, Y) \
> do { \
> if (unlikely(!((X) OP (Y)))) { \
> printk(KERN_ERR "\n"); \
> printk(KERN_ERR "AFS: Assertion failed\n"); \
> printk(KERN_ERR "%lu " #OP " %lu is false\n", \
> (unsigned long)(X), (unsigned long)(Y)); \
> printk(KERN_ERR "0x%lx " #OP " 0x%lx is false\n", \
> (unsigned long)(X), (unsigned long)(Y)); \
> BUG(); \
> } \
> } while(0)
>
> which I could then call like this:
>
> ASSERTCMP(A, ==, B);
>
> and if the assertion failed, it prints A and B explicitly. This is much
> easier than trying to pick the values out of a register dump, especially as
> the compiler may be free to clobber A or B immediately after testing them.
>
This is great if you'r in a development environment, and can focus on a
single, well
characterized case. Unfortunately, I'm staring at hundreds of thousands
of systems
in the field, all which which have a large number of panic() statements
for which this
approach has not been taken. So, I have no alternative but to pick the
value out of
a register dump.
> David
>
Martin Schwidefsky wrote:
> On Mon, 12 Apr 2010 13:27:45 +0100
> Russell King <[email protected]> wrote:
>
>
>> On Sun, Apr 11, 2010 at 11:06:09PM -0700, David VomLehn wrote:
>>
>>> This patch makes panic() and die() registers available to, for example,
>>> panic notifier functions. Panic notifier functions are quite useful
>>> for recording crash information, but they don't get passed the register
>>> values. This makes it hard to print register contents, do stack
>>> backtraces, etc. The changes in this patch save the register state when
>>> panic() is called and introduce a function for die() to call that allows
>>> it to pass in the registers it was passed.
>>>
>> Can you explain why you want this?
>>
>> I'm wondering about the value of saving the registers; normally when a panic
>> occurs, it's because of a well defined reason, and not because something
>> went wrong in some CPU register; to put it another way, a panic() is a
>> more controlled exception than a BUG() or a bad pointer dereference.
>>
>
> I'm curious about the potential use case as well. So far I only wanted
> to know the registers if the panic has been triggered due to an
> unexpected fault with panic_on_oops=1 or in_interrupt()==1. If that
> happens the die() handler prints the registers. An open coded panic is
> easy to analyze, imho no need for the registers
>
Good example, because helps focus the issue. In recording a subset of
kernel state
information from an embedded system for collection at a central point.
The register
values printed by die() are printed to the console, where they
disappear. One of the
things in this patch involves passed a pointer to those die() registers
to a register
panic notifier handler. So, there is a path to where panic handlers are
called from
die() and another one from panic() and this patch makes register values
available in
both cases.
David VomLehn <[email protected]> wrote:
> > Can the use of va_start() clobber lots of registers, thereby rendering the
> > exercise pointless on some arches?
> >
>
> The implementations I'm familiar with only need one or two registers. What
> it *does* do is to force the contents of registers being used to pass
> argument values onto the stack. This is roughly what gcc does for asm()
> statements when you tell it registers are clobbered.
How about something like Sparc, where you can pass up to 8 arguments (if I
remember correctly) in registers. I'm not sure how Sparc handles varargs
functions, though.
> > Also, can the save_ptregs() function be out of line asm? The FRV
> > constructed inline statement is huge (and wrong).
>
> With this implementation it has to be inline. One use of the saved registers
> is to backtrace the stack.
Indeed, but you've probably already lost that by using va_start().
> If you call a function to save the registers, the stack pointer and program
> counter would be those of the called function, which will not be valid after
> it returns.
Well, they'll be the context of panic() in your implementation.
> I expect that you could come up with an alternative out-of-line function
Yes. The easiest way might be to write the saver in assembly and call it from
within an inline asm statement.
> --on every processor I know, you could backtrace one frame to get reasonable
> values for those registers,. Unfortunately, you would run the risk of
> clobbering other registers by doing the function call. The more you change
> register values from those in the function that calls panic(), the less
> useful this becomes.
Indeed, but the more things panic() does, the more likely it is to clobber
registers anyway.
Note, also: panic() is __attribute__((noreturn)), which means that the
compiler calling it is not required to save the return address or registers
before jumping to panic() or even in panic() itself.
> In this case, I think an inline function is worth the effort to get working.
As I mentioned above, you can use asm for this. For instance, you can write
an inline asm statement that saves onto the stack the registers that need to
be clobbered to make a jump, then make the jump, and then have the saver
routine retrieve the register values from the stack and place them in the
storage area.
In fact, you could insert a prologue wrapper on panic() with a bit of asm to
save the registers, for example on FRV:
panic:
subi sp,#-8,sp
stdi.p gr4,@(sp,#0) # save GR4/GR5 on stack
addi sp,#8,gr5
sethi.p %hi(__panic_reg_save),gr4 # get the save space addr
setlo %lo(__panic_reg_save),gr4
sti gr5,@(gr4,#REG_SP)) # save orig stack pointer
stdi gr2,@(gr4,#REG_GR(2)) # save GR2/GR3
ldi @(sp,#0),gr5
sti gr5,@(gr4,#REG_GR(4)) # save orig GR4
ldi @(sp,#4),gr5
sti gr5,@(gr4,#REG_GR(5)) # save orig GR5
stdi gr6,@(gr4,#REG_GR(6)) # save GR6/GR7
stdi gr8,@(gr4,#REG_GR(8)) # save GR8/GR9
...
lddi.p @(sp,#0),gr4 # restore GR4/GR5 from stack
addi sp,#8,sp
bra real_panic # chain
Then real_panic() would be the original C panic function.
> (I'd be interested in know more details about how things are broken in the
> FRV)
Most load/store instructions come in two types, and you need to modify the
opcode according to the addressing mode and indicate that you're interpolating
a memory dereference argument rather than an address:
asm("ldd%I1 %M1,%0"
: "=e"(counter)
: "m"(v->counter));
You're also trying to load data into GR0 which won't achieve anything. GR0 is
hardwired to 0. It's used as the target of instructions where you don't care
about the calculated result (eg: compare is implemented as subtract to GR0),
and as a source of 0.
David
From: David Howells <[email protected]>
Date: Thu, 15 Apr 2010 00:52:14 +0100
> David VomLehn <[email protected]> wrote:
>
>> > Can the use of va_start() clobber lots of registers, thereby rendering the
>> > exercise pointless on some arches?
>> >
>>
>> The implementations I'm familiar with only need one or two registers. What
>> it *does* do is to force the contents of registers being used to pass
>> argument values onto the stack. This is roughly what gcc does for asm()
>> statements when you tell it registers are clobbered.
>
> How about something like Sparc, where you can pass up to 8 arguments (if I
> remember correctly) in registers. I'm not sure how Sparc handles varargs
> functions, though.
6 arguments, and all arguments get popped onto the stack into the
argument save area when doing varargs so you can access them as an
array.
Stack looks like:
struct register_window window;
unsigned long args[...];
On Sun, Apr 11, 2010 at 11:06:09PM -0700, David VomLehn wrote:
> o A save_ptregs() function for the processor. I've taken a whack at
> doing this for all of the processors. I have tested x86 and MIPS
> versions. I was able to find cross compilers for ARM, ... and the
> code compiles cleanly. Everything else, well, what you see is sheer
> fantasy. You are welcome to chortle with merriment.
Perhaps I'm missing something obvious, but is there some reason why you
can't just reuse the crash_setup_regs() code? MIPS doesn't implement it
presumably because it's lacking kexec crash kernel support, but it would
make sense to make current ptregs saving more generic if there are going
to be multiple users for it.