[lkml]   [2010]   [Feb]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [patch 1/3] Create spin lock/spin unlock with distinct memory barrier

On Sun, 31 Jan 2010, Mathieu Desnoyers wrote:
> Create the primitive family:
> spin_lock__no_acquire
> spin_unlock__no_release
> spin_lock_irq__no_acquire
> spin_unlock_irq__no_release

I really really hate this patch.

Locking is really hard to get right anyway, you already had one bug in
this, and on the major architectures, none of this will matter _anyway_,
since you cannot split the acquire from the lock and the release from the

So the only operation that actually makes any sense at all would be
"smp_mb__after_spin_lock" (and no new spinlock primitives at all), since
it can be optimized away on x86 (and then add "smp_mb__before_spin_unlock"
just to make them symmetric). But even that one is very dubious:

"The first user of these primitives is the scheduler code, where a full
memory barrier is wanted before and after runqueue data structure
modifications so these can be read safely by sys_membarrier without
holding the rq lock."

what kind of crazy model is this? That sounds insane. Locking together
with some new random optimistic usage that we don't even know how
performance-critical it is? Mixing locking and non-locking is simply
wrong. Why would it be any safer to read whatever that the lock protects
by adding smp barriers at the lock?

If you need other smp barriers at the lock, then what about the non-locked
accesses _while_ the lock is held? You get no ordering guarantees there.
The whole thing sounds highly dubious.

And all of this for something that is a new system call that nobody
actually uses? To optimize the new and experimental path with some insane
lockfree model, while making the core kernel more complex? A _very_
strong NAK from me.


 \ /
  Last update: 2010-02-01 16:25    [W:0.147 / U:0.148 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site