svn commit: r244112 - head/sys/kern

[Andriy Gapon]

on 13/12/2012 00:48 Alfred Perlstein said the following:
> 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.
> 
> So to put it in your own words, *without* KASSERT you get:
> 
>   The upside: there is no panic! There's **NO** log message (or etc). and chance
> to get more log messages because the insanity propagates.

I disagree with your base premise.  I think that the issue will get reported
anyway.  If the code has gone insane you will get a crash, it just would be much
harder to debug.

> Now let's get to the other points:
> 
> 2) Since this is not the default, then I do not understand why you are so
> concerned.

I am concerned about the code becoming less clear and thus harder to maintain.
I am also concerned about code with INVARIANTS becoming even more slow.

> 3) Can you explain to me why it is so upsetting to you that someone might be
> able to use this functionality?

That is not upsetting to me at all.
'Someone' could develop the code in his branch / repo.  Someone can provide the
code for testing and review to his customers and other interested users.
Someone may evaluate practical usefulness of the code and only then commit it to
the tree.
As far as I can see, no one had a chance yet to see how this code is really
useful in practice.  Yet the code is already in the tree.

> 4) *puts on flame retardant suit* ... Linux has had this for over a decade and
> it's allowed them to find bugs in different ways.  Mind you, in Linux it was the
> default.

Can't comment.
But if you wanted to add some equivalent of WARN_ON, then I would have zero
objection to that.  I want to have a clear distinction between "this can't
happen" and "it's weird that this happened".  KASSERT is not for error checking
or for spotting funny situations.  It's for asserting sanity in the code as its
name implies.

> 5) Adrian and I have both stated that we need this sort of functionality to
> avoid superfluous panics in our work environments while still getting bug feedback.

Good.  Please use this code.

> Can we now please stop arguing over a non-default option that will help some
> vendors report bugs to the project?

Please do use this code, please do report the bugs.
Please postpone putting this code into the tree until its benefits are seen.

-- 
Andriy Gapon