From owner-freebsd-arch@FreeBSD.ORG Thu Sep 25 19:21:25 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 E89D7637; Thu, 25 Sep 2014 19:21:24 +0000 (UTC) Received: from mho-02-ewr.mailhop.org (mho-02-ewr.mailhop.org [204.13.248.72]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id A3437214; Thu, 25 Sep 2014 19:21:24 +0000 (UTC) Received: from [73.34.117.227] (helo=ilsoft.org) by mho-02-ewr.mailhop.org with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.72) (envelope-from ) id 1XXDf7-0005AH-Bm; Thu, 25 Sep 2014 18:21:05 +0000 Received: from [172.22.42.240] (revolution.hippie.lan [172.22.42.240]) by ilsoft.org (8.14.9/8.14.9) with ESMTP id s8PIL4G7003660; Thu, 25 Sep 2014 12:21:04 -0600 (MDT) (envelope-from ian@FreeBSD.org) X-Mail-Handler: Dyn Standard SMTP by Dyn X-Originating-IP: 73.34.117.227 X-Report-Abuse-To: abuse@dyndns.com (see http://www.dyndns.com/services/sendlabs/outbound_abuse.html for abuse reporting information) X-MHO-User: U2FsdGVkX19e8RvCcEeiqcMsZXlOfnoC X-Authentication-Warning: paranoia.hippie.lan: Host revolution.hippie.lan [172.22.42.240] claimed to be [172.22.42.240] Subject: Re: KASSERT_WARN for asserting malloc(M_WAITOK) not in a non-sleepable thread From: Ian Lepore To: Davide Italiano In-Reply-To: References: <54236CD6.4050807@FreeBSD.org> <5424392D.9030201@FreeBSD.org> <1411668571.66615.247.camel@revolution.hippie.lan> Content-Type: text/plain; charset="us-ascii" Date: Thu, 25 Sep 2014 12:21:03 -0600 Message-ID: <1411669263.66615.249.camel@revolution.hippie.lan> Mime-Version: 1.0 X-Mailer: Evolution 2.32.1 FreeBSD GNOME Team Port Content-Transfer-Encoding: 7bit Cc: Adrian Chadd , "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 19:21:25 -0000 On Thu, 2014-09-25 at 11:16 -0700, Davide Italiano wrote: > On Thu, Sep 25, 2014 at 11:09 AM, Ian Lepore wrote: > > On Thu, 2014-09-25 at 10:51 -0700, Davide Italiano wrote: > >> On Thu, Sep 25, 2014 at 9:14 AM, Adrian Chadd wrote: > >> > 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.) > >> > > >> > > >> > >> If you put a KASSERT() inside your code -- probably you should be > >> careful enough to put that iff you're sure that it should be always > >> verified. No exceptions. > >> People tend to be very lazy (including me). I don't expect everybody > >> diligently upgrading KASSERT_WARN to KASSERT. So KASSERT_WARN start > >> becoming more and more widespread, and people realize all of these > >> need to be upgraded to KASSERT or removed. This generally happens > >> after years. Yet. Another. Crusade. > >> There's a lot of work in the kernel to remove old/wrong/naive KPI > >> from the kernel. jhb@ is looking at timeout()-> callout() conversion. > >> I'm personally looking at dev_clone() removal. There are a lot of > >> other examples. > >> Adding KASSERT_WARN is a step backward, not a step forward, IMHO. > >> That said, if you want to pollute the kernel, fine. I expressed my > >> opinion, and I'm personally not happy about this, but I never stated > >> I'm gonna stop you from doing that. > >> > >> Thanks, > >> > >> -- > > > > IMO, this entire argument is ridiculous. Some conditions are so insane > > that you've got to stop immediately rather than make things worse. > > Other conditions indicate problems, but the code can recover or > > otherwise continue to operate safely. Trying to define every possible > > anomalous condition as either fatal or not worth mentioning is insane. > > > > Everyone is free to write code such as > > > > #ifdef INVARIANTS > > if (some_condition) > > printf("whatever warning\n"); > > #endif > > > > So let's be clear here: the objections are to spelling that code > > sequence KASSERT_WARN. If you object, please explain what's wrong with > > that spelling and how you would prefer it to be spelled. > > > > -- Ian > > > > > > Take the assert out of the name. Call it DEBUG_WARN, or something else > if you like. > assert as a pretty *clear* and specific semantic, no need to mess > around with it. > > Thanks, > To me, another "clear and specific semantic" that's associated with the word 'assert' in C programming is that the test expression itself is automatically printed as part of the diagnostic message. That's not the case in the FreeBSD kernel, so I guess we need to rename KASSERT as well? -- Ian