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

[Jeff Roberson]

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?

>
> 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
>