From owner-cvs-all@FreeBSD.ORG Mon Aug 7 22:21:36 2006 Return-Path: X-Original-To: cvs-all@FreeBSD.org Delivered-To: cvs-all@FreeBSD.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id DAFA616A4E0; Mon, 7 Aug 2006 22:21:35 +0000 (UTC) (envelope-from bde@zeta.org.au) Received: from mailout2.pacific.net.au (mailout2.pacific.net.au [61.8.0.115]) by mx1.FreeBSD.org (Postfix) with ESMTP id BF93C43D46; Mon, 7 Aug 2006 22:21:34 +0000 (GMT) (envelope-from bde@zeta.org.au) Received: from mailproxy1.pacific.net.au (mailproxy1.pacific.net.au [61.8.2.162]) by mailout2.pacific.net.au (Postfix) with ESMTP id A7162109904; Tue, 8 Aug 2006 08:21:33 +1000 (EST) Received: from katana.zip.com.au (katana.zip.com.au [61.8.7.246]) by mailproxy1.pacific.net.au (8.13.4/8.13.4/Debian-3sarge1) with ESMTP id k77MLST2017701; Tue, 8 Aug 2006 08:21:29 +1000 Date: Tue, 8 Aug 2006 08:21:28 +1000 (EST) From: Bruce Evans X-X-Sender: bde@delplex.bde.org To: John Baldwin In-Reply-To: <200608041644.08533.jhb@freebsd.org> Message-ID: <20060808073829.T9516@delplex.bde.org> References: <200608030959.k739x9N6007207@repoman.freebsd.org> <200608041314.24161.jhb@freebsd.org> <20060804200154.GC31805@ns1.xcllnt.net> <200608041644.08533.jhb@freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: src-committers@FreeBSD.org, Yar Tikhiy , cvs-all@FreeBSD.org, Marcel Moolenaar , cvs-src@FreeBSD.org, Sam Leffler Subject: Re: cvs commit: src/sys/net if_vlan.c X-BeenThere: cvs-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 07 Aug 2006 22:21:36 -0000 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