Re: [ANNOUNCE] mdb-2.6.27-rc2-ia32-08-07-08.patch

[jmerkey@wolfmountaingroup.com]

> Jeremy Fitzhardinge wrote:
>> Stefan Richter wrote:
>>> jmerkey@wolfmountaingroup.com wrote:
>>>> ftp://ftp.wolfmountaingroup.org/pub/mdb/mdb-2.6.27-rc2-ia32-08-07-08.patch
> [...]
>>> The accessors rspin_lock() and rspin_try_lock() peek into spinlock_t
>>> and
>>> may therefore not be fully portable.  Also, they and rspin_unlock()
>>> don't look SMP safe:
>>>
>>>
>>>> +//
>>>> +//   returns   0 - atomic lock occurred, processor assigned
>>>> +//             1 - recusive count increased
>>>> +//
>>>> +
>>>> +unsigned long rspin_lock(volatile rlock_t *rlock)
>>>> +{
>>>> +#if defined(CONFIG_SMP)
>>>> +   register unsigned long proc = get_processor_id();
>>>> +   register unsigned long retCode;
>>>> +
>>>> +   if (rlock->lock.raw_lock.slock && rlock->processor == proc)
>>>>
>>
>> Ticket locks will almost always have a non-zero slock.  It doesn't
>> indicate anything about the locked/unlocked state.  But this looks like
>> it's effectively doing a trylock:
>>
>>     if (!spin_trylock(rlock) && rlock->processor == proc) {
>>         rlock->count++;
>>         ...
>>     } else {
>>         rlock->processor = proc;
>>         ...
>>     }
>
> Right.  This implemention also looks free of race conditions, provided
> that
>
>    - rspin_lock, rspin_try_lock, and rspin_unlock are only called in
>      contexts with disabled preemption and disabled local interrupts,
>
>    - rspin_unlock() rewrites rlock->processor to "no CPU" before
>      it drops the lock.  (The implementation in
>      mdb-2.6.27-rc2-ia32-08-07-08.patch does so.)
>
> BTW, the rspin_try_lock() in that patch wrong:  It always returns 0
> instead of having three branches of execution which return 0/1/-1.


.... On linux it does -- on another OS it does something quite different. 
I changed that case last night when I added spin)is_locked calls.   Is
there
to do a "debugger bust spinlocks" if the system ever hangs in the
debugger.  probably should code it different.

Jeff

:-)

Jeff

> --
> Stefan Richter
> -=====-==--- =--- -=--=
> http://arcgraph.de/sr/
>