* [PATCH RFC tip/core/rcu 1/6] rcu: Detect illegal rcu dereference in extended quiescent state
2011-07-08 15:43 [PATCH tip/core/rcu 0/6] Check for use of RCU from dyntick-idle mode Paul E. McKenney
@ 2011-07-08 15:43 ` Paul E. McKenney
2011-07-08 15:43 ` [PATCH RFC tip/core/rcu 2/6] rcu: Inform the user about dynticks-idle mode on PROVE_RCU warning Paul E. McKenney
` (5 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Paul E. McKenney @ 2011-07-08 15:43 UTC (permalink / raw)
To: linux-kernel
Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, niv, tglx,
peterz, rostedt, Valdis.Kletnieks, dhowells, eric.dumazet,
darren, patches, Frederic Weisbecker, Paul E. McKenney,
Peter Zijlstra
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH RFC tip/core/rcu 2/6] rcu: Inform the user about dynticks-idle mode on PROVE_RCU warning
2011-07-08 15:43 [PATCH tip/core/rcu 0/6] Check for use of RCU from dyntick-idle mode Paul E. McKenney
2011-07-08 15:43 ` [PATCH RFC tip/core/rcu 1/6] rcu: Detect illegal rcu dereference in extended quiescent state Paul E. McKenney
@ 2011-07-08 15:43 ` Paul E. McKenney
2011-07-08 15:43 ` [PATCH RFC tip/core/rcu 3/6] rcu: Warn when rcu_read_lock() is used in extended quiescent state Paul E. McKenney
` (4 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Paul E. McKenney @ 2011-07-08 15:43 UTC (permalink / raw)
To: linux-kernel
Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, niv, tglx,
peterz, rostedt, Valdis.Kletnieks, dhowells, eric.dumazet,
darren, patches, Frederic Weisbecker, Paul E. McKenney,
Peter Zijlstra
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH RFC tip/core/rcu 3/6] rcu: Warn when rcu_read_lock() is used in extended quiescent state
2011-07-08 15:43 [PATCH tip/core/rcu 0/6] Check for use of RCU from dyntick-idle mode Paul E. McKenney
2011-07-08 15:43 ` [PATCH RFC tip/core/rcu 1/6] rcu: Detect illegal rcu dereference in extended quiescent state Paul E. McKenney
2011-07-08 15:43 ` [PATCH RFC tip/core/rcu 2/6] rcu: Inform the user about dynticks-idle mode on PROVE_RCU warning Paul E. McKenney
@ 2011-07-08 15:43 ` Paul E. McKenney
2011-07-08 15:43 ` [PATCH RFC tip/core/rcu 4/6] rcu: Remove one layer of abstraction from PROVE_RCU checking Paul E. McKenney
` (3 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Paul E. McKenney @ 2011-07-08 15:43 UTC (permalink / raw)
To: linux-kernel
Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, niv, tglx,
peterz, rostedt, Valdis.Kletnieks, dhowells, eric.dumazet,
darren, patches, Frederic Weisbecker, Paul E. McKenney,
Peter Zijlstra
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH RFC tip/core/rcu 4/6] rcu: Remove one layer of abstraction from PROVE_RCU checking
2011-07-08 15:43 [PATCH tip/core/rcu 0/6] Check for use of RCU from dyntick-idle mode Paul E. McKenney
` (2 preceding siblings ...)
2011-07-08 15:43 ` [PATCH RFC tip/core/rcu 3/6] rcu: Warn when rcu_read_lock() is used in extended quiescent state Paul E. McKenney
@ 2011-07-08 15:43 ` Paul E. McKenney
2011-07-08 15:43 ` [PATCH RFC tip/core/rcu 5/6] rcu: Warn when srcu_read_lock() is used in an extended quiescent state Paul E. McKenney
` (2 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Paul E. McKenney @ 2011-07-08 15:43 UTC (permalink / raw)
To: linux-kernel
Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, niv, tglx,
peterz, rostedt, Valdis.Kletnieks, dhowells, eric.dumazet,
darren, patches, 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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH RFC tip/core/rcu 5/6] rcu: Warn when srcu_read_lock() is used in an extended quiescent state
2011-07-08 15:43 [PATCH tip/core/rcu 0/6] Check for use of RCU from dyntick-idle mode Paul E. McKenney
` (3 preceding siblings ...)
2011-07-08 15:43 ` [PATCH RFC tip/core/rcu 4/6] rcu: Remove one layer of abstraction from PROVE_RCU checking Paul E. McKenney
@ 2011-07-08 15:43 ` Paul E. McKenney
2011-07-08 15:43 ` [PATCH RFC tip/core/rcu 6/6] rcu: Make srcu_read_lock_held() call common lockdep-enabled function Paul E. McKenney
2011-07-11 16:03 ` [PATCH tip/core/rcu 0/6] Check for use of RCU from dyntick-idle mode Frederic Weisbecker
6 siblings, 0 replies; 12+ messages in thread
From: Paul E. McKenney @ 2011-07-08 15:43 UTC (permalink / raw)
To: linux-kernel
Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, niv, tglx,
peterz, rostedt, Valdis.Kletnieks, dhowells, eric.dumazet,
darren, patches, 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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH RFC tip/core/rcu 6/6] rcu: Make srcu_read_lock_held() call common lockdep-enabled function
2011-07-08 15:43 [PATCH tip/core/rcu 0/6] Check for use of RCU from dyntick-idle mode Paul E. McKenney
` (4 preceding siblings ...)
2011-07-08 15:43 ` [PATCH RFC tip/core/rcu 5/6] rcu: Warn when srcu_read_lock() is used in an extended quiescent state Paul E. McKenney
@ 2011-07-08 15:43 ` Paul E. McKenney
2011-07-11 16:03 ` [PATCH tip/core/rcu 0/6] Check for use of RCU from dyntick-idle mode Frederic Weisbecker
6 siblings, 0 replies; 12+ messages in thread
From: Paul E. McKenney @ 2011-07-08 15:43 UTC (permalink / raw)
To: linux-kernel
Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, niv, tglx,
peterz, rostedt, Valdis.Kletnieks, dhowells, eric.dumazet,
darren, patches, 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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH tip/core/rcu 0/6] Check for use of RCU from dyntick-idle mode
2011-07-08 15:43 [PATCH tip/core/rcu 0/6] Check for use of RCU from dyntick-idle mode Paul E. McKenney
` (5 preceding siblings ...)
2011-07-08 15:43 ` [PATCH RFC tip/core/rcu 6/6] rcu: Make srcu_read_lock_held() call common lockdep-enabled function Paul E. McKenney
@ 2011-07-11 16:03 ` Frederic Weisbecker
2011-07-11 16:38 ` Paul E. McKenney
6 siblings, 1 reply; 12+ messages in thread
From: Frederic Weisbecker @ 2011-07-11 16:03 UTC (permalink / raw)
To: Paul E. McKenney
Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
josh, niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells,
eric.dumazet, darren, patches
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" :)
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH tip/core/rcu 0/6] Check for use of RCU from dyntick-idle mode
2011-07-11 16:03 ` [PATCH tip/core/rcu 0/6] Check for use of RCU from dyntick-idle mode Frederic Weisbecker
@ 2011-07-11 16:38 ` Paul E. McKenney
2011-07-11 16:44 ` Frederic Weisbecker
0 siblings, 1 reply; 12+ messages in thread
From: Paul E. McKenney @ 2011-07-11 16:38 UTC (permalink / raw)
To: Frederic Weisbecker
Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
josh, niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells,
eric.dumazet, darren, patches
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH tip/core/rcu 0/6] Check for use of RCU from dyntick-idle mode
2011-07-11 16:38 ` Paul E. McKenney
@ 2011-07-11 16:44 ` Frederic Weisbecker
2011-07-11 17:00 ` Paul E. McKenney
0 siblings, 1 reply; 12+ messages in thread
From: Frederic Weisbecker @ 2011-07-11 16:44 UTC (permalink / raw)
To: Paul E. McKenney
Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
josh, niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells,
eric.dumazet, darren, patches
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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH tip/core/rcu 0/6] Check for use of RCU from dyntick-idle mode
2011-07-11 16:44 ` Frederic Weisbecker
@ 2011-07-11 17:00 ` Paul E. McKenney
2011-07-11 18:10 ` Frederic Weisbecker
0 siblings, 1 reply; 12+ messages in thread
From: Paul E. McKenney @ 2011-07-11 17:00 UTC (permalink / raw)
To: Frederic Weisbecker
Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
josh, niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells,
eric.dumazet, darren, patches
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH tip/core/rcu 0/6] Check for use of RCU from dyntick-idle mode
2011-07-11 17:00 ` Paul E. McKenney
@ 2011-07-11 18:10 ` Frederic Weisbecker
0 siblings, 0 replies; 12+ messages in thread
From: Frederic Weisbecker @ 2011-07-11 18:10 UTC (permalink / raw)
To: Paul E. McKenney
Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
josh, niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells,
eric.dumazet, darren, patches
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 ]---
^ permalink raw reply [flat|nested] 12+ messages in thread