Re: [PATCH 0/3] Synaptics - fix lockdep warnings

["Dmitry Torokhov" <dmitry.torokhov@gmail.com>]

On 9/15/06, Arjan van de Ven <arjan@infradead.org> wrote:
> On Thu, 2006-09-14 at 15:11 -0400, Dmitry Torokhov wrote:
> > On 9/14/06, Arjan van de Ven <arjan@infradead.org> wrote:
> > >
> > > >
> > > > I think it is - as far as I understand the reason for not tracking
> > > > every lock individually is just that it is too expensive to do by
> > > > default.
> > >
> > > that is not correct. While it certainly plays a role,
> > > the other reason is that you can find out "class" level locking rules
> > > (such as inode->i_mutex comes before <other lock>) for all inodes at a
> > > time; eg no need to see every inode do this before you can find the
> > > deadlock.
> > >
> >
> > OK, I can see that. However you must agree that for certain locks we
> > do want to track them individually, right?
>
> I agree that if locks really represent different objects with different
> locking semantics they should not share the class. Lockdep provides a
> mechanism for that; however I'm very afraid that for the input layer,
> they really are not that, they are not different objects with different
> semantics; they are the same objects with nesting semantics! In that
> case the "separate lock class" stuff has only disadvantages.

I'd say they are different objects with the same semantics...

> The worst thing is that as I understand it this separate class is
> *dynamic*. Eg it's not even "one class per driver" ;(
>

You are saying this as if was a bad thing. Pass-through ports just
implement PS/2 over PS/2 protocols and as such it is very natural that
the same driver that serves parent serves the child as well. That was
the goal - to reuse psmouse module instead of re-implementing all
re-probing and protocol decoding in synaptics driver. And trackpint
driver. And maybe somethng else down the road.

I also wonder that even if we had several drivers lockdep would still
complain about nestiness just because all PS/2 devices are initialized
via ps2_init (which initializes command mutex) and end up in the same
lock class.

I suspect that other driver implementing X-over-X or X-over-Y-over-X
may get hit the same way by lockdep.

I understand what Ingo is saying about detecting deadlocks across the
pool of locks of the same class not waiting till they really clash, it
is really useful. I also want to make my code as independent of
lockdep as possible. Having a speciall marking on the locks themselves
(done upon creation) instead of altering call sites is the cleanest
way IMHO. Can we have a flag in the lock structure that would tell
lockdep that it is OK for the given lock to be taken several times
(i.e. the locks are really on the different objects)? This would still
allow to detect incorrect locking across different classes.

-- 
Dmitry