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

["Ville Syrjälä" <ville.syrjala@nokia.com>]

On Fri, Mar 19, 2010 at 08:46:04AM +0100, Deak Imre (Nokia-D/Helsinki) wrote:
> 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)

Right. I didn't actually consider the case with so many threads. It's
good that lockdep was smarter than me :)

-- 
Ville Syrjälä