Messages in this thread | | | From | Geert Uytterhoeven <> | Date | Thu, 7 Nov 2019 08:27:29 +0100 | Subject | Re: [PATCH 00/50] Add log level to show_stack() |
| |
Hi Russell,
(reduced CC list)
On Thu, Nov 7, 2019 at 12:28 AM Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote: > On Wed, Nov 06, 2019 at 09:34:40PM +0100, Peter Zijlstra wrote: > > I suppose I'm surprised there are backtraces that are not important. > > Either badness happened and it needs printing, or the user asked for it > > and it needs printing. > > Or utterly meaningless. > > > Perhaps we should be removing backtraces if they're not important > > instead of allowing to print them as lower loglevels? > > Definitely! WARN_ON() is well overused - and as is typical, used > without much thought. Bound to happen after Linus got shirty about > BUG_ON() being over used. Everyone just grabbed the next nearest thing > to assert().
Which is what checkpatch.pl suggests...
> As a kind of example, I've recently come across one WARN_ON() in a > driver subsystem (that shall remain nameless at the moment) which very > likely has multiple different devices on a platform. The WARN_ON() > triggers as a result of a problem with the hardware, but because it's a > WARN_ON(), you've no idea which device has a problem. The backtrace is > mostly meaningless. So you know that a problem has occurred, but the > kernel prints *useless* backtrace to let you know, and totally omits > the *useful* information.
So that callsite should be converted to use dev_WARN(), with a suitable message.
Perhaps checkpatch should be updated, to suggest {,dev_}WARN() instead of WARN_ON(), and add a check for the latter, too?
Gr{oetje,eeting}s,
Geert
-- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
| |