From owner-cvs-all@FreeBSD.ORG Sat Aug 5 20:18:09 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 6AB5016A4DF; Sat, 5 Aug 2006 20:18:09 +0000 (UTC) (envelope-from marcel@xcllnt.net) Received: from ns1.xcllnt.net (209-128-86-226.BAYAREA.NET [209.128.86.226]) by mx1.FreeBSD.org (Postfix) with ESMTP id 1884943D5C; Sat, 5 Aug 2006 20:18:04 +0000 (GMT) (envelope-from marcel@xcllnt.net) Received: from [192.168.5.252] (dhcp52.wlan.xcllnt.net [192.168.5.252]) by ns1.xcllnt.net (8.13.6/8.13.6) with ESMTP id k75KHdhW002634; Sat, 5 Aug 2006 13:17:39 -0700 (PDT) (envelope-from marcel@xcllnt.net) In-Reply-To: <20060805184139.GD29277@comp.chem.msu.su> References: <200608030959.k739x9N6007207@repoman.freebsd.org> <20060804070348.GR97316@comp.chem.msu.su> <6B15CBE0-00E0-4F64-A652-4A4B614975C2@xcllnt.net> <200608041314.24161.jhb@freebsd.org> <20060804200154.GC31805@ns1.xcllnt.net> <20060805184139.GD29277@comp.chem.msu.su> Mime-Version: 1.0 (Apple Message framework v752.2) Content-Type: text/plain; charset=US-ASCII; delsp=yes; format=flowed Message-Id: <76B6A9D8-5BF2-4F5D-A711-C87A87B60696@xcllnt.net> Content-Transfer-Encoding: 7bit From: Marcel Moolenaar Date: Sat, 5 Aug 2006 13:17:26 -0700 To: Yar Tikhiy X-Mailer: Apple Mail (2.752.2) Cc: Sam Leffler , src-committers@freebsd.org, cvs-all@freebsd.org, John Baldwin , cvs-src@freebsd.org 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: Sat, 05 Aug 2006 20:18:09 -0000 On Aug 5, 2006, at 11:41 AM, Yar Tikhiy wrote: > On Fri, Aug 04, 2006 at 01:01:54PM -0700, 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? > > Anyway, they all will require KDB to register with. KDB is not conditional upon the KDB option. KDB is unconditionally present. >> 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... > > Then it would be great to have that backend enabled by default. I can see how some people would want that. >>> 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. > > At least kdb_backtrace() will just return if there is no active > backend. Yes. > The KDB option seems to be used in two distinct cases. One of them > is when a driver or subsystem can offer something to KDB. E.g., > sio(4) can do line break condition. The other case is when a > consumer call to KDB is made, e.g., that to kdb_backtrace(). Now > KDB seems to be able to handle such calls even if they are made > unconditionally. Of course, such calls will just return unless > there is an active backend. Putting such calls under "#ifdef KDB" > seems redundant now. No, not really. There can be a performance impact or a correctness impact of calling into KDB. A line break condition does not result in a '\0' character in the input stream with option KDB and it does when option KDB is not present. This is a change in behaviour and changes in behaviour may have compatibility impacts visible in user- space. > As for the first case, such drivers could call something like > kdb_is_on() to decide if special actions are to be taken. For the serial line break condition you could use a sysctl, but there's also a case to be made that it just opens up security holes. The point is that there are 2 ways we could react to a line break condition and we need some way to tell the kernel how we want to do it. It's for that purpose, among others, that we have the KDB option. > Using a > build-time option to control a link between two kernel parts not > depending on the option looks strange. E.g., the KDB code is always > there, sio(4) is controlled by "device sio", but the ability of the > latter to handle line break condition depends on "options KDB". > Counter-intuitive, eh? I think what might cause the confusion is the name "KDB", not what it does. -- Marcel Moolenaar USPA: A-39004 marcel@xcllnt.net