cvs commit: src/sys/net if_vlan.c

[Marcel Moolenaar]

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 at xcllnt.net