From owner-freebsd-arch@FreeBSD.ORG Thu Sep 25 04:19:14 2014 Return-Path: Delivered-To: arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id E5784F36; Thu, 25 Sep 2014 04:19:14 +0000 (UTC) Received: from mail-ie0-x234.google.com (mail-ie0-x234.google.com [IPv6:2607:f8b0:4001:c03::234]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id A2017B4E; Thu, 25 Sep 2014 04:19:14 +0000 (UTC) Received: by mail-ie0-f180.google.com with SMTP id ar1so10075927iec.11 for ; Wed, 24 Sep 2014 21:19:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=0Vh2DDjl2iMQdWZteIpnq110lalXz1ave0JXH319eoM=; b=irGZgCriYvltd/wAMkIiTh0AzuuJ+8DHsJxq7ZVMD7lYAF0WqLZ7Hyxnp9RRkImUX4 K5udphiaxzwl22TW6p27EG1zsFEweIEECVcApMtK6xOjBYbvQ1NRRLqloGMOv+rGnz1a VGLUnBuAb3yGdYA15R05MeImHYni568VqZx0wZ8tf57z/R5FDBdDjuA/o1fYlMDK5thc 6O18H6O4HNHwze4/NKl2S6HWpOiefZorGDgKNlvgVb505SGCJSEUs8ehqG6pQud6M4lK HlWANCmGkjRrm0Ir4NYxh3Jmn0FZA2iPEKV6L69RvgBlxICXj1njic7zbkTBiTyX/PDR T/Lw== MIME-Version: 1.0 X-Received: by 10.50.32.10 with SMTP id e10mr18785391igi.7.1411618754120; Wed, 24 Sep 2014 21:19:14 -0700 (PDT) Received: by 10.50.72.69 with HTTP; Wed, 24 Sep 2014 21:19:14 -0700 (PDT) In-Reply-To: References: <54236CD6.4050807@FreeBSD.org> Date: Wed, 24 Sep 2014 21:19:14 -0700 Message-ID: Subject: Re: KASSERT_WARN for asserting malloc(M_WAITOK) not in a non-sleepable thread From: NGie Cooper To: Davide Italiano Content-Type: text/plain; charset=UTF-8 Cc: "freebsd-arch@freebsd.org" , Bryan Drewery X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 25 Sep 2014 04:19:15 -0000 On Wed, Sep 24, 2014 at 7:56 PM, Davide Italiano wrote: > On Wed, Sep 24, 2014 at 6:16 PM, Bryan Drewery wrote: >> Hi, >> >> I've placed 2 reviews out in relation to >> https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=193696: >> >> Add KASSERT_WARN which will work just like KASSERT except that no panic >> will occur. My own expectation would be that any use of it would >> eventually be promoted to a full KASSERT. It would only be used where >> the impact is not known yet on all hardware/devices. We don't want to >> go adding a KASSERT and break boot for a whole class of systems. >> >> https://reviews.freebsd.org/D829 - KASSERT_WARN > > FYI, I'm not excited about the idea. If you introduce an assert you > want some invariant to not be violated. If it's violated, there's > something clearly going wrong and you need to stop and think about it. > I guess that in most cases is just better fail early, rather than keep > going with the system in a semi-functional state. Also, please note > that once a KPI is introduced in the kernel, everybody may start > abusing it. > A previous attempt (in my opinion wrong) was made to have KASSERT to > log rather than panic. It actually didn't lead to any benefit, > apparently. FWIW, at least your approach is more fine grained. The probability of hitting bug 193696 is unknown but the potential impact is great, so until most of the code paths are fixed and/or we have enough data to quantify the impact (and the data suggests that the probability is in fact low), it would be unadvisable to replace the KASSERT_WARN Bryan's introducing with a KASSERT. Eventually it should probably turn into a KASSERT. I agree that developers should use KASSERT_WARN sparingly and carefully. Maybe a comment should be added to note that? Thanks! -Garrett