All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Jiri Kosina <jikos@jikos.cz>
Cc: Andrew Morton <akpm@osdl.org>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] let WARN_ON() output the condition
Date: Wed, 6 Dec 2006 09:54:28 +0100	[thread overview]
Message-ID: <20061206085428.GA28160@elte.hu> (raw)
In-Reply-To: <Pine.LNX.4.64.0612060940130.28502@twin.jikos.cz>


* Jiri Kosina <jikos@jikos.cz> wrote:

> On Wed, 6 Dec 2006, Ingo Molnar wrote:
> 
> > a WARN_ON() also triggers a stack dump, which should pinpoint the exact 
> > location. (especially if combined with kallsyms) For example:
> 
> Actually, I was referring to something a little bit different. For example 
> kernel/mutex.c:__mutex_lock_common() calls spin_lock_mutex() on line 132. 
> spin_lock_mutex() contains
> 
>                 DEBUG_LOCKS_WARN_ON(in_interrupt());    \
>                 local_irq_save(flags);                  \
>                 __raw_spin_lock(&(lock)->raw_lock);     \
>                 DEBUG_LOCKS_WARN_ON(l->magic != l);     \
> 
> When one of these two WARN_ONs trigger, we get only
> 
> 	WARNING at kernel/mutex.c:132 __mutex_lock_common()

no, that's not all we get - we should also get a stackdump. Are you not 
getting a stackdump perhaps?

but i agree with you in theory that your proposed output is better, but 
the side-effect issue is a killer i think. Could you try to rework it to 
not evaluate the condition twice and to make it dependent on 
CONFIG_DEBUG_BUGVERBOSE? You can avoid the evaluation side-effect issue 
by doing something like:

	int __c = (c);							\
                                                                        \
        if (unlikely(__c)) {                                            \
                if (debug_locks_off())                                  \
                        WARN_ON(__c);                                   \
                __ret = 1;                                              \

	Ingo

  reply	other threads:[~2006-12-06  8:55 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-12-06  0:51 [PATCH] let WARN_ON() output the condition Jiri Kosina
2006-12-06  1:27 ` Andrew Morton
2006-12-06 12:52   ` Horst H. von Brand
2006-12-06 13:54     ` Ingo Molnar
2006-12-06 14:35       ` Jaswinder Singh
2006-12-06 15:08         ` Jiri Kosina
2006-12-06  8:37 ` Ingo Molnar
2006-12-06  8:47   ` Jiri Kosina
2006-12-06  8:54     ` Ingo Molnar [this message]
2006-12-06  9:04       ` Jiri Kosina
2006-12-06  9:07         ` Ingo Molnar
2006-12-06  9:11           ` Jiri Kosina
2006-12-06  9:27             ` Ingo Molnar
2006-12-06  9:14           ` Jan Engelhardt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20061206085428.GA28160@elte.hu \
    --to=mingo@elte.hu \
    --cc=akpm@osdl.org \
    --cc=jikos@jikos.cz \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.