mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Witold Baryluk" <baryluk@smp.if.uj.edu.pl>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-kernel@vger.kernel.org, Ingo Molnar <mingo@elte.hu>,
	Andrew Morton <akpm@linux-foundation.org>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Subject: Re: [PATCH 12/12] ftrace: Add internal recursive checks
Date: Thu, 26 May 2011 18:18:35 +0200	[thread overview]
Message-ID: <20110526161835.GL25813@smp.if.uj.edu.pl> (raw)
In-Reply-To: <20110526152958.961788971@goodmis.org>

[-- Attachment #1: Type: text/plain, Size: 1926 bytes --]

On 05-26 11:25, Steven Rostedt wrote:
> From: Steven Rostedt <rostedt@goodmis.org>
> 
> Witold reported a reboot caused by the selftests of the dynamic function
> tracer. He sent me a config and I used ktest to do a config_bisect on it
> (as my config did not cause the crash). It pointed out that the problem
> config was CONFIG_PROVE_RCU.

Just disabling PROVE_RCU in my config
make crash disapear, so it is good diagnosis. Good work.

> 
> What happened was that if multiple callbacks are attached to the
> function tracer, we iterate a list of callbacks. Because the list is
> managed by synchronize_sched() and preempt_disable, the access to the
> pointers uses rcu_dereference_raw().
> 
> When PROVE_RCU is enabled, the rcu_dereference_raw() calls some
> debugging functions, which happen to be traced. The tracing of the debug
> function would then call rcu_dereference_raw() which would then call the
> debug function and then... well you get the idea.
> 
> I first wrote two different patches to solve this bug.
> 
> 1) add a __rcu_dereference_raw() that would not do any checks.
> 2) add notrace to the offending debug functions.
> 
> Both of these patches worked.
> 
> Talking with Paul McKenney on IRC, he suggested to add recursion
> detection instead. This seemed to be a better solution, so I decided to
> implement it. As the task_struct already has a trace_recursion to detect
> recursion in the ring buffer, and that has a very small number it
> allows, I decided to use that same variable to add flags that can detect
> the recursion inside the infrastructure of the function tracer.
> 
> I plan to change it so that the task struct bit can be checked in
> mcount, but as that requires changes to all archs, I will hold that off
> to the next merge window.

I'm testing this patch now, and do not see any problem right now.

Regards,
Witek

-- 
Witold Baryluk

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

  reply	other threads:[~2011-05-26 16:23 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-26 15:25 [PATCH 00/12] [GIT PULL] tracing: fixes Steven Rostedt
2011-05-26 15:25 ` [PATCH 01/12] maccess,probe_kernel: Make write/read src const void * Steven Rostedt
2011-05-26 15:25 ` [PATCH 02/12] x86/ftrace: Fix compiler warning in ftrace.c Steven Rostedt
2011-05-26 15:25 ` [PATCH 03/12] scripts/tags.sh: Fix ctags for DEFINE_EVENT() Steven Rostedt
2011-05-26 15:25 ` [PATCH 04/12] scripts/tags.sh: Add magic for trace-events for etags too Steven Rostedt
2011-05-26 15:38   ` Peter Zijlstra
2011-05-26 15:25 ` [PATCH 05/12] ftrace/recordmcount: Avoid STT_FUNC symbols as base on ARM Steven Rostedt
2011-05-26 15:25 ` [PATCH 06/12] jump_label: Check entries limit in __jump_label_update Steven Rostedt
2011-05-26 15:25 ` [PATCH 07/12] ftrace: Have ftrace_startup() return failure code Steven Rostedt
2011-05-26 15:25 ` [PATCH 08/12] tracing: Have event with function tracer check error return Steven Rostedt
2011-05-26 15:25 ` [PATCH 09/12] ftrace: Set ops->flag to enabled even on static function tracing Steven Rostedt
2011-05-26 15:25 ` [PATCH 10/12] tracing: Add __print_symbolic_u64 to avoid warnings on 32bit machine Steven Rostedt
2011-05-26 15:25 ` [PATCH 11/12] tracing: Update btrfss tracepoints to use u64 interface Steven Rostedt
2011-05-26 15:25 ` [PATCH 12/12] ftrace: Add internal recursive checks Steven Rostedt
2011-05-26 16:18   ` Witold Baryluk [this message]
2011-05-26 16:54   ` Paul E. McKenney
2011-05-27 12:46 ` [PATCH 00/12] [GIT PULL] tracing: fixes Ingo Molnar

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=20110526161835.GL25813@smp.if.uj.edu.pl \
    --to=baryluk@smp.if.uj.edu.pl \
    --cc=akpm@linux-foundation.org \
    --cc=fweisbec@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome