On Wed, 11 Jul 2018 15:12:56 +0200 Peter Zijlstra peterz@infradead.org wrote:
On Thu, Jun 28, 2018 at 11:21:47AM -0700, Joel Fernandes wrote:
One note, I have to check for lockdep recursion in the code that calls the trace events API and bail out if we're in lockdep recursion
I'm not seeing any new lockdep_recursion checks...
I believe he's talking about this part:
+void trace_hardirqs_on(void) +{ + if (lockdep_recursing(current) || !this_cpu_read(tracing_irq_cpu)) + return; +
[etc]
protection to prevent something like the following case: a spin_lock is taken. Then lockdep_acquired is called. That does a raw_local_irq_save and then sets lockdep_recursion, and then calls __lockdep_acquired. In this function, a call to get_lock_stats happens which calls preempt_disable, which calls trace IRQS off somewhere which enters my tracepoint code and sets the tracing_irq_cpu flag to prevent recursion. This flag is then never cleared causing lockdep paths to never be entered and thus causing splats and other bad things.
Would it not be much easier to avoid that entirely, afaict all get/put_lock_stats() callers already have IRQs disabled, so that (traced) preempt fiddling is entirely superfluous.
Agreed. Looks like a good clean up.
-- Steve -- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html