svn commit: r244112 - head/sys/kern

[Navdeep Parhar]

On 12/12/12 16:09, Alfred Perlstein wrote:
> On 12/12/12 4:02 PM, Navdeep Parhar wrote:
>> On 12/12/12 14:48, Alfred Perlstein wrote:
>>> On 12/12/12 2:29 PM, Andriy Gapon wrote:
>>>> Now we get a new middle-ground: get both worse performance (because
>>>> KASSERTs are compiled in) and a risk of harming your data (because
>>>> KASSERTs no longer panic). The upside: there is no panic! There's just
>>>> a log message (or etc). and chance to get more log messages because
>>>> the insanity propagates. And a chance to lose your data (your
>>>> customer's) - but I've already mentioned this. I am not sure that I
>>>> like this kind of middle-ground.
>>> I have a number of points here:
>>>
>>> The most important one being:
>>> 1) without kassert you would still have the bug, just that it would be
>>> unreported.
>>>    The upside: there is no panic! There's **NO** log message (or etc).
>>> and chance to get more log messages because the insanity propagates.
>>>
>>> Terrible!
>>>
>>> Let me explain that again:
>>> If you don't compile in KASSERT, then it's not like the condition is
>>> never going to happen.  Instead it will just be unreported.
>> A KASSERT() really is for a condition that should never happen.  It is
>> primarily useful during development and testing (and when the code is
>> reworked or redesigned).  I agree with Andriy here -- a non-fatal assert
>> shouldn't really exist.
> 
> 
> What do you think happens to a FreeBSD kernel when INVARIANTS is
> compiled in and it trips an assertion after my change?

I know the new knob has sane defaults.  My point was that invariants
should be considered inviolable.  A knob that allows for
it's-really-not-supposed-to-fail-but-in-case-it-does... dilutes their
meaning, so it may have been better to introduce a new macro for the
kind of tests you had in mind.  I would use it too instead of the if
(!foo) kdb_backtrace() that I often resort to for conditions that I'm
not sure about.

Regards,
Navdeep