[PATCH] RCU: don't turn off lockdep when find suspicious rcu_dereference_check()usage

From: Lai Jiangshan
Date: Tue Apr 20 2010 - 04:22:53 EST


Paul E. McKenney wrote:
> On Mon, Apr 19, 2010 at 09:25:29PM -0400, Eric Paris wrote:
>> On Mon, 2010-04-19 at 16:01 -0700, Paul E. McKenney wrote:
>>
>>> Yep, different code path to the same location. Does the following
>>> patch help?
>>>
>>> Thanx, Paul
>>>
>>> ------------------------------------------------------------------------
>>>
>>> commit 2836f18139267ea918ed2cf39023fb0eb38c4361
>>> Author: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx>
>>> Date: Mon Apr 19 15:59:50 2010 -0700
>>>
>>> rcu: fix RCU lockdep splat on freezer_fork path
>>>
>>> Add an RCU read-side critical section to suppress this false positive.
>>>
>>> Located-by: Eric Paris <eparis@xxxxxxxxxxxxxx>
>>> Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx>
>> That one is also fixed so feel free to add a tested or something from
>> me. But we've got another, weeeee! If there some way I could get all
>> of these at once?

This patch fits your requirement.

>
> Sure! I -think- that if you remove the first "if" statement in
> lockdep_rcu_dereference() in kernel/lockdep.c, you will get lots of them
> all at once. Maybe more than your console log is able to hold...
>
> So another approach would be to print only the first 100 or some such.
>
> It -looks- to me that you could make __debug_locks_off() atomically
> decrement a counter rather than just setting it to zero, see
> include/linux/debug_locks.h. I suspect that atomic_dec_not_zero()
> would work very well for you here.
>

[PATCH] RCU: don't turn off lockdep when find suspicious rcu_dereference_check() usage

When suspicious rcu_dereference_check() usage is detected, lockdep is still
available actually, so we should not call debug_locks_off() in
lockdep_rcu_dereference().

For get rid of too much "suspicious rcu_dereference_check() usage"
output when the "if(!debug_locks_off())" statement is removed. This patch uses
static variable '__warned's for very usage of "rcu_dereference*()".

One variable per usage, so, Now, we can get multiple complaint
when we detect multiple different suspicious rcu_dereference_check() usage.

Requested-by: Eric Paris <eparis@xxxxxxxxxx>
Signed-off-by: Lai Jiangshan <laijs@xxxxxxxxxxxxxx>
---
diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 9f1ddfe..30b8d20 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -193,6 +193,15 @@ static inline int rcu_read_lock_sched_held(void)

#ifdef CONFIG_PROVE_RCU

+#define __do_rcu_dereference_check(c) \
+ do { \
+ static bool __warned; \
+ if (debug_lockdep_rcu_enabled() && !__warned && !(c)) { \
+ __warned = true; \
+ lockdep_rcu_dereference(__FILE__, __LINE__); \
+ } \
+ } while (0)
+
/**
* rcu_dereference_check - rcu_dereference with debug checking
* @p: The pointer to read, prior to dereferencing
@@ -222,8 +231,7 @@ static inline int rcu_read_lock_sched_held(void)
*/
#define rcu_dereference_check(p, c) \
({ \
- if (debug_lockdep_rcu_enabled() && !(c)) \
- lockdep_rcu_dereference(__FILE__, __LINE__); \
+ __do_rcu_dereference_check(c); \
rcu_dereference_raw(p); \
})

@@ -240,8 +248,7 @@ static inline int rcu_read_lock_sched_held(void)
*/
#define rcu_dereference_protected(p, c) \
({ \
- if (debug_lockdep_rcu_enabled() && !(c)) \
- lockdep_rcu_dereference(__FILE__, __LINE__); \
+ __do_rcu_dereference_check(c); \
(p); \
})

diff --git a/kernel/lockdep.c b/kernel/lockdep.c
index 78325f8..cc52ffe 100644
--- a/kernel/lockdep.c
+++ b/kernel/lockdep.c
@@ -3788,8 +3788,6 @@ void lockdep_rcu_dereference(const char *file, const int line)
{
struct task_struct *curr = current;

- if (!debug_locks_off())
- return;
printk("\n===================================================\n");
printk( "[ INFO: suspicious rcu_dereference_check() usage. ]\n");
printk( "---------------------------------------------------\n");
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/