* [ltt-dev] [PATCH] fix the "unknown" case [not found] ` <20100613212807.GF2428@linux.vnet.ibm.com> @ 2010-06-13 21:36 ` Mathieu Desnoyers 2010-06-14 0:52 ` Paul E. McKenney 2010-06-14 17:04 ` Paolo Bonzini 1 sibling, 1 reply; 18+ messages in thread From: Mathieu Desnoyers @ 2010-06-13 21:36 UTC (permalink / raw) * 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 ? Thanks, Mathieu > > Thanx, Paul > > > Thanks, > > > > Mathieu > > > > > > > > I will send two patches, one that incorporates your suggestions, and > > > another that removes sync_core(). > > > > > > Thanx, Paul > > > > > > > Thanks, > > > > > > > > Mathieu > > > > > > > > > + > > > > > +#ifdef __cplusplus > > > > > +} > > > > > +#endif > > > > > + > > > > > +#include <urcu/uatomic_generic.h> > > > > > + > > > > > +#endif /* _URCU_ARCH_UATOMIC_ARMV7_H */ > > > > > > > > -- > > > > Mathieu Desnoyers > > > > Operating System Efficiency R&D Consultant > > > > EfficiOS Inc. > > > > http://www.efficios.com > > > > -- > > Mathieu Desnoyers > > Operating System Efficiency R&D Consultant > > EfficiOS Inc. > > http://www.efficios.com -- Mathieu Desnoyers Operating System Efficiency R&D Consultant EfficiOS Inc. http://www.efficios.com ^ permalink raw reply [flat|nested] 18+ messages in thread
* [ltt-dev] [PATCH] fix the "unknown" case 2010-06-13 21:36 ` [ltt-dev] [PATCH] fix the "unknown" case Mathieu Desnoyers @ 2010-06-14 0:52 ` Paul E. McKenney 2010-06-14 2:08 ` Mathieu Desnoyers 0 siblings, 1 reply; 18+ messages in thread From: Paul E. McKenney @ 2010-06-14 0:52 UTC (permalink / raw) 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? Thanx, Paul > Thanks, > > Mathieu > > > > > Thanx, Paul > > > > > Thanks, > > > > > > Mathieu > > > > > > > > > > > I will send two patches, one that incorporates your suggestions, and > > > > another that removes sync_core(). > > > > > > > > Thanx, Paul > > > > > > > > > Thanks, > > > > > > > > > > Mathieu > > > > > > > > > > > + > > > > > > +#ifdef __cplusplus > > > > > > +} > > > > > > +#endif > > > > > > + > > > > > > +#include <urcu/uatomic_generic.h> > > > > > > + > > > > > > +#endif /* _URCU_ARCH_UATOMIC_ARMV7_H */ > > > > > > > > > > -- > > > > > Mathieu Desnoyers > > > > > Operating System Efficiency R&D Consultant > > > > > EfficiOS Inc. > > > > > http://www.efficios.com > > > > > > -- > > > Mathieu Desnoyers > > > Operating System Efficiency R&D Consultant > > > EfficiOS Inc. > > > http://www.efficios.com > > -- > Mathieu Desnoyers > Operating System Efficiency R&D Consultant > EfficiOS Inc. > http://www.efficios.com ^ permalink raw reply [flat|nested] 18+ messages in thread
* [ltt-dev] [PATCH] fix the "unknown" case 2010-06-14 0:52 ` Paul E. McKenney @ 2010-06-14 2:08 ` Mathieu Desnoyers 2010-06-14 2:46 ` Mathieu Desnoyers 0 siblings, 1 reply; 18+ messages in thread From: Mathieu Desnoyers @ 2010-06-14 2:08 UTC (permalink / raw) * 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. Thanks, Mathieu > > Thanx, Paul > > > Thanks, > > > > Mathieu > > > > > > > > Thanx, Paul > > > > > > > Thanks, > > > > > > > > Mathieu > > > > > > > > > > > > > > I will send two patches, one that incorporates your suggestions, and > > > > > another that removes sync_core(). > > > > > > > > > > Thanx, Paul > > > > > > > > > > > Thanks, > > > > > > > > > > > > Mathieu > > > > > > > > > > > > > + > > > > > > > +#ifdef __cplusplus > > > > > > > +} > > > > > > > +#endif > > > > > > > + > > > > > > > +#include <urcu/uatomic_generic.h> > > > > > > > + > > > > > > > +#endif /* _URCU_ARCH_UATOMIC_ARMV7_H */ > > > > > > > > > > > > -- > > > > > > Mathieu Desnoyers > > > > > > Operating System Efficiency R&D Consultant > > > > > > EfficiOS Inc. > > > > > > http://www.efficios.com > > > > > > > > -- > > > > Mathieu Desnoyers > > > > Operating System Efficiency R&D Consultant > > > > EfficiOS Inc. > > > > http://www.efficios.com > > > > -- > > Mathieu Desnoyers > > Operating System Efficiency R&D Consultant > > EfficiOS Inc. > > http://www.efficios.com -- Mathieu Desnoyers Operating System Efficiency R&D Consultant EfficiOS Inc. http://www.efficios.com ^ permalink raw reply [flat|nested] 18+ messages in thread
* [ltt-dev] [PATCH] fix the "unknown" case 2010-06-14 2:08 ` Mathieu Desnoyers @ 2010-06-14 2:46 ` Mathieu Desnoyers 2010-06-14 5:11 ` Paul E. McKenney 2010-06-14 5:41 ` Paul E. McKenney 0 siblings, 2 replies; 18+ messages in thread From: Mathieu Desnoyers @ 2010-06-14 2:46 UTC (permalink / raw) * 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 ? Thanks, Mathieu > > Thanks, > > Mathieu > > > > > Thanx, Paul > > > > > Thanks, > > > > > > Mathieu > > > > > > > > > > > Thanx, Paul > > > > > > > > > Thanks, > > > > > > > > > > Mathieu > > > > > > > > > > > > > > > > > I will send two patches, one that incorporates your suggestions, and > > > > > > another that removes sync_core(). > > > > > > > > > > > > Thanx, Paul > > > > > > > > > > > > > Thanks, > > > > > > > > > > > > > > Mathieu > > > > > > > > > > > > > > > + > > > > > > > > +#ifdef __cplusplus > > > > > > > > +} > > > > > > > > +#endif > > > > > > > > + > > > > > > > > +#include <urcu/uatomic_generic.h> > > > > > > > > + > > > > > > > > +#endif /* _URCU_ARCH_UATOMIC_ARMV7_H */ > > > > > > > > > > > > > > -- > > > > > > > Mathieu Desnoyers > > > > > > > Operating System Efficiency R&D Consultant > > > > > > > EfficiOS Inc. > > > > > > > http://www.efficios.com > > > > > > > > > > -- > > > > > Mathieu Desnoyers > > > > > Operating System Efficiency R&D Consultant > > > > > EfficiOS Inc. > > > > > http://www.efficios.com > > > > > > -- > > > Mathieu Desnoyers > > > Operating System Efficiency R&D Consultant > > > EfficiOS Inc. > > > http://www.efficios.com > > -- > Mathieu Desnoyers > Operating System Efficiency R&D Consultant > EfficiOS Inc. > http://www.efficios.com -- Mathieu Desnoyers Operating System Efficiency R&D Consultant EfficiOS Inc. http://www.efficios.com ^ permalink raw reply [flat|nested] 18+ messages in thread
* [ltt-dev] [PATCH] fix the "unknown" case 2010-06-14 2:46 ` Mathieu Desnoyers @ 2010-06-14 5:11 ` Paul E. McKenney 2010-06-14 5:41 ` Paul E. McKenney 1 sibling, 0 replies; 18+ messages in thread From: Paul E. McKenney @ 2010-06-14 5:11 UTC (permalink / raw) 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 ? Worth a try. If we have trouble with it (e.g., non-atomic implementations of the primitives), then there is always #error. ;-) Thanx, Paul > Thanks, > > Mathieu > > > > > > Thanks, > > > > Mathieu > > > > > > > > Thanx, Paul > > > > > > > Thanks, > > > > > > > > Mathieu > > > > > > > > > > > > > > Thanx, Paul > > > > > > > > > > > Thanks, > > > > > > > > > > > > Mathieu > > > > > > > > > > > > > > > > > > > > I will send two patches, one that incorporates your suggestions, and > > > > > > > another that removes sync_core(). > > > > > > > > > > > > > > Thanx, Paul > > > > > > > > > > > > > > > Thanks, > > > > > > > > > > > > > > > > Mathieu > > > > > > > > > > > > > > > > > + > > > > > > > > > +#ifdef __cplusplus > > > > > > > > > +} > > > > > > > > > +#endif > > > > > > > > > + > > > > > > > > > +#include <urcu/uatomic_generic.h> > > > > > > > > > + > > > > > > > > > +#endif /* _URCU_ARCH_UATOMIC_ARMV7_H */ > > > > > > > > > > > > > > > > -- > > > > > > > > Mathieu Desnoyers > > > > > > > > Operating System Efficiency R&D Consultant > > > > > > > > EfficiOS Inc. > > > > > > > > http://www.efficios.com > > > > > > > > > > > > -- > > > > > > Mathieu Desnoyers > > > > > > Operating System Efficiency R&D Consultant > > > > > > EfficiOS Inc. > > > > > > http://www.efficios.com > > > > > > > > -- > > > > Mathieu Desnoyers > > > > Operating System Efficiency R&D Consultant > > > > EfficiOS Inc. > > > > http://www.efficios.com > > > > -- > > Mathieu Desnoyers > > Operating System Efficiency R&D Consultant > > EfficiOS Inc. > > http://www.efficios.com > > -- > Mathieu Desnoyers > Operating System Efficiency R&D Consultant > EfficiOS Inc. > http://www.efficios.com ^ permalink raw reply [flat|nested] 18+ messages in thread
* [ltt-dev] [PATCH] fix the "unknown" case 2010-06-14 2:46 ` Mathieu Desnoyers 2010-06-14 5:11 ` Paul E. McKenney @ 2010-06-14 5:41 ` Paul E. McKenney 1 sibling, 0 replies; 18+ messages in thread From: Paul E. McKenney @ 2010-06-14 5:41 UTC (permalink / raw) 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* [ltt-dev] [PATCH] fix the "unknown" case [not found] ` <20100613212807.GF2428@linux.vnet.ibm.com> 2010-06-13 21:36 ` [ltt-dev] [PATCH] fix the "unknown" case Mathieu Desnoyers @ 2010-06-14 17:04 ` Paolo Bonzini 2010-06-14 18:25 ` Paul E. McKenney 1 sibling, 1 reply; 18+ messages in thread From: Paolo Bonzini @ 2010-06-14 17:04 UTC (permalink / raw) On 06/13/2010 11:28 PM, Paul E. McKenney 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. The default implementation of these __sync_* builtins is based on cmpxchg, and will cause a link error unless cmpxchg is also available (either in libgcc or with a compiler-provided inline implementation). Instead, the default implementation of __sync_synchronize is to just do a compiler barrier. ARM implements __sync_synchronize only for Linux, so at least there it is not needed. Strange that Paul needs it too. Anyway, a simple configure test is to compile this with -fdump-rtl-expand: int f() { __sync_synchronize(); } If the assembly output includes "__sync_synchronize", or the dump file includes the text "unspec:BLK", it should be fine. In particular, ia64, mips, and Alpha are ok. Else you can use the pthreads trick. I can try to make a patch if you're interested. Or, more simply, it's possible to hardcode the above three platforms since it's unlikely that others will be added soon. Paolo ^ permalink raw reply [flat|nested] 18+ messages in thread
* [ltt-dev] [PATCH] fix the "unknown" case 2010-06-14 17:04 ` Paolo Bonzini @ 2010-06-14 18:25 ` Paul E. McKenney 2010-06-15 8:52 ` Paolo Bonzini 0 siblings, 1 reply; 18+ messages in thread From: Paul E. McKenney @ 2010-06-14 18:25 UTC (permalink / raw) On Mon, Jun 14, 2010 at 07:04:43PM +0200, Paolo Bonzini wrote: > On 06/13/2010 11:28 PM, Paul E. McKenney 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. > > The default implementation of these __sync_* builtins is based on > cmpxchg, and will cause a link error unless cmpxchg is also > available (either in libgcc or with a compiler-provided inline > implementation). > > Instead, the default implementation of __sync_synchronize is to just > do a compiler barrier. ARM implements __sync_synchronize only for > Linux, so at least there it is not needed. Strange that Paul needs > it too. When I use __sync_synchronize(), I get failures from the rcutorture_urcu_mb stress test. When I use a "dmb" instruction, I don't get any failures. No idea why, to be honest. > Anyway, a simple configure test is to compile this with -fdump-rtl-expand: > > int > f() > { > __sync_synchronize(); > } > > If the assembly output includes "__sync_synchronize", or the dump > file includes the text "unspec:BLK", it should be fine. In > particular, ia64, mips, and Alpha are ok. Else you can use the > pthreads trick. I can try to make a patch if you're interested. > Or, more simply, it's possible to hardcode the above three platforms > since it's unlikely that others will be added soon. And I attached the input file, the .expand file, and the .s file. I see neither __sync_synchronize in the .s file nor "unspec" in the .expand file. Or was I confused about what to look for? Thanx, Paul -------------- next part -------------- A non-text attachment was scrubbed... Name: sync_sync.c Type: text/x-csrc Size: 35 bytes Desc: sync_sync.c URL: <http://lists.casi.polymtl.ca/pipermail/lttng-dev/attachments/20100614/20bd70cd/attachment-0003.c> -------------- next part -------------- .arch armv6 .eabi_attribute 27, 3 .fpu vfp .eabi_attribute 20, 1 .eabi_attribute 21, 1 .eabi_attribute 23, 3 .eabi_attribute 24, 1 .eabi_attribute 25, 1 .eabi_attribute 26, 2 .eabi_attribute 30, 6 .eabi_attribute 18, 4 .file "sync_sync.c" .text .align 2 .global f .type f, %function f: @ args = 0, pretend = 0, frame = 0 @ frame_needed = 1, uses_anonymous_args = 0 @ link register save eliminated. str fp, [sp, #-4]! add fp, sp, #0 add sp, fp, #0 ldmfd sp!, {fp} bx lr .size f, .-f .ident "GCC: (Ubuntu 4.4.1-4ubuntu9) 4.4.1" .section .note.GNU-stack,"",%progbits -------------- next part -------------- ;; Function f (f) ;; Generating RTL for gimple basic block 2 ;; ;; Full RTL generated for this function: ;; (note 1 0 3 NOTE_INSN_DELETED) (note 3 1 2 2 [bb 2] NOTE_INSN_BASIC_BLOCK) (note 2 3 4 2 NOTE_INSN_FUNCTION_BEG) (note 4 2 5 3 [bb 3] NOTE_INSN_BASIC_BLOCK) (insn 5 4 14 3 sync_sync.c:4 (parallel [ (asm_operands/v ("") ("") 0 [] [] 477) (clobber (mem:BLK (scratch) [0 A8])) ]) -1 (nil)) (note 14 5 8 4 [bb 4] NOTE_INSN_BASIC_BLOCK) (insn 8 14 9 4 sync_sync.c:5 (clobber (reg/i:SI 0 r0)) -1 (nil)) (insn 9 8 10 4 sync_sync.c:5 (clobber (reg:SI 133 [ <result> ])) -1 (nil)) (jump_insn 10 9 11 4 sync_sync.c:5 (set (pc) (label_ref 12)) -1 (nil)) (barrier 11 10 6) (code_label 6 11 15 5 1 "" [0 uses]) (note 15 6 7 5 [bb 5] NOTE_INSN_BASIC_BLOCK) (insn 7 15 12 5 sync_sync.c:5 (set (reg/i:SI 0 r0) (reg:SI 133 [ <result> ])) -1 (nil)) (code_label 12 7 16 6 2 "" [1 uses]) (note 16 12 13 6 [bb 6] NOTE_INSN_BASIC_BLOCK) (insn 13 16 0 6 sync_sync.c:5 (use (reg/i:SI 0 r0)) -1 (nil)) ^ permalink raw reply [flat|nested] 18+ messages in thread
* [ltt-dev] [PATCH] fix the "unknown" case 2010-06-14 18:25 ` Paul E. McKenney @ 2010-06-15 8:52 ` Paolo Bonzini 2010-06-15 14:57 ` Paul E. McKenney 0 siblings, 1 reply; 18+ messages in thread From: Paolo Bonzini @ 2010-06-15 8:52 UTC (permalink / raw) On 06/14/2010 08:25 PM, Paul E. McKenney wrote: >> Anyway, a simple configure test is to compile this with -fdump-rtl-expand: >> >> int >> f() >> { >> __sync_synchronize(); >> } >> >> If the assembly output includes "__sync_synchronize", or the dump >> file includes the text "unspec:BLK", it should be fine. In >> particular, ia64, mips, and Alpha are ok. Else you can use the >> pthreads trick. I can try to make a patch if you're interested. >> Or, more simply, it's possible to hardcode the above three platforms >> since it's unlikely that others will be added soon. > > And I attached the input file, the .expand file, and the .s file. > > I see neither __sync_synchronize in the .s file nor "unspec" in the > .expand file. Or was I confused about what to look for? No, the __sync_synchronize instruction is optimized out. I think I'll make GCC 4.6+ give a warning. Paolo ^ permalink raw reply [flat|nested] 18+ messages in thread
* [ltt-dev] [PATCH] fix the "unknown" case 2010-06-15 8:52 ` Paolo Bonzini @ 2010-06-15 14:57 ` Paul E. McKenney 0 siblings, 0 replies; 18+ messages in thread From: Paul E. McKenney @ 2010-06-15 14:57 UTC (permalink / raw) On Tue, Jun 15, 2010 at 10:52:23AM +0200, Paolo Bonzini wrote: > On 06/14/2010 08:25 PM, Paul E. McKenney wrote: > >>Anyway, a simple configure test is to compile this with -fdump-rtl-expand: > >> > >> int > >> f() > >> { > >> __sync_synchronize(); > >> } > >> > >>If the assembly output includes "__sync_synchronize", or the dump > >>file includes the text "unspec:BLK", it should be fine. In > >>particular, ia64, mips, and Alpha are ok. Else you can use the > >>pthreads trick. I can try to make a patch if you're interested. > >>Or, more simply, it's possible to hardcode the above three platforms > >>since it's unlikely that others will be added soon. > > > >And I attached the input file, the .expand file, and the .s file. > > > >I see neither __sync_synchronize in the .s file nor "unspec" in the > >.expand file. Or was I confused about what to look for? > > No, the __sync_synchronize instruction is optimized out. I think > I'll make GCC 4.6+ give a warning. Thank you for looking into this! Hmmm... One mistake I made last time was to forget the "-mcpu=cortex-a9 -mtune=cortex-a9" -- but adding this does not change the result. If I have other code in the function, I do get the following in the .s file: @ 10 "gcc_sync.c" 1 #__sync_synchronize But still no meaningful code. So for the moment I am using my own asm to generate a dmb instruction. Which raises the question as to whether I can really trust the other __sync_ instructions. They -seem- to be working for me, but one cannot prove them correct by testing. :-( Thanx, Paul ^ permalink raw reply [flat|nested] 18+ messages in thread
[parent not found: <20100614220600.GA15130@linux.vnet.ibm.com>]
* [ltt-dev] [PATCH] fix the "unknown" case [not found] <20100614220600.GA15130@linux.vnet.ibm.com> @ 2010-06-15 12:35 ` Ulrich Weigand 2010-06-15 15:07 ` Paul E. McKenney 0 siblings, 1 reply; 18+ messages in thread From: Ulrich Weigand @ 2010-06-15 12:35 UTC (permalink / raw) "Paul E. McKenney" <paulmck at linux.vnet.ibm.com> wrote on 06/15/2010 12:06:00 AM: > When I use __sync_synchronize(), I get failures from the rcutorture_urcu_mb > stress test. When I use a "dmb" instruction, I don't get any failures. > > No idea why, to be honest. If you get the code as below for __sync_synchronize, that's no wonder: this code is a plain compiler optimization barrier, it does not have any hardware memory barrier at all ... > int > f() > { > __sync_synchronize(); > } [snip] > str fp, [sp, #-4]! > add fp, sp, #0 > add sp, fp, #0 > ldmfd sp!, {fp} > bx lr No hardware barrier instruction ... > (insn 5 4 14 3 sync_sync.c:4 (parallel [ > (asm_operands/v ("") ("") 0 [] > [] 477) > (clobber (mem:BLK (scratch) [0 A8])) > ]) -1 (nil)) ... as this is just a optimization barrier here. Looking at the compiler sources, the __sync_synchronize call gets translated into: a. a hardware memory barrier instruction if provided by the platform b. otherwise, a call to a __sync_synchronize libgcc routine if provided by the platform c. otherwise, just a compiler optimization barrier On ARM (at least on GCC mainline), there is no memory barrier hardware instruction defined in the backend, so you'll never get a. There *is* support for b., i.e. a call to a library func, but only if you're using a quite recent compiler. Support was added to mainline on 2009-08-12; the first official GCC release to contain the change is 4.5.0; the change was later backported to 4.4.3. (As you seem to be using 4.4.1, this feature is probably just not there yet.) See also the discussion in this bugzilla and the mailing list threads linked from it: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42263 Mit freundlichen Gruessen / Best Regards Ulrich Weigand -- Dr. Ulrich Weigand | Phone: +49-7031/16-3727 STSM, GNU compiler and toolchain for Linux on System z and Cell/B.E. IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martin Jetter | Gesch?ftsf?hrung: Dirk Wittkopp Sitz der Gesellschaft: B?blingen | Registergericht: Amtsgericht Stuttgart, HRB 243294 ^ permalink raw reply [flat|nested] 18+ messages in thread
* [ltt-dev] [PATCH] fix the "unknown" case 2010-06-15 12:35 ` Ulrich Weigand @ 2010-06-15 15:07 ` Paul E. McKenney 2010-06-15 15:18 ` Paolo Bonzini 2010-06-15 16:07 ` Ulrich Weigand 0 siblings, 2 replies; 18+ messages in thread From: Paul E. McKenney @ 2010-06-15 15:07 UTC (permalink / raw) On Tue, Jun 15, 2010 at 02:35:16PM +0200, Ulrich Weigand wrote: > "Paul E. McKenney" <paulmck at linux.vnet.ibm.com> wrote on 06/15/2010 > 12:06:00 AM: > > > When I use __sync_synchronize(), I get failures from the > rcutorture_urcu_mb > > stress test. When I use a "dmb" instruction, I don't get any failures. > > > > No idea why, to be honest. > > If you get the code as below for __sync_synchronize, that's no wonder: this > code is a plain compiler optimization barrier, it does not have any > hardware > memory barrier at all ... That would explain it! ;-) > > int > > f() > > { > > __sync_synchronize(); > > } > [snip] > > str fp, [sp, #-4]! > > add fp, sp, #0 > > add sp, fp, #0 > > ldmfd sp!, {fp} > > bx lr > > No hardware barrier instruction ... > > > (insn 5 4 14 3 sync_sync.c:4 (parallel [ > > (asm_operands/v ("") ("") 0 [] > > [] 477) > > (clobber (mem:BLK (scratch) [0 A8])) > > ]) -1 (nil)) > > ... as this is just a optimization barrier here. > > Looking at the compiler sources, the __sync_synchronize call gets > translated into: > > a. a hardware memory barrier instruction if provided by the platform > b. otherwise, a call to a __sync_synchronize libgcc routine if provided > by the platform > c. otherwise, just a compiler optimization barrier > > On ARM (at least on GCC mainline), there is no memory barrier hardware > instruction defined in the backend, so you'll never get a. > > There *is* support for b., i.e. a call to a library func, but only if > you're using a quite recent compiler. Support was added to mainline > on 2009-08-12; the first official GCC release to contain the change > is 4.5.0; the change was later backported to 4.4.3. (As you seem to > be using 4.4.1, this feature is probably just not there yet.) Thank you very much for looking into this and for the information! > See also the discussion in this bugzilla and the mailing list threads > linked from it: > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42263 So I cannot expect the other __sync_ primitives to be generate memory barriers, either, correct? Hmmm... From looking at http://gcc.gnu.org/ml/gcc-patches/2009-12/msg00198.html, I wonder whether I can rely on them to be using atomic instructions -- though I do admit that __sync_lock_release() often does not need an atomic instruction. So, should I just bite the bullet and write the usual set of asms myself? People are going to want to build liburcu on old compilers, sad to say... Thanx, Paul > Mit freundlichen Gruessen / Best Regards > > Ulrich Weigand > > -- > Dr. Ulrich Weigand | Phone: +49-7031/16-3727 > STSM, GNU compiler and toolchain for Linux on System z and Cell/B.E. > IBM Deutschland Research & Development GmbH > Vorsitzender des Aufsichtsrats: Martin Jetter | Gesch?ftsf?hrung: Dirk > Wittkopp > Sitz der Gesellschaft: B?blingen | Registergericht: Amtsgericht > Stuttgart, HRB 243294 > ^ permalink raw reply [flat|nested] 18+ messages in thread
* [ltt-dev] [PATCH] fix the "unknown" case 2010-06-15 15:07 ` Paul E. McKenney @ 2010-06-15 15:18 ` Paolo Bonzini 2010-06-15 16:21 ` Paul E. McKenney 2010-06-15 16:07 ` Ulrich Weigand 1 sibling, 1 reply; 18+ messages in thread From: Paolo Bonzini @ 2010-06-15 15:18 UTC (permalink / raw) On 06/15/2010 05:07 PM, Paul E. McKenney wrote: > So, should I just bite the bullet and write the usual set of asms > myself? People are going to want to build liburcu on old compilers, > sad to say... If you care about old/buggy compilers you'll always have to anyway. Paolo ^ permalink raw reply [flat|nested] 18+ messages in thread
* [ltt-dev] [PATCH] fix the "unknown" case 2010-06-15 15:18 ` Paolo Bonzini @ 2010-06-15 16:21 ` Paul E. McKenney 0 siblings, 0 replies; 18+ messages in thread From: Paul E. McKenney @ 2010-06-15 16:21 UTC (permalink / raw) On Tue, Jun 15, 2010 at 05:18:13PM +0200, Paolo Bonzini wrote: > On 06/15/2010 05:07 PM, Paul E. McKenney wrote: > >So, should I just bite the bullet and write the usual set of asms > >myself? People are going to want to build liburcu on old compilers, > >sad to say... > > If you care about old/buggy compilers you'll always have to anyway. Well, maybe I can at least have a #ifdef that checks for compiler version or something. Thanx, Paul ^ permalink raw reply [flat|nested] 18+ messages in thread
* [ltt-dev] [PATCH] fix the "unknown" case 2010-06-15 15:07 ` Paul E. McKenney 2010-06-15 15:18 ` Paolo Bonzini @ 2010-06-15 16:07 ` Ulrich Weigand 2010-06-15 17:00 ` Paul E. McKenney 1 sibling, 1 reply; 18+ messages in thread From: Ulrich Weigand @ 2010-06-15 16:07 UTC (permalink / raw) "Paul E. McKenney" <paulmck at linux.vnet.ibm.com> wrote on 06/15/2010 05:07:27 PM: > > See also the discussion in this bugzilla and the mailing list threads > > linked from it: > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42263 > > So I cannot expect the other __sync_ primitives to be generate > memory barriers, either, correct? Hmmm... From looking at > http://gcc.gnu.org/ml/gcc-patches/2009-12/msg00198.html, I wonder > whether I can rely on them to be using atomic instructions -- though > I do admit that __sync_lock_release() often does not need an atomic > instruction. As mentioned in the bugzilla, there were bugs in __sync_synchronize (which was just a compiler optimization barrier), and in __sync_lock_release, which did use a memory barrier, but at the wrong place. With a compiler where these two are fixes (GCC 4.4.3 and above), all the __sync_ primitives will generate memory barriers, but not directly: the compiler will call the libgcc library function, which will call a special ARM kernel entry point, which will then use an appropriate instruction depending whether the kernel is compiled for SMP or UP, and depending on the target instruction set level. For example, the __kernel_dmb call uses this piece of code in the kernel: .macro smp_dmb #ifdef CONFIG_SMP #if __LINUX_ARM_ARCH__ >= 7 dmb #elif __LINUX_ARM_ARCH__ == 6 mcr p15, 0, r0, c7, c10, 5 @ dmb #endif #endif .endm > So, should I just bite the bullet and write the usual set of asms > myself? People are going to want to build liburcu on old compilers, > sad to say... As Paolo already said, you probably do want to support older compilers. On the other hand, if you hard-code the dmb instruction, it seems you're tied to the ARM 7 architecture level ... Maybe you want to directly use the __kernel_dmb etc. calls in liburcu? They seem to be official kernel ABI, so should be OK to use. Mit freundlichen Gruessen / Best Regards Ulrich Weigand -- Dr. Ulrich Weigand | Phone: +49-7031/16-3727 STSM, GNU compiler and toolchain for Linux on System z and Cell/B.E. IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martin Jetter | Gesch?ftsf?hrung: Dirk Wittkopp Sitz der Gesellschaft: B?blingen | Registergericht: Amtsgericht Stuttgart, HRB 243294 ^ permalink raw reply [flat|nested] 18+ messages in thread
* [ltt-dev] [PATCH] fix the "unknown" case 2010-06-15 16:07 ` Ulrich Weigand @ 2010-06-15 17:00 ` Paul E. McKenney 2010-06-15 17:03 ` Mathieu Desnoyers 0 siblings, 1 reply; 18+ messages in thread From: Paul E. McKenney @ 2010-06-15 17:00 UTC (permalink / raw) On Tue, Jun 15, 2010 at 06:07:20PM +0200, Ulrich Weigand wrote: > "Paul E. McKenney" <paulmck at linux.vnet.ibm.com> wrote on 06/15/2010 > 05:07:27 PM: > > > > See also the discussion in this bugzilla and the mailing list threads > > > linked from it: > > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42263 > > > > So I cannot expect the other __sync_ primitives to be generate > > memory barriers, either, correct? Hmmm... From looking at > > http://gcc.gnu.org/ml/gcc-patches/2009-12/msg00198.html, I wonder > > whether I can rely on them to be using atomic instructions -- though > > I do admit that __sync_lock_release() often does not need an atomic > > instruction. > > As mentioned in the bugzilla, there were bugs in __sync_synchronize > (which was just a compiler optimization barrier), and in > __sync_lock_release, > which did use a memory barrier, but at the wrong place. > > With a compiler where these two are fixes (GCC 4.4.3 and above), all the > __sync_ primitives will generate memory barriers, but not directly: the > compiler will call the libgcc library function, which will call a special > ARM kernel entry point, which will then use an appropriate instruction > depending whether the kernel is compiled for SMP or UP, and depending on > the target instruction set level. For example, the __kernel_dmb call > uses this piece of code in the kernel: > > .macro smp_dmb > #ifdef CONFIG_SMP > #if __LINUX_ARM_ARCH__ >= 7 > dmb > #elif __LINUX_ARM_ARCH__ == 6 > mcr p15, 0, r0, c7, c10, 5 @ dmb > #endif > #endif > .endm OK, so GCC 4.4.3 and above can be trusted. > > So, should I just bite the bullet and write the usual set of asms > > myself? People are going to want to build liburcu on old compilers, > > sad to say... > > As Paolo already said, you probably do want to support older compilers. > On the other hand, if you hard-code the dmb instruction, it seems you're > tied to the ARM 7 architecture level ... Maybe you want to directly > use the __kernel_dmb etc. calls in liburcu? They seem to be official > kernel ABI, so should be OK to use. My current autoconf code checks for "armv7l", so I am OK being ARMv7 specific. But it would be really good to cover the rest of the ARM family, and __kernel_dmb and friends might be a way to do that! Thanx, Paul ^ permalink raw reply [flat|nested] 18+ messages in thread
* [ltt-dev] [PATCH] fix the "unknown" case 2010-06-15 17:00 ` Paul E. McKenney @ 2010-06-15 17:03 ` Mathieu Desnoyers 2010-06-15 17:32 ` Ulrich Weigand 0 siblings, 1 reply; 18+ messages in thread From: Mathieu Desnoyers @ 2010-06-15 17:03 UTC (permalink / raw) * Paul E. McKenney (paulmck at linux.vnet.ibm.com) wrote: > On Tue, Jun 15, 2010 at 06:07:20PM +0200, Ulrich Weigand wrote: > > "Paul E. McKenney" <paulmck at linux.vnet.ibm.com> wrote on 06/15/2010 > > 05:07:27 PM: > > > > > > See also the discussion in this bugzilla and the mailing list threads > > > > linked from it: > > > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42263 > > > > > > So I cannot expect the other __sync_ primitives to be generate > > > memory barriers, either, correct? Hmmm... From looking at > > > http://gcc.gnu.org/ml/gcc-patches/2009-12/msg00198.html, I wonder > > > whether I can rely on them to be using atomic instructions -- though > > > I do admit that __sync_lock_release() often does not need an atomic > > > instruction. > > > > As mentioned in the bugzilla, there were bugs in __sync_synchronize > > (which was just a compiler optimization barrier), and in > > __sync_lock_release, > > which did use a memory barrier, but at the wrong place. > > > > With a compiler where these two are fixes (GCC 4.4.3 and above), all the > > __sync_ primitives will generate memory barriers, but not directly: the > > compiler will call the libgcc library function, which will call a special > > ARM kernel entry point, which will then use an appropriate instruction > > depending whether the kernel is compiled for SMP or UP, and depending on > > the target instruction set level. For example, the __kernel_dmb call > > uses this piece of code in the kernel: > > > > .macro smp_dmb > > #ifdef CONFIG_SMP > > #if __LINUX_ARM_ARCH__ >= 7 > > dmb > > #elif __LINUX_ARM_ARCH__ == 6 > > mcr p15, 0, r0, c7, c10, 5 @ dmb > > #endif > > #endif > > .endm > > OK, so GCC 4.4.3 and above can be trusted. > > > > So, should I just bite the bullet and write the usual set of asms > > > myself? People are going to want to build liburcu on old compilers, > > > sad to say... > > > > As Paolo already said, you probably do want to support older compilers. > > On the other hand, if you hard-code the dmb instruction, it seems you're > > tied to the ARM 7 architecture level ... Maybe you want to directly > > use the __kernel_dmb etc. calls in liburcu? They seem to be official > > kernel ABI, so should be OK to use. > > My current autoconf code checks for "armv7l", so I am OK being ARMv7 > specific. But it would be really good to cover the rest of the ARM > family, and __kernel_dmb and friends might be a way to do that! I wonder starting with which Linux kernel version __kernel_dmb appeared. Tying ourself directly to a Linux kernel ABI might complicate things. Is this ABI presented in a vDSO or userland have to go through a system call ? Is there any way to probe for its availability ? Thanks, Mathieu > > Thanx, Paul -- Mathieu Desnoyers Operating System Efficiency R&D Consultant EfficiOS Inc. http://www.efficios.com ^ permalink raw reply [flat|nested] 18+ messages in thread
* [ltt-dev] [PATCH] fix the "unknown" case 2010-06-15 17:03 ` Mathieu Desnoyers @ 2010-06-15 17:32 ` Ulrich Weigand 0 siblings, 0 replies; 18+ messages in thread From: Ulrich Weigand @ 2010-06-15 17:32 UTC (permalink / raw) Mathieu Desnoyers <mathieu.desnoyers at efficios.com> wrote on 06/15/2010 07:03:15 PM: > I wonder starting with which Linux kernel version __kernel_dmb appeared. > Tying ourself directly to a Linux kernel ABI might complicate things. > > Is this ABI presented in a vDSO or userland have to go through a system call ? > Is there any way to probe for its availability ? This looks sort-of like a vDSO, except without the DSO part :-) The kernel simply makes the code available at a fixed address that is directly callable by user space. See the comments in linux/arch/arm/kernel/entry-armv.S: /* * User helpers. * * These are segment of kernel provided user code reachable from user space * at a fixed address in kernel memory. This is used to provide user space * with some operations which require kernel help because of unimplemented * native feature and/or instructions in many ARM CPUs. The idea is for * this code to be executed directly in user mode for best efficiency but * which is too intimate with the kernel counter part to be left to user * libraries. In fact this code might even differ from one CPU to another * depending on the available instruction set and restrictions like on * SMP systems. In other words, the kernel reserves the right to change * this code as needed without warning. Only the entry points and their * results are guaranteed to be stable. * * Each segment is 32-byte aligned and will be moved to the top of the high * vector page. New segments (if ever needed) must be added in front of * existing ones. This mechanism should be used only for things that are * really small and justified, and not be abused freely. * * User space is expected to implement those things inline when optimizing * for a processor that has the necessary native support, but only if such * resulting binaries are already to be incompatible with earlier ARM * processors due to the use of unsupported instructions other than what * is provided here. In other words don't make binaries unable to run on * earlier processors just for the sake of not using these kernel helpers * if your compiled code is not going to use the new instructions for other * purpose. */ /* * Reference prototype: * * void __kernel_memory_barrier(void) * * Input: * * lr = return address * * Output: * * none * * Clobbered: * * none * * Definition and user space usage example: * * typedef void (__kernel_dmb_t)(void); * #define __kernel_dmb (*(__kernel_dmb_t *)0xffff0fa0) * * Apply any needed memory barrier to preserve consistency with data modified * manually and __kuser_cmpxchg usage. * * This could be used as follows: * * #define __kernel_dmb() \ * asm volatile ( "mov r0, #0xffff0fff; mov lr, pc; sub pc, r0, #95" \ * : : : "r0", "lr","cc" ) */ As far as I can see, the only provision to check whether a feature is available is this one: /* * Reference declaration: * * extern unsigned int __kernel_helper_version; * * Definition and user space usage example: * * #define __kernel_helper_version (*(unsigned int *)0xffff0ffc) * * User space may read this to determine the curent number of helpers * available. */ However, note that libgcc code does not perform this check, it simply assumes the above routine to be present. The __kernel_dmb (which is the most recently added helper available in current mainline) seems to have been available since kernel 2.6.15, so presumably code using any of the GCC sync primitives would simply fail on any older kernel, unless I'm missing something here ... Mit freundlichen Gruessen / Best Regards Ulrich Weigand -- Dr. Ulrich Weigand | Phone: +49-7031/16-3727 STSM, GNU compiler and toolchain for Linux on System z and Cell/B.E. IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martin Jetter | Gesch?ftsf?hrung: Dirk Wittkopp Sitz der Gesellschaft: B?blingen | Registergericht: Amtsgericht Stuttgart, HRB 243294 ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2010-06-15 17:32 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20100610225242.GA21978@linux.vnet.ibm.com>
[not found] ` <20100611014631.GA13838@Krystal>
[not found] ` <20100611171613.GD2394@linux.vnet.ibm.com>
[not found] ` <20100613212033.GF5427@Krystal>
[not found] ` <20100613212807.GF2428@linux.vnet.ibm.com>
2010-06-13 21:36 ` [ltt-dev] [PATCH] fix the "unknown" case Mathieu Desnoyers
2010-06-14 0:52 ` Paul E. McKenney
2010-06-14 2:08 ` Mathieu Desnoyers
2010-06-14 2:46 ` Mathieu Desnoyers
2010-06-14 5:11 ` Paul E. McKenney
2010-06-14 5:41 ` Paul E. McKenney
2010-06-14 17:04 ` Paolo Bonzini
2010-06-14 18:25 ` Paul E. McKenney
2010-06-15 8:52 ` Paolo Bonzini
2010-06-15 14:57 ` Paul E. McKenney
[not found] <20100614220600.GA15130@linux.vnet.ibm.com>
2010-06-15 12:35 ` Ulrich Weigand
2010-06-15 15:07 ` Paul E. McKenney
2010-06-15 15:18 ` Paolo Bonzini
2010-06-15 16:21 ` Paul E. McKenney
2010-06-15 16:07 ` Ulrich Weigand
2010-06-15 17:00 ` Paul E. McKenney
2010-06-15 17:03 ` Mathieu Desnoyers
2010-06-15 17:32 ` Ulrich Weigand
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox