From owner-freebsd-arch@FreeBSD.ORG Mon Feb 11 16:06:07 2013 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 72822D8E; Mon, 11 Feb 2013 16:06:07 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from bigwig.baldwin.cx (bigknife-pt.tunnel.tserv9.chi1.ipv6.he.net [IPv6:2001:470:1f10:75::2]) by mx1.freebsd.org (Postfix) with ESMTP id 49A8CB2C; Mon, 11 Feb 2013 16:06:07 +0000 (UTC) Received: from pakbsde14.localnet (unknown [38.105.238.108]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 1E28CB924; Mon, 11 Feb 2013 11:06:06 -0500 (EST) From: John Baldwin To: freebsd-arch@freebsd.org Subject: Re: Proposal: Unify printing the function name in panic messages() Date: Mon, 11 Feb 2013 11:05:57 -0500 User-Agent: KMail/1.13.5 (FreeBSD/8.2-CBSD-20110714-p22; KDE/4.5.5; amd64; ; ) References: <51141E33.4080103@gmx.de> <511622A2.2090601@FreeBSD.org> <5118C4E4.6020706@gmx.de> In-Reply-To: <5118C4E4.6020706@gmx.de> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201302111105.58186.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Mon, 11 Feb 2013 11:06:06 -0500 (EST) Cc: Kirk McKusick , Christoph Mallon , Andriy Gapon X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 11 Feb 2013 16:06:07 -0000 On Monday, February 11, 2013 5:16:04 am Christoph Mallon wrote: > On 09.02.2013 11:19, Andriy Gapon wrote: > > on 09/02/2013 11:28 Christoph Mallon said the following: > >> I do not understand, what the problem is. > >> There are bugs and cumbersome code. > >> This simple changes solves it. > > > > You conveniently omitted some questions of mine. > > I'll reproduce them: > > Well, have you experienced any problems with debugging due to those > > (absent/misleading) function names? Or do you see many reports about such problems? > > I have dropped them, because they are not relevant. Eh, showing that there is an actual problem being solved rather than making a change gratuitously is highly relevant. You are the one arguing for a change so you have to make the case for why it is a good one. I for one would _really_ not like it if you are magically changing the format string for panics. I am in the process of writing in-kernel regression tests that rely on being able to "catch" panics via a modified try-catch model where the catches do simple string compares on the panic message. Adding a silent ("not visible in the code") prefix to each panic message makes this process more error prone. A sample test to show what I mean: /* Tests for spin mutexes. */ static int spin_recurse(void) { struct mtx m; bzero(&m, sizeof(m)); mtx_init(&m, "test spin", NULL, MTX_SPIN | MTX_RECURSE); mtx_lock_spin(&m); mtx_lock_spin(&m); mtx_unlock_spin(&m); mtx_unlock_spin(&m); mtx_destroy(&m); mtx_init(&m, "test spin", NULL, MTX_SPIN); mtx_lock_spin(&m); PANIC_TRY { mtx_lock_spin(&m); } PANIC_CATCH { IGNORE_PANIC_STARTS_WITH( "mtx_lock_spin: recursed on non-recursive mutex"); } PANIC_END; mtx_unlock_spin(&m); mtx_destroy(&m); return (TEST_OK); } UNIT_TEST("spin lock recurse", spin_recurse); Also, this is a case where your script will claim that the KASSERT() is "wrong" but in actual fact it is correct. The programmer is using mtx_lock_spin() and expects to see that in a panic message not __mtx_lock_spin_flags(). Only someone working with the implementation of spin locks should have to know about __mtx_lock_spin_flags(). For someone working on a device driver or other code, using "mtx_lock_spin" is more correct and far more useful. IOW, your changes will break just about every panic that is part of a macro where the macro is the public API and __func__ is not relevant. -- John Baldwin