cvs commit: src/share/man/man9 locking.9 rmlock.9 src/sys/conf files src/sys/kern kern_rmlock.c subr_lock.c subr_pcpu.c subr_smp.c src/sys/sys _rmlock.h lock.h pcpu.h rmlock.h smp.h

[Julian Elischer]

Daniel Eischen wrote:
> On Sun, 25 Nov 2007, Julian Elischer wrote:
> 
>> Daniel Eischen wrote:
>>> On Sat, 24 Nov 2007, Darren Reed wrote:
>>>
>>>> Stephan Uphoff wrote:
>>>>> ups         2007-11-08 14:47:55 UTC
>>>>>
>>>>>   FreeBSD src repository
>>>>>
>>>>>   Modified files:
>>>>>     share/man/man9       locking.9     sys/conf             files 
>>>>> sys/kern             subr_lock.c subr_pcpu.c subr_smp.c     sys/sys 
>>>>> lock.h pcpu.h smp.h   Added files:
>>>>>     share/man/man9       rmlock.9     sys/kern             
>>>>> kern_rmlock.c sys/sys              _rmlock.h rmlock.h   Log:
>>>>>   Initial checkin for rmlock (read mostly lock) a multi reader 
>>>>> single writer
>>>>>   lock optimized for almost exclusive reader access. (see also 
>>>>> rmlock.9)
>>>>>
>>>>
>>>> Is there a white paper or other documentation around somewhere that
>>>> discusses the benefits/tradeoffs with using rmlock vs rwlock?
>>>
>>> Why aren't we using the rwlock interfaces, but just allowing a different
>>> behavior when the lock is created (rwlock_init2() or something)?  It
>>> would seem simpler to keep the same interface and allow easy toggling
>>> between rwlocks and rmlocks.  The same way we can initialize kernel
>>> mutexes differently (MTX_DEF, MTX_SPIN) could be applied here.
>>>
>>
>> I think that If anything, we should be going in the other direction..
>> firstly, mutexes are just rw_locks with no readers. So we might
>> as well make them the same thing..
> 
> Robert already answered my question sufficiently...
> 
> I am looking at the way Solaris does its kernel synchronizations.
> They have mutexes, CVs, and rwlocks as far as I can see.  It is very 
> convenient to have mutexes and CVs just as their userland counterparts.
> Mutexes are mutual exclusion and by definition not rwlock-capable.
> You don't want to mix the mutex API with the rwlock or rmlock APIs,
> and also, the CV APIs (cv_waitXXX, cv_timedwaitXXX) only take mutex
> types as arguments.  If the implementations can share code, ok, but
> don't share the mutex/CV APIs with the r{mw}lock APIs :-)
> 
>> Spin and blocking mutexes should in my opinion be defined as different 
>> structures, at least in name so that the compiler hits you with a 
>> clue-bat when you try use a spin-lock with non-spinlock ops etc.
> 
> That seems nice, but doesn't go along well with trying to keep a
> similar API as Solaris. 

We're not actually consciously following Solaris APIs.

> It is convenient to share the APIs so
> that it is easy to change the mtx_init() from a default to a spin
> type without changing the rest of the code.  We really shouldn't
> have a need for spin mutexes if the default mutexes are adaptive,
> and if mutexes taken by interrupt handlers are initialized in a
> manner similar to Solaris.  There really shouldn't be a separate
> API for spin mutexes.  Once a mutex is initialized as MTX_DEF
> or MTX_SPIN, that should be sufficient.

Very nice in theory, but in practice in the kernel, you can not use
them in the same places. Spin locks in the wrong place will lead 
to crashes and lockups and blocking (descheduling) mutexes will lead 
to crashes and panics if used in the many of the places where spin
locks are usually used. locks that are visible in both places are 
too easy to call with the wrong semantics.  We currently detect it 
at runtime but compile time would be nicer..

When you say "[one can] change the mtx_init() from a default to a spin
type without changing the rest of the code", that is in some cases 
actually a BAD thing... The required semantics surrounding spin and 
sleep locks are generally that they must be placed in the code in such a way
that the surrounding code effectively knows which they are. flipping the
type of the lock without going through all the surrounding code and
checking those assumptions would lead to all sorts of "Really hard to find"
bugs. 

Note; I am talking about in the (FreeBSD) kernel only. This is probably
not true at all for Userland.


> 
>> not sure why sx-locks exist at all, as they seem to be a variant of 
>> sleep.
>> I think it's just a convenience function set to allow one to implement
>> a sleep-derived synchronisation.
> 
> Hmm, sx locks seem similar to rmlocks, at least according to the
> description:
> 
>   Shared/exclusive locks are used to protect data that are read far
>   more often than they are written.

the trick is in what they do to block, and how they interract with 
sleep.

To 'sleep' you need to drop all mutexes and rwlocks (and rmlocks I think)
as you may ba asleep for a LONG time.  Same goes to sx locks.
they are designed to be used for things where you want a reader/writer 
semantic, but the result of a lock out may be a LONG delay.. 

i.e. after you gain exclusive access to a resource with sx-locks 
you may then decide to do something that could put you to sleep..
for rw-locks or mutexes this would cause a panic.


> 
> Do we need both?

Well, we need something but it could be that the naming was unfortunate.

>