[Xenomai-core] Re: [patch] memory barriers in intr.c :: xnintr_lock/unlock()

[Jan Kiszka <jan.kiszka@domain.hid>]

[-- Attachment #1: Type: text/plain, Size: 2678 bytes --]

Dmitry Adamushko wrote:
> On 20/11/06, Jan Kiszka <jan.kiszka@domain.hid> wrote:
> 
>> > Ok, maybe it's just in theory. e.g. lock and unlock for x86 seem to
>> imply a
>> > full memory barrier (and, probably, all the rest systems does the
>> same).
>>
>> > Just look at definitions of mb() and spinlocks() for x86. asm("lock;
>> some
>> > write ops") does the trick in both cases.
>> >
>>
>> Well, the essence is likely that we should put a real compiler barrier
>> in spinunlock so that such /potential/ leaking of post-lock code into
>> the critical region can be excluded.
> 
> 
> But some CPUs still may reorder write ops. (the ones that have write-op
> queues) and a compiler barrier doesn't help here. Moreover, a spinlock
> (here
> I'm referring to the linux one) does seem to include a compiler barrier
> (any
> real memory barrier also implies it).
> 
> Think about it : spinlocks act as "one-way permeable barriers" and lesser
> (one-side) variants of compiler barriers do not exist (it's from the same
> document :) - just barrier().
> So if spinlock doesn't imply a compiler barrier by default (+ (one-way
> where
> it's available) CPU memory barrier), it's just broken.

As we learned, spin-unlock comes with a one-way barrier. I was referring
two a full compiler barrier to avoid reordering of post-lock with
in-lock code. Or is it even a memory barrier? I hate this. It confuses me.

Well, I guess it's best to leave xnlocks as they are (when they really
match Linux characteristics) and put the required but missing barrier
for xnintr_synchronize() explicitly into the code. To play save (and as
performance is not important in that path), we can use a memory barrier.

> 
> Errr... in fact, xnlock_put() doesn't seem to be safe in this respect.
> Although, nobody uses it as is. xnlock_put_irqrestore() seems to be safe
> though.
> 
> xnlock_get() makes use of atomic_cmpxchg() which is mapped on linux's
> cmpxchg() which, in turn, is said to be smp_mb() alike.

My whole feeling on the xnlock is not that positive. Maybe it would be
good to go through all "home-made" locking primitives with barriers in
mind and an eye on the Linux implementation.

> 
> Ok, I'm stopping further speculations here... Back to practice, so do we go
> for smp_mb__before/after_atomic_* counterparts or live it as is? I guess,
> those would be more optimal.
> 

As I said, I'm still not sure if we finally need it when we may use
per-cpu counters for efficiency reasons on the long-term. If this will
add code or - even more important - an interface (of the system layer)
that will soon be removed again...

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]