Re: [RFC][PATCH] lockdep: Introduce wait-type checks

[Peter Zijlstra <peterz@infradead.org>]

On Thu, Jan 09, 2014 at 05:31:20PM +0100, Oleg Nesterov wrote:
> This is really minor, but it seems you can simplify it a little bit.
> We do not really need curr_inner, the main loop can do
> 
> 	for (...) {
> 		...
> 
> 		if (prev_inner && prev_inner < next_outer)
> 			return print_lock_invalid_wait_context(...);
> 	}
> 
> 	return 0;

Indeed, that might work.. My brain still finds it easier to understand
the min() version though.

> Off-topic question... I can't understand the "int check" argument of
> lock_acquire(). First of all, __lock_acquire() does
> 
> 	if (!prove_locking)
> 		check = 1;
> 
> Doesn't this mean lock_acquire_*() do not depend on CONFIG_PROVE_LOCKING?
> IOW, can't we do
> 
> 	--- x/include/linux/lockdep.h
> 	+++ x/include/linux/lockdep.h
> 	@@ -479,15 +479,9 @@ static inline void print_irqtrace_events
> 	  * on the per lock-class debug mode:
> 	  */
> 	 
> 	-#ifdef CONFIG_PROVE_LOCKING
> 	- #define lock_acquire_exclusive(l, s, t, n, i)		lock_acquire(l, s, t, 0, 2, n, i)
> 	- #define lock_acquire_shared(l, s, t, n, i)		lock_acquire(l, s, t, 1, 2, n, i)
> 	- #define lock_acquire_shared_recursive(l, s, t, n, i)	lock_acquire(l, s, t, 2, 2, n, i)
> 	-#else
> 	- #define lock_acquire_exclusive(l, s, t, n, i)		lock_acquire(l, s, t, 0, 1, n, i)
> 	- #define lock_acquire_shared(l, s, t, n, i)		lock_acquire(l, s, t, 1, 1, n, i)
> 	- #define lock_acquire_shared_recursive(l, s, t, n, i)	lock_acquire(l, s, t, 2, 1, n, i)
> 	-#endif
> 	+#define lock_acquire_exclusive(l, s, t, n, i)		lock_acquire(l, s, t, 0, 2, n, i)
> 	+#define lock_acquire_shared(l, s, t, n, i)		lock_acquire(l, s, t, 1, 2, n, i)
> 	+#define lock_acquire_shared_recursive(l, s, t, n, i)	lock_acquire(l, s, t, 2, 2, n, i)

I suppose we could; note however that the if (!prove_locking) logic was
added later.

> But what I really can't understans is what "check == 0" means? It
> seems that in fact it can be 1 or 2? Or, iow, "check == 0" is actually
> equivalent to "check == 1" ?

Hmm indeed, the comment in lockdep.h says 0 means no checks at all, but
the code doesn't actually appear to work like that. I'm not sure it ever
did or not, I'd have to go dig through history.

That said, given the current state it certainly looks like we can remove
the check argument.

Ingo?