From owner-freebsd-arch@FreeBSD.ORG Thu Sep 25 18:16:14 2014 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 0DD98BA4; Thu, 25 Sep 2014 18:16:14 +0000 (UTC) Received: from mail-la0-x231.google.com (mail-la0-x231.google.com [IPv6:2a00:1450:4010:c03::231]) (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 026A9A81; Thu, 25 Sep 2014 18:16:12 +0000 (UTC) Received: by mail-la0-f49.google.com with SMTP id pn19so13174409lab.36 for ; Thu, 25 Sep 2014 11:16:10 -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=zYNZSzOC0mpiTYeWopuRIv5Tt1wIZTQTF2TcgBjmNFk=; b=DwcmQ2uVgaFReshYmIDEK0OwCI1EhpBKYNaQ47/VFM7b8FC6CQ82JxLmIHI/TDAXr3 /j1fi8s4OEwDuA+6Eqmf82nIV2w4nBisl3iTqGjUerAbWvSnmOHub7uPRauim69KhBDn g8VmJUAign+vJEkjBLLZQBfsbCYp8fbVZs52EpJKsl10KgWf0YL2aKauTwPswpYF+8tn 9lx+wIqsnkrZfV1L/N9YRfPZ5iY7NgbvoTGBgfPRKQPJvLQwVEVB//bx6UAn+GmaZ+u6 bG2yaCZ0KIwQGbxOR/lBNqG7bxpmYDdWI0EuIANAlpT+o8D0yJzmiDDrWwADcYQyfIMp hE4w== MIME-Version: 1.0 X-Received: by 10.152.88.97 with SMTP id bf1mr15156788lab.58.1411668970822; Thu, 25 Sep 2014 11:16:10 -0700 (PDT) Sender: davide.italiano@gmail.com Received: by 10.25.207.74 with HTTP; Thu, 25 Sep 2014 11:16:10 -0700 (PDT) In-Reply-To: <1411668571.66615.247.camel@revolution.hippie.lan> References: <54236CD6.4050807@FreeBSD.org> <5424392D.9030201@FreeBSD.org> <1411668571.66615.247.camel@revolution.hippie.lan> Date: Thu, 25 Sep 2014 11:16:10 -0700 X-Google-Sender-Auth: nX_F4wYQsxQMLbkLHQwHHGqEWCA Message-ID: Subject: Re: KASSERT_WARN for asserting malloc(M_WAITOK) not in a non-sleepable thread From: Davide Italiano To: Ian Lepore Content-Type: text/plain; charset=UTF-8 Cc: Adrian Chadd , Bryan Drewery , "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 18:16:14 -0000 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, -- Davide "There are no solved problems; there are only problems that are more or less solved" -- Henri Poincare