cvs commit: src/sys/net if_vlan.c

[John Baldwin]

On Friday 04 August 2006 16:01, Marcel Moolenaar wrote:
> On Fri, Aug 04, 2006 at 01:14:22PM -0400, John Baldwin wrote:
> > > 
> > > So, putting the kdb_backtrace() under KDB is not a matter of said
> > > function not being present without KDB, it's that we don't want
> > > to emit backtraces when debugging is not enabled. Backtraces are
> > > a debugging tool and it makes sense to emit them only when the
> > > kernel is configured for debugging.
> > 
> > In practice this ends up being redundant though as to have kdb_backtrace() 
> > actually do anything you have to have DDB in your kernel config, which 
> > requires KDB.
> 
> That's really an implementation detail. What if we get a new debugger
> backend that allows backtraces? What if the GDB backend is extended to
> allow backtraces?

In that case the user has specifically requested debugger support in their 
kernel (whether it be DDB, GDB, or FOODB).

> 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. :)

> > 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.
> 
> 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.

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.

-- 
John Baldwin