From owner-svn-src-all@FreeBSD.ORG Sun Dec 16 04:36:43 2012 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id B94541D7 for ; Sun, 16 Dec 2012 04:36:43 +0000 (UTC) (envelope-from peter@wemm.org) Received: from mail-vb0-f54.google.com (mail-vb0-f54.google.com [209.85.212.54]) by mx1.freebsd.org (Postfix) with ESMTP id 536F78FC0A for ; Sun, 16 Dec 2012 04:36:43 +0000 (UTC) Received: by mail-vb0-f54.google.com with SMTP id l1so5924318vba.13 for ; Sat, 15 Dec 2012 20:36:42 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=wemm.org; s=google; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=f3g9ZRLCVXopTQlG2P1y0QTs9UYUKnjEErjOJ1/vfv4=; b=kAhmQr7U7lP3O1bbwxS1JWmFcAqTYGIIxDZEu1Y49k23WZ8CmckCdkZXpivUYq4Q2Z cA63WBjRtRQq4HFFmzA/BxRTs0n7IbrlKcE5j8/IkWNWWDBpWUYNCv5gtMC0C3BbAbrt 2hCJ0lt/XqWbHYWf7QWybUF36/FewlZzZIiSY= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:x-gm-message-state; bh=f3g9ZRLCVXopTQlG2P1y0QTs9UYUKnjEErjOJ1/vfv4=; b=Re62fp1181KVR+xz2AUQYUAEOkAYvIO+f88tqH0asWKsOfg3KzF4SlPTU1GyArRTMe yN/XHNea+M5exhxwIzq/5IPFsks8uoVKriKRB347dEDEI1fIG6UaTN/WHb3wud+GhMGW KMxRN9OgOM1JcYpLkPNtuWqPNAvCt/Jj1AOVsib/SAuNMsSHsvjt/l7DF8XQet4P06bq qyvoKGaIq9ZgE4CCieVJJ5pOw8GzoioacggdA3PcfbeluQZVT17YnbYoZedNWUpH1ixF au6VPLHzEiJIJT7mvQtbM1h8+m6TBTka2Pw3MgtRJRkMCBwSzjNe0JzRi9V6xOdqW72K 7DiQ== MIME-Version: 1.0 Received: by 10.52.20.108 with SMTP id m12mr62018vde.11.1355632602618; Sat, 15 Dec 2012 20:36:42 -0800 (PST) Received: by 10.220.38.71 with HTTP; Sat, 15 Dec 2012 20:36:42 -0800 (PST) In-Reply-To: <20121216040717.GG35245@server.rulingia.com> References: <201212110708.qBB78EWx025288@svn.freebsd.org> <201212121046.43706.jhb@freebsd.org> <201212121658.49048.jhb@freebsd.org> <50C90567.8080406@FreeBSD.org> <50C909BD.9090709@mu.org> <50C91B32.4080904@FreeBSD.org> <20121215205202.GF1411@garage.freebsd.pl> <20121216040717.GG35245@server.rulingia.com> Date: Sat, 15 Dec 2012 20:36:42 -0800 Message-ID: Subject: Re: svn commit: r244112 - head/sys/kern From: Peter Wemm To: Peter Jeremy Content-Type: text/plain; charset=ISO-8859-1 X-Gm-Message-State: ALoCoQmhUZ7ZUBeqlqXUQ7TxBXiE5GE1WoZ2zk1aQCHfczs8MQU3PhIlf9XCKyHqi0+YvcFFoCYI Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 16 Dec 2012 04:36:43 -0000 On Sat, Dec 15, 2012 at 8:07 PM, Peter Jeremy wrote: > [pruning the CC list] > On 2012-Dec-15 21:52:03 +0100, Pawel Jakub Dawidek wrote: >>On Wed, Dec 12, 2012 at 04:02:58PM -0800, Navdeep Parhar wrote: >>> A KASSERT() really is for a condition that should never happen. > > and can't be practically recovered from. > >>I have sort of mixed feelings about this change, but in reality we have >>three cases: >> >>1. Fatal conditions that shouldn't happen, but may happen for some >> reason and we definiately want to stop running (corrupted file system >> metadata that can mess up the file system more badly). For those we have >> direct panic(9) calls. >> >>2. Fatal conditions that cannot happen and for those we have KASSERT(9). > > I don't see the difference between these two cases. In both of them, > the system has entered a state that the designer/programmer didn't > envisage and can't recover from. The best option is to abort as > quickly to minimise further corruption. Actually it was a pretty good description of the situation as it existed for the last N years (where N > 10). Back in the early days we had #ifdef DIAGNOSTIC which was a general catch-all bucket to put anything remotely related to diagnostic code. The problem was that it gathered so much debugging code that so much was changed by all the added code that it was usually the case that a DIAGNOSTIC kernel added new problems, if it even booted at all. It had everything from extra functions that existed just for ddb's benefit through extra assertions. There was also the problem that there was no knob to compile in things you could call from ddb to check the state of things as it was all in one lump. That lead to the great DIAGNOSTIC -> INVARIANTS / INVARIANT_SUPPORT split. INVARIANTS was for extra assertions, and INVARIANT_SUPPORT was ddb callable functions and code for INVARIANTS assertions to call. panic remained for fatal conditions, while INVARIANTS (and KASSERT) were introduced to add additional non-invasive sanity checks to detect things that were going off into the weeds before too big a mess was made. The point of KASSERT was that it was something you could turn on during development and/or or while trying to get a look at a problem sooner in DDB before things got too messed up. It was an early warning to get you into ddb faster/sooner. We were not supposed to need it on during production use because it was for the kinds of problems that would be detected later via other means. eg: a panic or a trap. eg: one of the earlier uses was checking priorities before beginning a context switch. If you had an invalid priority state, you could get fairly deep into mi_switch() before a run queue slot was unable to be found. But by the time that happened, the context switch code had changed things a lot before it reached that fatal condition. So we'd add a KASSERT at the start so we could get into DDB before we'd started messing with run queues and had a better chance at seeing how we got into the mess. I must say I'm rather concerned about the direction of recent changes in this area. -- Peter Wemm - peter@wemm.org; peter@FreeBSD.org; peter@yahoo-inc.com; KI6FJV "All of this is for nothing if we don't go to the stars" - JMS/B5 "If Java had true garbage collection, most programs would delete themselves upon execution." -- Robert Sewell