Subject: Re: CVS commit: syssrc
To: John Hawkinson <jhawk@MIT.EDU>
From: None <jchacon@genuity.net>
List: source-changes
Date: 05/16/2001 01:57:19
>
>| All other DEBUG options are set via 
>| 
>| options <foo>DEBUG
>
>That's not true. in the same dir, RAY_DEBUG isn't like that.
>In dev/ic, AIC_DEBUG, HMEDEBUG, PCIDEBUG, WDCDEBUG, are all
>on by default. I could go on.

Read through those. They are a small handfull overall and in the AIC_DEBUG
case it's not even wrapping DPRINTF's but just enabling some additional 
functions.

The rest should be fixed. Most users are not developers. They shouldn't be
paying a  performance penalty and have zero choice about compiling
in debug code for a standard production build.

If you want debug code compiled in, set the options. The fact remains that
a fair majority of the ones I've looked at recently do it in this manner and
it makes the most sense since then you only pull into your build what you
expect to pull in.

>
>| It'll bail out with a redefinition error. That's 
>| 
>| 1. Non obvious as hell
>
>The *DEBUG interface isn't documented, and I don't think we
>have to stick to it. It might be nice to, but that would
>be a good reason to wrap the #define with a #ifndef, not
>a good reason to do as you have done.

The *DEBUG interface may not be, but I'm also looking at it from a derivation
of DEBUG itself and most commonly used practices. Anyone looking up DEBUG is
going to see:

options         DEBUG           # expensive debugging checks/support

and notice it's not turned on by default which implies of course that it's
extra overhead. The same applies to the DPRINTF/*DEBUG interface. It's extra
overhead that shouldn't be turned on by default unless requested by the
kernel builder.

>
>| 2. Completely opposite of how every other standard debug #define gets set
>
>I think there's more variety than you're aware of.

Fine, but a quick perusal shows a lot of them doing this and it's much much
easier to deal with if it's done this way rather than force everuone who
uses pcmcia to get debug code compiled into their kernels. 

Every DPRINTF is now real code that the end user has zero choice about
compiling in which causes an extra check as those are run across. And people
wonder why performance keeps going down. This certainly isn't a large factor
but people consistantly throwing the kitchen sink in by default doesn't help.

>
>| This should be setup the same as any other debug arguments. If you want to
>| have debug turned on for something, set it via an option in the conf file.
>| Otherwise the extra code shouldn't even be generated.
>
>Again, this is debatable. If the extra code doesn't incur a performacne
>penalty, then there's no compelling reason why it must not be present,
>especially when it can be very useful to have in all kernels.

It does incur a penaly. Simply checking against the debug var on every
DPRINTF vs. no code adds approx 4-5 instructions or so on an x86, including
a branch. Sprinkle enough DPRINTF's in (especially in something like an
interrupt routine) and it really starts to impact.


>
>| Past that I agree with being able to set the check var via ddb.
>
>Parse failure.

Setting pcmcia_debug via ddb on the fly. I use that quite often myself 
depending, but I also only do this when I expect to be debugging something or
have setup a kernel I want to possibly debug on. With a production system, the
last thing I want is extra overheard.

>
>| But this all goes back to DEBUG not getting set in GENERIC builds
>| which this violates the premise of being able to control.
>
>No. "DEBUG" doesn't mean "*DEBUG".

That's getting pedantic for zero real gain. See above. This directly incurs
performance hits (albeit small, but they are there).

>
>In summary, I don't see a compelling reason that you're offering why
>PCMCIADEBUG must be off by default, and you have just removed a valuable
>debugging feature from the operating system, and I object to that.
>
>Response?

options PCMCIADEBUG

That's not tough. Otherwise with your arguments I should have *DEBUG turned
on in every file and live with the performance hits I'll take from those.
That's illogical unless you're strictly a kernel developer who wants to worry
about debugging on the fly. Considering a vast majority of the users of NetBSD
are not kernel hackers making them incur this penalty so you don't have
to set an option seems fairly ridiculous and a horrible precedent to be
setting.

James