Cleanup: cast pthread_self() return value to unsigned long
pthread_t can map to other things that unsigned long (e.g. pointer).
Cast it to unsigned long for debug printing and for debug delay random
value purposes.
Fallback mechanism not working on platform where TLS is unsupported
The CONFIG_RCU_TLS entry in config.h.in is defined by default to "TLS".
This has the unfortunate consequence of defining CONFIG_RCU_TLS on
platform where TLS is unsupported and effectively disabling the pthread
based fallback mechanism. This macro should be #undef by default and the
AX_TLS m4 macro will properly detect if TLS is supported.
Signed-off-by: Christian Babeux <christian.babeux@efficios.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
* Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
> * Lai Jiangshan (laijs@cn.fujitsu.com) wrote:
> > test code:
> > ./tests/test_urcu_lfs 100 10 10
> >
> > bug produce rate > 60%
> >
> > {{{
> > I didn't see any bug when "./tests/test_urcu_lfs 10 10 10" Or
> +"./tests/test_urcu_lfs 100 100 10"
> > But I just test it about 5 times
> > }}}
> >
> > 4cores*1threads: Intel(R) Core(TM) i5 CPU 760
> > RCU_MB (no time to test for other rcu type)
> > test commit: 768fba83676f49eb73fd1d8ad452016a84c5ec2a
> >
> > I didn't see any bug when "./tests/test_urcu_mb 10 100 10"
> >
> > Sorry, I tried, but I failed to find out the root cause currently.
>
> I think I managed to narrow down the issue:
>
> 1) the master branch does not reproduce it, but commit
> 768fba83676f49eb73fd1d8ad452016a84c5ec2a repdroduces it about 50% of the
> time.
>
> 2) the main change between 768fba83676f49eb73fd1d8ad452016a84c5ec2a and
> current master (f94061a3df4c9eab9ac869a19e4228de54771fcb) is call_rcu
> moving to wfcqueue.
>
> 3) the bug always arise, for me, at the end of the 10 seconds.
> However, it might be simply due to the fact that most of the memory
> get freed at the end of program execution.
>
> 4) I've been able to get a backtrace, and it looks like we have some
> call_rcu callback-invokation threads still working while
> call_rcu_data_free() is invoked. In the backtrace, call_rcu_data_free()
> is nicely waiting for the next thread to stop, and during that time,
> two callback-invokation threads are invoking callbacks (and one of
> them triggers the segfault).
>
> So I expect that commit
>
> commit 5161f31e09ce33dd79afad8d08a2372fbf1c4fbe
> Author: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Date: Tue Sep 25 10:50:49 2012 -0500
>
> call_rcu: use wfcqueue, eliminate false-sharing
>
> Eliminate false-sharing between call_rcu (enqueuer) and worker threads
> on the queue head and tail.
>
> Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>
> Could have managed to fix the issue, or change the timing enough that it
> does not reproduces. I'll continue investigating.
The bug was in call rcu. It is not required for master, because we fixed
it while moving to wfcqueue. We were erroneously writing to the head
field of the default call_rcu_data rather than tail.
The conditions to reproduce this bug:
1) setup per-cpu callback-invokation threads,
2) use call_rcu
3) call call_rcu_data_free() while there are still some pending
callbacks that have not yet been executed by the callback-invokation
threads,
4) we then get corruption due to the "default" callback invokation
that walks through a corrupted queue.
Ensure that read-side functions meet 10-line LGPL criterion
This commit ensures that all read-side functions meet the 10-line LGPL
criterion that permits them to be expanded directly into non-LGPL code,
without function-call instructions. It also documents this as the
intent.
[ paulmck: Spelling fixes called out by Josh Triplett and name
change called out by Mathieu Desnoyers (_rcu_read_lock_help() ->
_rcu_read_lock_update(). ]
[ Mathieu Desnoyers: _rcu_read_unlock_help renamed to
_rcu_read_unlock_update_and_wakeup, and spelling fix for
preced -> precede. ]
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Lai Jiangshan [Thu, 9 Aug 2012 14:24:38 +0000 (10:24 -0400)]
urcu: move busy-wait code and name it ___cds_wfq_node_sync_next()
This code which waits for a node's next pointer until it appears, will
be used many times, move it to a help function and name it
___cds_wfq_node_sync_next().
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Lai Jiangshan [Thu, 9 Aug 2012 14:19:14 +0000 (10:19 -0400)]
urcu: fix compat_futex_noasync()
This patch fix two critical problems in the compatibility fallback of
compact_futex_noasync():
1) compat_futex_cond is not bound to any @uaddr, it services all @uaddr,
if you wakeup only one thread(pthread_cond_signal), the @uaddr of
this waking thread and the @uaddr of the woken-up thread may be different.
The woken-up thread will very probably go to sleep again
because his own condition is not true.
*And* this waking thread(FUTEX_WAKE) wake up NOTHING.
2) If the caller want to wake up all waiting threads, he will use INT_MAX
for @val, and:
for (i = 0; i < INT_MAX; i++)
pthread_cond_signal(&compat_futex_cond);
becomes almost infinity loop.
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
[ Edit by Mathieu Desnoyers: add explanations about supported
MIPS architectures, extracted from conversation with Ralf Baechle:
* Supported architectures
Ralf Baechle (edited by Mathieu Desnoyers):
This code works on all MIPS architecture variants. The memory barrier
instruction, SYNC, was introduced for MIPS II. The original MIPS I
instruction set doesn't have SYNC nor SMP support in the processor
architecture itself so SMP required horrible kludges in the system
hardware. I think it's safe to say that Linux/MIPS will never support
any of these MIPS I SMP systems. In the unlikely case this happens
anyway, we have a (Linux) kernel emulation of the SYNC instruction.
Voila - full binary compatibility across all MIPS processors and the
oldest hardware pays the performance penalty.
* Choice of barrier for cmm_mb()/cmm_rmb()/cmm_wmb()
Ralf Baechle:
"RMI (aka Netlogic and now Broadcom) XLR processor cores can be
configured to permit LD-LD, LD-ST, ST-LD and ST-ST reordering; default
is only ST-ST reordering. To allow Linux to eventually enable full
reordering cmm_mb(), cmm_rmb() and cmm_wmb() all should perform SYNC
and a compiler barrier."
* No-op choice for cmm_read_barrier_depends():
Ralf Baechle:
"Technically there is nothing in the MIPS architecture spec that would
keep a MIPS implementation from reordering as freely as an Alpha or
even more liberally. In practice most do strong ordering. However
there is no MIPS implementation that makes full use of all the rope
provided. So in theory a paranoid implementation of
cmm_read_barrier_depends() for MIPS should perform a SYNC. In reality
it's not necessary and no sane MIPS core designer would implement
something that would design a core that need a non-empty
cmm_read_barrier_depends(). The reason why my patch had an empty one
is that I was using the Alpha code as a template."
Mathieu Desnoyers:
Moreover, the Linux kernel chooses a no-op for MIPS
read_barrier_depends() implementation, so any MIPS architecture that
would be as weak as Alpha would break the Linux kernel before breaking
the userspace RCU library.
* No need to put ".set noreorder" in cmm_mb() inline assembly:
Ralf Baechle:
"Certain instructions such as SYNC won't get reordered." ]
> I tried to build the latest urcu (git master e51500) on a Centos 6.2 box, and got:
>
> jscott@dxi0-62:~/src/userspace-rcu$ make -j4
> CDPATH="${ZSH_VERSION+.}:" && cd . && /bin/sh /users/jscott/src/userspace-rcu/config/missing --run aclocal-1.11 -I
> +config
> CDPATH="${ZSH_VERSION+.}:" && cd . && /bin/sh /users/jscott/src/userspace-rcu/config/missing --run autoconf
> cd . && /bin/sh /users/jscott/src/userspace-rcu/config/missing --run automake-1.11 --foreign
> configure:4010: error: possibly undefined macro: m4_ifnblank
> If this token and others are legitimate, please use m4_pattern_allow.
> See the Autoconf documentation.
> make: *** [configure] Error 1
> make: *** Waiting for unfinished jobs....
>
> Some digging showed that the macro m4_ifnblank requires autoconf 2.64. Centos 6.2 has autoconf 2.63. :(
>
> I just worked around it by reverting commit a767fd locally, then I can build fine.
Reported-by: John Steele Scott <toojays@toojays.net> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> CC rcutorture_urcu-urcutorture.o
> In file included from urcutorture.c:9:
> api.h: In function '__smp_thread_id':
> api.h:160: warning: cast from pointer to integer of different size
> api.h:160: warning: cast from pointer to integer of different size
> api.h: In function 'wait_thread':
> api.h:210: warning: cast from pointer to integer of different size
> api.h:210: warning: cast from pointer to integer of different size
Commit 4d0d66bb795d1ed938e11a97a4e5f71326e20c71, implementing
tls-compat.h for pthread TLS compatibility, adds a prefix in front of
each TLS symbol (__tls_*). However, some of these symbols are exported
by the URCU library (e.g. rcu_reader_mb, defined in urcu.c as
"rcu_reader", which is overloaded by the urcu/map/urcu.h) to
applications. Therefore, this breaks binary compatibility with 0.6.x
versions of the library. This is not intended, and therefore is a bug,
so we remove this __tls_* prefix from the variables declared, defined
and referenced to through the tls-compat.h API for compilers supporting
"__thread".
NetBSD 5 implements a mremap with a different semantic. Rename our
wrapper symbol name so it does not clash with the NetBSD 5 symbol.
Eventually, we could envision doing a special-case that uses the NetBSD
5 version instead of the fallback, but let's first get it working before
going into optimization land.
Suggested-by: Marek Vavruša <marek.vavrusa@nic.cz> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
We choose to provide full memory barriers before and after successful
hash table update operations. Eventually, new API with weaker semantic
can be added, but let's make the basic API as fool-proof as possible.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> Reviewed-by: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Use cmm_smp_mb__before_uatomic_or() prior to the uatomic_or() in
_rcu_lfht_del() to ensure correct memory barrier semantic when we relax
(in the future) the barrier implementation of some architectures.
These currently translate into simple compiler barriers on all
architectures, but the and/or/add/sub/inc/dec uatomics do not provide
memory ordering guarantees (only uatomic_add_return, uatomic_sub_return,
uatomic_xchg, and uatomic_cmpxchg provides full memory barrier
guarantees before and after the atomic operations).
T0 T1
replace A by B
del A
read A->next
-> check REMOVED flag, not set yet.
read A->next
-> check REMOVED flag, not set yet.
cmpxchg A->next to set REMOVED flag
-> cmpxchg succeeds
uatomic_or to set REMOVED flag
uatomic_xchg to atomically set the REMOVAL_OWNER flag
-> first to set the flag.
Replace returns node -> free(A) Del success -> free(A)
With this race, we have a double-free.
The problem with the replace code is that it does not set the
"REMOVAL_OWNER" flag.
Test case to reproduce the bug:
test_urcu_hash 0 2 20 -A -s -M 1 -N 1 -O 1
(2 threads, doing replace/del, with a hash table that has only a single
key for all values). After just a couple of seconds, either the program
hangs, or, more often, it does:
rculfhash: replace unneeded rcu_dereference by CMM_LOAD_SHARED
The difference between the two is that CMM_LOAD_SHARED() does not imply
a read barrier between the read and following uses of the data pointed
to by the pointer read.
All sites that only use the pointer load for its bits (never
dereference) don't need the read barrier implied by rcu_dereference.
Reviewed-by: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
avoid empty statement.
-------------------------
if (condition)
dbg_printf() /* forget ";", but compiler say nothing if dbg_printf() is empty */
statement;
-------------------------
also add printf format check.
(we can use gcc extention "__printf(1, 2)" to declare a dummy inline function
to do the check, but I use "printf()" directly here)
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Passing an unsigned int to uatomic_sub does not honor sign extend to
long, as we should be allowed by assume.
Fix this by introducing caa_cast_long_keep_sign(), which casts either to
long or unsigned long depending on the signedness of the argument
received. It is used in uatomic_sub before applying the "-" operator,
since this operator needs to operate on the "long" type size (since sign
extension might not be performed if the argument received is unsigned).
In file included from urcutorture.c:9:0:
api.h:67:0: warning: "__USE_GNU" redefined [enabled by default]
/usr/include/features.h:304:0: note: this is the location of the previous definition
Some thoughts on how to use the RCU lock-free hash table brought me to
figure out that using the lock-free hash table along with a per-node
mutex is a quite interesting way to deal with lookup vs teardown
coherency.
A per-node lock can be used to protect concurrent modifications of an
entry from one another, as well as concurrent read vs modification. In
addition, if we ensure that each reader/updater of the node checks if
the node has been removed right after taking the mutex, and if we
perform the node removal from the hash table with the per-node mutex
held, we can ensure that readers/updaters will never access unlinked
data.
To perform this efficiently, we need an API function to return whether
the node previously looked-up has been deleted since then.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com> CC: Lai Jiangshan <laijs@cn.fujitsu.com> CC: Stephen Hemminger <shemminger@vyatta.com>
Gerlando Falauto [Wed, 21 Dec 2011 14:50:38 +0000 (09:50 -0500)]
readme: state correct GCC dependency for ARM
If you are trying to compile liburcu for ARM and get errors like:
/usr/local/include/urcu/uatomic/generic.h:180: undefined reference to
`__sync_add_and_fetch_4'
usr/local/lib/liburcu-common.so: undefined reference to
`__sync_lock_test_and_set_4'
/usr/local/lib/liburcu.so: undefined reference to
`__sync_or_and_fetch_4'
rculfhash: use node instead of iter argument for deletion
Using a node instead of an iterator as argument for deletion allows
passing a node pointer (that would have been looked up from another data
structure, thus not using the iterator) as argument for deletion.
Deletion still returns -ENOENT if asked to delete the NULL node. This
simplifies the caller usage.
Suggested-by: Lai Jiangshan <laijs@cn.fujitsu.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
rculfhash: number of logically removed nodes should not appear in API
This is an implementation artefact that should not appear in the API. So
only count the non-removed nodes. Print a debug message showing the
number of logically removed nodes instead within the count nodes
function.
projects / urcu.git / log
