Q. lockdep_assert_held() and lockdep_off/on()

Hello Peter Zijlstra and Ingo Molnar,

May I ask you a question about the commit

	f607c66 2009-08-02 lockdep: Introduce lockdep_assert_held()

In short, should lockdep_assert_held() support ->lockdep_recursion?

Its current definition is
#define lockdep_assert_held(l) WARN_ON(debug_locks && !lockdep_is_held(l))

When someone somewhere calls lockdep_off() and executes some memory allocation or something, then the functions to shrink dentry cache happens to run. And cond_resched_lock() in __shrink_dcache_sb() may produce a false warning.

fs/dcache.c:
__shrink_dcache_sb()

{
	:::
	spin_lock(&dcache_lru_lock);
	while (...) {
		:::
		cond_resched_lock(&dcache_lru_lock);
		:::
	}
	:::
}

The function __shrink_dcache_sb() acquires dcache_lru_lock correctly and comfirms it by cond_resched_lock() which calls lockdep_assert_held(). When the caller already called lockdep_off(), lock_is_held() always return 0 which leads to WARN_ON(true). Obviously the warning is false positive.

Setting FALSE to debug_locks may be one solution, but this variable doesn't seem to expect to return to TRUE. So it is better for lockdep_assert_held() to test ->lockdep_recursion too I think.

index 413c754..8658138 100644
@@ -341,7 +341,9 @@ extern void lockdep_trace_alloc(gfp_t mask);
 
 #define lockdep_depth(tsk)	(debug_locks ? (tsk)->lockdep_depth : 0)
 
-#define lockdep_assert_held(l)	WARN_ON_ONCE(debug_locks && !lockdep_is_held(l))
+#define lockdep_assert_held(l)	WARN_ON_ONCE(debug_locks \
+					     && !current->lockdep_recursion \
+					     && !lockdep_is_held(l))
 
 #else /* !LOCKDEP */
 

J. R. Okajima

Probably, however

you're not supposed to do that..