diff --git a/configure.ac b/configure.ac index 7c6b842f5..79b82845b 100644 --- a/configure.ac +++ b/configure.ac @@ -246,6 +246,15 @@ fi # If everything else fails use libatomic_ops need_libatomic_ops=yes +AC_CACHE_CHECK([whether $CC knows __atomic_store_n()], + pulseaudio_cv_atomic_store_n, [ + AC_LINK_IFELSE( + [AC_LANG_PROGRAM([], [[int c = 0; __atomic_store_n(&c, 4, __ATOMIC_SEQ_CST);]])], + [pulseaudio_cv_atomic_store_n=yes], + [pulseaudio_cv_atomic_store_n=no]) + ]) + + AC_CACHE_CHECK([whether $CC knows __sync_bool_compare_and_swap()], pulseaudio_cv_sync_bool_compare_and_swap, [ AC_LINK_IFELSE( @@ -256,6 +265,9 @@ AC_CACHE_CHECK([whether $CC knows __sync_bool_compare_and_swap()], if test "$pulseaudio_cv_sync_bool_compare_and_swap" = "yes" ; then AC_DEFINE([HAVE_ATOMIC_BUILTINS], 1, [Have __sync_bool_compare_and_swap() and friends.]) + if test "$pulseaudio_cv_atomic_store_n" = "yes" ; then + AC_DEFINE([HAVE_ATOMIC_BUILTINS_MEMORY_MODEL], 1, [Have __atomic_store_n() and friends.]) + fi need_libatomic_ops=no else # HW specific atomic ops stuff diff --git a/meson.build b/meson.build index c50382c7e..320cf8414 100644 --- a/meson.build +++ b/meson.build @@ -285,12 +285,23 @@ if shm_dep.found() cdata.set('HAVE_SHM_OPEN', 1) endif + atomictest = '''void func() { volatile int atomic = 2; __sync_bool_compare_and_swap (&atomic, 2, 3); } ''' if cc.compiles(atomictest) + newatomictest = '''void func() { + int c = 0; + __atomic_store_n(&c, 4, __ATOMIC_SEQ_CST); + } + ''' + + if(cc.compiles(newatomictest)) + cdata.set('HAVE_ATOMIC_BUILTINS_MEMORY_MODEL', true) + endif + cdata.set('HAVE_ATOMIC_BUILTINS', true) else # FIXME: check if we need libatomic_ops diff --git a/src/Makefile.am b/src/Makefile.am index 928fe74ab..438bf57fe 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -293,7 +293,8 @@ TESTS_norun = \ sig2str-test \ stripnul \ echo-cancel-test \ - lo-latency-test + lo-latency-test \ + atomic-test # These tests need a running pulseaudio daemon TESTS_daemon = \ @@ -412,6 +413,11 @@ srbchannel_test_CFLAGS = $(AM_CFLAGS) $(LIBCHECK_CFLAGS) srbchannel_test_LDADD = $(AM_LDADD) libpulse.la libpulsecommon-@PA_MAJORMINOR@.la srbchannel_test_LDFLAGS = $(AM_LDFLAGS) $(BINLDFLAGS) $(LIBCHECK_LIBS) +atomic_test_SOURCES = tests/atomic-test.c +atomic_test_CFLAGS = $(AM_CFLAGS) $(LIBCHECK_CFLAGS) +atomic_test_LDADD = $(AM_LDADD) libpulsecommon-@PA_MAJORMINOR@.la libpulse.la +atomic_test_LDFLAGS = $(AM_LDFLAGS) $(BINLDFLAGS) $(LIBCHECK_LIBS) + get_binary_name_test_SOURCES = tests/get-binary-name-test.c get_binary_name_test_CFLAGS = $(AM_CFLAGS) $(LIBCHECK_CFLAGS) get_binary_name_test_LDADD = $(AM_LDADD) libpulse.la libpulsecommon-@PA_MAJORMINOR@.la diff --git a/src/pulsecore/atomic.h b/src/pulsecore/atomic.h index 765714449..a82ca83c5 100644 --- a/src/pulsecore/atomic.h +++ b/src/pulsecore/atomic.h @@ -50,6 +50,20 @@ typedef struct pa_atomic { #define PA_ATOMIC_INIT(v) { .value = (v) } +#ifdef HAVE_ATOMIC_BUILTINS_MEMORY_MODEL + +/* __atomic based implementation */ + +static inline int pa_atomic_load(const pa_atomic_t *a) { + return __atomic_load_n(&a->value, __ATOMIC_SEQ_CST); +} + +static inline void pa_atomic_store(pa_atomic_t *a, int i) { + __atomic_store_n(&a->value, i, __ATOMIC_SEQ_CST); +} + +#else + static inline int pa_atomic_load(const pa_atomic_t *a) { __sync_synchronize(); return a->value; @@ -60,6 +74,9 @@ static inline void pa_atomic_store(pa_atomic_t *a, int i) { __sync_synchronize(); } +#endif + + /* Returns the previously set value */ static inline int pa_atomic_add(pa_atomic_t *a, int i) { return __sync_fetch_and_add(&a->value, i); @@ -91,6 +108,20 @@ typedef struct pa_atomic_ptr { #define PA_ATOMIC_PTR_INIT(v) { .value = (long) (v) } +#ifdef HAVE_ATOMIC_BUILTINS_MEMORY_MODEL + +/* __atomic based implementation */ + +static inline void* pa_atomic_ptr_load(const pa_atomic_ptr_t *a) { + return (void*) __atomic_load_n(&a->value, __ATOMIC_SEQ_CST); +} + +static inline void pa_atomic_ptr_store(pa_atomic_ptr_t *a, void* p) { + __atomic_store_n(&a->value, p, __ATOMIC_SEQ_CST); +} + +#else + static inline void* pa_atomic_ptr_load(const pa_atomic_ptr_t *a) { __sync_synchronize(); return (void*) a->value; @@ -101,6 +132,8 @@ static inline void pa_atomic_ptr_store(pa_atomic_ptr_t *a, void *p) { __sync_synchronize(); } +#endif + static inline bool pa_atomic_ptr_cmpxchg(pa_atomic_ptr_t *a, void *old_p, void* new_p) { return __sync_bool_compare_and_swap(&a->value, (long) old_p, (long) new_p); } diff --git a/src/tests/atomic-test.c b/src/tests/atomic-test.c new file mode 100644 index 000000000..eb986e79d --- /dev/null +++ b/src/tests/atomic-test.c @@ -0,0 +1,135 @@ +/*** + This file is part of PulseAudio. + + Copyright 2014 David Henningsson, Canonical Ltd. + + PulseAudio is free software; you can redistribute it and/or modify + it under the terms of the GNU Lesser General Public License as published + by the Free Software Foundation; either version 2.1 of the License, + or (at your option) any later version. + + PulseAudio is distributed in the hope that it will be useful, but + WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + General Public License for more details. + + You should have received a copy of the GNU Lesser General Public License + along with PulseAudio; if not, see . +***/ + +/* This test spawns two threads on distinct cpu-cores that pass a value + * between each other through shared memory protected by pa_atomic_t. + * Thread "left" continuously increments a value and writes its contents to memory. + * Thread "right" continuously reads the value and checks whether it was incremented. + * + * With the pa_atomic_load/pa_atomic_store implementations based on __sync_synchronize, + * this will fail after some time (sometimes 2 seconds, sometimes 8 hours) at least + * on ARM Cortex-A53 and ARM Cortex-A57 systems. + * + * On x86_64, it does not. + * + * The chosen implementation in some way mimics a situation that can also occur + * using memfd srbchannel transport. + * + * NOTE: This is a long-running test, so don't execute in normal test suite. + * + * */ + +#ifdef HAVE_CONFIG_H +#include +#endif + +#include +#include + +#include +#include +#include +#include +#include +#include + +#define MEMORY_SIZE (8 * 2 * 1024 * 1024) + + +typedef struct io_t { + pa_atomic_t *flag; + char* memory; + cpu_set_t cpuset; +} io_t; + +static void read_func(void* data) { + io_t *io = (io_t *) data; + size_t expect = 0; + size_t value = 0; + pthread_setaffinity_np(pthread_self(), sizeof(cpu_set_t), &io->cpuset); + while(1) { + if(pa_atomic_load(io->flag) == 1) { + memcpy(&value, io->memory, sizeof(value)); + pa_atomic_sub(io->flag, 1); + ck_assert_uint_eq(value, expect); + ++expect; + } + } +} + +static void write_func(void* data) { + io_t *io = (io_t *) data; + size_t value = 0; + pthread_setaffinity_np(pthread_self(), sizeof(cpu_set_t), &io->cpuset); + while(1) { + if(pa_atomic_load(io->flag) == 0) { + memcpy(io->memory, &value, sizeof(value)); + pa_atomic_add(io->flag, 1); + ++value; + } + } +} + +START_TEST (atomic_test) { + pa_thread *thread1, *thread2; + io_t io1, io2; + + char* memory = pa_xmalloc0(MEMORY_SIZE); + pa_atomic_t flag = PA_ATOMIC_INIT(0); + memset(memory, 0, MEMORY_SIZE); + + /* intentionally misalign memory since srbchannel also does not + * always read/write aligned. Might be a red hering. */ + io1.memory = io2.memory = memory + 1025; + io1.flag = io2.flag = &flag; + + CPU_ZERO(&io1.cpuset); + CPU_SET(1, &io1.cpuset); + thread1 = pa_thread_new("left", &write_func, &io1); + + CPU_ZERO(&io2.cpuset); + CPU_SET(3, &io2.cpuset); + thread2 = pa_thread_new("right", &read_func, &io2); + pa_thread_free(thread1); + pa_thread_free(thread2); + pa_xfree(memory); +} +END_TEST + +int main(int argc, char *argv[]) { + int failed = 0; + Suite *s; + TCase *tc; + SRunner *sr; + + if (!getenv("MAKE_CHECK")) + pa_log_set_level(PA_LOG_DEBUG); + + s = suite_create("atomic"); + tc = tcase_create("atomic"); + tcase_add_test(tc, atomic_test); + suite_add_tcase(s, tc); + + sr = srunner_create(s); + srunner_run_all(sr, CK_NORMAL); + failed = srunner_ntests_failed(sr); + srunner_free(sr); + + return (failed == 0) ? EXIT_SUCCESS : EXIT_FAILURE; +}