[PATCH tip/core/rcu 0/6] Check for use of RCU from dyntick-idle mode

[PATCH tip/core/rcu 0/6] Check for use of RCU from dyntick-idle mode

[Paul E. McKenney]

Hello!

This set of patches adds checks for use of RCU from "extended quiescent
states" such as dyntick-idle mode.  Such use is grossly illegal because
RCU by definition ignores CPUs that are in extended quiescent states.
In the case of dyntick-idle mode, the only way for RCU to avoid ignoring
such CPUs would be to wake them up periodically, which would defeat the
whole purpose of dyntick-idle mode.

The good news is that Frederic got this effort started.  The bad news is
that there are several cases where RCU read-side critical sections appear
in dyntick-idle mode.  Frederic is working on a solution, but because there
is no point in giving warnings in cases where there is no known fix, I will
not push these patches until a fix is available.  (The most likely fix is
to move the code that informs RCU of entry to and exit from dyntick-idle
mode into the arch-specific idle loops, which would legitimize current RCU
usage -- and potentially simplify RCU's quiescent-state detection.)

The individual patches are as follows:

1.	Make rcu_read_lock_held(), rcu_read_lock_sched_held(),
	and rcu_read_lock_bh_held() invoke a new rcu_check_extended_qs()
	function to check for calls from extended quiescent states,
	courtesy of Frederic Weisbecker.
2.	Make lockdep RCU splats call out use from within extended
	quiescent states, also courtesy of Frederic Weisbecker.
3.	Make rcu_read_lock(), rcu_read_lock_sched(), and
	rcu_read_lock_bh() also check for (illegal) use from within
	an extended quiescent state, again courtesy of Frederic
	Weisbecker.
4.	Remove an unnecessary level of abstraction from the PROVE_RCU
	checking.
5.	Warn when srcu_read_lock() and srcu_read_lock_held() are called
	from within an extended quiescent state.
6.	Make srcu_read_lock_held() invoke the debug_lockdep_rcu_enabled()
	common function that determines whether or not RCU lockdep splats
	are appropriate.

These changes are also available on the subject-to-rebase branch in the
git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-2.6-rcu.git eqscheck.2011.07.08a

							Thanx, Paul

------------------------------------------------------------------------

 b/include/linux/rcupdate.h |   36 +++++++++++++++
 b/include/linux/srcu.h     |   25 ++++++----
 b/kernel/lockdep.c         |   20 ++++++++
 b/kernel/rcupdate.c        |   17 ++++++-
 b/kernel/rcutiny.c         |   14 ++++++
 b/kernel/rcutree.c         |   16 ++++++
 include/linux/rcupdate.h   |  105 +++++++++++++++++++++------------------------
 include/linux/srcu.h       |    5 +-
 8 files changed, 171 insertions(+), 67 deletions(-)

[PATCH RFC tip/core/rcu 1/6] rcu: Detect illegal rcu dereference in extended quiescent state

[Paul E. McKenney]

From: Frederic Weisbecker <fweisbec@gmail.com>

Report that none of the rcu read lock maps are held while in an RCU
extended quiescent state (in this case, the RCU extended quiescent state
is dyntick-idle mode). This helps detect any use of rcu_dereference()
and friends from within dyntick-idle mode.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Lai Jiangshan <laijs@cn.fujitsu.com>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 include/linux/rcupdate.h |   36 ++++++++++++++++++++++++++++++++++++
 kernel/rcupdate.c        |   17 ++++++++++++++++-
 kernel/rcutiny.c         |   14 ++++++++++++++
 kernel/rcutree.c         |   16 ++++++++++++++++
 4 files changed, 82 insertions(+), 1 deletions(-)

diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 99f9aa7..0a33075 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -179,6 +179,14 @@ static inline void destroy_rcu_head_on_stack(struct rcu_head *head)
 }
 #endif	/* #else !CONFIG_DEBUG_OBJECTS_RCU_HEAD */
 
+
+#if defined(CONFIG_PROVE_RCU) && defined(CONFIG_NO_HZ)
+extern bool rcu_check_extended_qs(void);
+#else
+static inline bool rcu_check_extended_qs(void) { return false; }
+#endif
+
+
 #ifdef CONFIG_DEBUG_LOCK_ALLOC
 
 extern struct lockdep_map rcu_lock_map;
@@ -210,11 +218,25 @@ extern int debug_lockdep_rcu_enabled(void);
  *
  * Checks debug_lockdep_rcu_enabled() to prevent false positives during boot
  * and while lockdep is disabled.
+ *
+ * Note that if the CPU is in an extended quiescent state, for example,
+ * if the CPU is in dyntick-idle mode, then rcu_read_lock_held() returns
+ * false even if the CPU did an rcu_read_lock().  The reason for this is
+ * that RCU ignores CPUs that are in extended quiescent states, so such
+ * a CPU is effectively never in an RCU read-side critical section
+ * regardless of what RCU primitives it invokes.  This state of affairs
+ * is required -- RCU would otherwise need to periodically wake up
+ * dyntick-idle CPUs, which would defeat the whole purpose of dyntick-idle
+ * mode.
  */
 static inline int rcu_read_lock_held(void)
 {
 	if (!debug_lockdep_rcu_enabled())
 		return 1;
+
+	if (rcu_check_extended_qs())
+		return 0;
+
 	return lock_is_held(&rcu_lock_map);
 }
 
@@ -238,6 +260,16 @@ extern int rcu_read_lock_bh_held(void);
  *
  * Check debug_lockdep_rcu_enabled() to prevent false positives during boot
  * and while lockdep is disabled.
+ *
+ * Note that if the CPU is in an extended quiescent state, for example,
+ * if the CPU is in dyntick-idle mode, then rcu_read_lock_held() returns
+ * false even if the CPU did an rcu_read_lock().  The reason for this is
+ * that RCU ignores CPUs that are in extended quiescent states, so such
+ * a CPU is effectively never in an RCU read-side critical section
+ * regardless of what RCU primitives it invokes.  This state of affairs
+ * is required -- RCU would otherwise need to periodically wake up
+ * dyntick-idle CPUs, which would defeat the whole purpose of dyntick-idle
+ * mode.
  */
 #ifdef CONFIG_PREEMPT
 static inline int rcu_read_lock_sched_held(void)
@@ -246,6 +278,10 @@ static inline int rcu_read_lock_sched_held(void)
 
 	if (!debug_lockdep_rcu_enabled())
 		return 1;
+
+	if (rcu_check_extended_qs())
+		return 0;
+
 	if (debug_locks)
 		lockdep_opinion = lock_is_held(&rcu_sched_lock_map);
 	return lockdep_opinion || preempt_count() != 0 || irqs_disabled();
diff --git a/kernel/rcupdate.c b/kernel/rcupdate.c
index 7784bd2..a0e7e59 100644
--- a/kernel/rcupdate.c
+++ b/kernel/rcupdate.c
@@ -82,12 +82,27 @@ EXPORT_SYMBOL_GPL(debug_lockdep_rcu_enabled);
  * that require that they be called within an RCU read-side critical
  * section.
  *
- * Check debug_lockdep_rcu_enabled() to prevent false positives during boot.
+ * Check debug_lockdep_rcu_enabled() to prevent false positives during boot
+ * and while lockdep is disabled.
+ *
+ * Note that if the CPU is in an extended quiescent state, for example,
+ * if the CPU is in dyntick-idle mode, then rcu_read_lock_held() returns
+ * false even if the CPU did an rcu_read_lock().  The reason for this is
+ * that RCU ignores CPUs that are in extended quiescent states, so such
+ * a CPU is effectively never in an RCU read-side critical section
+ * regardless of what RCU primitives it invokes.  This state of affairs
+ * is required -- RCU would otherwise need to periodically wake up
+ * dyntick-idle CPUs, which would defeat the whole purpose of dyntick-idle
+ * mode.
  */
 int rcu_read_lock_bh_held(void)
 {
 	if (!debug_lockdep_rcu_enabled())
 		return 1;
+
+	if (rcu_check_extended_qs())
+		return 0;
+
 	return in_softirq() || irqs_disabled();
 }
 EXPORT_SYMBOL_GPL(rcu_read_lock_bh_held);
diff --git a/kernel/rcutiny.c b/kernel/rcutiny.c
index 7bbac7d..d01d390 100644
--- a/kernel/rcutiny.c
+++ b/kernel/rcutiny.c
@@ -77,6 +77,20 @@ void rcu_exit_nohz(void)
 	rcu_dynticks_nesting++;
 }
 
+
+#ifdef CONFIG_PROVE_RCU
+
+bool rcu_check_extended_qs(void)
+{
+	if (!rcu_dynticks_nesting)
+		return true;
+
+	return false;
+}
+EXPORT_SYMBOL_GPL(rcu_check_extended_qs);
+
+#endif
+
 #endif /* #ifdef CONFIG_NO_HZ */
 
 /*
diff --git a/kernel/rcutree.c b/kernel/rcutree.c
index 7e59ffb..485cdc9 100644
--- a/kernel/rcutree.c
+++ b/kernel/rcutree.c
@@ -438,6 +438,22 @@ void rcu_irq_exit(void)
 	rcu_enter_nohz();
 }
 
+#ifdef CONFIG_PROVE_RCU
+
+bool rcu_check_extended_qs(void)
+{
+	struct rcu_dynticks *rdtp;
+
+	rdtp = &per_cpu(rcu_dynticks, raw_smp_processor_id());
+	if (atomic_read(&rdtp->dynticks) & 0x1)
+		return false;
+
+	return true;
+}
+EXPORT_SYMBOL_GPL(rcu_check_extended_qs);
+
+#endif /* CONFIG_PROVE_RCU */
+
 #ifdef CONFIG_SMP
 
 /*
-- 
1.7.3.2

[PATCH RFC tip/core/rcu 2/6] rcu: Inform the user about dynticks-idle mode on PROVE_RCU warning

[Paul E. McKenney]

From: Frederic Weisbecker <fweisbec@gmail.com>

Inform the user if an RCU usage error is detected by lockdep while in
an extended quiescent state (in this case, dyntick-idle mode).  This
is accomplished by adding a line to the RCU lockdep splat indicating
whether or not the splat occurred in dyntick-idle mode.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Lai Jiangshan <laijs@cn.fujitsu.com>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 kernel/lockdep.c |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/kernel/lockdep.c b/kernel/lockdep.c
index 298c927..f2bff04 100644
--- a/kernel/lockdep.c
+++ b/kernel/lockdep.c
@@ -3998,6 +3998,26 @@ void lockdep_rcu_dereference(const char *file, const int line)
 			file, line);
 	printk("\nother info that might help us debug this:\n\n");
 	printk("\nrcu_scheduler_active = %d, debug_locks = %d\n", rcu_scheduler_active, debug_locks);
+
+	/*
+	 * If a CPU is in dyntick-idle mode (CONFIG_NO_HZ), then RCU
+	 * considers that CPU to be in an "extended quiescent state",
+	 * which means that RCU will be completely ignoring that CPU.
+	 * Therefore, rcu_read_lock() and friends have absolutely no
+	 * effect on a dyntick-idle CPU.  In other words, even if a
+	 * dyntick-idle CPU has called rcu_read_lock(), RCU might well
+	 * delete data structures out from under it.  RCU really has no
+	 * choice here: if it were to consult the CPU, that would wake
+	 * the CPU up, and the whole point of dyntick-idle mode is to
+	 * allow CPUs to enter extremely deep sleep states.
+	 *
+	 * So complain bitterly if someone does call rcu_read_lock(),
+	 * rcu_read_lock_bh() and so on from extended quiescent states
+	 * such as dyntick-idle mode.
+	 */
+	if (rcu_check_extended_qs())
+		printk("RCU used illegally from extended quiescent state!\n");
+
 	lockdep_print_held_locks(curr);
 	printk("\nstack backtrace:\n");
 	dump_stack();
-- 
1.7.3.2

[PATCH RFC tip/core/rcu 3/6] rcu: Warn when rcu_read_lock() is used in extended quiescent state

[Paul E. McKenney]

From: Frederic Weisbecker <fweisbec@gmail.com>

We are currently able to detect uses of rcu_dereference_check() inside
extended quiescent states (such as dyntick-idle mode). But rcu_read_lock()
and friends can be used without rcu_dereference(), so that the earlier
commit checking for use of rcu_dereference() and friends while in
dyntick-idle mode miss some error conditions.  This commit therefore adds
dyntick-idle checking to rcu_read_lock() and friends.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Lai Jiangshan <laijs@cn.fujitsu.com>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 include/linux/rcupdate.h |   52 +++++++++++++++++++++++++++++++++++++--------
 1 files changed, 42 insertions(+), 10 deletions(-)

diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 0a33075..bf2c316 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -189,21 +189,53 @@ static inline bool rcu_check_extended_qs(void) { return false; }
 
 #ifdef CONFIG_DEBUG_LOCK_ALLOC
 
+static inline void rcu_lock_acquire(struct lockdep_map *map)
+{
+	WARN_ON_ONCE(rcu_check_extended_qs());
+	lock_acquire(map, 0, 0, 2, 1, NULL, _THIS_IP_);
+}
+
+static inline void rcu_lock_release(struct lockdep_map *map)
+{
+	WARN_ON_ONCE(rcu_check_extended_qs());
+	lock_release(map, 1, _THIS_IP_);
+}
+
 extern struct lockdep_map rcu_lock_map;
-# define rcu_read_acquire() \
-		lock_acquire(&rcu_lock_map, 0, 0, 2, 1, NULL, _THIS_IP_)
-# define rcu_read_release()	lock_release(&rcu_lock_map, 1, _THIS_IP_)
+
+static inline void rcu_read_acquire(void)
+{
+	rcu_lock_acquire(&rcu_lock_map);
+}
+
+static inline void rcu_read_release(void)
+{
+	rcu_lock_release(&rcu_lock_map);
+}
 
 extern struct lockdep_map rcu_bh_lock_map;
-# define rcu_read_acquire_bh() \
-		lock_acquire(&rcu_bh_lock_map, 0, 0, 2, 1, NULL, _THIS_IP_)
-# define rcu_read_release_bh()	lock_release(&rcu_bh_lock_map, 1, _THIS_IP_)
+
+static inline void rcu_read_acquire_bh(void)
+{
+	rcu_lock_acquire(&rcu_bh_lock_map);
+}
+
+static inline void rcu_read_release_bh(void)
+{
+	rcu_lock_release(&rcu_bh_lock_map);
+}
 
 extern struct lockdep_map rcu_sched_lock_map;
-# define rcu_read_acquire_sched() \
-		lock_acquire(&rcu_sched_lock_map, 0, 0, 2, 1, NULL, _THIS_IP_)
-# define rcu_read_release_sched() \
-		lock_release(&rcu_sched_lock_map, 1, _THIS_IP_)
+
+static inline void rcu_read_acquire_sched(void)
+{
+	rcu_lock_acquire(&rcu_sched_lock_map);
+}
+
+static inline void rcu_read_release_sched(void)
+{
+	rcu_lock_release(&rcu_sched_lock_map);
+}
 
 extern int debug_lockdep_rcu_enabled(void);
 
-- 
1.7.3.2

[PATCH RFC tip/core/rcu 4/6] rcu: Remove one layer of abstraction from PROVE_RCU checking

[Paul E. McKenney]

Simplify things a bit by substituting the definitions of the single-line
rcu_read_acquire(), rcu_read_release(), rcu_read_acquire_bh(),
rcu_read_release_bh(), rcu_read_acquire_sched(), and
rcu_read_release_sched() functions at their call points.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 include/linux/rcupdate.h |   53 +++++++---------------------------------------
 1 files changed, 8 insertions(+), 45 deletions(-)

diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index bf2c316..3fc3aeb 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -202,41 +202,8 @@ static inline void rcu_lock_release(struct lockdep_map *map)
 }
 
 extern struct lockdep_map rcu_lock_map;
-
-static inline void rcu_read_acquire(void)
-{
-	rcu_lock_acquire(&rcu_lock_map);
-}
-
-static inline void rcu_read_release(void)
-{
-	rcu_lock_release(&rcu_lock_map);
-}
-
 extern struct lockdep_map rcu_bh_lock_map;
-
-static inline void rcu_read_acquire_bh(void)
-{
-	rcu_lock_acquire(&rcu_bh_lock_map);
-}
-
-static inline void rcu_read_release_bh(void)
-{
-	rcu_lock_release(&rcu_bh_lock_map);
-}
-
 extern struct lockdep_map rcu_sched_lock_map;
-
-static inline void rcu_read_acquire_sched(void)
-{
-	rcu_lock_acquire(&rcu_sched_lock_map);
-}
-
-static inline void rcu_read_release_sched(void)
-{
-	rcu_lock_release(&rcu_sched_lock_map);
-}
-
 extern int debug_lockdep_rcu_enabled(void);
 
 /**
@@ -327,12 +294,8 @@ static inline int rcu_read_lock_sched_held(void)
 
 #else /* #ifdef CONFIG_DEBUG_LOCK_ALLOC */
 
-# define rcu_read_acquire()		do { } while (0)
-# define rcu_read_release()		do { } while (0)
-# define rcu_read_acquire_bh()		do { } while (0)
-# define rcu_read_release_bh()		do { } while (0)
-# define rcu_read_acquire_sched()	do { } while (0)
-# define rcu_read_release_sched()	do { } while (0)
+# define rcu_lock_acquire(a)		do { } while (0)
+# define rcu_lock_release(a)		do { } while (0)
 
 static inline int rcu_read_lock_held(void)
 {
@@ -659,7 +622,7 @@ static inline void rcu_read_lock(void)
 {
 	__rcu_read_lock();
 	__acquire(RCU);
-	rcu_read_acquire();
+	rcu_lock_acquire(&rcu_lock_map);
 }
 
 /*
@@ -679,7 +642,7 @@ static inline void rcu_read_lock(void)
  */
 static inline void rcu_read_unlock(void)
 {
-	rcu_read_release();
+	rcu_lock_release(&rcu_lock_map);
 	__release(RCU);
 	__rcu_read_unlock();
 }
@@ -700,7 +663,7 @@ static inline void rcu_read_lock_bh(void)
 {
 	__rcu_read_lock_bh();
 	__acquire(RCU_BH);
-	rcu_read_acquire_bh();
+	rcu_lock_acquire(&rcu_bh_lock_map);
 }
 
 /*
@@ -710,7 +673,7 @@ static inline void rcu_read_lock_bh(void)
  */
 static inline void rcu_read_unlock_bh(void)
 {
-	rcu_read_release_bh();
+	rcu_lock_release(&rcu_bh_lock_map);
 	__release(RCU_BH);
 	__rcu_read_unlock_bh();
 }
@@ -727,7 +690,7 @@ static inline void rcu_read_lock_sched(void)
 {
 	preempt_disable();
 	__acquire(RCU_SCHED);
-	rcu_read_acquire_sched();
+	rcu_lock_acquire(&rcu_sched_lock_map);
 }
 
 /* Used by lockdep and tracing: cannot be traced, cannot call lockdep. */
@@ -744,7 +707,7 @@ static inline notrace void rcu_read_lock_sched_notrace(void)
  */
 static inline void rcu_read_unlock_sched(void)
 {
-	rcu_read_release_sched();
+	rcu_lock_release(&rcu_sched_lock_map);
 	__release(RCU_SCHED);
 	preempt_enable();
 }
-- 
1.7.3.2

[PATCH RFC tip/core/rcu 5/6] rcu: Warn when srcu_read_lock() is used in an extended quiescent state

[Paul E. McKenney]

Catch SRCU up to the other variants of RCU by making PROVE_RCU
complain if either srcu_read_lock() or srcu_read_lock_held() are
used from within dyntick-idle mode.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 include/linux/srcu.h |   25 +++++++++++++++----------
 1 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/include/linux/srcu.h b/include/linux/srcu.h
index 58971e8..fcbaee7 100644
--- a/include/linux/srcu.h
+++ b/include/linux/srcu.h
@@ -28,6 +28,7 @@
 #define _LINUX_SRCU_H
 
 #include <linux/mutex.h>
+#include <linux/rcupdate.h>
 
 struct srcu_struct_array {
 	int c[2];
@@ -60,18 +61,10 @@ int __init_srcu_struct(struct srcu_struct *sp, const char *name,
 	__init_srcu_struct((sp), #sp, &__srcu_key); \
 })
 
-# define srcu_read_acquire(sp) \
-		lock_acquire(&(sp)->dep_map, 0, 0, 2, 1, NULL, _THIS_IP_)
-# define srcu_read_release(sp) \
-		lock_release(&(sp)->dep_map, 1, _THIS_IP_)
-
 #else /* #ifdef CONFIG_DEBUG_LOCK_ALLOC */
 
 int init_srcu_struct(struct srcu_struct *sp);
 
-# define srcu_read_acquire(sp)  do { } while (0)
-# define srcu_read_release(sp)  do { } while (0)
-
 #endif /* #else #ifdef CONFIG_DEBUG_LOCK_ALLOC */
 
 void cleanup_srcu_struct(struct srcu_struct *sp);
@@ -90,11 +83,23 @@ long srcu_batches_completed(struct srcu_struct *sp);
  * read-side critical section.  In absence of CONFIG_DEBUG_LOCK_ALLOC,
  * this assumes we are in an SRCU read-side critical section unless it can
  * prove otherwise.
+ *
+ * Note that if the CPU is in an extended quiescent state, for example,
+ * if the CPU is in dyntick-idle mode, then rcu_read_lock_held() returns
+ * false even if the CPU did an rcu_read_lock().  The reason for this is
+ * that RCU ignores CPUs that are in extended quiescent states, so such
+ * a CPU is effectively never in an RCU read-side critical section
+ * regardless of what RCU primitives it invokes.  This state of affairs
+ * is required -- RCU would otherwise need to periodically wake up
+ * dyntick-idle CPUs, which would defeat the whole purpose of dyntick-idle
+ * mode.
  */
 static inline int srcu_read_lock_held(struct srcu_struct *sp)
 {
 	if (debug_locks)
 		return lock_is_held(&sp->dep_map);
+	if (rcu_check_extended_qs())
+		return 0;
 	return 1;
 }
 
@@ -150,7 +155,7 @@ static inline int srcu_read_lock(struct srcu_struct *sp) __acquires(sp)
 {
 	int retval = __srcu_read_lock(sp);
 
-	srcu_read_acquire(sp);
+	rcu_lock_acquire(&(sp)->dep_map);
 	return retval;
 }
 
@@ -164,7 +169,7 @@ static inline int srcu_read_lock(struct srcu_struct *sp) __acquires(sp)
 static inline void srcu_read_unlock(struct srcu_struct *sp, int idx)
 	__releases(sp)
 {
-	srcu_read_release(sp);
+	rcu_lock_release(&(sp)->dep_map);
 	__srcu_read_unlock(sp, idx);
 }
 
-- 
1.7.3.2

[PATCH RFC tip/core/rcu 6/6] rcu: Make srcu_read_lock_held() call common lockdep-enabled function

[Paul E. McKenney]

A common debug_lockdep_rcu_enabled() function is used to check whether
RCU lockdep splats should be reported, but srcu_read_lock() does not
use it.  This commit therefore brings srcu_read_lock_held() up to date.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 include/linux/srcu.h |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/include/linux/srcu.h b/include/linux/srcu.h
index fcbaee7..54a70b7 100644
--- a/include/linux/srcu.h
+++ b/include/linux/srcu.h
@@ -84,6 +84,9 @@ long srcu_batches_completed(struct srcu_struct *sp);
  * this assumes we are in an SRCU read-side critical section unless it can
  * prove otherwise.
  *
+ * Checks debug_lockdep_rcu_enabled() to prevent false positives during boot
+ * and while lockdep is disabled.
+ *
  * Note that if the CPU is in an extended quiescent state, for example,
  * if the CPU is in dyntick-idle mode, then rcu_read_lock_held() returns
  * false even if the CPU did an rcu_read_lock().  The reason for this is
@@ -96,7 +99,7 @@ long srcu_batches_completed(struct srcu_struct *sp);
  */
 static inline int srcu_read_lock_held(struct srcu_struct *sp)
 {
-	if (debug_locks)
+	if (!debug_lockdep_rcu_enabled())
 		return lock_is_held(&sp->dep_map);
 	if (rcu_check_extended_qs())
 		return 0;
-- 
1.7.3.2

Re: [PATCH tip/core/rcu 0/6] Check for use of RCU from dyntick-idle mode

[Frederic Weisbecker]

On Fri, Jul 08, 2011 at 08:43:31AM -0700, Paul E. McKenney wrote:
> Hello!
> 
> This set of patches adds checks for use of RCU from "extended quiescent
> states" such as dyntick-idle mode.  Such use is grossly illegal because
> RCU by definition ignores CPUs that are in extended quiescent states.
> In the case of dyntick-idle mode, the only way for RCU to avoid ignoring
> such CPUs would be to wake them up periodically, which would defeat the
> whole purpose of dyntick-idle mode.
> 
> The good news is that Frederic got this effort started.  The bad news is
> that there are several cases where RCU read-side critical sections appear
> in dyntick-idle mode.

Ok, let me send to you the patch that splits up rcu/tickless logic and I'll try
to fix up what you reported to me in ppc.

BTW, are you aware of other cases? You mentioned "several" :)

Re: [PATCH tip/core/rcu 0/6] Check for use of RCU from dyntick-idle mode

[Paul E. McKenney]

On Mon, Jul 11, 2011 at 06:03:32PM +0200, Frederic Weisbecker wrote:
> On Fri, Jul 08, 2011 at 08:43:31AM -0700, Paul E. McKenney wrote:
> > Hello!
> > 
> > This set of patches adds checks for use of RCU from "extended quiescent
> > states" such as dyntick-idle mode.  Such use is grossly illegal because
> > RCU by definition ignores CPUs that are in extended quiescent states.
> > In the case of dyntick-idle mode, the only way for RCU to avoid ignoring
> > such CPUs would be to wake them up periodically, which would defeat the
> > whole purpose of dyntick-idle mode.
> > 
> > The good news is that Frederic got this effort started.  The bad news is
> > that there are several cases where RCU read-side critical sections appear
> > in dyntick-idle mode.
> 
> Ok, let me send to you the patch that splits up rcu/tickless logic and I'll try
> to fix up what you reported to me in ppc.

Very good, thank you!

> BTW, are you aware of other cases? You mentioned "several" :)

PowerPC's hypercall-exit trace event will also cause this complaint.
Plus I thought you saw some others.

							Thanx, Paul

Re: [PATCH tip/core/rcu 0/6] Check for use of RCU from dyntick-idle mode

[Frederic Weisbecker]

On Mon, Jul 11, 2011 at 09:38:48AM -0700, Paul E. McKenney wrote:
> On Mon, Jul 11, 2011 at 06:03:32PM +0200, Frederic Weisbecker wrote:
> > On Fri, Jul 08, 2011 at 08:43:31AM -0700, Paul E. McKenney wrote:
> > > Hello!
> > > 
> > > This set of patches adds checks for use of RCU from "extended quiescent
> > > states" such as dyntick-idle mode.  Such use is grossly illegal because
> > > RCU by definition ignores CPUs that are in extended quiescent states.
> > > In the case of dyntick-idle mode, the only way for RCU to avoid ignoring
> > > such CPUs would be to wake them up periodically, which would defeat the
> > > whole purpose of dyntick-idle mode.
> > > 
> > > The good news is that Frederic got this effort started.  The bad news is
> > > that there are several cases where RCU read-side critical sections appear
> > > in dyntick-idle mode.
> > 
> > Ok, let me send to you the patch that splits up rcu/tickless logic and I'll try
> > to fix up what you reported to me in ppc.
> 
> Very good, thank you!
> 
> > BTW, are you aware of other cases? You mentioned "several" :)
> 
> PowerPC's hypercall-exit trace event will also cause this complaint.

Ok looking at this.

> Plus I thought you saw some others.

Nope, mine were spurious. In my v1 rcu_dereference_check warned if rcu read lock
wasn't held but didn't handle the rest of the conditional (another lock held or
simply 1 in rcu_dereference_raw()).

In the v3, the one you applied, they legitimately disappeared.

Re: [PATCH tip/core/rcu 0/6] Check for use of RCU from dyntick-idle mode

[Paul E. McKenney]

On Mon, Jul 11, 2011 at 06:44:08PM +0200, Frederic Weisbecker wrote:
> On Mon, Jul 11, 2011 at 09:38:48AM -0700, Paul E. McKenney wrote:
> > On Mon, Jul 11, 2011 at 06:03:32PM +0200, Frederic Weisbecker wrote:
> > > On Fri, Jul 08, 2011 at 08:43:31AM -0700, Paul E. McKenney wrote:
> > > > Hello!
> > > > 
> > > > This set of patches adds checks for use of RCU from "extended quiescent
> > > > states" such as dyntick-idle mode.  Such use is grossly illegal because
> > > > RCU by definition ignores CPUs that are in extended quiescent states.
> > > > In the case of dyntick-idle mode, the only way for RCU to avoid ignoring
> > > > such CPUs would be to wake them up periodically, which would defeat the
> > > > whole purpose of dyntick-idle mode.
> > > > 
> > > > The good news is that Frederic got this effort started.  The bad news is
> > > > that there are several cases where RCU read-side critical sections appear
> > > > in dyntick-idle mode.
> > > 
> > > Ok, let me send to you the patch that splits up rcu/tickless logic and I'll try
> > > to fix up what you reported to me in ppc.
> > 
> > Very good, thank you!
> > 
> > > BTW, are you aware of other cases? You mentioned "several" :)
> > 
> > PowerPC's hypercall-exit trace event will also cause this complaint.
> 
> Ok looking at this.

Oh, and I removed some of my RCU dyntick-idle trace events because
they triggered this warning.  This would of course be a problem regardless
of where the RCU dyntick-idle APIs were called from, so I just took the
approach of more carefully placing the trace events.

> > Plus I thought you saw some others.
> 
> Nope, mine were spurious. In my v1 rcu_dereference_check warned if rcu read lock
> wasn't held but didn't handle the rest of the conditional (another lock held or
> simply 1 in rcu_dereference_raw()).
> 
> In the v3, the one you applied, they legitimately disappeared.

OK, good to know!

							Thanx, Paul

Re: [PATCH tip/core/rcu 0/6] Check for use of RCU from dyntick-idle mode

[Frederic Weisbecker]

On Mon, Jul 11, 2011 at 10:00:14AM -0700, Paul E. McKenney wrote:
> On Mon, Jul 11, 2011 at 06:44:08PM +0200, Frederic Weisbecker wrote:
> > On Mon, Jul 11, 2011 at 09:38:48AM -0700, Paul E. McKenney wrote:
> > > On Mon, Jul 11, 2011 at 06:03:32PM +0200, Frederic Weisbecker wrote:
> > > > On Fri, Jul 08, 2011 at 08:43:31AM -0700, Paul E. McKenney wrote:
> > > > > Hello!
> > > > > 
> > > > > This set of patches adds checks for use of RCU from "extended quiescent
> > > > > states" such as dyntick-idle mode.  Such use is grossly illegal because
> > > > > RCU by definition ignores CPUs that are in extended quiescent states.
> > > > > In the case of dyntick-idle mode, the only way for RCU to avoid ignoring
> > > > > such CPUs would be to wake them up periodically, which would defeat the
> > > > > whole purpose of dyntick-idle mode.
> > > > > 
> > > > > The good news is that Frederic got this effort started.  The bad news is
> > > > > that there are several cases where RCU read-side critical sections appear
> > > > > in dyntick-idle mode.
> > > > 
> > > > Ok, let me send to you the patch that splits up rcu/tickless logic and I'll try
> > > > to fix up what you reported to me in ppc.
> > > 
> > > Very good, thank you!
> > > 
> > > > BTW, are you aware of other cases? You mentioned "several" :)
> > > 
> > > PowerPC's hypercall-exit trace event will also cause this complaint.
> > 
> > Ok looking at this.
> 
> Oh, and I removed some of my RCU dyntick-idle trace events because
> they triggered this warning.  This would of course be a problem regardless
> of where the RCU dyntick-idle APIs were called from, so I just took the
> approach of more carefully placing the trace events.
> 
> > > Plus I thought you saw some others.
> > 
> > Nope, mine were spurious. In my v1 rcu_dereference_check warned if rcu read lock
> > wasn't held but didn't handle the rest of the conditional (another lock held or
> > simply 1 in rcu_dereference_raw()).
> > 
> > In the v3, the one you applied, they legitimately disappeared.
> 
> OK, good to know!

Ah, actually:

[    0.649412] WARNING: at include/linux/rcupdate.h:262 __atomic_notifier_call_chain+0xf8/0x110()
[    0.649414] Hardware name: AMD690VM-FMH
[    0.649415] Modules linked in: [<ffffffff81051ed5>] warn_slowpath_null+0x15/0x20
[    0.649420] 
[    0.649422] Pid: 0, comm: kworker/0:1 Tainted: G        W   3.0.0-rc6+ #250
[    0.649433] Call Trace:
[    0.649437]  [<ffffffff81051e8a>] warn_slowpath_common+0x7a/0xb0
[    0.649441]  [<ffffffff81051ed5>] warn_slowpath_null+0x15/0x20
[    0.649449]  [<ffffffff817da5c8>] __atomic_notifier_call_chain+0xf8/0x110
[    0.649468]  [<ffffffff817da5c8>] __atomic_notifier_call_chain+0xf8/0x110
[    0.649480]  [<ffffffff810018a0>] enter_idle+0x20/0x30
[    0.649483]  [<ffffffff817da5f1>] atomic_notifier_call_chain+0x11/0x20
[    0.649487]  [<ffffffff81001995>] cpu_idle+0xa5/0x110
[    0.649494]  [<ffffffff817ce3d5>] start_secondary+0x1df/0x1e6
[    0.649500] ---[ end trace f17e946d22a56016 ]---