Re: [PATCH 12/12] ftrace: Add internal recursive checks

[Witold Baryluk]

On 05-26 11:25, Steven Rostedt wrote:
> From: Steven Rostedt <rostedt@xxxxxxxxxxx>
> 
> 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: signature.asc
Description: Digital signature