Re: [PATCH 1/8] lockdep: fix recursive read lock validation

Peter Zijlstra
2008-04-29T15:15:31Z

On Tue, 2008-04-29 at 17:03 +0200, Bart Van Assche wrote:
> On Tue, Apr 29, 2008 at 4:57 PM, Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> >
> > On Tue, 2008-04-29 at 15:16 +0200, Bart Van Assche wrote:
> > > On Tue, Apr 29, 2008 at 2:57 PM, Gautham R Shenoy <ego@in.ibm.com> wrote:
> > > > Subject: lockdep: fix recursive read lock validation
> > > > This means that the following sequence is now invalid, whereas previously
> > > > it was considered valid:
> > > >
> > > > rlock(a); rlock(b); runlock(b); runlock(a)
> > > > rlock(b); rlock(a);
> > >
> > > Why are you marking this sequence as invalid ? Although it can be
> > > debated whether it is good programming practice to be inconsistent
> > > about the order of read-locking, the above sequence can't be involved
> > > in a deadlock.
> >
> > Not for pure read locks, but when you add write locks to it, it does get
> > deadlocky. Lockdep does not keep separate chains for read and write
> > locks.
>
> Nesting writer locks inside reader locks is always a bad idea. So
> please come up with an example of how varying the reader lock nesting
> order can trigger a deadlock (when no writer locks are nested inside
> reader locks and nested writer locks are always nested in the same
> order).

It can't deadlock when only readers are involved, but lockdep will not
be able to distinguish between the cases where only read locks are
involved and a mix of readers and writers is involved.

Hence disallow both.

But hitting this requiers you do a series of rather unfortunate things:

1) use recursive locking
2) don't have strict lock order
3) make it work by using read locks only

Seriously, any code that triggers this might want to have its locking
re-throught.