From owner-freebsd-arch@FreeBSD.ORG Thu Sep 25 16:15:01 2014 Return-Path: Delivered-To: freebsd-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 396D95A2; Thu, 25 Sep 2014 16:15:01 +0000 (UTC) Received: from mail-wg0-x22e.google.com (mail-wg0-x22e.google.com [IPv6:2a00:1450:400c:c00::22e]) (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 A2E199C3; Thu, 25 Sep 2014 16:15:00 +0000 (UTC) Received: by mail-wg0-f46.google.com with SMTP id a1so7818713wgh.5 for ; Thu, 25 Sep 2014 09:14:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=ZnxAR8YQYefbeAInVbmUkWd8j2IfbpYszgJQLmkOprQ=; b=Ett4t7Pyy5d/O99L06Wdrm4RHE8vUcMmWLOm9TUEhtPvF65yP3qOGvigXEd24jYaYN m7GPqfv1GejsEGjt/mv85BpDKLz8ELEin0zT1TTVKmql4vGbMn37x3IHPKggHxl3y1a8 ZUnCIV0TKGnUCskx4j/D+o6jyz3BIZzq5EC92TshEK8Smy5M9mCrcjv9dJhP7IhTDVtP qP71NCtfLCv8ZbXeM8YzstgEcE+MaiIAJ6GFeDKK0hajpaXNBlpl0XAZ2fkro9aSw4Xw rNGvexSXDesjTK1FmqQcbtsF9eNrASsO4v3cumf37TJ6PZag9gzYbTkNv/i/zz7Btfav cU2Q== MIME-Version: 1.0 X-Received: by 10.180.9.73 with SMTP id x9mr20052765wia.20.1411661698802; Thu, 25 Sep 2014 09:14:58 -0700 (PDT) Sender: adrian.chadd@gmail.com Received: by 10.216.106.136 with HTTP; Thu, 25 Sep 2014 09:14:58 -0700 (PDT) In-Reply-To: <5424392D.9030201@FreeBSD.org> References: <54236CD6.4050807@FreeBSD.org> <5424392D.9030201@FreeBSD.org> Date: Thu, 25 Sep 2014 09:14:58 -0700 X-Google-Sender-Auth: -425onM09PD7em5kHnAAN6UAwwg Message-ID: Subject: Re: KASSERT_WARN for asserting malloc(M_WAITOK) not in a non-sleepable thread From: Adrian Chadd To: Bryan Drewery Content-Type: text/plain; charset=UTF-8 Cc: "freebsd-arch@freebsd.org" 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 16:15:01 -0000 Hi, Please bring in KASSERT_WARN(). I'm grown up enough to use KASSERT_WARN() along with handling the invariant check myself in code. Having KASSERT_WARN() means I can add in this rather than printf()s or device_printf()'s with various knobs to remove it. (This is absolutely _not_ the "should KASSERT() optionally just log" argument. I'm not going to get into that a second time.) -a On 25 September 2014 08:47, Bryan Drewery wrote: > On 9/24/2014 9: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. >> >> -- >> Davide > > I would be comfortable adding it in as a full KASSERT (and not bringing > in KASSERT_WARN) if other people test the patch in > https://reviews.freebsd.org/D830 and change them to KASSERT. If the > fallout is not too bad then we can commit the real assert. > > -- > Regards, > Bryan Drewery >