[lkml]   [2008]   [Aug]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [ANNOUNCE] mdb: Merkey's Linux Kernel Debugger 2.6.27-rc4 released wrote:
[Stefan Richter wrote]
>> I'm having a quick look at mdb-2.6.27-rc4-ia32-08-20-08.patch at the
>> moment. Speaking of debug_lock()...:
>> Major problem: rlock->flags is wrong in this call. Use an on-stack
>> flags variable for the initial spin_trylock_irqsave. Ditto in the
>> following call of spin_trylock_irqsave.
>> Next major problem with debug_lock() and debug_unlock(): The reference
>> counting doesn't work. You need an atomic_t counter. Have a look at
>> the struct kref accessors for example, or even make use of the kref API.
>> Or if it isn't feasible to fix with atomic_t, add a second spinlock to
>> rlock_t to ensure integrity of .count (and of the .processor if
>> necessary).
> The code works in debug lock provided this memory location is actually
> SHARED between the processors. The various race conditions you describe
> are valid cases, but he only modifier of .count and .lock is the processor
> that obtains the spinlock -- the rest are readers. This code works well,
> of course, when this memory location is actually SHARED between the
> processors and the read/write operations serialized.
> Even in SMP, at various times it is necessary for the processors to
> perform serializing operations. You cannot in checker-scoreboard land for
> everything.

Regarding rlock->count, I stand corrected: It works correctly if the
debug_lock()...debug_unlock() region can be entered by up to two
contexts and if the second one to enter cannot be preempted by the first

However, since these regions are enclosed in preempt_disable/enable and
since the first one to grab the rlock->lock keeps local interrupts
disabled until debug_unlock() or even longer, there is always only a
single context in the debug_lock()...debug_unlock() region --- which
defeats the whole purpose of the rlock_t. Or what am I missing /now/?

Independently of this, you cannot use rlock->flags like you currently
do. spin_trylock_irqsave would overwrite the flags of CPU A by the
flags of CPU B, making the results of spin_unlock_irqrestore in
debug_unlock unpredictable.
Stefan Richter
-=====-==--- =--- =-=-=

 \ /
  Last update: 2008-08-21 17:27    [from the cache]
©2003-2014 Jasper Spaans. hosted at Digital Ocean