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

[Daniel Eischen]

On Sun, 25 Nov 2007, Julian Elischer wrote:

> Daniel Eischen wrote:
>> On Sun, 25 Nov 2007, Julian Elischer wrote:
>> 
>>> Daniel Eischen wrote:
>>
>> 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..

Are there not other ways to analyze the code to find consumers of
spin mutexes that may potentially block?   Compiler-time detection
doesn't help if the API is being followed but the consumer is
still blocking on a non-spin mutex or something else after taking
a spin mutex.

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

The general rule is you should never ever intentionally sleep after
taking a mutex unless it is with cv_wait() and friends.  msleep() 
should go away hopefully someday, and we shouldn't encourage its usage.
The consumer could be taking multiple mutexes or other synch primitives,
but hopefully that isn't too common.  Mutexes should be held briefly,
so maybe we should be looking for code paths that can block while any
type of mutex is held (other than cv_waitXXX and msleep)?

-- 
DE