Date: Mon, 11 Feb 2013 21:32:09 -0600 From: Mike Karels <mike@karels.net> To: Kirk McKusick <mckusick@mckusick.com> Cc: Adrian Chadd <adrian@freebsd.org>, Christoph Mallon <christoph.mallon@gmx.de>, Andriy Gapon <avg@freebsd.org>, freebsd-arch@freebsd.org Subject: Re: Proposal: Unify printing the function name in panic messages() Message-ID: <201302120332.r1C3W9i3005646@mail.karels.net> In-Reply-To: Your message of Mon, 11 Feb 2013 17:34:38 -0800. <201302120134.r1C1Ycfh026347@chez.mckusick.com>
next in thread | previous in thread | raw e-mail | index | archive | help
> I have recently gone down several ratholes trying to understand a > panic message which had the wrong function name (prefix ufs1_ instead > of ufs2_). I can definitely say that it matters to me. And I think > that fixing the problem as Christoph has outlined will be a big > step in the right direction. > John, in your code you cannot expect to match the entire panic > string if it has any % formats in it. So to be useful, you must be > able to work with a constant subset of the string. The addition of > other information such as a function name at the start should not > affect that constant part that you are trying to match. > Though a bit ugly, I do think that the change should use "PANIC" > rather than overloading "panic". First, it lets the feature be > introduced over time rather than requiring that every panic be > changed at once. And, it allows historic panic's to work as expected. > Finally, having it as a macro means that folks can readily and > consistently add file and/or line numbers to panic messages if they > wish to do so. > Summary: I think that this is an excellent idea that will both help > in finding the location of panic errors and also will allow folks > trying to minimize kernel size to do so by cutting out the overhead. I don't think that the proposal is to introduce this feature over time, but instead to mechanically change as many instances as possible. However, if I understand correctly, that would include primarily the instances now using the function name "correctly", not those using the wrong function name. I really don't want to see the kernel half-converted from panic() to PANIC(). I'd rather see lower-case panic as an available macro depending on a kernel option. However, as John notes, this option is of reduced utility if it isn't on by default, and I don't see sufficient reason to turn it on by default. Perhaps I haven't spent as much time looking at the wrong function for panics that have been cloned badly, but I tend to use glimpse if I don't know where the panic is coming from. It seems to me that a script looking for duplicate panic strings would solve the real problem much more quickly. Mike
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201302120332.r1C3W9i3005646>