History

rcu: Restore checks for blocking in RCU read-side critical sections

Long ago, using TREE_RCU with PREEMPT would result in "scheduling while atomic" diagnostics if you blocked in an RCU read-side critical section. However, PREEMPT now implies TREE_PREEMPT_RCU, which defeats this diagnostic. This commit therefore adds a replacement diagnostic based on PROVE_RCU. Because rcu_lockdep_assert() and lockdep_rcu_dereference() are now being used for things that have nothing to do with rcu_dereference(), rename lockdep_rcu_dereference() to lockdep_rcu_suspicious() and add a third argument that is a string indicating what is suspicious. This third argument is passed in from a new third argument to rcu_lockdep_assert(). Update all calls to rcu_lockdep_assert() to add an informative third argument. Also, add a pair of rcu_lockdep_assert() calls from within rcu_note_context_switch(), one complaining if a context switch occurs in an RCU-bh read-side critical section and another complaining if a context switch occurs in an RCU-sched read-side critical section. These are present only if the PROVE_RCU kernel parameter is enabled. Finally, fix some checkpatch whitespace complaints in lockdep.c. Again, you must enable PROVE_RCU to see these new diagnostics. But you are enabling PROVE_RCU to check out new RCU uses in any case, aren't you?

Paul E. McKenney committed 15 years ago

Commit b3fbab01 parent d5988af5 files · +75 −45Browse files

Unified · Split

5 files
  1. include/linux/lockdep.h +1 −1
  2. include/linux/rcupdate.h +22 −6
  3. kernel/lockdep.c +47 −37
  4. kernel/pid.c +3 −1
  5. kernel/sched.c +2 −0
include/linux/lockdep.h+1−1
@@ -548,7 +548,7 @@ do { \ · 547 unmodified lines
548548 #endif
549549
550550 #ifdef CONFIG_PROVE_RCU
551−extern void lockdep_rcu_dereference(const char *file, const int line);
551+void lockdep_rcu_suspicious(const char *file, const int line, const char *s);
552552 #endif
553553
554554 #endif /* __LINUX_LOCKDEP_H */
include/linux/rcupdate.h+22−6
@@ -297,19 +297,31 @@ extern int rcu_my_thread_group_empty(void); · 296 unmodified lines
297297 /**
298298 * rcu_lockdep_assert - emit lockdep splat if specified condition not met
299299 * @c: condition to check
300+ * @s: informative message
300301 */
301−#define rcu_lockdep_assert(c) \
302+#define rcu_lockdep_assert(c, s) \
302303 do { \
303304 static bool __warned; \
304305 if (debug_lockdep_rcu_enabled() && !__warned && !(c)) { \
305306 __warned = true; \
306− lockdep_rcu_dereference(__FILE__, __LINE__); \
307+ lockdep_rcu_suspicious(__FILE__, __LINE__, s); \
307308 } \
308309 } while (0)
309310
311+#define rcu_sleep_check() \
312+ do { \
313+ rcu_lockdep_assert(!lock_is_held(&rcu_bh_lock_map), \
314+ "Illegal context switch in RCU-bh" \
315+ " read-side critical section"); \
316+ rcu_lockdep_assert(!lock_is_held(&rcu_sched_lock_map), \
317+ "Illegal context switch in RCU-sched"\
318+ " read-side critical section"); \
319+ } while (0)
320+
310321 #else /* #ifdef CONFIG_PROVE_RCU */
311322
312−#define rcu_lockdep_assert(c) do { } while (0)
323+#define rcu_lockdep_assert(c, s) do { } while (0)
324+#define rcu_sleep_check() do { } while (0)
313325
314326 #endif /* #else #ifdef CONFIG_PROVE_RCU */
315327
@@ -338,14 +350,16 @@ extern int rcu_my_thread_group_empty(void); · 22 unmodified lines
338350 #define __rcu_dereference_check(p, c, space) \
339351 ({ \
340352 typeof(*p) *_________p1 = (typeof(*p)*__force )ACCESS_ONCE(p); \
341− rcu_lockdep_assert(c); \
353+ rcu_lockdep_assert(c, "suspicious rcu_dereference_check()" \
354+ " usage"); \
342355 rcu_dereference_sparse(p, space); \
343356 smp_read_barrier_depends(); \
344357 ((typeof(*p) __force __kernel *)(_________p1)); \
345358 })
346359 #define __rcu_dereference_protected(p, c, space) \
347360 ({ \
348− rcu_lockdep_assert(c); \
361+ rcu_lockdep_assert(c, "suspicious rcu_dereference_protected()" \
362+ " usage"); \
349363 rcu_dereference_sparse(p, space); \
350364 ((typeof(*p) __force __kernel *)(p)); \
351365 })
@@ -359,7 +373,9 @@ extern int rcu_my_thread_group_empty(void); · 7 unmodified lines
359373 #define __rcu_dereference_index_check(p, c) \
360374 ({ \
361375 typeof(p) _________p1 = ACCESS_ONCE(p); \
362− rcu_lockdep_assert(c); \
376+ rcu_lockdep_assert(c, \
377+ "suspicious rcu_dereference_index_check()" \
378+ " usage"); \
363379 smp_read_barrier_depends(); \
364380 (_________p1); \
365381 })
kernel/lockdep.c+47−37
@@ -1129,10 +1129,11 @@ print_circular_bug_header(struct lock_list *entry, unsigned int depth, · 1128 unmodified lines
11291129 if (debug_locks_silent)
11301130 return 0;
11311131
1132− printk("\n=======================================================\n");
1133− printk( "[ INFO: possible circular locking dependency detected ]\n");
1132+ printk("\n");
1133+ printk("======================================================\n");
1134+ printk("[ INFO: possible circular locking dependency detected ]\n");
11341135 print_kernel_version();
1135− printk( "-------------------------------------------------------\n");
1136+ printk("-------------------------------------------------------\n");
11361137 printk("%s/%d is trying to acquire lock:\n",
11371138 curr->comm, task_pid_nr(curr));
11381139 print_lock(check_src);
@@ -1463,11 +1464,12 @@ print_bad_irq_dependency(struct task_struct *curr, · 324 unmodified lines
14631464 if (!debug_locks_off_graph_unlock() || debug_locks_silent)
14641465 return 0;
14651466
1466− printk("\n======================================================\n");
1467− printk( "[ INFO: %s-safe -> %s-unsafe lock order detected ]\n",
1467+ printk("\n");
1468+ printk("======================================================\n");
1469+ printk("[ INFO: %s-safe -> %s-unsafe lock order detected ]\n",
14681470 irqclass, irqclass);
14691471 print_kernel_version();
1470− printk( "------------------------------------------------------\n");
1472+ printk("------------------------------------------------------\n");
14711473 printk("%s/%d [HC%u[%lu]:SC%u[%lu]:HE%u:SE%u] is trying to acquire:\n",
14721474 curr->comm, task_pid_nr(curr),
14731475 curr->hardirq_context, hardirq_count() >> HARDIRQ_SHIFT,
@@ -1692,10 +1694,11 @@ print_deadlock_bug(struct task_struct *curr, struct held_lock *prev, · 218 unmodified lines
16921694 if (!debug_locks_off_graph_unlock() || debug_locks_silent)
16931695 return 0;
16941696
1695− printk("\n=============================================\n");
1696− printk( "[ INFO: possible recursive locking detected ]\n");
1697+ printk("\n");
1698+ printk("=============================================\n");
1699+ printk("[ INFO: possible recursive locking detected ]\n");
16971700 print_kernel_version();
1698− printk( "---------------------------------------------\n");
1701+ printk("---------------------------------------------\n");
16991702 printk("%s/%d is trying to acquire lock:\n",
17001703 curr->comm, task_pid_nr(curr));
17011704 print_lock(next);
@@ -2177,10 +2180,11 @@ print_usage_bug(struct task_struct *curr, struct held_lock *this, · 475 unmodified lines
21772180 if (!debug_locks_off_graph_unlock() || debug_locks_silent)
21782181 return 0;
21792182
2180− printk("\n=================================\n");
2181− printk( "[ INFO: inconsistent lock state ]\n");
2183+ printk("\n");
2184+ printk("=================================\n");
2185+ printk("[ INFO: inconsistent lock state ]\n");
21822186 print_kernel_version();
2183− printk( "---------------------------------\n");
2187+ printk("---------------------------------\n");
21842188
21852189 printk("inconsistent {%s} -> {%s} usage.\n",
21862190 usage_str[prev_bit], usage_str[new_bit]);
@@ -2241,10 +2245,11 @@ print_irq_inversion_bug(struct task_struct *curr, · 54 unmodified lines
22412245 if (!debug_locks_off_graph_unlock() || debug_locks_silent)
22422246 return 0;
22432247
2244− printk("\n=========================================================\n");
2245− printk( "[ INFO: possible irq lock inversion dependency detected ]\n");
2248+ printk("\n");
2249+ printk("=========================================================\n");
2250+ printk("[ INFO: possible irq lock inversion dependency detected ]\n");
22462251 print_kernel_version();
2247− printk( "---------------------------------------------------------\n");
2252+ printk("---------------------------------------------------------\n");
22482253 printk("%s/%d just changed the state of lock:\n",
22492254 curr->comm, task_pid_nr(curr));
22502255 print_lock(this);
@@ -3065,9 +3070,10 @@ print_unlock_inbalance_bug(struct task_struct *curr, struct lockdep_map *lock, · 814 unmodified lines
30653070 if (debug_locks_silent)
30663071 return 0;
30673072
3068− printk("\n=====================================\n");
3069− printk( "[ BUG: bad unlock balance detected! ]\n");
3070− printk( "-------------------------------------\n");
3073+ printk("\n");
3074+ printk("=====================================\n");
3075+ printk("[ BUG: bad unlock balance detected! ]\n");
3076+ printk("-------------------------------------\n");
30713077 printk("%s/%d is trying to release lock (",
30723078 curr->comm, task_pid_nr(curr));
30733079 print_lockdep_cache(lock);
@@ -3478,9 +3484,10 @@ print_lock_contention_bug(struct task_struct *curr, struct lockdep_map *lock, · 404 unmodified lines
34783484 if (debug_locks_silent)
34793485 return 0;
34803486
3481− printk("\n=================================\n");
3482− printk( "[ BUG: bad contention detected! ]\n");
3483− printk( "---------------------------------\n");
3487+ printk("\n");
3488+ printk("=================================\n");
3489+ printk("[ BUG: bad contention detected! ]\n");
3490+ printk("---------------------------------\n");
34843491 printk("%s/%d is trying to contend lock (",
34853492 curr->comm, task_pid_nr(curr));
34863493 print_lockdep_cache(lock);
@@ -3839,9 +3846,10 @@ print_freed_lock_bug(struct task_struct *curr, const void *mem_from, · 352 unmodified lines
38393846 if (debug_locks_silent)
38403847 return;
38413848
3842− printk("\n=========================\n");
3843− printk( "[ BUG: held lock freed! ]\n");
3844− printk( "-------------------------\n");
3849+ printk("\n");
3850+ printk("=========================\n");
3851+ printk("[ BUG: held lock freed! ]\n");
3852+ printk("-------------------------\n");
38453853 printk("%s/%d is freeing memory %p-%p, with a lock still held there!\n",
38463854 curr->comm, task_pid_nr(curr), mem_from, mem_to-1);
38473855 print_lock(hlock);
@@ -3895,9 +3903,10 @@ static void print_held_locks_bug(struct task_struct *curr) · 47 unmodified lines
38953903 if (debug_locks_silent)
38963904 return;
38973905
3898− printk("\n=====================================\n");
3899− printk( "[ BUG: lock held at task exit time! ]\n");
3900− printk( "-------------------------------------\n");
3906+ printk("\n");
3907+ printk("=====================================\n");
3908+ printk("[ BUG: lock held at task exit time! ]\n");
3909+ printk("-------------------------------------\n");
39013910 printk("%s/%d is exiting with locks still held!\n",
39023911 curr->comm, task_pid_nr(curr));
39033912 lockdep_print_held_locks(curr);
@@ -3991,16 +4000,17 @@ void lockdep_sys_exit(void) · 87 unmodified lines
39914000 if (unlikely(curr->lockdep_depth)) {
39924001 if (!debug_locks_off())
39934002 return;
3994− printk("\n================================================\n");
3995− printk( "[ BUG: lock held when returning to user space! ]\n");
3996− printk( "------------------------------------------------\n");
4003+ printk("\n");
4004+ printk("================================================\n");
4005+ printk("[ BUG: lock held when returning to user space! ]\n");
4006+ printk("------------------------------------------------\n");
39974007 printk("%s/%d is leaving the kernel with locks still held!\n",
39984008 curr->comm, curr->pid);
39994009 lockdep_print_held_locks(curr);
40004010 }
40014011 }
40024012
4003−void lockdep_rcu_dereference(const char *file, const int line)
4013+void lockdep_rcu_suspicious(const char *file, const int line, const char *s)
40044014 {
40054015 struct task_struct *curr = current;
40064016
@@ -4009,15 +4019,15 @@ void lockdep_rcu_dereference(const char *file, const int line) · 2 unmodified lines
40094019 return;
40104020 #endif /* #ifdef CONFIG_PROVE_RCU_REPEATEDLY */
40114021 /* Note: the following can be executed concurrently, so be careful. */
4012− printk("\n===================================================\n");
4013− printk( "[ INFO: suspicious rcu_dereference_check() usage. ]\n");
4014− printk( "---------------------------------------------------\n");
4015− printk("%s:%d invoked rcu_dereference_check() without protection!\n",
4016− file, line);
4022+ printk("\n");
4023+ printk("===============================\n");
4024+ printk("[ INFO: suspicious RCU usage. ]\n");
4025+ printk("-------------------------------\n");
4026+ printk("%s:%d %s!\n", file, line, s);
40174027 printk("\nother info that might help us debug this:\n\n");
40184028 printk("\nrcu_scheduler_active = %d, debug_locks = %d\n", rcu_scheduler_active, debug_locks);
40194029 lockdep_print_held_locks(curr);
40204030 printk("\nstack backtrace:\n");
40214031 dump_stack();
40224032 }
4023−EXPORT_SYMBOL_GPL(lockdep_rcu_dereference);
4033+EXPORT_SYMBOL_GPL(lockdep_rcu_suspicious);
kernel/pid.c+3−1
@@ -418,7 +418,9 @@ EXPORT_SYMBOL(pid_task); · 417 unmodified lines
418418 */
419419 struct task_struct *find_task_by_pid_ns(pid_t nr, struct pid_namespace *ns)
420420 {
421− rcu_lockdep_assert(rcu_read_lock_held());
421+ rcu_lockdep_assert(rcu_read_lock_held(),
422+ "find_task_by_pid_ns() needs rcu_read_lock()"
423+ " protection");
422424 return pid_task(find_pid_ns(nr, ns), PIDTYPE_PID);
423425 }
424426
kernel/sched.c+2−0
@@ -4237,6 +4237,7 @@ static inline void schedule_debug(struct task_struct *prev) · 4236 unmodified lines
42374237 */
42384238 if (unlikely(in_atomic_preempt_off() && !prev->exit_state))
42394239 __schedule_bug(prev);
4240+ rcu_sleep_check();
42404241
42414242 profile_hit(SCHED_PROFILING, __builtin_return_address(0));
42424243
@@ -8230,6 +8231,7 @@ void __might_sleep(const char *file, int line, int preempt_offset) · 3987 unmodified lines
82308231 {
82318232 static unsigned long prev_jiffy; /* ratelimiting */
82328233
8234+ rcu_sleep_check(); /* WARN_ON_ONCE() by default, no rate limit reqd. */
82338235 if ((preempt_count_equals(preempt_offset) && !irqs_disabled()) ||
82348236 system_state != SYSTEM_RUNNING || oops_in_progress)
82358237 return;