Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 12 Dec 2012 16:41:45 -0800
From:      Adrian Chadd <adrian@freebsd.org>
To:        Navdeep Parhar <np@freebsd.org>
Cc:        Alfred Perlstein <alfred@freebsd.org>, John Baldwin <jhb@freebsd.org>, svn-src-all@freebsd.org, Alfred Perlstein <bright@mu.org>, Andriy Gapon <avg@freebsd.org>, src-committers@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r244112 - head/sys/kern
Message-ID:  <CAJ-VmomAj9Yu=zrF2uKnAUSt4DDxNpNuyCUUE%2Bg8j4E3=WhP=g@mail.gmail.com>
In-Reply-To: <50C9206D.6080502@FreeBSD.org>
References:  <201212110708.qBB78EWx025288@svn.freebsd.org> <201212121046.43706.jhb@freebsd.org> <CAJ-Vmo=U04GX%2BZyKuzXLwV%2BPpzU6_dm5BCmL=DWfsmhTVAR%2BsA@mail.gmail.com> <201212121658.49048.jhb@freebsd.org> <50C90567.8080406@FreeBSD.org> <50C909BD.9090709@mu.org> <50C91B32.4080904@FreeBSD.org> <50C91CD3.7030900@mu.org> <50C9206D.6080502@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
If a kassert is inviolable, then make it a panic() and include in the
default kernel.

Right now there's no distinction between "kassert for a condition that
shouldn't happen, but we fail gracefully" and "kassert for a condition
that shouldn't happen, and we don't handle at all."

Ideally we'd create two separate macros to express that, t.hen we
should make all kassert's that don't fail gracefully to fail
gracefully.
Anything left over should've been a panic() in the first place.

Right now, with no INVARIANTS compiled in, we don't check KASSERT().
Again, the difference between panic and KASSERT.

TL;DR - we're doing panic/assert sub-optimally; we don't have a
consistent definition of what an invariant check is; Alfred's work
doesn't change that.




Adrian

On 12 December 2012 16:25, Navdeep Parhar <np@freebsd.org> wrote:
> 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



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJ-VmomAj9Yu=zrF2uKnAUSt4DDxNpNuyCUUE%2Bg8j4E3=WhP=g>