On Sat, 21 Apr 2018, Jonathan T. Looney wrote:
Log: When running with INVARIANTS, the kernel contains extra checks. However, these assumptions may not hold true once we've panic'd. Therefore, the checks hold less value after a panic. Additionally, if one of the checks fails while we are already panic'd, this creates a double-panic which can interfere with debugging the original panic.Therefore, this commit allows an administrator to suppress a response to KASSERT checks after a panic by setting a tunable/sysctl. The tunable/sysctl (debug.kassert.suppress_in_panic) defaults to being enabled. ... Modified: head/sys/kern/kern_shutdown.c ============================================================================== --- head/sys/kern/kern_shutdown.c Sat Apr 21 15:15:47 2018 (r332859) +++ head/sys/kern/kern_shutdown.c Sat Apr 21 17:05:00 2018 (r332860) @@ -642,6 +642,7 @@ static int kassert_do_log = 1; static int kassert_log_pps_limit = 4; static int kassert_log_mute_at = 0; static int kassert_log_panic_at = 0; +static int kassert_suppress_in_panic = 1; static int kassert_warnings = 0; SYSCTL_NODE(_debug, OID_AUTO, kassert, CTLFLAG_RW, NULL, "kassert options"); @@ -676,6 +677,10 @@ SYSCTL_INT(_debug_kassert, OID_AUTO, log_pps_limit, CT SYSCTL_INT(_debug_kassert, OID_AUTO, log_mute_at, CTLFLAG_RWTUN, &kassert_log_mute_at, 0, "max number of KASSERTS to log"); +SYSCTL_INT(_debug_kassert, OID_AUTO, suppress_in_panic, CTLFLAG_RWTUN, + &kassert_suppress_in_panic, 0, + "KASSERTs will be suppressed while handling a panic"); + static int kassert_sysctl_kassert(SYSCTL_HANDLER_ARGS); SYSCTL_PROC(_debug_kassert, OID_AUTO, kassert, @@ -707,6 +712,10 @@ kassert_panic(const char *fmt, ...) { static char buf[256]; va_list ap; + + /* If we already panic'd, don't create a double-fault. */ + if (panicstr != NULL && kassert_suppress_in_panic) + return; va_start(ap, fmt); (void)vsnprintf(buf, sizeof(buf), fmt, ap);
panic() can't return, but I see that KASSERT() has already been broken to use kassert_panic() which does return in some cases including this new one. KASSERT(9) is still documented to call panic(), and none of the options to break it including this new one, or kassert_panic() itself are documented in KASSERT(9) or in any other section 9 man page. Lots of code was and still is written under the assumption that KASSERT() works as documented. E.g., after a null pointer check using KASSERT(), the pointer is normally used and the page fault for this should cause a panic anyway. This is actually a feature -- it works around KASSERT() breaking restartability of the fault. Other cases are not so good. There man be a cascade of KASSERT() failures, and the new option prevents even printing a message about even one of them. If you are lucky then you get a clean null pointer recursive panic. The only example in KASSERT(9) is to give the worst use of it (for breaking clean null pointer panics when KASSERT() works as documented, but now doesn't even break them when KASSERT() returns). Bruce _______________________________________________ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
