mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: peterz@infradead.org
To: Guenter Roeck <linux@roeck-us.net>
Cc: linux-kernel@vger.kernel.org, mingo@kernel.org, will@kernel.org,
	npiggin@gmail.com, elver@google.com, jgross@suse.com,
	paulmck@kernel.org, rostedt@goodmis.org, rjw@rjwysocki.net,
	joel@joelfernandes.org, svens@linux.ibm.com, tglx@linutronix.de,
	ulf.hansson@linaro.org, viresh.kumar@linaro.org,
	vincent.guittot@linaro.org
Subject: Re: [PATCH v2 11/11] lockdep,trace: Expose tracepoints
Date: Wed, 2 Sep 2020 10:56:36 +0200	[thread overview]
Message-ID: <20200902085636.GV1362448@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <20200902035146.GA45826@roeck-us.net>

On Tue, Sep 01, 2020 at 08:51:46PM -0700, Guenter Roeck wrote:

> [   27.056457] include/trace/events/lock.h:13 suspicious rcu_dereference_check() usage!

> [   27.057006] Hardware name: Generic OMAP3-GP (Flattened Device Tree)
> [   27.057098] [<c03125b4>] (unwind_backtrace) from [<c030c32c>] (show_stack+0x10/0x14)
> [   27.057189] [<c030c32c>] (show_stack) from [<c08e7a4c>] (dump_stack+0xd8/0xf8)
> [   27.057312] [<c08e7a4c>] (dump_stack) from [<c03b04bc>] (lock_acquire+0x4d8/0x4dc)
> [   27.057464] [<c03b04bc>] (lock_acquire) from [<c12457e8>] (_raw_spin_lock_irqsave+0x58/0x74)
> [   27.057617] [<c12457e8>] (_raw_spin_lock_irqsave) from [<c0338198>] (pwrdm_lock+0x10/0x18)
> [   27.057739] [<c0338198>] (pwrdm_lock) from [<c033a214>] (clkdm_deny_idle+0x10/0x24)
> [   27.057891] [<c033a214>] (clkdm_deny_idle) from [<c0332e10>] (omap3_enter_idle_bm+0xd4/0x1b8)
> [   27.058044] [<c0332e10>] (omap3_enter_idle_bm) from [<c0f740d4>] (cpuidle_enter_state+0x16c/0x620)

ARM cpuidle is a trainwreck :/

So it looks like we have:

 - clkdm_
 - pwrdm_
 - cpu_pm_
 - pm_runtime_

In that approximate order, and then there's the coupled idle muck.
Sometimes cpuidle core calls cpu_pm_*(), but mostly it's sprinkled
around in drivers.

How about we unconditionally kill tracing when RCU is idle? Yes this is
a hack, and we really should turn it into a WARN in due time.

The thing is, we're shutting down clock/power domains and god knows
what, the CPU is in a crap state, we'll not take interrupts, but tracing
must happen! Hell no.

Let's make the rule that if you want something traced, you get to pull
it out of the cpuidle driver and stick it in the generic code with a
CPUIDLE_FLAG, before rcu_idle_enter().

Totally untested patch below..

---

diff --git a/include/linux/preempt.h b/include/linux/preempt.h
index 7d9c1c0e149c..878bac893e41 100644
--- a/include/linux/preempt.h
+++ b/include/linux/preempt.h
@@ -27,17 +27,20 @@
  *         SOFTIRQ_MASK:	0x0000ff00
  *         HARDIRQ_MASK:	0x000f0000
  *             NMI_MASK:	0x00f00000
+ *         RCUIDLE_MASK:	0x01000000
  * PREEMPT_NEED_RESCHED:	0x80000000
  */
 #define PREEMPT_BITS	8
 #define SOFTIRQ_BITS	8
 #define HARDIRQ_BITS	4
 #define NMI_BITS	4
+#define RCUIDLE_BITS	1
 
 #define PREEMPT_SHIFT	0
 #define SOFTIRQ_SHIFT	(PREEMPT_SHIFT + PREEMPT_BITS)
 #define HARDIRQ_SHIFT	(SOFTIRQ_SHIFT + SOFTIRQ_BITS)
 #define NMI_SHIFT	(HARDIRQ_SHIFT + HARDIRQ_BITS)
+#define RCUIDLE_SHIFT	(NMI_SHIFT     + NMI_BITS)
 
 #define __IRQ_MASK(x)	((1UL << (x))-1)
 
@@ -45,11 +48,13 @@
 #define SOFTIRQ_MASK	(__IRQ_MASK(SOFTIRQ_BITS) << SOFTIRQ_SHIFT)
 #define HARDIRQ_MASK	(__IRQ_MASK(HARDIRQ_BITS) << HARDIRQ_SHIFT)
 #define NMI_MASK	(__IRQ_MASK(NMI_BITS)     << NMI_SHIFT)
+#define RCUIDLE_MASK	(__IRQ_MASK(RCUIDLE_BITS) << RCUIDLE_SHIFT)
 
 #define PREEMPT_OFFSET	(1UL << PREEMPT_SHIFT)
 #define SOFTIRQ_OFFSET	(1UL << SOFTIRQ_SHIFT)
 #define HARDIRQ_OFFSET	(1UL << HARDIRQ_SHIFT)
 #define NMI_OFFSET	(1UL << NMI_SHIFT)
+#define RCUIDLE_OFFSET	(1UL << RCUIDLE_SHIFT)
 
 #define SOFTIRQ_DISABLE_OFFSET	(2 * SOFTIRQ_OFFSET)
 
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index 598fec9f9dbf..997472c662c4 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -164,7 +164,7 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
 		void *__data;						\
 		int __maybe_unused __idx = 0;				\
 									\
-		if (!(cond))						\
+		if (!(cond) || (preempt_count() & RCUIDLE_MASK))	\
 			return;						\
 									\
 		/* srcu can't be used from NMI */			\
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 8ce77d9ac716..ad9fb4f12c63 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -269,6 +269,8 @@ static noinstr void rcu_dynticks_eqs_enter(void)
 	/* Better not have special action (TLB flush) pending! */
 	WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
 		     (seq & RCU_DYNTICK_CTRL_MASK));
+
+	__preempt_count_add(RCUIDLE_OFFSET);
 }
 
 /*
@@ -281,6 +283,8 @@ static noinstr void rcu_dynticks_eqs_exit(void)
 	struct rcu_data *rdp = this_cpu_ptr(&rcu_data);
 	int seq;
 
+	__preempt_count_sub(RCUIDLE_OFFSET);
+
 	/*
 	 * CPUs seeing atomic_add_return() must see prior idle sojourns,
 	 * and we also must force ordering with the next RCU read-side

  parent reply	other threads:[~2020-09-02  8:57 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-21  8:47 [PATCH v2 00/11] TRACE_IRQFLAGS wreckage Peter Zijlstra
2020-08-21  8:47 ` [PATCH v2 01/11] lockdep: Use raw_cpu_*() for per-cpu variables Peter Zijlstra
2020-08-27  1:05   ` Joel Fernandes
2020-08-27  7:54   ` [tip: locking/core] " tip-bot2 for Peter Zijlstra
2020-08-21  8:47 ` [PATCH v2 02/11] cpuidle: Fixup IRQ state Peter Zijlstra
2020-08-21 17:36   ` Rafael J. Wysocki
2020-08-27  1:06     ` Joel Fernandes
2020-08-27  7:54   ` [tip: locking/core] " tip-bot2 for Peter Zijlstra
2020-08-21  8:47 ` [PATCH v2 03/11] sched,idle,rcu: Push rcu_idle deeper into the idle path Peter Zijlstra
2020-08-27  1:18   ` Joel Fernandes
2020-08-27  1:24     ` Joel Fernandes
2020-08-27  7:47       ` peterz
2020-08-27  7:53         ` Thomas Gleixner
2020-08-27 22:30         ` Joel Fernandes
2020-09-02  7:08           ` peterz
2020-08-27  7:54   ` [tip: locking/core] " tip-bot2 for Peter Zijlstra
2020-08-21  8:47 ` [PATCH v2 04/11] cpuidle: Make CPUIDLE_FLAG_TLB_FLUSHED generic Peter Zijlstra
2020-08-27  7:54   ` [tip: locking/core] " tip-bot2 for Peter Zijlstra
2020-08-21  8:47 ` [PATCH v2 05/11] cpuidle: Move trace_cpu_idle() into generic code Peter Zijlstra
2020-08-27  7:54   ` [tip: locking/core] " tip-bot2 for Peter Zijlstra
2020-08-21  8:47 ` [PATCH v2 06/11] x86/entry: Remove unused THUNKs Peter Zijlstra
2020-08-27  7:54   ` [tip: locking/core] " tip-bot2 for Peter Zijlstra
2020-08-21  8:47 ` [PATCH v2 07/11] locking/lockdep: Cleanup Peter Zijlstra
2020-08-27  7:54   ` [tip: locking/core] " tip-bot2 for Peter Zijlstra
2020-08-21  8:47 ` [PATCH v2 08/11] nds32: Implement arch_irqs_disabled() Peter Zijlstra
2020-08-27  7:54   ` [tip: locking/core] " tip-bot2 for Peter Zijlstra
2020-08-21  8:47 ` [PATCH v2 09/11] arm64: " Peter Zijlstra
2020-08-27  7:54   ` [tip: locking/core] " tip-bot2 for Peter Zijlstra
2020-08-21  8:47 ` [PATCH v2 10/11] lockdep: Only trace IRQ edges Peter Zijlstra
2020-08-26  0:47   ` Thomas Gleixner
2020-09-02  4:21   ` Guenter Roeck
2020-09-02  9:09     ` peterz
2020-09-02  9:12       ` peterz
2020-09-02 13:48         ` Guenter Roeck
2020-09-08 14:22           ` peterz
2020-09-08 14:40             ` Guenter Roeck
2020-09-08 15:41               ` [PATCH] sparc64: Fix irqtrace warnings on Ultra-S peterz
2020-08-21  8:47 ` [PATCH v2 11/11] lockdep,trace: Expose tracepoints Peter Zijlstra
2020-08-27  7:54   ` [tip: locking/core] " tip-bot2 for Peter Zijlstra
2020-09-02  3:51   ` [PATCH v2 11/11] " Guenter Roeck
2020-09-02  7:24     ` peterz
2020-09-02 13:21       ` Guenter Roeck
2020-09-02  8:56     ` peterz [this message]
2020-09-02 13:57       ` Guenter Roeck
2020-09-03 14:00         ` peterz
2020-09-03 14:13           ` peterz
2020-09-03 15:19           ` Guenter Roeck
2020-09-03 15:36             ` peterz
2020-09-03 15:54               ` Paul E. McKenney
2020-09-03 18:11               ` Guenter Roeck
2020-08-21 12:58 ` [PATCH v2 00/11] TRACE_IRQFLAGS wreckage peterz
2020-08-26 10:16 ` [PATCH v2 12/11] mips: Implement arch_irqs_disabled() peterz
2020-08-27  7:54   ` [tip: locking/core] " tip-bot2 for Peter Zijlstra

Reply instructions:

You may reply publicly 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=20200902085636.GV1362448@hirez.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=elver@google.com \
    --cc=jgross@suse.com \
    --cc=joel@joelfernandes.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=mingo@kernel.org \
    --cc=npiggin@gmail.com \
    --cc=paulmck@kernel.org \
    --cc=rjw@rjwysocki.net \
    --cc=rostedt@goodmis.org \
    --cc=svens@linux.ibm.com \
    --cc=tglx@linutronix.de \
    --cc=ulf.hansson@linaro.org \
    --cc=vincent.guittot@linaro.org \
    --cc=viresh.kumar@linaro.org \
    --cc=will@kernel.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome