On Wed, Jul 10, 2013 at 08:27:34AM -0400, Steven Rostedt wrote: > On Wed, 2013-07-10 at 11:52 +0200, Peter Zijlstra wrote: > > > Fun.. :-) we trace __local_bh_enable() and hit a ftrace callback between > > telling lockdep we enabled softirqs and actually doing so. > > > > I'm just a tad confused by the trace; it says we go: > > lock_is_held() > > check_flags() > > > > Looking at perf_tp_event() this would most likely be from: > > > > ctx = rcu_dereference(task->perf_event_ctxp[perf_sw_context]); > > Function tracing should not be treated as a normal trace point. It is > much more invasive, and there's things that one should be weary about > when using it. Clearly.. yet someone forgot this when merging ced39002 :/ > > > > Where the lock_is_held() would be from rcu_dereference_check()'s > > rcu_read_lock_sched_held(). However, by there we've already passed > > rcu_read_lock() which includes rcu_lock_acquire() -> lock_acquire() -> > > check_flags(). So it should've triggered there. > > > > Ideally we'd not trace __local_bh_enable() at all, seeing as how any RCU usage > > in there would be bound to trigger this. > > I find it very useful to trace __local_bh_enable(). I also trace RCU > calls. > > When using function tracing, you need to use > rcu_dereference_raw_notrace(). > > Also, function tracing callbacks should avoid rcu_read_lock(), as that's > traced as well. You can use preempt_disable_notrace() for rcu usage. but but but preempt_disable_notrace() isn't an rcu_read_lock().. You can only do that for rcu_sched. Anyway, I don't see a nice way out of this mess :/ the entire perf core uses regular RCU and converting all that is going to me a nasty big patch. -- To unsubscribe from this list: send the line "unsubscribe trinity" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
