[ltt-dev] [PATCH] fix the "unknown" case

[Paul E. McKenney]

On Sun, Jun 13, 2010 at 10:46:35PM -0400, Mathieu Desnoyers wrote:
> * Mathieu Desnoyers (mathieu.desnoyers at efficios.com) wrote:
> > * Paul E. McKenney (paulmck at linux.vnet.ibm.com) wrote:
> > > On Sun, Jun 13, 2010 at 05:36:05PM -0400, Mathieu Desnoyers wrote:
> > > > * Paul E. McKenney (paulmck at linux.vnet.ibm.com) wrote:
> > > > > On Sun, Jun 13, 2010 at 05:20:34PM -0400, Mathieu Desnoyers wrote:
> > > > > > * Paul E. McKenney (paulmck at linux.vnet.ibm.com) wrote:
> > > > > > > On Thu, Jun 10, 2010 at 09:46:31PM -0400, Mathieu Desnoyers wrote:
> > > > > > 
> > > > > > [...]
> > > > > > 
> > > > > > (will reply to the rest in the individual patches)
> > > > > > 
> > > > > > > > Can we trust __sync_lock_test_and_set/__sync_add_and_fetch given that
> > > > > > > > __sync_synchronize is broken ?
> > > > > > > 
> > > > > > > I don't know yet.  If it turns out that we cannot, then I will use some
> > > > > > > form of global locking.  But the __sync_lock_test_and_set() do at least
> > > > > > > generate instructions, unlike __sync_synchronize().  ;-)
> > > > > > 
> > > > > > I'm concerned about the fact that their synchronization primitives might have
> > > > > > the assembly all with, except for the memory barriers.
> > > > > 
> > > > > OK.  How about if I used a hashed array of locks, indexed by a hash of
> > > > > the cacheline number of the access in question?  Then the "unknown" case
> > > > > doesn't depend at all on the __sync_ primitives.
> > > > 
> > > > I'm afraid this won't work with signal handlers. :-/ It would be a shame to have
> > > > to make these primitives non-signal-safe just for the sake of the "unknown"
> > > > arch. Or maybe it could make sense to disable signals around these, even though
> > > > it will be really slow ?
> > > 
> > > Hmmm...
> > > 
> > > Maybe the right thing to do is to make the build fail in the "unknown"
> > > case.  By the time we make this safe, the performance will be pretty
> > > bad!
> > > 
> > > My thought is to simply not have the two "unknown" include files, so
> > > that configuration dies when trying to copy them over.  Perhaps better
> > > yet, have the two "unknown" include files have nothing but a #error
> > > statement.  Seem like a reasonable approach?
> > 
> > Yep, I like the #error approach. It will provide a meaningful error message.
> 
> Hrm. OK, I think I got a compromise for you so we can have a working "unknown"
> architecture.
> 
> smp_mb(): double-fake-mutex (stack-local)
>           Note: we depend on having correct memory barriers in the pthread mutex
> implementation. Hopefully they got, at least, that right for ARM. ;)
> 
> uatomic_xchg():
> 
>   smp_mb();
>   ret = __sync_lock_test_and_set(addr, v);
>   smp_mb();
>   ret;
> 
> uatomic_cmpxchg():
> 
>   smp_mb();
>   ret = __sync_val_compare_and_swap(addr, old, _new)
>   smp_mb();
>   ret;
> 
> uatomic_add_return():
> 
>   smp_mb();
>   ret = __sync_add_and_fetch(addr, v);
>   smp_mb();
>   ret;
> 
> get_cycles(): use gettimeofday.
> 
> This is based on the assumption that all architecture's __sync_*() primitives
> are right, modulo possibly incorrect memory barriers (so we provide our own).
> This would be basically showing extreme distrust in gcc's __sync_*() memory
> barriers.
> 
> Thoughts ?

Interesting...  I just managed to convince myself that "unknown" should
refuse to build.  I will think some more about it and send a patch
tomorrow.  ;-)

							Thanx, Paul