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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <44066.166.70.238.43.1218293041.squirrel@webmail.wolfmountaingroup.com>
Date:	Sat, 9 Aug 2008 08:44:01 -0600 (MDT)
From:	jmerkey@...fmountaingroup.com
To:	"Stefan Richter" <stefanr@...6.in-berlin.de>
Cc:	"Jeremy Fitzhardinge" <jeremy@...p.org>,
	jmerkey@...fmountaingroup.com, linux-kernel@...r.kernel.org
Subject: Re: [ANNOUNCE] mdb-2.6.27-rc2-ia32-08-07-08.patch

> Jeremy Fitzhardinge wrote:
>> Stefan Richter wrote:
>>> jmerkey@...fmountaingroup.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/
>


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/