Messages in this thread | | | Date | Sun, 12 Jan 2014 21:58:14 +0100 | From | Peter Zijlstra <> | Subject | Re: check && lockdep_no_validate (Was: lockdep: Introduce wait-type checks) |
| |
On Thu, Jan 09, 2014 at 06:54:48PM +0100, Oleg Nesterov wrote: > But this connects to lockdep_no_validate. Not sure I understand what > this class should actually do, but consider this code: > > DEFINE_MUTEX(m1); > DEFINE_MUTEX(m2); > DEFINE_MUTEX(mx); > > void lockdep_should_complain(void) > { > lockdep_set_novalidate_class(&mx); > > // m1 -> mx -> m2 > mutex_lock(&m1); > mutex_lock(&mx); > mutex_lock(&m2); > mutex_unlock(&m2); > mutex_unlock(&mx); > mutex_unlock(&m1); > > > // m2 -> m1 ; should trigger the warning > mutex_lock(&m2); > mutex_lock(&m1); > mutex_unlock(&m1); > mutex_unlock(&m2); > } > > lockdep doesn't not detect the trivial possible deadlock. > > The patch below seems to work but most probably it is not right, and > I forgot everything (not too much) I knew about lockdep internals. > > Oleg. > > --- a/kernel/locking/lockdep.c > +++ b/kernel/locking/lockdep.c > @@ -1939,7 +1939,8 @@ check_prevs_add(struct task_struct *curr, struct held_lock *next) > * Only non-recursive-read entries get new dependencies > * added: > */ > - if (hlock->read != 2) { > + if (hlock->read != 2 && > + hlock->instance->key != &__lockdep_no_validate__) { > if (!check_prev_add(curr, hlock, next, > distance, trylock_loop)) > return 0; >
Hmm, you are quite right indeed; although I would write it like:
if (hlock->read != 2 && hlock->check == 2)
because the __lockdep_no_validate__ thing forces the ->check value to 1.
| |