Re: [PATCH v3 1/4] DSS2: OMAPFB: Add support for switching memory regions

Imre Deak
2010-03-19T07:46:22Z

On Thu, Mar 18, 2010 at 04:26:04PM +0100, Syrjala Ville (Nokia-D/Helsinki) wrote:
> [...]
> > > Just tried it and seems to be mostly OK. We get lockdep checking as a
> > > bonus. It didn't like setup_plane taking the same rwsem twice so I
> > > added a check to see if the old and new regions are the same and just
> > > lock once in that case. I thought rwsem was supposed to be OK with
> > > read recursion but perhaps I was mitaken, or perhaps it's just lockdep
> > > that's misbehaving.
> >
> > Ah ok, so it's not so obvious change. Nested read locks could really lead
> > to a deadlock I think. A read lock will block if there is a write waiter
> > in the queue to avoid write starvation..
>
> Yes but I think in out case it should be fine because if we hit this:
>
> t thread 1 thread 2
> |
> | down_read(0)
> | down_write(1)
> v down_read(1)
>
> then thread 2 will eventually do a up_write() without taking any
> other region rwsem, and thread 1 can then continue.

Yes and things will work fine with the extra ordering you added. But
lockdep was right in that without the ordering you can get - the not
too likely - scenario:

t thread 1 thread 2 thread 3 thread 4
| down_read(0)
| down_write(0)
| down_read(1)
| down_write(1)
| down_read(1)
v down_read(0)

--Imre