cvs commit: src/lib/libthr/thread thr_mutex.c src/lib/libkse/thread thr_mutex.c src/include pthread.h

[Daniel Eischen]

On Tue, 30 Oct 2007, David Xu wrote:

> Kris Kennaway wrote:
>> kris        2007-10-29 21:01:47 UTC
>>
>>   FreeBSD src repository
>>
>>   Modified files:
>>     lib/libthr/thread    thr_mutex.c     lib/libkse/thread    thr_mutex.c 
>> include              pthread.h   Log:
>>   Add a new "non-portable" mutex type, PTHREAD_MUTEX_ADAPTIVE_NP.  This
>>   is also implemented in glibc and is used by a number of existing
>>   applications (mysql, firefox, etc).
>>     This mutex type is a default mutex with the additional property that
>>   it spins briefly when attempting to acquire a contested lock, doing
>>   trylock operations in userland before entering the kernel to block if
>>   eventually unsuccessful.
>>     The expectation is that applications requesting this mutex type know
>>   that the mutex is likely to be only held for very brief periods, so it
>>   is faster to spin in userland and probably succeed in acquiring the
>>   mutex, than to enter the kernel and sleep, only to be woken up almost
>>   immediately.  This can help significantly in certain cases when
>>   pthread mutexes are heavily contended and held for brief durations
>>   (such as mysql).
>>     Spin up to 200 times before entering the kernel, which represents only
>>   a few us on modern CPUs.  No performance degradation was observed with
>>   this value and it is sufficient to avoid a large performance drop in
>>   mysql performance in the heavily contended pthread mutex case.
>>     The libkse implementation is a NOP.
>>     Reviewed by:      jeff
>>   MFC after:        3 days
>>     Revision  Changes    Path
>>   1.41      +2 -0      src/include/pthread.h
>>   1.54      +3 -0      src/lib/libkse/thread/thr_mutex.c
>>   1.55      +29 -0     src/lib/libthr/thread/thr_mutex.c
>> 
>
> I am not sure PTHREAD_MUTEX_ADAPTIVE_NP is a correct solution, in fact
> I think this is Linux crap, shouldn't PTHREAD_PRIO_PROTECT and
> PTHREAD_PRIO_INHERIT mutex be adaptivly spinned ?
> also this commit does not change mutex_self_lock() to handle the
> PTHREAD_MUTEX_ADAPTIVE_NP, what is the PTHREAD_MUTEX_ADAPTIVE_NP
> definition when the mutex is already locked by the currect thread ?
> deadlock or return error code ?

I tend to agree with the "Linux crap" comment, but I hesitate
to use those words considering the recent sensor framework
incident ;-)

As I said in previous email, I would rather see our default
mutex implementations improved instead of adding new interfaces.
If it's really necessary in the short term, perhaps an
environment variable that can be set to force all mutexes
to be adaptive (and when kern.smp.cpus > 1 perhaps?).

-- 
DE