linux-kernel - Re: [PATCH] tracing/core: Add current context on tracing recursion warning

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20090419140154.GB28919@elte.hu>
Date:	Sun, 19 Apr 2009 16:01:54 +0200
From:	Ingo Molnar <mingo@...e.hu>
To:	Frederic Weisbecker <fweisbec@...il.com>
Cc:	Li Zefan <lizf@...fujitsu.com>,
	Steven Rostedt <rostedt@...dmis.org>,
	Zhaolei <zhaolei@...fujitsu.com>,
	Tom Zanussi <tzanussi@...il.com>,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH] tracing/core: Add current context on tracing recursion
	warning


* Frederic Weisbecker <fweisbec@...il.com> wrote:

> On Sun, Apr 19, 2009 at 02:34:32PM +0200, Frederic Weisbecker wrote:
> > On Sun, Apr 19, 2009 at 02:14:54PM +0800, Li Zefan wrote:
> > > Frederic Weisbecker wrote:
> > > > Hi,
> > > > 
> > > > Here is the v3 of the __string() field patchset.
> > > > It applies suggestions from Steven and Peter with some arrangements.
> > > > 
> > > > This time, filtering is not supported (though it is ready in a pending patch).
> > > > I wanted to provide it but it looks like filtering has been broken recently.
> > > > Once I set a usual string filter, no more traces appear, and clearing it
> > > > doesn't change anything.
> > > > 
> > > 
> > > I tried it, and triggered a WARNING, and ring buffers was
> > > disabled permanently:
> > 
> > 
> > I've also seen this warning but on another event.
> > I don't think this is related to this patchset but
> > more about the tracing recursion detection.
> > 
> > For exemple, here we are in an Irq event, which doesn't
> > use the __string() thing. For such off-case, the only change
> > is a variable declaration and a + 0 operation.
> > 
> > Another thing: I've only seen it in a selftest.
> 
> 
> Worst: I can't reproduce it anymore.
> What were you doing when you got such warning? Were you
> in a selftest, or trying a usual event?
> 
> Also, could you test the following patch. It will give us
> more informations about the tracing recursion.
> 
> You can find it on:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing tracing/recursion
> 
> It's against tip/tracing/core
> 
> Thanks!
> 
> ---
> >From d13bf59ca011b976c561f623e3189a4a5b94370e Mon Sep 17 00:00:00 2001
> From: Frederic Weisbecker <fweisbec@...il.com>
> Date: Sun, 19 Apr 2009 15:30:19 +0200
> Subject: [PATCH] tracing/core: Add current context on tracing recursion warning

> 
> In case of tracing recursion detection, we only get the stacktrace.
> But the current context may be very useful to debug the issue.
> 
> This patch adds the softirq/hardirq/nmi context with the warning
> using lockdep context display to have a familiar output.
> 
> [ Impact: more information in tracing recursion ]
> 
> Signed-off-by: Frederic Weisbecker <fweisbec@...il.com>
> ---
>  kernel/trace/ring_buffer.c |   13 +++++++++++++
>  1 files changed, 13 insertions(+), 0 deletions(-)
> 
> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index b421b0e..27a6e7d 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -1493,8 +1493,21 @@ static int trace_recursive_lock(void)
>  	level = trace_irq_level();
>  
>  	if (unlikely(current->trace_recursion & (1 << level))) {
> +		static atomic_t warned;
> +
>  		/* Disable all tracing before we do anything else */
>  		tracing_off_permanent();
> +
> +		if (atomic_inc_return(&warned) == 1) {
> +			printk(KERN_WARNING "Tracing recursion: "
> +				"[HC%u[%lu]:SC%u[%lu]:NMI[%lu]:HE%u:SE%u]\n",
> +				current->hardirq_context,
> +				hardirq_count() >> HARDIRQ_SHIFT,
> +				current->softirq_context,
> +				softirq_count() >> SOFTIRQ_SHIFT,
> +				in_nmi(), current->hardirqs_enabled,
> +				current->softirqs_enabled);
> +		}

It would be nice to have this ... but there's no need to do that 
atomic thing - just use printk_once() please. (if we race with 
another instance and get two messages that's not a problem)

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