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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140109170823.GF7572@laptop.programming.kicks-ass.net>
Date:	Thu, 9 Jan 2014 18:08:23 +0100
From:	Peter Zijlstra <peterz@...radead.org>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...nel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Steven Rostedt <rostedt@...dmis.org>,
	Paul McKenney <paulmck@...ux.vnet.ibm.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [RFC][PATCH] lockdep: Introduce wait-type checks

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?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/