Date: Tue, 8 Aug 2006 08:21:28 +1000 (EST) From: Bruce Evans <bde@zeta.org.au> To: John Baldwin <jhb@FreeBSD.org> Cc: src-committers@FreeBSD.org, Yar Tikhiy <yar@comp.chem.msu.su>, cvs-all@FreeBSD.org, Marcel Moolenaar <marcel@xcllnt.net>, cvs-src@FreeBSD.org, Sam Leffler <sam@errno.com> Subject: Re: cvs commit: src/sys/net if_vlan.c Message-ID: <20060808073829.T9516@delplex.bde.org> In-Reply-To: <200608041644.08533.jhb@freebsd.org> References: <200608030959.k739x9N6007207@repoman.freebsd.org> <200608041314.24161.jhb@freebsd.org> <20060804200154.GC31805@ns1.xcllnt.net> <200608041644.08533.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 4 Aug 2006, John Baldwin wrote:
> On Friday 04 August 2006 16:01, Marcel Moolenaar wrote:
>
>> The point is that kdb_backtrace() is there if you want a backtrace and
>> you call it based on whatever option that makes sense at the call-site
>> or even unconditionally if that's the right thing.
>> Whether there's actually a backend that can make a backtrace is really
>> a seperate issue. We just happen to implement backtracing and unwinding
>> by debuggers, but with an unwinder in the kernel on ia64, we really
>> don't need a debugger in order to make a backtrace and it's not that
>> unrealistic that I create a backend that can only do backtraces...
>
> To be honest, as someone who works with bug reports, I'd actually like
> backtraces up front w/o requiring the user to compile a custom kernel, etc.
> Having a simple backend in place and kdb_backtrace()'s where relevant would
> be very handy. :)
This was non-broken in the original implementation of backtrace():
% RCS file: /home/ncvs/src/sys/kern/kern_shutdown.c,v
% Working file: kern_shutdown.c
% ...
% ----------------------------
% revision 1.138
% date: 2003/01/04 20:54:58; author: phk; state: Exp; lines: +14 -0
% Introduce the
% void backtrace(void);
% function which will print a backtrace if DDB is in the kernel and an
% explanation if not.
%
% This is useful for recording backtraces in non-fatal circumstances and
% does not require pollution with DDB #includes in the files where it
% is used.
%
% It would of course be nice to have a non-DDB dependent version too,
% but since the meat of a backtrace is MD it is probably not worth it.
% ----------------------------
Debugger() has been broken similarly. It was designed to be called
independently of the configuration of a debugger, and print a message
if it is called when no debugger is configured (not quite right since
Debugger() was often used as a quick replacement for panic()). Now
Debugger() is misspelled kdb_enter() and is silent if there is no
low-level debugger to enter.
>>> Places that call kdb_enter() aren't all #ifdef KDB IIRC. It's
>>> just a feature that kdb_foo() functions become NOPs when the kernel isn't
>>> configured for debugging, so I think the #ifdef KDB's would be redundant.
Just a bug, for kdb_enter() at least.
>> None of the kdb_*() functions in src/sys/kern/subr_kdb.c turn into
>> NOPs when option KDB is not present. They are all unconditionally
>> functional by design and should therefore be called conditionally
>> by consequence.
Some of them need to at least print a message if they are called when
no low-level debugger is present, since most calls in that case are
errors -- debugger functions shouldn't be called in production kernels.
Panicing would be too much for most calls.
> Well, given that separation, I'm not sure KDB is the right option to make
> calls conditional. Rather, some specific is-debugging-enabled? option (like
> INARIANTS or FOO_DEBUG) should be used instead. i.e.:
>
> #ifdef FOO_DEBUG
> if (foo_bad) {
> printf("foo is bad\n");
> kdb_backtrace();
> }
> #endif
>
> I don't think that warrants an extra #ifdef KDB.
Yes, most calls that belong in production kernels are near kern_shutdown.c
and already have control variables. For backtrace in panics, it is harmless
for kdb to print a message saying that backtrace is unavailable. For calls
from other places, where you want some debugging info but don't want to
enter a debugger even if one is available, it should be possible to just
call backtrace() and then a message that backtrace is unavailable is good
as a reminder that the call should be conditional in production kernels.
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20060808073829.T9516>