Sorry for the delay here; I got swamped yesterday and much of today.
I think this has exceeded the reasonable scope for a source-changes
discussion, and so I'm moving it to tech-kern (source-changes is in the
envelope).

To bring folks up to speed, James committed a change recently
to no longer make PCMCIADEBUG the default, and so it must
be specified explicitly in kernels that want it.

I objected ;-)

As I understand it, James argument is essentially:

  a) Philosophically, debugging options should not be on by default.

  b) Compiling in debugging code causes users to pay a performance
  penalty.

  c) Having PCMCIADEBUG on by default is not intuitively obvious.

My argument has been essentially:

  a) PCMCIADEBUG provides some functions that is invaluable to be able
  to enable, especially as part of debugging initial installations
  (where you would patch pcmciadebug's value with ddb), and where
  rebuilding a kernel is not easy.

James Chacon <jchacon@genuity.net> wrote on Wed, 16 May 2001
at 01:57:19 -0400 in <200105160557.BAA28891@dragon.tools.gtei.net>:

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

There is no performance penalty associated with PCMCIADEBUG. It causes
some additional printfs in pcmcia_function_enable(), and some
DPRINTFs in pcmcia_card_detach(), pcmcia_card_deactivate(),
pcmcia_function_enable(), pcmcia_function_disable(), and
pcmcia_intr_establish(). None of these are functions that
are executed frequently or repeatedly. They basically happen
O(1) times.

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

I argue that not every case is the same. There is value associated with
having debugging code in the default kernel, and there is value associated
with improving performance by not doing so. You have to look at the debugging
code in order to decide what the trade-off is. In this case,
I think it's pretty clear. There's basically no performance penalty,
and the value is significant.

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

This is not correct. "DEBUG" implies expensive debugging checks.
*DEBUG does not. That is a misimpression.

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

Please go look at the DPRINTFs. They are negligable.

If they were not, then it might be reasonable to try to see if we could
find a balance point between useful debugging for users and for
debugging for developers.

Of course, PCMCIAVERBOSE is one such thing. I think at times earlier
in this thread I had conflated PCMCIADEBUG and PCMCIAVERBOSE. Nonetheless,
I think the PCMCIADEBUG information is useful enough that it should be
in the default kernel, given the lack of penalty.

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

You can't simply say that because someone COULD sprinkle a lot of DPRINTFs
in, then we can't have debugging on by default. The fact is that there
are not a lot of DPRINTFs here, and they are not in timing-critical
sections of code.

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

I find myself debugging pcmcia problems on machines where I'm installing
NetBSD for the first time, and I'm afraid this seems unlikely to go away.

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

No, it is not zero gain. There are plenty of really expensive things
that happen with DEBUG that slow down performance in non-negligable ways.
That is not the case with all *DEBUGs. It may be the case for some of them,
but they are each individual cases. There is a very clear gain associated
with differentiating.

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

You have not offered compelling arguments. The arguments that I see
seem to be easily debunked (above).

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

No. You have to strike a balance between debugging that is useful in a wide
array of cases (especially at install time), and performance penalties.
In this case, I think it is quite clear that there is no performance penalty.

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

Please look at the code.

--jhawk