Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 18 Dec 2012 15:23:05 -0500
From:      John Baldwin <jhb@freebsd.org>
To:        Adrian Chadd <adrian@freebsd.org>
Cc:        Ian Lepore <freebsd@damnhippie.dyndns.org>, src-committers@freebsd.org, Peter Wemm <peter@wemm.org>, svn-src-all@freebsd.org, Andriy Gapon <avg@freebsd.org>, svn-src-head@freebsd.org, Peter Jeremy <peter@rulingia.com>
Subject:   Re: svn commit: r244112 - head/sys/kern
Message-ID:  <201212181523.05713.jhb@freebsd.org>
In-Reply-To: <CAJ-Vmo=XPXAAV90ZNqbky8kBfxGdQ6iB6GX4YQ_oQayeKiNWFQ@mail.gmail.com>
References:  <201212121658.49048.jhb@freebsd.org> <20121216233213.GA1451@itx> <CAJ-Vmo=XPXAAV90ZNqbky8kBfxGdQ6iB6GX4YQ_oQayeKiNWFQ@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Sunday, December 16, 2012 10:05:48 pm Adrian Chadd wrote:
> On 16 December 2012 15:32, Navdeep Parhar <nparhar@gmail.com> wrote:
> 
> 
> >> The status quo _does not change_ by default.
> >
> > So now we have a knob that could be used to change the behaviour of all
> > the KASSERTs in the system; one that hints that it may be possible to
> > continue even if an assertion in the FreeBSD kernel doesn't hold good
> > (this is the part that bothers me).  I know all the KASSERTs I've looked
> > at or written are genuine assertions -- the code simply wouldn't be able
> > to cope if they were violated.  You'd get NULL dereferences, or worse,
> > access protected structures without corresponding locks held, etc.
> 
> In that case, those failures should be handled gracefully, or they
> should immediately panic the kernel.
> 
> Claiming that a KASSERT() is optional at this point is basically us as
> a project saying "We know that if the kernel gets to this point and it
> fails this check, everything is busted after this." Ie, "Hey, if you
> disable KASSERT(), your data is potentially toast."
> 
> Yet we ship with KASSERT() disabled. Silent data corruption, race
> conditions, etc. Not everything leads to a NULL pointer dereference.
> 
> Again, we ship with KASSERT disabled in GENERIC on shipping production
> releases. The concerns you have with KASSERT printing out when
> Alfred's modification is enabled -does not change the fact that the
> kernel does _EXACTLY THIS_ kind of "oh well, I'll keep going"
> behaviour in a GENERIC, production, release kernel-.

It seems to me that if you are willing to pay the cost of extra sanity checks, 
you should gain the extra protection against data corruption.  We have made 
the design decision to trade the extra data protection for increased 
performance in GENERIC since we feel the cases of data corruption should be 
rare.  In HEAD we have altered the tradeoff since code in HEAD has had less 
testing and is thus assumed to be more risky.  In your case you seem to be 
trading performance for reduced data protection.  Can you at least appreciate 
that argument?

Also, to Alfred's point that "we have spare CPU lying around".  Performance
is not only measured in throughput, it can also be measured in latency.

One other note: have you noticed that there is not a similar "whine but don't 
core" option for the userland assert()?  OTOH, there are such things in glib 
and the core KDE libraries.  Try firing up a gnome or KDE application in a 
terminal window.  You will be greeted by an endless stream of various debug 
messages.  The noise is so great that I haven't bothered to report any of them 
as bugs.  OTOH, I do open bugs if a KDE app cores.  It would not surprise me 
to find that enabling KASSERTS but having them only log doesn't actually help 
with anything as the extra logging could prove so noisy that no one bothers 
reporting them (and/or you end up with spurious noise due to secondary 
failures similar to the noise one can get with compiler warnings).

-- 
John Baldwin



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201212181523.05713.jhb>