lkml.org 
[lkml]   [2014]   [Jan]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [RFC][PATCH] lockdep: Introduce wait-type checks
On 01/09, Peter Zijlstra wrote:
>
> +static int check_context(struct task_struct *curr, struct held_lock *next)
> +{
> + short next_inner = hlock_class(next)->wait_type_inner;
> + short next_outer = hlock_class(next)->wait_type_outer;
> + short curr_inner = LD_WAIT_MAX;
> + int depth;
> +
> + if (!curr->lockdep_depth || !next_inner)
> + return 0;
> +
> + if (!next_outer)
> + next_outer = next_inner;
> +
> + for (depth = 0; depth < curr->lockdep_depth; depth++) {
> + struct held_lock *prev = curr->held_locks + depth;
> + short prev_inner = hlock_class(prev)->wait_type_inner;
> +
> + if (prev_inner) {
> + /*
> + * we can have a bigger inner than a previous one
> + * when outer is smaller than inner, as with RCU.
> + */
> + curr_inner = min(curr_inner, prev_inner);
> + }
> + }
> +
> + if (next_outer > curr_inner)
> + return print_lock_invalid_wait_context(curr, next);
> +
> + return 0;
> +}

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;


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)

#define spin_acquire(l, s, t, i) lock_acquire_exclusive(l, s, t, NULL, i)
#define spin_acquire_nest(l, s, t, n, i) lock_acquire_exclusive(l, s, t, n, i)


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" ?

Oleg.



\
 
 \ /
  Last update: 2014-01-09 18:21    [W:0.466 / U:0.592 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site