Re: [PATCH] let WARN_ON() output the condition

[Ingo Molnar <mingo@elte.hu>]

* Jiri Kosina <jkosina@suse.cz> wrote:

> [PATCH] let WARN_ON() output the condition
> 
> It is possible, in some cases, that the output of WARN_ON() is 
> ambiguous and can't be properly used to identify the exact condition 
> which caused the warning to trigger. This happens whenever there is a 
> macro that contains multiple WARN_ONs inside. Notable example is 
> spin_lock_mutex(). If any of the two WARN_ONs trigger, we are not able 
> to say which one was the cause (as we get only line number, which 
> however belongs to the place where the macro was expanded).

a WARN_ON() also triggers a stack dump, which should pinpoint the exact 
location. (especially if combined with kallsyms) For example:

posix_cpu_timer/13[CPU#1]: BUG in trace_stop_sched_switched at kernel/latency_trace.c:2142

Call Trace:
 [<ffffffff8020b272>] dump_trace+0xaf/0x3f4
 [<ffffffff8020b5f6>] show_trace+0x3f/0x5d
 [<ffffffff8020b8c1>] dump_stack+0x1a/0x1c
 [<ffffffff8022ef09>] __WARN_ON+0x65/0x80
 [<ffffffff80252c37>] trace_stop_sched_switched+0xad/0x30a
 [<ffffffff804ae810>] thread_return+0xa5/0x123
 [<ffffffff804aea15>] schedule+0xdd/0x101
 [<ffffffff8024298f>] posix_cpu_timers_thread+0x86/0xe5
 [<ffffffff80240c26>] kthread+0xd6/0x100
 [<ffffffff8020a938>] child_rip+0xa/0x12

here the "trace_stop_sched_switched+0xad/0x30a" is a perfect 
identification of the WARN_ON() code location - if there's any doubt 
about why the problem happened.

> This patch lets WARN_ON() to output also the condition and fixes the 
> DEBUG_LOCKS_WARN_ON() macro to pass the condition properly to WARN_ON. 
> The possible drawback could be when someone passes a condition which 
> has sideeffects. Then it would be evaluated twice, instead of current 
> one evaluation. On the other hand, when anyone passes expression with 
> sideeffects to WARN_ON(), he is asking for problems anyway.

side-effects happen regularly in WARN_ON()s and while they should be 
avoided, they are not noticed by the compiler and can cause nasty bugs 
if executed twice. Do we really need this change?

	Ingo
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/