Re: automatic stacktrace on panic, was: Re: Boot fail as kvm guest

[Elad Efrat <elad%NetBSD.org@localhost>]

On 2012-02-18 10:04 AM, Manuel Bouyer wrote:
> On Sat, Feb 18, 2012 at 06:28:25PM +0400, Aleksej Saushev wrote:
> > Manuel Bouyer<bouyer%antioche.eu.org@localhost>  writes:
> >
> > > As for automatically outputting stack trace on panic:
> > > this can be a long output, much longer than a .e.g VGA display.
> > > This means that the user has only the tail of the stack trace and
> > > doesn't see the panic message, which, actually, is the most important.
> >
> > The output should go in the opposite direction then: top frames should
> > be printed last.
>
> that's not enough: you still miss the panic message.
> Also, if ddb has problems while decoding the stack frame, you loose
> all important informations. This situation is not uncommon, when
> things start to go wrong ...
>
> I think a few lines explaining basic ddb commands would be much better

You know, there is a very worrying theme going on in several threads on
the mailing lists right now, which is basically making life a living
hell for anyone who has bothered to install NetBSD or wants to
contribute.

Ignoring the issue of the wiki, which has brought the community full
circle back to square one, do you really think the best user interface
for a kernel panic is to provide a ddb field guide? I have been using
NetBSD for over a decade and I still have no idea how to get pkgsrc
working, and you want to ask people who just want to use NetBSD to
learn about the kernel debugger?

Attached is a very simple diff that adds a ddb variable called
simple_stacktrace. When it's 1, it prints a condensed form of the
trace with just function names and arrows indicating the call sequence.
I.e., func1 <- func2 <- func3. It assumes there's a right side to the
screen as well and uses it to save space. Here's a picture comparing
the sizes of a standard stack trace and a simple stack trace:

    http://netbsd.org/~elad/simple_stacktrace.jpg

Having this enabled by default should make all panic reports be easily
accompanied by a stack trace that gives you a hint as to what happened.
And of course, if it's not enough, you can always revert to exactly what
we do today and say "could you get a [standard] trace?"

Just an idea.

Elad

Index: arch/x86/x86/db_trace.c
===================================================================
RCS file: /cvsroot/src/sys/arch/x86/x86/db_trace.c,v
retrieving revision 1.3
diff -u -p -r1.3 db_trace.c
--- arch/x86/x86/db_trace.c     21 Apr 2011 00:24:07 -0000      1.3
+++ arch/x86/x86/db_trace.c     18 Feb 2012 05:30:18 -0000
@@ -50,6 +50,7 @@ __KERNEL_RCSID(0, "$NetBSD: db_trace.c,v
 #include <ddb/db_user.h>
 #include <ddb/db_proc.h>
 #include <ddb/db_command.h>
+#include <ddb/ddbvar.h>
 #include <x86/db_machdep.h>
 
 int
@@ -232,6 +233,11 @@ db_stack_trace_print(db_expr_t addr, boo
                                narg = db_numargs(frame);
                }
 
+               if (db_simple_stacktrace) {
+                       (*pr)("%s%s", name, lastframe == 0 ? "\n" : " <- ");
+                       goto move_on;
+               }
+
                (*pr)("%s(", name);
 
                if (lastframe == 0 && offset == 0 && !have_addr) {
@@ -258,6 +264,7 @@ db_stack_trace_print(db_expr_t addr, boo
                db_printsym(callpc, DB_STGY_PROC, pr);
                (*pr)("\n");
 
+ move_on:
                if (lastframe == 0 && offset == 0 && !have_addr) {
                        /* Frame really belongs to next callpc */
                        struct x86_frame *fp = (void *)
Index: ddb/db_variables.c
===================================================================
RCS file: /cvsroot/src/sys/ddb/db_variables.c,v
retrieving revision 1.42
diff -u -p -r1.42 db_variables.c
--- ddb/db_variables.c  11 Mar 2009 23:22:57 -0000      1.42
+++ ddb/db_variables.c  18 Feb 2012 05:30:18 -0000
@@ -66,6 +66,13 @@ int          db_fromconsole = DDB_FROMCONSOLE;
 #endif
 int            db_tee_msgbuf = DDB_TEE_MSGBUF;
 
+/*
+ * Show a simple, condensed version of the stack trace?
+ */
+#ifndef DDB_SIMPLE_STACKTRACE
+#define DDB_SIMPLE_STACKTRACE 1
+#endif
+int            db_simple_stacktrace = DDB_SIMPLE_STACKTRACE;
 
 static int     db_rw_internal_variable(const struct db_variable *, db_expr_t *,
                    int);
@@ -81,6 +88,7 @@ const struct db_variable db_vars[] = {
        { "onpanic",    (void *)&db_onpanic,    db_rw_internal_variable, NULL },
        { "fromconsole", (void *)&db_fromconsole, db_rw_internal_variable, NULL 
},
        { "tee_msgbuf", (void *)&db_tee_msgbuf, db_rw_internal_variable, NULL },
+       { "simple_stacktrace",  (void *)&db_simple_stacktrace,  
db_rw_internal_variable, NULL },
 };
 const struct db_variable * const db_evars = db_vars + 
sizeof(db_vars)/sizeof(db_vars[0]);
 
@@ -167,6 +175,13 @@ SYSCTL_SETUP(sysctl_ddb_setup, "sysctl d
                       SYSCTL_DESCR("Command to be executed on each ddb enter"),
                       NULL, 0, &db_cmd_on_enter, DB_LINE_MAXLEN,
                       CTL_DDB, CTL_CREATE, CTL_EOL);
+       sysctl_createv(clog, 0, NULL, NULL,
+                      CTLFLAG_PERMANENT|CTLFLAG_READWRITE,
+                      CTLTYPE_INT, "simple_stacktrace",
+                      SYSCTL_DESCR("Whether to print a simple, condensed "
+                                   "version of the stack trace"),
+                      NULL, 0, &db_simple_stacktrace, 0,
+                      CTL_DDB, CTL_CREATE, CTL_EOL);
 }
 #endif /* _KERNEL */
 
Index: ddb/ddbvar.h
===================================================================
RCS file: /cvsroot/src/sys/ddb/ddbvar.h,v
retrieving revision 1.9
diff -u -p -r1.9 ddbvar.h
--- ddb/ddbvar.h        28 Apr 2008 20:23:46 -0000      1.9
+++ ddb/ddbvar.h        18 Feb 2012 05:30:18 -0000
@@ -39,6 +39,7 @@
 extern int db_onpanic;
 extern int db_fromconsole;
 extern int db_tee_msgbuf;
+extern int db_simple_stacktrace;
 
 int read_symtab_from_file(struct proc *,struct vnode *,const char *);
 #endif /* !_DDBVAR_H_ */