LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: Joel Fernandes <joel@joelfernandes.org>
Cc: linux-kernel@vger.kernel.org, kernel-team@android.com,
	Boqun Feng <boqun.feng@gmail.com>,
	Byungchul Park <byungchul.park@lge.com>,
	Ingo Molnar <mingo@redhat.com>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	Namhyung Kim <namhyung@kernel.org>,
	Paul McKenney <paulmck@linux.vnet.ibm.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Thomas Glexiner <tglx@linutronix.de>,
	Tom Zanussi <tom.zanussi@linux.intel.com>
Subject: Re: [PATCH v12 3/3] tracing: Centralize preemptirq tracepoints and unify their usage
Date: Mon, 6 Aug 2018 15:50:58 -0400
Message-ID: <20180806155058.5ee875f4@gandalf.local.home> (raw)
In-Reply-To: <20180730222423.196630-4-joel@joelfernandes.org>


With this patch applied, I'm constantly getting lockdep errors. Instead
of doing a full revert of the patch, I did this, which makes all those
errors go away. I may apply this for now, and we can revisit having
lockdep use the tracepoint code. But since it's currently always
enabled, I'm thinking of just leaving this as is. The macros are still
clean from Joel's patch.

Thoughts?

-- Steve

diff --git a/include/linux/irqflags.h b/include/linux/irqflags.h
index 50edb9cbbd26..a93476c6c954 100644
--- a/include/linux/irqflags.h
+++ b/include/linux/irqflags.h
@@ -19,6 +19,8 @@
 #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)
diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index a8113357ceeb..30d0eb857f2e 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 @@ void lockdep_reset_lock(struct lockdep_map *lock)
 	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..8326adcab0dc 100644
--- a/kernel/trace/trace_preemptirq.c
+++ b/kernel/trace/trace_preemptirq.c
@@ -20,40 +20,52 @@ static DEFINE_PER_CPU(int, tracing_irq_cpu);
 void trace_hardirqs_on(void)
 {
 	if (!this_cpu_read(tracing_irq_cpu))
-		return;
+		goto out;
 
 	trace_irq_enable_rcuidle(CALLER_ADDR0, CALLER_ADDR1);
 	this_cpu_write(tracing_irq_cpu, 0);
+
+ out:
+	lockdep_hardirqs_on(CALLER_ADDR0);
 }
 EXPORT_SYMBOL(trace_hardirqs_on);
 
 void trace_hardirqs_off(void)
 {
 	if (this_cpu_read(tracing_irq_cpu))
-		return;
+		goto out;
 
 	this_cpu_write(tracing_irq_cpu, 1);
 	trace_irq_disable_rcuidle(CALLER_ADDR0, CALLER_ADDR1);
+
+ out:
+	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;
+		goto out;
 
 	trace_irq_enable_rcuidle(CALLER_ADDR0, caller_addr);
 	this_cpu_write(tracing_irq_cpu, 0);
+
+ out:
+	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;
+		goto out;
 
 	this_cpu_write(tracing_irq_cpu, 1);
 	trace_irq_disable_rcuidle(CALLER_ADDR0, caller_addr);
+
+ out:
+	lockdep_hardirqs_off(CALLER_ADDR0);
 }
 EXPORT_SYMBOL(trace_hardirqs_off_caller);
 #endif /* CONFIG_TRACE_IRQFLAGS */

  reply index

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-30 22:24 [PATCH v12 0/3] " Joel Fernandes
2018-07-30 22:24 ` [PATCH v12 1/3] lockdep: use this_cpu_ptr instead of get_cpu_var stats Joel Fernandes
2018-07-30 22:24 ` [PATCH v12 2/3] tracepoint: Make rcuidle tracepoint callers use SRCU Joel Fernandes
2018-07-30 23:10   ` Steven Rostedt
2018-08-10 15:35   ` Steven Rostedt
2018-08-10 16:32     ` Steven Rostedt
2018-07-30 22:24 ` [PATCH v12 3/3] tracing: Centralize preemptirq tracepoints and unify their usage Joel Fernandes
2018-08-06 19:50   ` Steven Rostedt [this message]
2018-08-07  0:43     ` Joel Fernandes
2018-08-07  1:43       ` Steven Rostedt
2018-08-07 13:33         ` Joel Fernandes
2018-08-07 13:49           ` Steven Rostedt
2018-08-07 14:10             ` Joel Fernandes
2018-08-07 14:34               ` Steven Rostedt
2018-08-07 14:48                 ` Joel Fernandes
2018-08-07 15:09                   ` Steven Rostedt
2018-08-07 15:24                     ` Joel Fernandes
2018-08-07 23:45                       ` Steven Rostedt
2018-08-07 23:54                         ` Joel Fernandes
2018-08-08  0:48                           ` Steven Rostedt
2018-08-08  1:17                             ` Joel Fernandes
2018-08-08  1:55                               ` Steven Rostedt
2018-08-08  2:13                                 ` Joel Fernandes
2018-08-08  2:28                                   ` Steven Rostedt
2018-08-08  3:44                                     ` Joel Fernandes
2018-08-08  3:53                                       ` Joel Fernandes
2018-08-08  5:06                                         ` Joel Fernandes
2018-08-08 12:46                                         ` Steven Rostedt
2018-08-08 13:03                                           ` Paul E. McKenney
2018-08-08 13:07                                             ` Steven Rostedt
2018-08-08 14:33                                               ` Paul E. McKenney
2018-08-08 14:49                                                 ` Steven Rostedt
2018-08-08 15:05                                                   ` Paul E. McKenney
2018-08-08 15:23                                                     ` Steven Rostedt
2018-08-08 16:02                                                       ` Paul E. McKenney
2018-08-08 16:24                                                         ` Steven Rostedt
2018-08-08 17:21                                                           ` Paul E. McKenney
2018-08-08 13:00                                         ` Paul E. McKenney
2018-08-08 14:10                                           ` Joel Fernandes
2018-08-08 14:49                                             ` Paul E. McKenney
2018-08-08 19:24                                               ` Joel Fernandes
2018-08-08 20:18                                                 ` Paul E. McKenney
2018-08-08 22:15                                                   ` Joel Fernandes
2018-08-08 22:47                                                     ` Paul E. McKenney
2018-08-09 12:18                                                       ` joel
2018-08-08 14:27                                           ` Steven Rostedt
2018-08-08 14:42                                             ` Paul E. McKenney
2018-08-08 15:27                                               ` Steven Rostedt
2018-08-08 16:03                                                 ` Paul E. McKenney
2018-08-02 14:55 ` [PATCH v12 0/3] " Masami Hiramatsu
2018-08-03  2:57   ` Joel Fernandes
2018-08-03  7:23     ` Masami Hiramatsu
2018-08-04  4:51       ` Joel Fernandes
2018-08-05 16:46         ` Joel Fernandes
2018-08-06  2:07           ` Masami Hiramatsu
2018-08-06 15:24             ` Joel Fernandes
2018-08-03  7:34     ` Masami Hiramatsu

Reply instructions:

You may reply publically 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=20180806155058.5ee875f4@gandalf.local.home \
    --to=rostedt@goodmis.org \
    --cc=boqun.feng@gmail.com \
    --cc=byungchul.park@lge.com \
    --cc=joel@joelfernandes.org \
    --cc=kernel-team@android.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mhiramat@kernel.org \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=tom.zanussi@linux.intel.com \
    /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

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org linux-kernel@archiver.kernel.org
	public-inbox-index lkml


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox