From 8854255d7ae6084d61dd572578b2dee13cf6cfd2 Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Fri, 15 Nov 2019 09:22:18 +0100 Subject: [PATCH] tests/bitarithm_timings: Fix use of volatile The tests used the volatile qualifier for two this: 1. Prevent the compiler to optimize out calls to the inline-able functions bitarithm_msb, bitarithm_lsb, bitarithm_bits_set 2. Communication between IRQ context and thread context While the first use is valid, the second is dangerous, see [1], [2], [3], [4]. This commit replaces the second use with C11 atomics, which were explicitly added to the C standard to address this use case. [1]: https://www.kernel.org/doc/html/latest/process/volatile-considered-harmful.html [2]: http://c.isvolatileusefulwiththreads.com/ [3]: https://web.archive.org/web/20181124154026/http://software.intel.com/en-us/blogs/2007/11/30/volatile-almost-useless-for-multi-threaded-programming/ [4]: https://blog.regehr.org/archives/28 --- tests/bitarithm_timings/main.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/tests/bitarithm_timings/main.c b/tests/bitarithm_timings/main.c index 7264ab9228..3b4fc14d46 100644 --- a/tests/bitarithm_timings/main.c +++ b/tests/bitarithm_timings/main.c @@ -29,6 +29,8 @@ * @} */ +#include +#include #include #include "bitarithm.h" @@ -38,22 +40,21 @@ #define TIMEOUT (TIMEOUT_S * US_PER_SEC) #define PER_ITERATION (4) -static void callback(void *done_) +static atomic_bool done; + +static void callback(void *unused) { - volatile int *done = done_; - *done = 1; + (void)unused; + atomic_store(&done, true); } static void run_test(const char *name, unsigned (*test)(unsigned)) { - volatile int done = 0; unsigned i = 0; unsigned long count = 0; + atomic_store(&done, false); - xtimer_t xtimer; - xtimer.callback = callback; - xtimer.arg = (void *) &done; - + xtimer_t xtimer = { .callback = callback }; xtimer_set(&xtimer, TIMEOUT); do { @@ -78,7 +79,7 @@ static void run_test(const char *name, unsigned (*test)(unsigned)) } ++count; - } while (done == 0); + } while (atomic_load(&done) == false); printf("+ %s: %lu iterations per second\r\n", name, (4*PER_ITERATION) * count / TIMEOUT_S); }