Messages in this thread | | | Date | Sat, 9 Aug 2008 08:44:01 -0600 (MDT) | Subject | Re: [ANNOUNCE] mdb-2.6.27-rc2-ia32-08-07-08.patch | From | jmerkey@wolfmoun ... |
| |
> 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/ >
| |