svn commit: r244112 - head/sys/kern

[John Baldwin]

On Thursday, December 13, 2012 4:02:15 am Gleb Smirnoff wrote:
> On Wed, Dec 12, 2012 at 04:53:48PM -0800, Alfred Perlstein wrote:
> A> The problem again is that not all the KASSERTS are inviolable, if you 
> A> want to do a project to split them, then please do, it would really be 
> A> helpful, as for now, they are a mis-mash of death/warnings and there are 
> A> at least three vendors who approve of this as well as 3 long term 
> A> committers that approved my change (not including Adrian).
> 
> Can you show examples of not inviolable KASSERTs?

There are none.  They are all assertions for a reason.  However, in my 
experience at several large consumers of FreeBSD, no one wants to run with 
INVARIANTS in production.  Not because we don't want panics (believe me, 
Yahoo! gets plenty of panics even with INVARIANTS disabled), but because the 
performance overhead, and redefining INVARIANTS into printf doesn't address 
that at all.

Also, in regards to "if you think an a condition should be inviolable, make it 
a panic".  I _did_ do that in WITNESS and you just reverted them!  In all the 
cases of things like mismatching slock -> xlock you are about to corrupt 
WITNESS' internal state (leading to false positives or missed warnings) as 
well as the state of the locks themselves resulting in either hangs or random 
data corruption.

Also, if you don't have a console wired up on all your machines (which not 
everyone does these days) a hang is _far_ worse than a crashdump, as when the 
machine hangs, you have to power cycle it, and you won't find the messages in 
/var/log/messages.  With a straight-up panic if someone wants to run with 
INVARIANTS enabled they would instead have a nice crashdump that could be 
examined after the machine comes back up that points to the offending 
location.

So in general, I will never use this and find it doesn't add any benefit 
whatsoever.  OTOH, if it is not invasive (e.g. your original commit), then I 
think it might be ok if there are some folks who might actually find it 
useful.  That said, I think direct use of kassert_panic() such as your changes 
to WITNESS is wrong.  If you are going to change explicit panics, make them 
into a KASSERT() instead of changing a panic into a kassert_panic().

-- 
John Baldwin