Date: Mon, 9 Feb 1998 07:51:27 +0100 From: Eivind Eklund <eivind@FreeBSD.ORG> To: Michael Hancock <michaelh@cet.co.jp> Cc: FreeBSD Hackers <Hackers@FreeBSD.ORG> Subject: Re: DIAGNOSTICS and DEBUG LOGGING (was Re: cvs commit: src/sys/conf options) Message-ID: <19980209075127.63680@follo.net> In-Reply-To: <Pine.SV4.3.95.980209150818.661A-100000@parkplace.cet.co.jp>; from Michael Hancock on Mon, Feb 09, 1998 at 03:23:45PM %2B0900 References: <199802061241.EAA24833@freefall.freebsd.org> <Pine.SV4.3.95.980209150818.661A-100000@parkplace.cet.co.jp>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Feb 09, 1998 at 03:23:45PM +0900, Michael Hancock wrote:
> Eivind,
>
> I'd like to see "sanity checks" (assertions) and diagnostic logging
> separated. DIAGNOSTICS turns on both, but I'd like to be able to run an
> assertion checking kernel without all the logging.
Absolutely agreed. I was thinking of
_INVARIANTS - Enable invariant/postcondition checking (expensive)
_ASSERTS - Enable precondition and other cheap assertions
INVARIANT_CODE - Compile in invariant functions.
DIAGNOSTIC - Messages to help tracing errors; non-overwhelming amount.
The difference between _INVARIANTS and INVARIANT_CODE is that INVARIANT_CODE
just includes the actual code necessary to be able to check an invariant,
while _INVARIANTS actually throw in the code that do calls to check the
invariant.
The reason for the underscores is that header files are likely to depend on
those options. The separation of the INVARIANT_CODE and _INVARIANTS is to
be able to support enabling invariant-checks in only some files. Separation
of _INVARIANTS and _ASSERTS is that there is often a factor of >1000
difference between the cost of checking pre-conditions and the cost of
checking post-conditions/data-structure invariants.
Example of how the above would affect source-code:
The following would be the diffs to kern/tty_subr.c to implement a
clist invariant. (It is some of the code I've got lying around of
the type nobody-seems-to-be-interested-so-I-won't-commit-it-until-
it-seems-I-can-find-a-nice-way-to-fit-it-in).
Index: tty_subr.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/tty_subr.c,v
retrieving revision 1.28
diff -u -r1.28 tty_subr.c
--- tty_subr.c 1997/10/12 20:24:09 1.28
+++ tty_subr.c 1998/02/09 06:37:34
@@ -112,6 +112,10 @@
cblock_free(cblockp)
struct cblock *cblockp;
{
+#ifdef _ASSERTS
+ if ((unsigned long)cblockp & (CBLOCK-1))
+ panic("Unaligned cblock in cblock_free");
+#endif
if (isset(cblockp->c_quote, CBQSIZE * NBBY - 1))
bzero(cblockp->c_quote, sizeof cblockp->c_quote);
cblockp->c_next = cfreelist;
@@ -136,6 +140,10 @@
"clist_alloc_cblocks: M_NOWAIT malloc failed, trying M_WAITOK\n");
cbp = malloc(sizeof *cbp, M_TTYS, M_WAITOK);
}
+#ifdef _ASSERTS
+ if ((unsigned long)cbp & (CBLOCK-1))
+ panic("Unaligned cblock alloced in cblock_alloc_cblocks");
+#endif
/*
* Freed cblocks have zero quotes and garbage elsewhere.
* Set the may-have-quote bit to force zeroing the quotes.
@@ -260,6 +268,117 @@
return (chr);
}
+#if defined(INVARIANT_CODE) || defined(_INVARIANTS)
+/* Verify the clist invariant. Will panic if passed an invalid clist.
+ Intended for debugging of tty-drivers. */
+void
+clist_invariant(
+ struct clist *clistp, /* clist to verify */
+ char *descr /* Section of program causing panic */
+) {
+ struct cblock *cblockp; /* Presently examined cblock */
+ struct cblock *cblockp2; /* Loop limit cblock */
+ int real_cc; /* Real character count */
+ int max_loop; /* Number of cblocks to check (max) */
+
+ if (descr == NULL)
+ descr = "undescribed";
+
+ if (clistp == NULL)
+ panic("clist invariant: NULL clist in %s", descr);
+
+ /* Check for negative counts */
+ if (clistp->c_cc < 0)
+ panic("clist invariant: negative character count(%d) in %s",
+ clistp->c_cc, descr);
+ if (clistp->c_cbcount < 0)
+ panic("clist invariant: negative block count (%d) in %s",
+ clistp->c_cbcount, descr);
+ if (clistp->c_cbmax < 0)
+ panic("clist invariant: negative max block count (%d) in %s",
+ clistp->c_cbmax, descr);
+ if (clistp->c_cbreserved < 0)
+ panic("clist invariant: negative reserved block count (%d) in %s",
+ clistp->c_cbreserved, descr);
+
+ /* Check for generally invalid counts */
+
+ if (clistp->c_cc && roundup(clistp->c_cc, CBSIZE) / CBSIZE > clistp->c_cbcount)
+ panic("clist invariant: too few cblocks for c_cc (%d vs %d) in %s",
+ clistp->c_cbcount, clistp->c_cc, descr);
+ if (roundup(clistp->c_cc, CBSIZE) / CBSIZE + 2 < clistp->c_cbcount)
+ panic("clist invariant: too many cblocks for c_cc (%d vs %d) in %s",
+ clistp->c_cbcount, clistp->c_cc, descr);
+ if (clistp->c_cbcount > clistp->c_cbmax)
+ panic("clist invariant: more cblocks than cb_max (%d vs %d) in %s",
+ clistp->c_cbcount, clistp->c_cbmax, descr);
+
+ if (clistp->c_cc == 0) {
+ if (clistp->c_cf && clistp->c_cl &&
+ clistp->c_cf != clistp->c_cl)
+ panic("clist invariant: cf (%p) and cl (%p) differ for empty clist in %s",
+ clistp->c_cf, clistp->c_cl, descr);
+ return;
+ }
+ if (clistp->c_cf == NULL)
+ panic("clist invariant: cf is NULL with c_cc=%d (cl=%p) in %s",
+ clistp->c_cc, clistp->c_cl, descr);
+ if (clistp->c_cl == NULL)
+ panic("clist invariant: cl is NULL with c_cc=%d (cf=%p) in %s",
+ clistp->c_cc, clistp->c_cf, descr);
+
+ /* Traverse clist to find actual character count and verify that the
+ last pointer exists */
+ cblockp = (struct cblock *)((unsigned long)clistp->c_cf & ~CROUND);
+ real_cc = (char*)cblockp->c_info - clistp->c_cf;
+ if (real_cc > 0)
+ panic("clist invariant: cf (%p) points outside c_info block in %s",
+ clistp->c_cf, descr);
+
+ /* Limit looping */
+ max_loop = clistp->c_cbcount*2 + 2;
+ cblockp2 = cblockp;
+
+ while (cblockp) {
+ if ((void*)clistp->c_cl > (void*)cblockp &&
+ (char*)clistp->c_cl <= (char*)cblockp + CBLOCK) {
+
+ /* We've found the last block in the list - handle character
+ count and other checks */
+ if (clistp->c_cl < (char*)cblockp->c_info)
+ panic("clist invariant: cl (%p) points outside c_info block in %s",
+ clistp->c_cl);
+ real_cc += clistp->c_cl - (char*)cblockp->c_info;
+ if (real_cc != clistp->c_cc)
+ panic("clist invariant: real cc (%d) does not match c_cc (%d) in %s",
+ real_cc, clistp->c_cc, descr);
+ if (cblockp->c_next)
+ panic("clist invariant: non-NULL c_next (%p) on last cblock in %s",
+ cblockp->c_next, descr);
+ /* Invariant OK */
+ return;
+ }
+
+ /* Increase character count */
+ real_cc += CBSIZE;
+ if (max_loop-- <= 0) {
+ panic("clist invariant: too long ");
+ }
+ cblockp = cblockp->c_next;
+ if (cblockp == cblockp2)
+ panic("clist invariant: loop in clist (%p) in %s",
+ clistp, descr);
+ if ((max_loop & 1) == 0) {
+ cblockp2 = cblockp2->c_next;
+ }
+ if (cblockp == cblockp2)
+ panic("clist invariant: loop in clist (%p) in %s",
+ clistp, descr);
+ }
+ panic("clist invariant: terminated before finding c_cl");
+}
+#endif
+
/*
* Copy 'amount' of chars, beginning at head of clist 'clistp' to
* destination linear buffer 'dest'. Return number of characters
@@ -277,8 +396,15 @@
int numc;
int s;
+#ifdef _ASSERTS
+ if (dest == NULL)
+ panic("destination is NULL in q_to_b");
+#endif
s = spltty();
+#ifdef _INVARIANTS
+ clist_invariant(clistp, "q_to_b");
+#endif
while (clistp && amount && (clistp->c_cc > 0)) {
cblockp = (struct cblock *)((long)clistp->c_cf & ~CROUND);
cblockn = cblockp + 1; /* pointer arithmetic! */
@@ -435,6 +561,10 @@
int startbit, endbit, num_between, numc;
int s;
+#ifdef _ASSERTS
+ if (src == NULL)
+ panic("clist b_to_q on NULL block");
+#endif
/*
* Avoid allocating an initial cblock and then not using it.
* c_cc == 0 must imply c_cbount == 0.
@@ -444,6 +574,9 @@
s = spltty();
+#ifdef _INVARIANTS
+ clist_invariant(clistp, "b_to_q");
+#endif
/*
* If there are no cblocks assigned to this clist yet,
* then get one.
> Do you think that DIAGNOSTICS can be separated into these 2 categories
> without upsetting too many people?
I haven't got a clue. I've been planning to try it, though. Do the above
scheme look good to you?
> What does DEBUG do? Can all sanity checks be moved to DEBUG?
DEBUG does an insane pletoria of different things, all depending on what the
driver-author (or whatever) wanted it to do. It usually turn on insane
amounts of debugging information.
> This would give us more clearly defined debugging flags:
>
> DIAGNOSTICS - Debug logging
> DEBUG - Sanity checks
> DDS - GDB callable debug functions.
I like the distinctions, but not the names :-)
Eivind.
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe hackers" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?19980209075127.63680>