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

[Jeff Roberson]

On Tue, 1 Apr 2008, 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
>

I should mention one more variation on this.  If the following preceeded 
the diagram below there would be no deadlock:

                  rw_rlock(rw);
                  ^^^^^^^^^^^^^
> 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

The recursive acquisition is not blocked by a write waiter because that 
could certainly lead to deadlock.  Note that this reverts us to a strict 
order which can be verified by witness regardless of the reader/write type 
of the lock.

Jeff

>
> Does that help?
>
>> 
>> 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
>> 
>