cvs commit: src/sys/kern kern_rwlock.c src/sys/sys rwlock.h

[Max Laier]

On Wednesday 02 April 2008 09:15:25 Jeff Roberson wrote:
> On Wed, 2 Apr 2008, Max Laier wrote:
> > On Wednesday 02 April 2008 03:11:11 Jeff Roberson wrote:
> >> On Wed, 2 Apr 2008, Attilio Rao wrote:
> >>> 2008/4/2, Max Laier <max at love2party.net>:
> >>>> On Wednesday 02 April 2008 00:52:45 Jeff Roberson wrote:
> >>>>> On Wed, 2 Apr 2008, Max Laier wrote:
> >>>>>> On Tuesday 01 April 2008 22:31:55 Attilio Rao wrote:
> >>>>>>> attilio     2008-04-01 20:31:55 UTC
> >>>>>>>
> >>>>>>>   FreeBSD src repository
> >>>>>>>
> >>>>>>>   Modified files:
> >>>>>>>     sys/kern             kern_rwlock.c
> >>>>>>>     sys/sys              rwlock.h
> >>>>>>>   Log:
> >>>>>>>   Add rw_try_rlock() and rw_try_wlock() to rwlocks.
> >>>>>>>   These functions try the specified operation (rlocking and
> >>>>>>> wlocking) and true is returned if the operation completes,
> >>>>>>> false otherwise.
> >>>>>>
> >>>>>> hmmm ... I'm certainly missing something here, but what's a
> >>>>>> possible usecase for these?  It seems there is not much you can
> >>>>>> do if you can't obtain a rw_lock.  I can understand the need for
> >>>>>> sx_try_* where you want to avoid sleeping, but I can't figure
> >>>>>> out the need for it on a locking primitive that will only spin
> >>>>>> or wait (not 100% sure about the terminology here).  This is
> >>>>>> especially strange for rw_try_wlock, unless you plan to sleep
> >>>>>> manually on fail.  But then again you'd have a good chance that
> >>>>>> you have to do it over and over again if timing is unfortunate.
> >>>>>
> >>>>> I asked for it.  We have a try operation for mtx already.  I was
> >>>>> experimenting with converting some code to use rwlocks from mtx
> >>>>> and it required it.  The specific case is in the softdep code
> >>>>> where it uses trylock to avoid deadlocking.  With trylock you can
> >>>>> violate the lockorder.
> >>>>
> >>>> Makes sense, thanks!  A little follow-up, though about something
> >>>> I'm wondering about for quite some time now.  Take the following
> >>>> scenario:
> >>>>
> >>>>  Thread A:  rw_rlock(RW) ... mtx_lock(MTX) ... UNLOCK
> >>>>  Thread B:  mtx_lock(MTX) ... rw_rlock(RW) ... UNLOCK
> >>>>  Thread C:  rw_wlock(RW) ... UNLOCK
> >>>
> >>> This can't deadlock simply because rw_rlock() is not mutually
> >>> exclusive.
> >>
> >> It can deadlock if there is a writer waiting in queue depending on
> >> whether we prefer readers or writers.  I think we should consider
> >> the reader/writer perference an implementation detail to prevent
> >> code like this from cropping up.
> >
> > Sorry, I still don't understand this.  Even if there is a writer
> > (thread C) waiting and we prefer writers, the reader (A or B) has to
> > wait, but eventually the writer will give up the lock (as it can make
> > progress independently of whether the mutex is held or not) and the
> > readers can progress.  What am I missing?
>
> Thread A:  rw_rlock(RW) ... mtx_lock(MTX) ... UNLOCK
> Thread B:  mtx_lock(MTX) ... rw_rlock(RW) ... UNLOCK
> Thread C:  rw_wlock(RW) ... UNLOCK
>
> Thread A:       Thread B:       Thread C:
> rw_rlock(rw)
>                  mtx_lock(mtx)
>                                  rw_wlock(rw) <- Blocked waiting for a
>                  rw_rlock(rw) <- Blocked waiting for c due to write
> fairness mtx_lock(mtx) <- Blocked waiting for B
>
> Does that help?

Yes it does, thanks.  Do you know what the situation for rm_locks is?  If 
I understand correctly they implement full priority propagation.  In this 
scenario C would boost A via the RM and A would boost B via MTX so B 
could run and avoid the deadlock.  Or am I missing something again?

In any case doing "while (!rw_try_wlock(rw));" instead of a 
plain "rw_wlock(rw);" would now be a possible workaround - if we don't 
care about starvation of thread C, right?

> > I don't think this is a good thing either, but I also think that
> > there are some cases where there just are different access orders. 
> > I'd rather want a clean way out of this than a lot of difficult
> > per-instance hacks.  This does not mean that these can't be fixed
> > cleanly, but I think it's really hard sometimes especially for code
> > we import from elsewhere (hence the personal interest).
>
> I think the best solution is to treat rlocks as wlocks in terms of
> access orders.
>
> >> Readers are only allowed to proceed with a read lock if they already
> >> own a read lock, not just if the lock is already read locked.  This
> >> changed in current recently.  So a single recursive read acqusition
> >> can't deadlock but get multiple threads and a writer involved with
> >> writer preference and you can.
>
> Thanks,
> Jeff
>
> > --
> > /"\  Best regards,                      | mlaier at freebsd.org
> > \ /  Max Laier                          | ICQ #67774661
> > X   http://pf4freebsd.love2party.net/  | mlaier at EFnet
> > / \  ASCII Ribbon Campaign              | Against HTML Mail and News



-- 
/"\  Best regards,                      | mlaier at freebsd.org
\ /  Max Laier                          | ICQ #67774661
 X   http://pf4freebsd.love2party.net/  | mlaier at EFnet
/ \  ASCII Ribbon Campaign              | Against HTML Mail and News