From c3bc8fd637a9623f5c507bd18f9677effbddf584 Mon Sep 17 00:00:00 2001 From: "Joel Fernandes (Google)" Date: Mon, 30 Jul 2018 15:24:23 -0700 Subject: tracing: Centralize preemptirq tracepoints and unify their usage This patch detaches the preemptirq tracepoints from the tracers and keeps it separate. Advantages: * Lockdep and irqsoff event can now run in parallel since they no longer have their own calls. * This unifies the usecase of adding hooks to an irqsoff and irqson event, and a preemptoff and preempton event. 3 users of the events exist: - Lockdep - irqsoff and preemptoff tracers - irqs and preempt trace events The unification cleans up several ifdefs and makes the code in preempt tracer and irqsoff tracers simpler. It gets rid of all the horrific ifdeferry around PROVE_LOCKING and makes configuration of the different users of the tracepoints more easy and understandable. It also gets rid of the time_* function calls from the lockdep hooks used to call into the preemptirq tracer which is not needed anymore. The negative delta in lines of code in this patch is quite large too. In the patch we introduce a new CONFIG option PREEMPTIRQ_TRACEPOINTS as a single point for registering probes onto the tracepoints. With this, the web of config options for preempt/irq toggle tracepoints and its users becomes: PREEMPT_TRACER PREEMPTIRQ_EVENTS IRQSOFF_TRACER PROVE_LOCKING | | \ | | \ (selects) / \ \ (selects) / TRACE_PREEMPT_TOGGLE ----> TRACE_IRQFLAGS \ / \ (depends on) / PREEMPTIRQ_TRACEPOINTS Other than the performance tests mentioned in the previous patch, I also ran the locking API test suite. I verified that all tests cases are passing. I also injected issues by not registering lockdep probes onto the tracepoints and I see failures to confirm that the probes are indeed working. This series + lockdep probes not registered (just to inject errors): [ 0.000000] hard-irqs-on + irq-safe-A/21: ok | ok | ok | [ 0.000000] soft-irqs-on + irq-safe-A/21: ok | ok | ok | [ 0.000000] sirq-safe-A => hirqs-on/12:FAILED|FAILED| ok | [ 0.000000] sirq-safe-A => hirqs-on/21:FAILED|FAILED| ok | [ 0.000000] hard-safe-A + irqs-on/12:FAILED|FAILED| ok | [ 0.000000] soft-safe-A + irqs-on/12:FAILED|FAILED| ok | [ 0.000000] hard-safe-A + irqs-on/21:FAILED|FAILED| ok | [ 0.000000] soft-safe-A + irqs-on/21:FAILED|FAILED| ok | [ 0.000000] hard-safe-A + unsafe-B #1/123: ok | ok | ok | [ 0.000000] soft-safe-A + unsafe-B #1/123: ok | ok | ok | With this series + lockdep probes registered, all locking tests pass: [ 0.000000] hard-irqs-on + irq-safe-A/21: ok | ok | ok | [ 0.000000] soft-irqs-on + irq-safe-A/21: ok | ok | ok | [ 0.000000] sirq-safe-A => hirqs-on/12: ok | ok | ok | [ 0.000000] sirq-safe-A => hirqs-on/21: ok | ok | ok | [ 0.000000] hard-safe-A + irqs-on/12: ok | ok | ok | [ 0.000000] soft-safe-A + irqs-on/12: ok | ok | ok | [ 0.000000] hard-safe-A + irqs-on/21: ok | ok | ok | [ 0.000000] soft-safe-A + irqs-on/21: ok | ok | ok | [ 0.000000] hard-safe-A + unsafe-B #1/123: ok | ok | ok | [ 0.000000] soft-safe-A + unsafe-B #1/123: ok | ok | ok | Link: http://lkml.kernel.org/r/20180730222423.196630-4-joel@joelfernandes.org Acked-by: Peter Zijlstra (Intel) Reviewed-by: Namhyung Kim Signed-off-by: Joel Fernandes (Google) Signed-off-by: Steven Rostedt (VMware) --- init/main.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'init/main.c') diff --git a/init/main.c b/init/main.c index 3b4ada11ed52..44fe43be84c1 100644 --- a/init/main.c +++ b/init/main.c @@ -648,6 +648,9 @@ asmlinkage __visible void __init start_kernel(void) profile_init(); call_function_init(); WARN(!irqs_disabled(), "Interrupts were enabled early\n"); + + lockdep_init_early(); + early_boot_irqs_disabled = false; local_irq_enable(); @@ -663,7 +666,7 @@ asmlinkage __visible void __init start_kernel(void) panic("Too many boot %s vars at `%s'", panic_later, panic_param); - lockdep_info(); + lockdep_init(); /* * Need to run this when irqs are enabled, because it wants -- cgit v1.2.3 From bff1b208a5d1dbb2355822ef859edcb9be0379e4 Mon Sep 17 00:00:00 2001 From: "Steven Rostedt (VMware)" Date: Mon, 6 Aug 2018 15:50:58 -0400 Subject: tracing: Partial revert of "tracing: Centralize preemptirq tracepoints and unify their usage" Joel Fernandes created a nice patch that cleaned up the duplicate hooks used by lockdep and irqsoff latency tracer. It made both use tracepoints. But it caused lockdep to trigger several false positives. We have not figured out why yet, but removing lockdep from using the trace event hooks and just call its helper functions directly (like it use to), makes the problem go away. This is a partial revert of the clean up patch c3bc8fd637a9 ("tracing: Centralize preemptirq tracepoints and unify their usage") that adds direct calls for lockdep, but also keeps most of the clean up done to get rid of the horrible preprocessor if statements. Link: http://lkml.kernel.org/r/20180806155058.5ee875f4@gandalf.local.home Cc: Peter Zijlstra Reviewed-by: Joel Fernandes (Google) Fixes: c3bc8fd637a9 ("tracing: Centralize preemptirq tracepoints and unify their usage") Signed-off-by: Steven Rostedt (VMware) --- include/linux/irqflags.h | 8 ++++++-- include/linux/lockdep.h | 2 -- init/main.c | 2 -- kernel/locking/lockdep.c | 14 ++------------ kernel/trace/trace_preemptirq.c | 36 ++++++++++++++++++++---------------- 5 files changed, 28 insertions(+), 34 deletions(-) (limited to 'init/main.c') diff --git a/include/linux/irqflags.h b/include/linux/irqflags.h index 50edb9cbbd26..21619c92c377 100644 --- a/include/linux/irqflags.h +++ b/include/linux/irqflags.h @@ -19,9 +19,13 @@ #ifdef CONFIG_PROVE_LOCKING extern void trace_softirqs_on(unsigned long ip); extern void trace_softirqs_off(unsigned long ip); + extern void lockdep_hardirqs_on(unsigned long ip); + extern void lockdep_hardirqs_off(unsigned long ip); #else -# define trace_softirqs_on(ip) do { } while (0) -# define trace_softirqs_off(ip) do { } while (0) + static inline void trace_softirqs_on(unsigned long ip) { } + static inline void trace_softirqs_off(unsigned long ip) { } + static inline void lockdep_hardirqs_on(unsigned long ip) { } + static inline void lockdep_hardirqs_off(unsigned long ip) { } #endif #ifdef CONFIG_TRACE_IRQFLAGS diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h index a8113357ceeb..b0d0b51c4d85 100644 --- a/include/linux/lockdep.h +++ b/include/linux/lockdep.h @@ -267,7 +267,6 @@ struct held_lock { * Initialization, self-test and debugging-output methods: */ extern void lockdep_init(void); -extern void lockdep_init_early(void); extern void lockdep_reset(void); extern void lockdep_reset_lock(struct lockdep_map *lock); extern void lockdep_free_key_range(void *start, unsigned long size); @@ -408,7 +407,6 @@ static inline void lockdep_on(void) # define lock_set_class(l, n, k, s, i) do { } while (0) # define lock_set_subclass(l, s, i) do { } while (0) # define lockdep_init() do { } while (0) -# define lockdep_init_early() do { } while (0) # define lockdep_init_map(lock, name, key, sub) \ do { (void)(name); (void)(key); } while (0) # define lockdep_set_class(lock, key) do { (void)(key); } while (0) diff --git a/init/main.c b/init/main.c index 44fe43be84c1..5d42e577643a 100644 --- a/init/main.c +++ b/init/main.c @@ -649,8 +649,6 @@ asmlinkage __visible void __init start_kernel(void) call_function_init(); WARN(!irqs_disabled(), "Interrupts were enabled early\n"); - lockdep_init_early(); - early_boot_irqs_disabled = false; local_irq_enable(); diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c index 03bfaeb9f4e6..e406c5fdb41e 100644 --- a/kernel/locking/lockdep.c +++ b/kernel/locking/lockdep.c @@ -2840,8 +2840,7 @@ static void __trace_hardirqs_on_caller(unsigned long ip) debug_atomic_inc(hardirqs_on_events); } -static void lockdep_hardirqs_on(void *none, unsigned long ignore, - unsigned long ip) +void lockdep_hardirqs_on(unsigned long ip) { if (unlikely(!debug_locks || current->lockdep_recursion)) return; @@ -2885,8 +2884,7 @@ static void lockdep_hardirqs_on(void *none, unsigned long ignore, /* * Hardirqs were disabled: */ -static void lockdep_hardirqs_off(void *none, unsigned long ignore, - unsigned long ip) +void lockdep_hardirqs_off(unsigned long ip) { struct task_struct *curr = current; @@ -4315,14 +4313,6 @@ out_restore: raw_local_irq_restore(flags); } -void __init lockdep_init_early(void) -{ -#ifdef CONFIG_PROVE_LOCKING - register_trace_prio_irq_disable(lockdep_hardirqs_off, NULL, INT_MAX); - register_trace_prio_irq_enable(lockdep_hardirqs_on, NULL, INT_MIN); -#endif -} - void __init lockdep_init(void) { printk("Lock dependency validator: Copyright (c) 2006 Red Hat, Inc., Ingo Molnar\n"); diff --git a/kernel/trace/trace_preemptirq.c b/kernel/trace/trace_preemptirq.c index e76b78bf258e..fa656b25f427 100644 --- a/kernel/trace/trace_preemptirq.c +++ b/kernel/trace/trace_preemptirq.c @@ -19,41 +19,45 @@ static DEFINE_PER_CPU(int, tracing_irq_cpu); void trace_hardirqs_on(void) { - if (!this_cpu_read(tracing_irq_cpu)) - return; + if (this_cpu_read(tracing_irq_cpu)) { + trace_irq_enable_rcuidle(CALLER_ADDR0, CALLER_ADDR1); + this_cpu_write(tracing_irq_cpu, 0); + } - trace_irq_enable_rcuidle(CALLER_ADDR0, CALLER_ADDR1); - this_cpu_write(tracing_irq_cpu, 0); + lockdep_hardirqs_on(CALLER_ADDR0); } EXPORT_SYMBOL(trace_hardirqs_on); void trace_hardirqs_off(void) { - if (this_cpu_read(tracing_irq_cpu)) - return; + if (!this_cpu_read(tracing_irq_cpu)) { + this_cpu_write(tracing_irq_cpu, 1); + trace_irq_disable_rcuidle(CALLER_ADDR0, CALLER_ADDR1); + } - this_cpu_write(tracing_irq_cpu, 1); - trace_irq_disable_rcuidle(CALLER_ADDR0, CALLER_ADDR1); + lockdep_hardirqs_off(CALLER_ADDR0); } EXPORT_SYMBOL(trace_hardirqs_off); __visible void trace_hardirqs_on_caller(unsigned long caller_addr) { - if (!this_cpu_read(tracing_irq_cpu)) - return; + if (this_cpu_read(tracing_irq_cpu)) { + trace_irq_enable_rcuidle(CALLER_ADDR0, caller_addr); + this_cpu_write(tracing_irq_cpu, 0); + } - trace_irq_enable_rcuidle(CALLER_ADDR0, caller_addr); - this_cpu_write(tracing_irq_cpu, 0); + lockdep_hardirqs_on(CALLER_ADDR0); } EXPORT_SYMBOL(trace_hardirqs_on_caller); __visible void trace_hardirqs_off_caller(unsigned long caller_addr) { - if (this_cpu_read(tracing_irq_cpu)) - return; + if (!this_cpu_read(tracing_irq_cpu)) { + this_cpu_write(tracing_irq_cpu, 1); + trace_irq_disable_rcuidle(CALLER_ADDR0, caller_addr); + } - this_cpu_write(tracing_irq_cpu, 1); - trace_irq_disable_rcuidle(CALLER_ADDR0, caller_addr); + lockdep_hardirqs_off(CALLER_ADDR0); } EXPORT_SYMBOL(trace_hardirqs_off_caller); #endif /* CONFIG_TRACE_IRQFLAGS */ -- cgit v1.2.3