On Wed, May 07, 2008 at 04:49:53PM -0700, David VomLehn wrote:
> This patch contains the kernel stack backtrace code we've been running for
> about a year on our MIPS-based settop box. It is considerably larger than
> the existing backtrace implementation and so is configurable in the Kernel
> Hacking section. It also requires that KALLSYMS be enabled. In return, I
> think it offers some advantages over the existing backtrace code:
>
> o It will backtrace over nested interrupts and exceptions, allowing
> detailed analysis of was going on when it was invoked.
> o It handles a number of corner cases involving instructions in branch
> delay slots.
> o It is very careful to use __get_user when fetching stack data and
> instructions, meaning that it will fail gracefully even in the presence of
> stack corruption.
> o It identifies whether the $sp register or another register is being used
> as the frame pointer. Assuming people are happy with this submission, there
> is a small subsequent patch I'll submit that dumps the frame pointer value
> as part of the backtrace.
> o It segregates the backtrace code into a subdirectory of arch/mips/kernel
> rather than cluttering up traps.c or the kernel directory.
>
> The main reason I am submitting this as a request for comments rather than
> as a normal patch is that, though I wrote it with 64-bit systems in mind, I
> don't have access to a 64-bit system on which to test it. I am happy to
> merge any 64-bit-specific changes. For 32-bit systems, I'll claim it's
> ready to go.
>
> The other reason is that this is an RFC is that it is such a large, single
> chunk of code that there are certainly lots of comments that people will
> have. I'm not naive enough to think it's really ready without more review.
>
> [Note: for anyone who was at the MIPS backtrace session at the CELF
> Conference, this is the code I was talking about.]
I wasn't even aware of the session.
> Signed-off-by: David VomLehn <dvomlehn@cisco.com>
> +#ifndef numberof
> +#define numberof(a) (sizeof(a) / sizeof((a) [0]))
> +#endif
I really just glanced over the patch. I think I like the promise of more
accurate backtraces because frequently one (1) backtrace is all information
which there ever will be available for debugging.
o Use ARRAY_SIZE() from <linux/kernel.h> instead of numberof.
o Please run the patch through scripts/checkpatch.pl and fix the resulting
errors.
o Your patch got linewrapped by your mail client.
Ralf
|