Fix: call_rcu: teardown default call_rcu worker on application exit
Teardown the default call_rcu worker thread if there are no queued
callbacks on process exit. This prevents leaking memory.
Here is how an application can ensure graceful teardown of this
worker thread:
- An application queuing call_rcu callbacks should invoke
rcu_barrier() before it exits.
- When chaining call_rcu callbacks, the number of calls to
rcu_barrier() on application exit must match at least the maximum
number of chained callbacks.
- If an application chains callbacks endlessly, it would have to be
modified to stop chaining callbacks when it detects an application
exit (e.g. with a flag), and wait for quiescence with rcu_barrier()
after setting that flag.
- The statements above apply to a library which queues call_rcu
callbacks, only it needs to invoke rcu_barrier in its library
destructor.
Fix a deadlock for auto-resize hash tables when cds_lfht_destroy
is called with RCU read-side lock held.
Example stack track of a hang:
Thread 2 (Thread 0x7f21ba876700 (LWP 26114)):
#0 syscall () at ../sysdeps/unix/sysv/linux/x86_64/syscall.S:38
#1 0x00007f21beba7aa0 in futex (val3=0, uaddr2=0x0, timeout=0x0, val=-1, op=0, uaddr=0x7f21bedac308 <urcu_memb_gp+8>) at ../include/urcu/futex.h:81
#2 futex_noasync (timeout=0x0, uaddr2=0x0, val3=0, val=-1, op=0, uaddr=0x7f21bedac308 <urcu_memb_gp+8>) at ../include/urcu/futex.h:90
#3 wait_gp () at urcu.c:265
#4 wait_for_readers (input_readers=input_readers@entry=0x7f21ba8751b0, cur_snap_readers=cur_snap_readers@entry=0x0,
qsreaders=qsreaders@entry=0x7f21ba8751c0) at urcu.c:357
#5 0x00007f21beba8339 in urcu_memb_synchronize_rcu () at urcu.c:498
#6 0x00007f21be99f93f in fini_table (last_order=<optimized out>, first_order=13, ht=0x5651cec75400) at rculfhash.c:1489
#7 _do_cds_lfht_shrink (new_size=<optimized out>, old_size=<optimized out>, ht=0x5651cec75400) at rculfhash.c:2001
#8 _do_cds_lfht_resize (ht=ht@entry=0x5651cec75400) at rculfhash.c:2023
#9 0x00007f21be99fa26 in do_resize_cb (work=0x5651e20621a0) at rculfhash.c:2063
#10 0x00007f21be99dbfd in workqueue_thread (arg=0x5651cec74a00) at workqueue.c:234
#11 0x00007f21bd7c06db in start_thread (arg=0x7f21ba876700) at pthread_create.c:463
#12 0x00007f21bd4e961f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
Thread 1 (Thread 0x7f21bf285300 (LWP 26098)):
#0 syscall () at ../sysdeps/unix/sysv/linux/x86_64/syscall.S:38
#1 0x00007f21be99d8b7 in futex (val3=0, uaddr2=0x0, timeout=0x0, val=-1, op=0, uaddr=0x5651d8b38584) at ../include/urcu/futex.h:81
#2 futex_async (timeout=0x0, uaddr2=0x0, val3=0, val=-1, op=0, uaddr=0x5651d8b38584) at ../include/urcu/futex.h:113
#3 futex_wait (futex=futex@entry=0x5651d8b38584) at workqueue.c:135
#4 0x00007f21be99e2c8 in urcu_workqueue_wait_completion (completion=completion@entry=0x5651d8b38580) at workqueue.c:423
#5 0x00007f21be99e3f9 in urcu_workqueue_flush_queued_work (workqueue=0x5651cec74a00) at workqueue.c:452
#6 0x00007f21be9a0c83 in cds_lfht_destroy (ht=0x5651d8b2fcf0, attr=attr@entry=0x0) at rculfhash.c:1906
This deadlock is easy to reproduce when rapidly adding a large number of
entries in the cds_lfht, removing them, and calling cds_lfht_destroy().
The deadlock will occur if the call to cds_lfht_destroy() takes place
while a resize of the hash table is ongoing.
Fix this by moving the teardown of the lfht worker thread to libcds
library destructor, so it does not have to wait on synchronize_rcu from
a resize callback from within a read-side critical section. As a
consequence, the atfork callbacks are left registered within each urcu
flavor for which a resizeable hash table is created until the end of the
executable lifetime.
The other part of the fix is to move the hash table destruction to the
worker thread for auto-resize hash tables. This prevents having to wait
for resize callbacks from RCU read-side critical section. This is
guaranteed by the fact that the worker thread serializes previously
queued resize callbacks before the destroy callback.
Eric Wong [Sun, 2 Oct 2022 16:13:43 +0000 (12:13 -0400)]
Fix: Always check pthread_create for failures
pthread_create may fail with EAGAIN (which is no fault of the
programmer), so don't allow the check to be compiled out.
Signed-off-by: Eric Wong <normalperson@yhbt.net> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Change-Id: Ia2695ea6953b589ac8ab8b444fb668daee06a614
Simon Marchi [Wed, 17 Aug 2022 15:24:25 +0000 (11:24 -0400)]
Fix: change method used by _rcu_dereference to strip type constness
Commit 1e41ec3b07e4 ("Make temporary variable in _rcu_dereference
non-const") used the trick to add 0 to the pointer passed as a parameter
to the macro to get rid of its constness, should it be const (with the
end goal of avoiding compiler warnings). This is problematic (as shown
in [1]) if it is a pointer to an opaque type though, as the compiler
cannot perform pointer arithmetic on such a pointer (even though it
wouldn't really need to here, as we add 0).
Change it to use another trick to strip away the constness, that
shouldn't hit this problem. It was found in the same stackoverflow post
as the original trick [2]. It consists of using a statement expression
like so:
__typeof__(({ const int foo; foo; }))
The statement expression yields a value of type `int`. Statement
expressions are extensions to the C language, but we already use them
here.
Simon Marchi [Wed, 17 Aug 2022 17:11:21 +0000 (13:11 -0400)]
Fix: remove type constness in URCU_FORCE_CAST's C++ version
The test added by the following patch wouldn't compile, when built
without _LGPL_SOURCE:
CXX test_build_dynlink_cxx-test_build_cxx.o
In file included from ../../include/urcu/arch.h:25,
from /home/simark/src/urcu/tests/unit/test_build.c:28,
from /home/simark/src/urcu/tests/unit/test_build_cxx.cpp:3:
/home/simark/src/urcu/tests/unit/test_build.c: In function ‘void test_build_rcu_dereference()’:
/home/simark/src/urcu/include/urcu/compiler.h:85:42: error: type qualifiers ignored on cast result type [-Werror=ignored-qualifiers]
85 | #define URCU_FORCE_CAST(type, arg) (reinterpret_cast<type>(arg))
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/simark/src/urcu/include/urcu/pointer.h:71:49: note: in expansion of macro ‘URCU_FORCE_CAST’
71 | __typeof__(p) _________p1 = URCU_FORCE_CAST(__typeof__(p), \
| ^~~~~~~~~~~~~~~
/home/simark/src/urcu/tests/unit/test_build.c:133:9: note: in expansion of macro ‘rcu_dereference’
133 | rcu_dereference(opaque_const);
| ^~~~~~~~~~~~~~~
The compiler complains that we do a cast to a const type, equivalent to:
reinterpret_cast<const int>(arg)
... and that the const is meaningless in this context.
Use std::remove_cv to strip away any const or volatile qualifiers from
the type (using a volatile type would result in the same warning).
Change-Id: I94e79fcccfc2108021752f65977e1548084c646a Signed-off-by: Simon Marchi <simon.marchi@efficios.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Simon Marchi [Wed, 17 Aug 2022 16:49:50 +0000 (12:49 -0400)]
Move extern "C" down in include/urcu/urcu-bp.h
A following patch adds a <type_traits> include in
urcu/compiler.h. However, compiler.h gets included by urcu/pointer.h,
which gets included by urcu/urcu-bp.h inside an extern "C" scope.
Including the C++ header file <type_traits> inside an extern "C" scope
doesn't work:
In file included from /home/simark/src/urcu/include/urcu/compiler.h:25,
from /home/simark/src/urcu/include/urcu/pointer.h:29,
from /home/simark/src/urcu/include/urcu/urcu-bp.h:58,
from /home/simark/src/urcu/include/urcu-bp.h:2,
from /home/simark/src/urcu/tests/unit/test_urcu_multiflavor-bp.c:28,
from /home/simark/src/urcu/tests/unit/test_urcu_multiflavor-bp_cxx.cpp:3:
/usr/include/c++/12.1.1/type_traits:44:3: error: template with C linkage
44 | template<typename _Tp>
| ^~~~~~~~
/home/simark/src/urcu/include/urcu/urcu-bp.h:41:1: note: ‘extern "C"’ linkage started here
41 | extern "C" {
| ^~~~~~~~~~
Move the extern "C" in urcu-bp.h down, so that the includes are not
inside it. Each header file is responsible to use extern "C" where
relevant, and we should avoid including files inside such a scope.
Change-Id: I42bdfa6ab445e8c40f5bcac1c1ae0786d443626c Signed-off-by: Simon Marchi <simon.marchi@efficios.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Michael Jeanson [Mon, 15 Aug 2022 15:11:54 +0000 (11:11 -0400)]
fix: ifdef linux specific cpu count compat
Expand the '#ifdef __linux__' block in src/compat-cpu.h to all static
inline functions related to sysfs since they are only useful on Linux
and fail to build on some non-Linux platforms. This issue was reported
on QNX.
Thanks to Elad Lahav <e2lahav@gmail.com> for reporting this issue.
Change-Id: I17c88a9a2fb5b9be6cf5325234a18ff40788cd09 Signed-off-by: Michael Jeanson <mjeanson@efficios.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Michael Jeanson [Wed, 27 Jul 2022 14:44:00 +0000 (10:44 -0400)]
fix: sysconf(_SC_NPROCESSORS_CONF) can be less than max cpu id
We rely on sysconf(_SC_NPROCESSORS_CONF) to get the maximum possible
number of CPUs that can be attached to the system for the lifetime of an
application.
As such we expect that the highest possible CPU id would be one less
than the number returned by sysconf(_SC_NPROCESSORS_CONF) which is
unfortunatly not always the case and can vary across libc
implementations and versions.
Glibc up to 2.35 will count the number of "cpuX" directories in
"/sys/devices/system/cpu" which doesn't include CPUS that were
hot-unplugged.
This information is however provided by the Linux kernel in
"/sys/devices/system/cpu/possible" in the form of a mask listing all the
CPUs that could possibly be hot-plugged in the system.
This patch replaces sysconf(_SC_NPROCESSORS_CONF) with an internal
function that first tries parsing the possible CPU mask to extract the
highest possible value and if this fails fallback to the previous
behavior.
Change-Id: I68dfed42ebbab02728a02eeefd4a395a22bb1bea Signed-off-by: Michael Jeanson <mjeanson@efficios.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Shao-Tse Hung [Tue, 2 Aug 2022 17:44:00 +0000 (01:44 +0800)]
Fix: revise obsolete command in README.md
The obsolete command `make bench` was replaced by `make short_bench` and
`make long_bench` in 2015. However, this command wasn't revised in
README, so I follow the previous commit and rewrite it.
The urcu-qsbr wait_gp() implements a futex wait/wakeup scheme identical to
the workqueue code, which has an issue with spurious wakeups.
A spurious wakeup on wait_gp can cause wait_gp to return with a
urcu_qsbr_gp.futex state of -1, which is unexpected. It would cause the
following loops in wait_for_readers() to decrement the
urcu_qsbr_gp.futex to values below -1, thus actively using CPU as values
will be decremented to very low negative values until it reaches 0
through underflow, or until the input_readers list is found to be empty.
The state is restored to 0 when the input_readers list is found to be
empty, which restores the futex state to a correct state for the
following calls to wait_for_readers().
This issue will cause spurious unexpected high CPU use, but will not
lead to data corruption.
Cause
=====
From futex(5):
FUTEX_WAIT
Returns 0 if the caller was woken up. Note that a wake-up can
also be caused by common futex usage patterns in unrelated code
that happened to have previously used the futex word's memory
location (e.g., typical futex-based implementations of Pthreads
mutexes can cause this under some conditions). Therefore, call‐
ers should always conservatively assume that a return value of 0
can mean a spurious wake-up, and use the futex word's value
(i.e., the user-space synchronization scheme) to decide whether
to continue to block or not.
Solution
========
We therefore need to validate whether the value differs from -1 in
user-space after the call to FUTEX_WAIT returns 0.
The urcu wait_gp() implements a futex wait/wakeup scheme identical to
the workqueue code, which has an issue with spurious wakeups.
A spurious wakeup on wait_gp can cause wait_gp to return with a
rcu_gp.futex state of -1, which is unexpected. It would cause the
following loops in wait_for_readers() to decrement the
rcu_gp.futex to values below -1, thus actively using CPU as values
will be decremented to very low negative values until it reaches 0
through underflow, or until the input_readers list is found to be empty.
The state is restored to 0 when the input_readers list is found to be
empty, which restores the futex state to a correct state for the
following calls to wait_for_readers().
This issue will cause spurious unexpected high CPU use, but will not
lead to data corruption.
Cause
=====
From futex(5):
FUTEX_WAIT
Returns 0 if the caller was woken up. Note that a wake-up can
also be caused by common futex usage patterns in unrelated code
that happened to have previously used the futex word's memory
location (e.g., typical futex-based implementations of Pthreads
mutexes can cause this under some conditions). Therefore, call‐
ers should always conservatively assume that a return value of 0
can mean a spurious wake-up, and use the futex word's value
(i.e., the user-space synchronization scheme) to decide whether
to continue to block or not.
Solution
========
We therefore need to validate whether the value differs from -1 in
user-space after the call to FUTEX_WAIT returns 0.
The urcu-wait urcu_adaptative_busy_wait() implements a futex wait/wakeup
scheme similar to the workqueue code, which has an issue with spurious
wakeups.
A spurious wakeup on urcu_adaptative_busy_wait can cause
urcu_adaptative_busy_wait to reach label skip_futex_wait with a
wait->state state of URCU_WAIT_WAITING, which is unexpected. It would
cause busy-waiting on URCU_WAIT_TEARDOWN state to start early. The
wait-teardown stage is done with URCU_WAIT_ATTEMPTS active attempts,
following by attempts spaced by 10ms sleeps. I do not expect that these
spurious wakeups will cause user-observable effects other than being
slightly less efficient that it should be.
urcu-wait is used by all urcu flavor's synchronize_rcu() to implement
the grace period batching scheme.
This issue will cause spurious unexpected high CPU use, but will not
lead to data corruption.
Cause
=====
From futex(5):
FUTEX_WAIT
Returns 0 if the caller was woken up. Note that a wake-up can
also be caused by common futex usage patterns in unrelated code
that happened to have previously used the futex word's memory
location (e.g., typical futex-based implementations of Pthreads
mutexes can cause this under some conditions). Therefore, call‐
ers should always conservatively assume that a return value of 0
can mean a spurious wake-up, and use the futex word's value
(i.e., the user-space synchronization scheme) to decide whether
to continue to block or not.
Solution
========
We therefore need to validate whether the value differs from
URCU_WAIT_WAITING in user-space after the call to FUTEX_WAIT returns 0.
The urcu-defer wait_defer() implements a futex wait/wakeup scheme identical to
the workqueue code, which has an issue with spurious wakeups.
A spurious wakeup on wait_defer can cause wait_defer to return with a
defer_thread_futex state of -1, which is unexpected. It would cause the
following loops in thr_defer() to decrement the defer_thread_futex to
values below -1, thus actively using CPU as values will be decremented
to very low negative values until it reaches 0 through underflow, or
until callbacks are eventually queued. The state is restored to 0 when
callbacks are found, which restores the futex state to a correct state
for the following calls to wait_defer().
This issue will cause spurious unexpected high CPU use, but will not
lead to data corruption.
Cause
=====
From futex(5):
FUTEX_WAIT
Returns 0 if the caller was woken up. Note that a wake-up can
also be caused by common futex usage patterns in unrelated code
that happened to have previously used the futex word's memory
location (e.g., typical futex-based implementations of Pthreads
mutexes can cause this under some conditions). Therefore, call‐
ers should always conservatively assume that a return value of 0
can mean a spurious wake-up, and use the futex word's value
(i.e., the user-space synchronization scheme) to decide whether
to continue to block or not.
Solution
========
We therefore need to validate whether the value differs from -1 in
user-space after the call to FUTEX_WAIT returns 0.
The urcu call_rcu() and rcu_barrier() each implement a futex wait/wakeup
scheme identical to the workqueue code, which has an issue with spurious
wakeups.
* call_rcu
A spurious wakeup on call_rcu_wait can cause call_rcu_wait to return
with a crdp->futex state of -1, which is unexpected. It would cause the
following loops in call_rcu_thread() to decrement the crdp->futex to
values below -1, thus actively using CPU time as values will be
decremented to very low negative values until the futex value underflows
back to 0. The state is *not* restored to 0 when the callback list is
found to be non-empty, so this unexpected state will persist until the
crdp->futex state underflows back to 0, or until the call_rcu_thread is
stopped. What prevents this from having too much user-observable effects
is that the call rcu thread has a 10ms sleep between loops, to favor
batching of callbacks. Therefore, rather than being a purely 100% active
busy-wait, this scenario leads to a busy-wait which is paced by 10ms
sleeps.
Therefore the observed issue will be that the call_rcu_thread will
unexpectedly wake up the CPU each 10ms after this spurious wakeup
happens.
* rcu_barrier
A spurious wakeup on call_rcu_completion_wait can cause
call_rcu_completion_wait to return with a completion->futex state of -1,
which is unexpected. It would cause the following loops in rcu_barrier()
to decrement the completion->futex to values below -1, thus actively
using CPU time as values will be decremented to very low negative values
until either the barrier count reaches 0 or until the futex value
underflows to 0.
Therefore the observed issue will be that rcu_barrier() will
unexpectedly use a lot of CPU time when this spurious wakeup happens.
These issues will cause spurious unexpected high CPU use, but will not
lead to data corruption.
Cause
=====
From futex(5):
FUTEX_WAIT
Returns 0 if the caller was woken up. Note that a wake-up can
also be caused by common futex usage patterns in unrelated code
that happened to have previously used the futex word's memory
location (e.g., typical futex-based implementations of Pthreads
mutexes can cause this under some conditions). Therefore, call‐
ers should always conservatively assume that a return value of 0
can mean a spurious wake-up, and use the futex word's value
(i.e., the user-space synchronization scheme) to decide whether
to continue to block or not.
Solution
========
We therefore need to validate whether the value differs from -1 in
user-space after the call to FUTEX_WAIT returns 0.
The workqueue thread futex_wait() returns with a workqueue->futex state
of -1, which is unexpected. In this situation, the workqueue thread is
observed to use 99% of CPU as workqueue->futex values are decremented to
very low negative values while the workqueue is empty.
This issue will cause spurious unexpected high CPU use, but will not
lead to data corruption.
Cause
=====
From futex(5):
FUTEX_WAIT
Returns 0 if the caller was woken up. Note that a wake-up can
also be caused by common futex usage patterns in unrelated code
that happened to have previously used the futex word's memory
location (e.g., typical futex-based implementations of Pthreads
mutexes can cause this under some conditions). Therefore, call‐
ers should always conservatively assume that a return value of 0
can mean a spurious wake-up, and use the futex word's value
(i.e., the user-space synchronization scheme) to decide whether
to continue to block or not.
Solution
========
We therefore need to validate whether the value differs from -1 in
user-space after the call to FUTEX_WAIT returns 0.
Simon Marchi [Fri, 30 Jul 2021 03:06:11 +0000 (23:06 -0400)]
Make temporary variable in _rcu_dereference non-const
When building the lttng-tools project with Ubuntu's gcc 11, I get the
following error:
CC agent.lo
In file included from /tmp/lttng/include/urcu/arch.h:25,
from /tmp/lttng/include/urcu/uatomic.h:23,
from /home/simark/src/lttng-tools/src/bin/lttng-sessiond/agent.c:11:
/home/simark/src/lttng-tools/src/bin/lttng-sessiond/agent.c: In function ‘agent_update’:
/tmp/lttng/include/urcu/static/pointer.h:96:33: error: argument 2 of ‘__atomic_load’ discards ‘const’ qualifier [-Werror=incompatible-pointer-types]
96 | __atomic_load(&(p), &_________p1, __ATOMIC_CONSUME); \
| ^~~~~~~~~~~~~
/tmp/lttng/include/urcu/compiler.h:69:70: note: in definition of macro ‘caa_container_of’
69 | const __typeof__(((type *) NULL)->member) * __ptr = (ptr); \
| ^~~
/tmp/lttng/include/urcu/rculist.h:87:20: note: in expansion of macro ‘cds_list_entry’
87 | for (pos = cds_list_entry(rcu_dereference((head)->next), __typeof__(*(pos)), member); \
| ^~~~~~~~~~~~~~
/tmp/lttng/include/urcu/pointer.h:47:33: note: in expansion of macro ‘_rcu_dereference’
47 | #define rcu_dereference _rcu_dereference
| ^~~~~~~~~~~~~~~~
/tmp/lttng/include/urcu/rculist.h:87:35: note: in expansion of macro ‘rcu_dereference’
87 | for (pos = cds_list_entry(rcu_dereference((head)->next), __typeof__(*(pos)), member); \
| ^~~~~~~~~~~~~~~
/home/simark/src/lttng-tools/src/bin/lttng-sessiond/agent.c:1551:9: note: in expansion of macro ‘cds_list_for_each_entry_rcu’
1551 | cds_list_for_each_entry_rcu(ctx, &agt->app_ctx_list, list_node) {
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
This is because the pointer passed to _rcu_dereference is const (the
pointer itself is const, IIUC, not necessarily the data it points to),
so the temporary _________p1 is also declared as const. We therefore
can't pass a non-const pointer to it to a function that modifies it.
I applied the trick found here [1] with success to get rid of the
constness of the variable. With this change, lttng-tools compiles
successfully with gcc 11.
There may be other spots in the headers where this would be needed, but
it is hard to spot them. I think we would need to write some test file
that pass const pointers to all macros of the API and see if they
compile.
Fix: x86 and s390 uatomic: __hp() macro warning with gcc 11
The __hp() macro used in the x86 and s390 uatomic code generates the
following warning with gcc-11:
In file included from ../include/urcu/uatomic.h:27,
from ../include/urcu/static/wfcqueue.h:35,
from ../include/urcu/wfcqueue.h:133,
from workqueue.c:39:
workqueue.c: In function ‘workqueue_thread’:
../include/urcu/uatomic/x86.h:155:17: warning: array subscript ‘struct __uatomic_dummy[0]’ is partly outside array bounds of ‘struct cds_wfcq_tail[1]’ [-Warray-bounds]
155 | __asm__ __volatile__(
| ^~~~~~~
workqueue.c:184:38: note: while referencing ‘cbs_tmp_tail’
184 | struct cds_wfcq_tail cbs_tmp_tail;
| ^~~~~~~~~~~~
The (previously undocumented) reason for this macro is to allow passing the
"void *" parameter as "m" or "+m" operand to the inline assembly. That
motivation was explained in commit 53b8ed6836363 ("s390 uatomic arch fix").
The out of bound access is detected by gcc because struct
__uatomic_dummy's length is quite large: an array of 10 unsigned long,
which is larger than the size pointed to by the void pointer.
So rather than using a fixed-size type, cast to a structure containing
an array of characters of a size matching the @addr input argument.
While we are at it and digging out git archeology, properly document the
__hp() macro for posterity.
Michael Jeanson [Fri, 7 May 2021 15:34:33 +0000 (11:34 -0400)]
fix: clock_gettime on macOs
Newer version of macOs have an implementation of clock_gettime() that
requires additionnal setup, move the platform specific code first so it
is always used.
Change-Id: I12fcdeff6c0ae59bc1a13f4e2cd7f4ebcedfc253 Signed-off-by: Michael Jeanson <mjeanson@efficios.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Fix: rculist header: use parenthesis around macro parameters
The coding style followed across liburcu is to use parenthesis around
macro parameters when it would otherwise lead to unexpected results due
to priority of operators. Fix rculist.h to follow this coding style.
Fix: rcuhlist header: use parenthesis around macro parameters
The coding style followed across liburcu is to use parenthesis around
macro parameters when it would otherwise lead to unexpected results due
to priority of operators. Fix rcuhlist.h to follow this coding style.
Fix: hlist header: use parenthesis around macro parameters
The coding style followed across liburcu is to use parenthesis around
macro parameters when it would otherwise lead to unexpected results due
to priority of operators. Fix hlist.h to follow this coding style.
Fix: list.h: use parenthesis around macro parameters, caa_container_of()
The coding style followed across liburcu is to use parenthesis around
macro parameters when it would otherwise lead to unexpected results due
to priority of operators. Fix list.h to follow this coding style.
Use caa_container_of() for cds_list_entry rather than open-code the
pointer arithmetic.
Comparing an offset from an object with NULL is undefined behavior
and the compiler may assume that this is never true.
This is indeed what is observed with gcc-10 miscompiling
cds_hlist_for_each_entry_rcu_2().
Fix this by introducing cds_hlist_entry_safe() rather than open-coding
the NULL check comparisons, and move cds_hlist_for_each_entry_2()
and cds_hlist_for_each_entry_safe_2() to this scheme as well.
Fix: use __atomic_load() rather than atomic load explicit
Use __atomic_load (gcc extension) rather than atomic load explict
(C11/C++11) for rcu_dereference because it does not require the input
type to be _Atomic. This fixes a regression with clang introduced by
commit 380f4b19052 ("Fix: use atomic load memory_order_consume for
rcu_dereference on C11/C++11").
Note that the cmm_smp_read_barrier_depends is removed when using
__ATOMIC_CONSUME because their memory ordering effect is redundant.
Michael Jeanson [Tue, 13 Apr 2021 20:19:06 +0000 (16:19 -0400)]
fix: HAVE_SCHED_SETAFFINITY is not defined
Use '#ifdef' instead of '#if' to test if HAVE_SCHED_SETAFFINITY is
defined. Both work but using '#if' on an undefined macro will generate a
warning with '-Wundef'.
Signed-off-by: Michael Jeanson <mjeanson@efficios.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Change-Id: Ib8e13defb79e271da880196fd0a7f7f642999048
Michael Jeanson [Thu, 1 Apr 2021 18:39:01 +0000 (14:39 -0400)]
cleanup: explicitly mark unused parameters (-Wunused-parameter)
Add the 'unused' attribute to function parameters that are unused to
allow turning on -Wunused-parameter and distinguish unused parameters
that are actual errors.
Change-Id: Ie585e37f9d38718543a31aee2e7ab3428cdfd0a5 Signed-off-by: Michael Jeanson <mjeanson@efficios.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Michael Jeanson [Mon, 25 Jan 2021 18:36:24 +0000 (13:36 -0500)]
fix: exclude clang from GCC version blacklists
URCU_GCC_VERSION is used to blacklist specific GCC versions with known
bugs, clang also defines these macros to an equivalent GCC version it
claims to support, so exclude it.
Signed-off-by: Michael Jeanson <mjeanson@efficios.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Change-Id: Idf0980fddca6533313a3367601ddda8d8e13bfdf
The newly-released autoconf 2.70 introduces a number of breaking
changes [1] and is being rolled-out by some distros.
Amongst those changes, the AC_PROG_CC_STDC macro is marked as obsolete
and was merged into AC_PROG_CC, which we already use. On 2.70, this
results in a warning which we handle as an error.
A version check is added to invoke the AC_PROG_CC_STDC macro only when
running a pre-2.70 version of autoconf, fixing the issue.
Also, the AX_PTHREAD macro makes use of the $as_echo built-in shell
variable which no longer exists in 2.70. A patch was submitted to the
GNU Autoconf archive in March, but there have been no signs of life
given since then [2].
As such, our local copy is updated to the latest version and the patch
(which looks fairly straight-forward / safe) is applied. This should
minimize changes once we go back to an "official" version of the macro.
Michael Jeanson [Wed, 9 Dec 2020 17:05:31 +0000 (12:05 -0500)]
fix: bump tests thread limit to 4096
Machines with more than 128 CPUs are becomming more common. A
future-proof fix here would be to dynamically allocate the array, but in
the meantime bump the limit to 4096 to fix the problem on a 160 CPUs
ppc64el system where this was reported.
Signed-off-by: Michael Jeanson <mjeanson@efficios.com> Cc: Paul E. McKenney <paulmck@kernel.org> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Change-Id: Ib3cb5d8cb4515e6f626be33c2685fa38cb081782
Michael Jeanson [Fri, 30 Oct 2020 19:39:56 +0000 (15:39 -0400)]
fix: add -lurcu-common to pkg-config libs for each flavor
The urcu-common library contains common code like the write-free queue
and compat code, each urcu flavor library is dynamicly linked with it.
Most but not all toolchains will automatically link an executable with a
transitive depency of an explicitly linked library if said binary uses a
symbol from the transitive dependency.
Since this behavior is not present in all toolchains, add
'-lurcu-common' to the 'Libs' field of each flavors pkg-config file so
that executables using symbols from urcu-common can be reliably linked
using pkg-config.
Signed-off-by: Michael Jeanson <mjeanson@efficios.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Paul E. McKenney [Thu, 22 Oct 2020 22:30:21 +0000 (15:30 -0700)]
call_rcu: Fix race between rcu_barrier() and call_rcu_data_free()
The current code can lose RCU callbacks at shutdown time, which can
result in hangs. This lossage can happen as follows:
o A thread invokes call_rcu_data_free(), which executes up through
the wake_call_rcu_thread(). At this point, the call_rcu_data
structure has been drained of callbacks, but is still on the
call_rcu_data_list. Note that this thread does not hold the
call_rcu_mutex.
o Another thread invokes rcu_barrier(), which traverses the
call_rcu_data_list under the protection of call_rcu_mutex,
a list which still includes the above newly drained structure.
This thread therefore adds a callback to the newly drained
call_rcu_data structure. It then releases call_rcu_mutex and
enters a mystifying loop that does futex stuff.
o The first thread finishes executing call_rcu_data_free(),
which acquires call_rcu_mutex just long enough to remove the
newly drained call_rcu_data structure from call_rcu_data_list.
Which causes one of the rcu_barrier() invocation's callbacks to
be leaked.
o The second thread's rcu_barrier() invocation never returns
resulting in a hang.
This commit therefore changes call_rcu_data_free() to acquire
call_rcu_mutex before checking the call_rcu_data structure for callbacks.
In the case where there are no callbacks, call_rcu_mutex is held across
both the check and the removal from call_rcu_data_list, thus preventing
rcu_barrier() from adding a callback in the meantime. In the case where
there are callbacks, call_rcu_mutex must be momentarily dropped across
the call to get_default_call_rcu_data(), which can itself acquire
call_rcu_mutex. This momentary drop is not a problem because any
callbacks that rcu_barrier() might queue during that period of time will
be moved to the default call_rcu_data structure, and the lock will be
held across the full time including moving those callbacks and removing
the call_rcu_data structure that was passed into call_rcu_data_free()
from call_rcu_data_list.
With this fix, a several-hundred-CPU test successfully completes more
than 5,000 executions. Without this fix, it fails within a few tens
of executions. Although the failures happen more quickly on larger
systems, in theory this could happen on a single-CPU system, courtesy
of preemption.
Signed-off-by: Paul E. McKenney <paulmck@kernel.org> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> Cc: Stephen Hemminger <stephen@networkplumber.org> Cc: Alan Stern <stern@rowland.harvard.edu> Cc: Lai Jiangshan <jiangshanlai@gmail.com> Cc: <lttng-dev@lists.lttng.org> Cc: <linux-kernel@vger.kernel.org> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Fix: tls-compat.h exposes compiler-dependent public configuration
Exposing the storage class chosen by ax_tls.m4 in a public header is
a bad idea, because if a recent gcc is used when configuring
liburcu, thus detecting C11, it will choose _Thread_local. Then, if an
external project uses urcu/tls-compat.h with an older gcc (e.g. 4.8),
it will fail to build, because that storage class is unknown, and
__thread should be used instead.
Therefore, use a preprocessor conditional on __cplusplus to detect C++11
(and use thread_local). Else, the STDC version is used to select
_Thread_local. Else check if _MSC_VER is defined to select
__declspec(thread), or else rely on __thread as fallback.
On architectures where "char" is signed, it should be cast to unsigned
char before being passed as parameter to isdigit or isspace. Based on
their man page:
These functions check whether c, which must have the value of an
unsigned char or EOF, falls into a certain character class according to
the specified locale.
Passing a signed char as parameter is invalid if the values fall into
the negative range of the signed char.
urcu-bp: perform thread registration on urcu_bp_register_thread
Some real-time use-cases (e.g. Xenomai) require to perform urcu-bp
thread registration early in the thread life-time before it starts
performing real-time tasks.
Currently, this can be achieved by issuing a urcu_bp_read_lock() and
urcu_bp_read_unlock() pair, or by using urcu_bp_read_ongoing(), while in
the initialization phrase.
However, it seems natural to expect that calling urcu_bp_register_thread()
would have the side effect to perform the lazy thread registration
immediately rather than being a no-op.
cds_lfht_is_node_deleted parameter can be marked const
Mark the cds_lfht_node pointer parameter of cds_lfht_is_node_deleted
as const. The fact that this parameter is mutable makes it harder to
use liburcu in const-correct code.
In urcu-signal flavor, call_rcu_thread calls synchronize_rcu which
will send SIGRCU signal to all registed threads, and then loops to
wait need_mb to be cleared. However, the registed workqueue_thread
does not process the SIGRCU signal, and never clear the need_mb.
Based on above, call_rcu_thread and workqueue_thread will wait
forever for completion of the grace period: call_rcu_thread which holds
the rcu_registry_lock, waits for workqueue_thread to do cmm_smp_mb.
While workqueue thread never does cmm_smp_mb because of signal blocking,
and it will eventually wait to get rcu_registry_lock in do_resize_cb.
The phenomenon is as follows, which is easy to be triggered:
(gdb) t 2
[Switching to thread 2 (Thread 0xffff83c3b080 (LWP 27116))]
0 0x0000ffff845296c4 in poll () from /lib64/libc.so.6
(gdb) bt
0 0x0000ffff845296c4 in poll () from /lib64/libc.so.6
1 0x0000ffff8461b93c in force_mb_all_readers () at urcu.c:241
2 0x0000ffff8461c748 in smp_mb_master () at urcu.c:249
3 urcu_signal_synchronize_rcu () at urcu.c:445
4 0x0000ffff8461d004 in call_rcu_thread at urcu-call-rcu-impl.h:364
5 0x0000ffff845eb8bc in start_thread () from /lib64/libpthread.so.0
6 0x0000ffff845335cc in thread_start () from /lib64/libc.so.6
(gdb) t 3
[Switching to thread 3 (Thread 0xffff8443c080 (LWP 27191))]
0 0x0000ffff845f51c4 in __lll_lock_wait () from /lib64/libpthread.so.0
(gdb) bt
0 0x0000ffff845f51c4 in __lll_lock_wait () from /lib64/libpthread.so.0
1 0x0000ffff845ee048 in pthread_mutex_lock () from /lib64/libpthread.so.0
2 0x0000ffff8461b814 in mutex_lock ( <rcu_registry_lock>) at urcu.c:157
3 0x0000ffff8461b9e4 in urcu_signal_unregister_thread () at urcu.c:564
4 0x0000ffff8463e62c in do_resize_cb (work=0x11e2e790) at rculfhash.c:2042
5 0x0000ffff8463c940 in workqueue_thread (arg=0x11e1d260) at workqueue.c:228
6 0x0000ffff845eb8bc in start_thread () from /lib64/libpthread.so.0
7 0x0000ffff845335cc in thread_start () from /lib64/libc.so.6
So we should not block SIGRCU in workqueue thread to avoid blocking
forever in the grace period awaiting on the worker thread when using
urcu-signal flavor.
Signed-off-by: hewenliang <hewenliang4@huawei.com> Co-developed-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Found by Coverity:
CID 1357055 (#1 of 1): Unchecked return value (CHECKED_RETURN)
4. check_return: Calling pthread_mutex_lock without checking return value
(as is done elsewhere 44 out of 48 times).
Michael Jeanson [Mon, 3 Jun 2019 20:36:43 +0000 (16:36 -0400)]
Fix: SONAME bump to 6.1.0
In commit d6c78161aed9b2d550ce201b0a8cd5b3ee515ac8 we bumped the 'age'
part of the library version with the intention of keeping the same major
SONAME because we only introduced new symbols. However by bumping the
'age' and not the 'current' we substracted 1 to the major SONAME which
we did not intend. Seems like we missed this in testing.
Fix it by bumping the 'current' to end up with an SONAME of 6.1.0 which
is what we originally intended.
From the libtool manual for reference :
Programs using the previous version may use the new version as drop-in
replacement, but programs using the new version may use APIs not present
in the previous one. In other words, a program linking against the new
version may fail with “unresolved symbols” if linking against the old
version at runtime: set revision to 0, bump current and age.
Signed-off-by: Michael Jeanson <mjeanson@efficios.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cleanup: update code layout to fix old gcc warning
Some CI jobs show:
urcu-pointer.o
13:46:22 In file included from urcu.c:49:0:
13:46:22 urcu-wait.h:70:9: warning: missing initializer for field 'lock' of 'struct cds_wfs_stack' [-Wmissing-field-initializers]
13:46:22 struct urcu_wait_queue name = URCU_WAIT_QUEUE_HEAD_INIT(name)
13:46:22 ^
13:46:22 urcu.c:150:8: note: in expansion of macro 'DEFINE_URCU_WAIT_QUEUE'
13:46:22 static DEFINE_URCU_WAIT_QUEUE(gp_waiters);
13:46:22 ^
13:46:22 In file included from urcu-wait.h:27:0,
13:46:22 from urcu.c:49:
13:46:22 ../include/urcu/wfstack.h:92:18: note: 'lock' declared here
13:46:22 pthread_mutex_t lock;
13:46:22
Building liburcu with --enable-cds-lfht-iter-debug and rebuilding
application to match the ABI change allows finding cases where the
hash table iterator is re-purposed to be used on a different hash
table while still being used to iterate on a hash table.
This is a common programming mistake that happens often enough
to justify creating a debugging mode to track this automatically.
Michael Jeanson [Wed, 12 Dec 2018 20:01:37 +0000 (15:01 -0500)]
Port: no symbols aliases on MacOS
There is no equivalent to symbols aliases on MacOS, this will
unfortunatly break the ABI for SONAME(6) and will require a rebuild of
client applications.
Signed-off-by: Michael Jeanson <mjeanson@efficios.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Michael Jeanson [Fri, 30 Nov 2018 19:28:51 +0000 (14:28 -0500)]
Add -Wextra to CFLAGS
Edited by Mathieu Desnoyers:
Use /* fall through */ rather than __attribute__((fallthrough)) to
stay compatible with clang and gcc < 7. The fallthrough attribute
was introduced in gcc 7.
Signed-off-by: Michael Jeanson <mjeanson@efficios.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
projects / urcu.git / log
