Skip to content

Commit

Permalink
lockdep: Deal with many similar locks
Browse files Browse the repository at this point in the history
spin_lock_nest_lock() allows to take many instances of the same
class, this can easily lead to overflow of MAX_LOCK_DEPTH.

To avoid this overflow, we'll stop accounting instances but
start reference counting the class in the held_lock structure.

[ We could maintain a list of instances, if we'd move the hlock
  stuff into __lock_acquired(), but that would require
  significant modifications to the current code. ]

We restrict this mode to spin_lock_nest_lock() only, because it
degrades the lockdep quality due to lost of instance.

For lockstat this means we don't track lock statistics for any
but the first lock in the series.

Currently nesting is limited to 11 bits because that was the
spare space available in held_lock. This yields a 2048
instances maximium.

Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Marcelo Tosatti <mtosatti@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
  • Loading branch information
Peter Zijlstra authored and Ingo Molnar committed Aug 2, 2009
1 parent f607c66 commit bb97a91
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 13 deletions.
4 changes: 3 additions & 1 deletion include/linux/lockdep.h
Original file line number Diff line number Diff line change
Expand Up @@ -213,10 +213,12 @@ struct held_lock {
* interrupt context:
*/
unsigned int irq_context:2; /* bit 0 - soft, bit 1 - hard */
unsigned int trylock:1;
unsigned int trylock:1; /* 16 bits */

unsigned int read:2; /* see lock_acquire() comment */
unsigned int check:2; /* see lock_acquire() comment */
unsigned int hardirqs_off:1;
unsigned int references:11; /* 32 bits */
};

/*
Expand Down
89 changes: 77 additions & 12 deletions kernel/lockdep.c
Original file line number Diff line number Diff line change
Expand Up @@ -2708,13 +2708,15 @@ EXPORT_SYMBOL_GPL(lockdep_init_map);
*/
static int __lock_acquire(struct lockdep_map *lock, unsigned int subclass,
int trylock, int read, int check, int hardirqs_off,
struct lockdep_map *nest_lock, unsigned long ip)
struct lockdep_map *nest_lock, unsigned long ip,
int references)
{
struct task_struct *curr = current;
struct lock_class *class = NULL;
struct held_lock *hlock;
unsigned int depth, id;
int chain_head = 0;
int class_idx;
u64 chain_key;

if (!prove_locking)
Expand Down Expand Up @@ -2762,17 +2764,32 @@ static int __lock_acquire(struct lockdep_map *lock, unsigned int subclass,
if (DEBUG_LOCKS_WARN_ON(depth >= MAX_LOCK_DEPTH))
return 0;

class_idx = class - lock_classes + 1;

if (depth) {
hlock = curr->held_locks + depth - 1;
if (hlock->class_idx == class_idx && nest_lock) {
if (hlock->references)
hlock->references++;
else
hlock->references = 2;

return 1;
}
}

hlock = curr->held_locks + depth;
if (DEBUG_LOCKS_WARN_ON(!class))
return 0;
hlock->class_idx = class - lock_classes + 1;
hlock->class_idx = class_idx;
hlock->acquire_ip = ip;
hlock->instance = lock;
hlock->nest_lock = nest_lock;
hlock->trylock = trylock;
hlock->read = read;
hlock->check = check;
hlock->hardirqs_off = !!hardirqs_off;
hlock->references = references;
#ifdef CONFIG_LOCK_STAT
hlock->waittime_stamp = 0;
hlock->holdtime_stamp = sched_clock();
Expand Down Expand Up @@ -2881,6 +2898,30 @@ static int check_unlock(struct task_struct *curr, struct lockdep_map *lock,
return 1;
}

static int match_held_lock(struct held_lock *hlock, struct lockdep_map *lock)
{
if (hlock->instance == lock)
return 1;

if (hlock->references) {
struct lock_class *class = lock->class_cache;

if (!class)
class = look_up_lock_class(lock, 0);

if (DEBUG_LOCKS_WARN_ON(!class))
return 0;

if (DEBUG_LOCKS_WARN_ON(!hlock->nest_lock))
return 0;

if (hlock->class_idx == class - lock_classes + 1)
return 1;
}

return 0;
}

static int
__lock_set_class(struct lockdep_map *lock, const char *name,
struct lock_class_key *key, unsigned int subclass,
Expand All @@ -2904,7 +2945,7 @@ __lock_set_class(struct lockdep_map *lock, const char *name,
*/
if (prev_hlock && prev_hlock->irq_context != hlock->irq_context)
break;
if (hlock->instance == lock)
if (match_held_lock(hlock, lock))
goto found_it;
prev_hlock = hlock;
}
Expand All @@ -2923,7 +2964,8 @@ __lock_set_class(struct lockdep_map *lock, const char *name,
if (!__lock_acquire(hlock->instance,
hlock_class(hlock)->subclass, hlock->trylock,
hlock->read, hlock->check, hlock->hardirqs_off,
hlock->nest_lock, hlock->acquire_ip))
hlock->nest_lock, hlock->acquire_ip,
hlock->references))
return 0;
}

Expand Down Expand Up @@ -2962,20 +3004,34 @@ lock_release_non_nested(struct task_struct *curr,
*/
if (prev_hlock && prev_hlock->irq_context != hlock->irq_context)
break;
if (hlock->instance == lock)
if (match_held_lock(hlock, lock))
goto found_it;
prev_hlock = hlock;
}
return print_unlock_inbalance_bug(curr, lock, ip);

found_it:
lock_release_holdtime(hlock);
if (hlock->instance == lock)
lock_release_holdtime(hlock);

if (hlock->references) {
hlock->references--;
if (hlock->references) {
/*
* We had, and after removing one, still have
* references, the current lock stack is still
* valid. We're done!
*/
return 1;
}
}

/*
* We have the right lock to unlock, 'hlock' points to it.
* Now we remove it from the stack, and add back the other
* entries (if any), recalculating the hash along the way:
*/

curr->lockdep_depth = i;
curr->curr_chain_key = hlock->prev_chain_key;

Expand All @@ -2984,7 +3040,8 @@ lock_release_non_nested(struct task_struct *curr,
if (!__lock_acquire(hlock->instance,
hlock_class(hlock)->subclass, hlock->trylock,
hlock->read, hlock->check, hlock->hardirqs_off,
hlock->nest_lock, hlock->acquire_ip))
hlock->nest_lock, hlock->acquire_ip,
hlock->references))
return 0;
}

Expand Down Expand Up @@ -3014,7 +3071,7 @@ static int lock_release_nested(struct task_struct *curr,
/*
* Is the unlock non-nested:
*/
if (hlock->instance != lock)
if (hlock->instance != lock || hlock->references)
return lock_release_non_nested(curr, lock, ip);
curr->lockdep_depth--;

Expand Down Expand Up @@ -3065,7 +3122,9 @@ static int __lock_is_held(struct lockdep_map *lock)
int i;

for (i = 0; i < curr->lockdep_depth; i++) {
if (curr->held_locks[i].instance == lock)
struct held_lock *hlock = curr->held_locks + i;

if (match_held_lock(hlock, lock))
return 1;
}

Expand Down Expand Up @@ -3148,7 +3207,7 @@ void lock_acquire(struct lockdep_map *lock, unsigned int subclass,

current->lockdep_recursion = 1;
__lock_acquire(lock, subclass, trylock, read, check,
irqs_disabled_flags(flags), nest_lock, ip);
irqs_disabled_flags(flags), nest_lock, ip, 0);
current->lockdep_recursion = 0;
raw_local_irq_restore(flags);
}
Expand Down Expand Up @@ -3252,14 +3311,17 @@ __lock_contended(struct lockdep_map *lock, unsigned long ip)
*/
if (prev_hlock && prev_hlock->irq_context != hlock->irq_context)
break;
if (hlock->instance == lock)
if (match_held_lock(hlock, lock))
goto found_it;
prev_hlock = hlock;
}
print_lock_contention_bug(curr, lock, ip);
return;

found_it:
if (hlock->instance != lock)
return;

hlock->waittime_stamp = sched_clock();

contention_point = lock_point(hlock_class(hlock)->contention_point, ip);
Expand Down Expand Up @@ -3299,14 +3361,17 @@ __lock_acquired(struct lockdep_map *lock, unsigned long ip)
*/
if (prev_hlock && prev_hlock->irq_context != hlock->irq_context)
break;
if (hlock->instance == lock)
if (match_held_lock(hlock, lock))
goto found_it;
prev_hlock = hlock;
}
print_lock_contention_bug(curr, lock, _RET_IP_);
return;

found_it:
if (hlock->instance != lock)
return;

cpu = smp_processor_id();
if (hlock->waittime_stamp) {
now = sched_clock();
Expand Down

0 comments on commit bb97a91

Please sign in to comment.