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