tst_atomic: Add load, store and use __atomic builtins
PATCH v3 1/7 tst_atomic: Add load, store and use __atomic builtins
Also add more fallback inline assembly for the existing architectures and add SPARC64. Use the __atomic_* compiler intrinsics when available.
---
V3 - I have just updated the comments at the top of tst_atomic.h and tst_fuzzy_sync.h.
configure.ac | 1 + include/tst_atomic.h | 265 +++++++++++++++++++++++++++++++++++++++++------ include/tst_fuzzy_sync.h | 1 + m4/ltp-atomic.m4 | 34 ++++++ 4 files changed, 272 insertions(+), 29 deletions(-) create mode 100644 m4/ltp-atomic.m4
configure.ac +1 -0
@@ -194,5 +194,6 @@ LTP_CHECK_BUILTIN_CLEAR_CACHE LTP_CHECK_MMSGHDR LTP_CHECK_UNAME_DOMAINNAME LTP_CHECK_X_TABLES +LTP_CHECK_ATOMIC_MEMORY_MODEL AC_OUTPUT
include/tst_atomic.h +236 -29
330 lines not on this page · raw message
include/tst_fuzzy_sync.h +1 -0
@@ -32,6 +32,7 @@#include <sys/time.h> #include <time.h> +#include "tst_atomic.h" #ifndef CLOCK_MONOTONIC_RAW # define CLOCK_MONOTONIC_RAW CLOCK_MONOTONIC
m4/ltp-atomic.m4 new file +34 -0
@@ -0,0 +1,34 @@ +dnl +dnl Copyright (c) Linux Test Project, 2016 +dnl +dnl This program is free software; you can redistribute it and/or modify +dnl it under the terms of the GNU General Public License as published by +dnl the Free Software Foundation; either version 2 of the License, or +dnl (at your option) any later version. +dnl +dnl This program is distributed in the hope that it will be useful, +dnl but WITHOUT ANY WARRANTY; without even the implied warranty of +dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See +dnl the GNU General Public License for more details. +dnl + +AC_DEFUN([LTP_CHECK_ATOMIC_MEMORY_MODEL],[dnl + AC_MSG_CHECKING([for __atomic_* compiler builtins]) + AC_LINK_IFELSE([AC_LANG_SOURCE([ +int main(void) { + int i = 0, j = 0; + __atomic_add_fetch(&i, 1, __ATOMIC_ACQ_REL); + __atomic_load_n(&i, __ATOMIC_SEQ_CST); + __atomic_compare_exchange_n(&i, &j, 0, 0, __ATOMIC_RELEASE, __ATOMIC_ACQUIRE); + __atomic_store_n(&i, 0, __ATOMIC_RELAXED); + return i; +}])],[has_atomic_mm="yes"]) + +if test "x$has_atomic_mm" = xyes; then + AC_DEFINE(HAVE_ATOMIC_MEMORY_MODEL,1, + [Define to 1 if you have the __atomic_* compiler builtins]) + AC_MSG_RESULT(yes) +else + AC_MSG_RESULT(no) +fi +])
Signature · 1 line
2.14.1
PATCH v3 2/7 tst_atomic: Add atomic store and load tests
Similar to test09.
--- lib/newlib_tests/.gitignore | 1 + lib/newlib_tests/Makefile | 1 + lib/newlib_tests/test15.c | 132 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 134 insertions(+) create mode 100644 lib/newlib_tests/test15.c
lib/newlib_tests/.gitignore +1 -0
@@ -12,6 +12,7 @@ test11 test12 test13 test14 +test15 tst_device tst_safe_fileops tst_res_hexd
lib/newlib_tests/Makefile +1 -0
@@ -7,6 +7,7 @@ LDLIBS += -lltp test08: CFLAGS+=-pthread test09: CFLAGS+=-pthread +test15: CFLAGS+=-pthread ifeq ($(ANDROID),1) FILTER_OUT_MAKE_TARGETS += test08
lib/newlib_tests/test15.c new file +132 -0
132 lines not on this page · raw message
Signature · 1 line
2.14.1
PATCH v3 2/7 tst_atomic: Add atomic store and load tests
Hi!
Quoted 56 lines
lib/newlib_tests/test15.c new file +50
-0@@ -0,0 +1,132 @@ +/* +* Copyright (c) 2017 Richard Palethorpe <[email protected]> +* +* This program is free software: you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation, either version 2 of the License, or +* (at your option) any later version. +* +* This program 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 General Public License +* along with this program. If not, see <http://www.gnu.org/licenses/>. +*/ + +/* +* A basic regression test for tst_atomic_{load,store}. Also provides a +* limited check that atomic stores and loads order non-atomic memory +* accesses. That is, we are checking that they implement memory fences or +* barriers. +* +* Many architectures/machines will still pass the test even if you remove the +* atomic functions. X86 in particular has strong memory ordering by default +* so that should always pass (if you use volatile). However Aarch64 +* (Raspberry Pi 3 Model B) has been observed to fail without the atomic +* functions. +* +* A failure can occur if an update to seq_n is not made globally visible by +* the time the next thread needs to use it. +*/ + +#include <stdint.h> +#include <pthread.h> +#include "tst_test.h" +#include "tst_atomic.h" + +#define THREADS 64 +#define FILLER (1 << 20) + +/* Uncomment these to see what happens without atomics. To prevent the compiler +* from removing/reording atomic and seq_n, mark them as volatile. +*/ +/* #define tst_atomic_load(v) (*(v)) */ +/* #define tst_atomic_store(i, v) *(v) = (i) */ + +struct block { +int seq_n; +intptr_t id;
lib/newlib_tests/test15.c line 60
Quoted 10 lines
lib/newlib_tests/test15.c new file +10
-0+intptr_t filler[FILLER ]; +}; + +static int atomic; +static int *seq_n; +static struct block *m; + +static void *worker_load_store (void *_id) +{ +intptr_t id = (intptr_t )_id, i;
You can do:
int id = (intptr_t)_id, i;
Then you can drop the casts int the rest of the function.
Also identifiers starting with underscore or two are reserved for system libraries (libc/kernel headers).
lib/newlib_tests/test15.c line 65
Quoted 4 lines
lib/newlib_tests/test15.c new file +4
-0+for (i = (intptr_t )tst_atomic_load (&atomic); + i != id; + i = (intptr_t )tst_atomic_load (&atomic)) + ;
Okay this spins in place until atomic == id, hence only one thread gets to run the code below and they should get there in increasing order by id.
lib/newlib_tests/test15.c line 68
Quoted 2 lines
lib/newlib_tests/test15.c new file +2
-0+ (m + (*seq_n))->id = id; + *seq_n +=1 ;
But I do not get why seq_n points in the middle of the mmaped area. A short comment on the expected efect of this as well as of the cache ruiner would help a bit.
Quoted 11 lines
lib/newlib_tests/test15.c new file +11
-0+tst_atomic_store ((int )i +1 , &atomic); + +return NULL ; +} + +static void *cache_ruiner(void *vp LTP_ATTRIBUTE_UNUSED) +{ +intptr_t i =0 , j; +struct block *cur = m; + +tst_res (TINFO ,"cache ruiner started" );
lib/newlib_tests/test15.c line 89
Quoted 10 lines
lib/newlib_tests/test15.c new file +10
-0+while (tst_atomic_load (&atomic) >0 ) { +for (j =0 ; j <FILLER ; j++) + cur->filler[j] = j; + +if (i <THREADS -1 ) { + cur = m + (++i); + }else { + i =0 ; + cur = m; + }
i = (i + 1) % (THREADS - 1); cur = m + i;
lib/newlib_tests/test15.c line 101
Quoted 12 lines
lib/newlib_tests/test15.c new file +12
-0+ } + +return NULL ; +} + +static void do_test (void ) +{ +intptr_t i, id; +pthread_t threads[THREADS +1 ]; + + atomic =0 ; + m = (struct block *)SAFE_MMAP(NULL, sizeof(*m) * THREADS,
^ Useless cast.
Quoted 5 lines
lib/newlib_tests/test15.c new file +5
-0+PROT_READ |PROT_WRITE , +MAP_PRIVATE |MAP_ANONYMOUS , + -1, 0); + seq_n = &((m +THREADS /2 )->seq_n); +
lib/newlib_tests/test15.c line 116
Quoted 10 lines
lib/newlib_tests/test15.c new file +10
-0+pthread_create (threads+THREADS ,NULL , cache_ruiner,NULL ); +for (i =THREADS -1 ; i >=0 ; i--) +pthread_create (threads+i,NULL , worker_load_store, (void *)i); + +for (i =0 ; i <THREADS ; i++) { +tst_res (TINFO ,"Joining thread %li" , i); +pthread_join (threads[i],NULL ); + } +tst_atomic_store (-1 , &atomic); +pthread_join (*(threads+THREADS ),NULL );
^
Huh, why not just threads[THREADS] ?
Quoted 22 lines
lib/newlib_tests/test15.c new file +16
-0+ +tst_res (TINFO ,"Expected\tFound" ); +for (i =0 ; i <THREADS ; i++) { + id = (m + i)->id; +if (id != i) +tst_res (TFAIL ,"%d\t\t%d" , (int )i, (int )id); +else +tst_res (TPASS ,"%d\t\t%d" , (int )i, (int )id); + } + +SAFE_MUNMAP (m,sizeof (*m) *THREADS ); +} + +static struct tst_test test = { + .test_all = do_test, +};Signature · 5 lines
2.14.1 -- Mailing list info: https://lists.linux.it/listinfo/ltp
Signature · 2 lines
Cyril Hrubis [email protected]
PATCH v3 3/7 fzsync: Add long running thread support and deviation stats
In reply to Richard PalethorpeIntroduce a new synchronisation method which causes two threads to stop and wait for each other. Continuing only when both threads have signaled their readiness. Previously we would start a new thread in each iteration which creates a variable delay in either the parent or child.
Using two long running threads significantly reduces the time needed to perform a single race and may also increase timing stability on some architectures.
This does not replace the delay method of synchronisation which can be used in addition to the new method.
--- include/tst_fuzzy_sync.h | 169 +++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 142 insertions(+), 27 deletions(-)
include/tst_fuzzy_sync.h +142 -27
@@ -17,21 +17,30 @@/* * Fuzzy Synchronisation - abreviated to fzsync * -* This library is intended to help reproduce race conditions while running in -* a loop. You can use it to measure the time at which two functions are -* called in different threads. Then calculate the average time gap between -* the function calls and introduce a delay in one thread to synchronise the -* calls. +* This library is intended to help reproduce race conditions by providing two +* thread synchronisation mechanisms. The first is a 'checkpoint' system where +* each thread will wait indefinitely for the other to enter a checkpoint +* before continuing. This is acheived by calling tst_fzsync_wait() and/or +* tst_fzsync_wait_update() at the points you want to synchronise in each +* thread. This is functionally very similar to using pthread_barrier_wait() +* with two threads. However tst_fzsync_wait() can be inlined and is +* guaranteed not to call sleep or futex. * -* It is called 'fuzzy' synchronisation because the time gap will naturally vary -* due to environmental factors. It is not a 'hard' synchronisation mechanism -* like lockstepping. +* The second takes the form a of a delay which is calculated by measuring the +* time difference between two points in each thread and comparing it to the +* desired difference (default is zero). Using a delay allows you to +* synchronise the threads at a point outside of your direct control +* (e.g. inside the kernel) or just increase the accuracy for the first +* mechanism. It is acheived using tst_fzsync_delay_{a,b}(), +* tst_fzsync_time_{a,b}() and tst_fzsync[_wait_]update(). * -* For a usage example see testcases/cve/cve-2017-2671.c +* For a usage example see testcases/cve/cve-2016-7117.c or just run +* 'git grep tst_fuzzy_sync.h' */ #include <sys/time.h> #include <time.h> +#include <math.h> #include "tst_atomic.h" #ifndef CLOCK_MONOTONIC_RAW
@@ -40,30 +49,44 @@/** * struct tst_fzsync_pair - the state of a two way synchronisation -* @avg_diff: The average time difference over multiple iterations -* @avg_diff_trgt: The desired average time difference, defaults to 0 +* @avg_diff: Internal; the average time difference over multiple iterations. +* @avg_diff_trgt: The desired average time difference, defaults to 0. * @avg_alpha: The rate at which old diff samples are forgotten, -* defaults to 0.25 -* @a: The time at which call site A was last passed -* @b: The time at which call site B was last passed -* @delay: The size of the delay, positive to delay A, negative to delay B -* @delay_inc: The step size of a delay increment, defaults to 10 +* defaults to 0.25. +* @avg_dev: Internal; Absolute average deviation. +* @a: Internal; The time at which call site A was last passed. +* @b: Internal; The time at which call site B was last passed. +* @delay: Internal; The size of the delay, positive to delay A, +* negative to delay B. +* @delay_inc: The step size of a delay increment, defaults to 1. * @update_gap: The number of iterations between recalculating the delay. * Defaults to 0xF and must be of the form $2^n - 1$ +* @info_gap: The number of iterations between printing some statistics. +* Defaults to 0x7FFFF and must also be one less than a power of 2. +* @a_cntr: Internal; Atomic counter used by fzsync_pair_wait() +* @b_cntr: Internal; Atomic counter used by fzsync_pair_wait() +* @exit: Internal; Used by tst_fzsync_pair_exit() and fzsync_pair_wait() * * This contains all the necessary state for synchronising two points A and * B. Where A is the time of an event in one process and B is the time of an * event in another process. +* +* Internal fields should only be accessed by library functions. */ struct tst_fzsync_pair {double avg_diff;double avg_diff_trgt;double avg_alpha; +double avg_dev;struct timespec a;struct timespec b;long delay;long delay_inc;int update_gap; +int info_gap; +int a_cntr; +int b_cntr; +int exit; };/**
@@ -71,14 +94,19 @@ struct tst_fzsync_pair {*/ #define TST_FZSYNC_PAIR_INIT { \ .avg_alpha = 0.25, \ - .delay_inc = 10, \ - .update_gap = 0xF \ + .delay_inc = 1, \ + .update_gap = 0xF, \ + .info_gap = 0x7FFFF \ } +/** +* tst_fzsync_pair_info - Print some synchronisation statistics +*/ static void tst_fzsync_pair_info (struct tst_fzsync_pair *pair) { -tst_res (TINFO ,"avg_diff = %.5gns, delay = %05ld loops" , - pair->avg_diff, pair->delay); +tst_res (TINFO , +"avg_diff = %.0fns, avg_dev = %.0fns, delay = %05ld loops" , + pair->avg_diff, pair->avg_dev, pair->delay); }/**
@@ -133,18 +161,15 @@ static inline void tst_fzsync_time_b(struct tst_fzsync_pair *pair) }/** -* tst_exp_moving_avg - Exponential moving average +* TST_EXP_MOVING_AVG - Exponential moving average * @alpha: The preference for recent samples over old ones. * @sample: The current sample * @prev_avg: The average of the all the previous samples * * Returns average including the current sample. */ -static inline double tst_exp_moving_avg (double alpha,long sample, -double prev_avg) -{ -return alpha * sample + (1.0 - alpha) * prev_avg; -} +#define TST_EXP_MOVING_AVG (alpha, sample, prev_avg)\ + (alpha * sample + (1.0 - alpha) * prev_avg)/** * tst_fzsync_pair_update - Recalculate the delay
@@ -169,8 +194,16 @@ static void tst_fzsync_pair_update(int loop_index, struct tst_fzsync_pair *pair)double target = pair->avg_diff_trgt;double avg = pair->avg_diff; +if (pair->a.tv_sec > pair->b.tv_sec) + pair->a.tv_nsec +=1000000000 ; +else if (pair->a.tv_sec < pair->b.tv_sec) + pair->b.tv_nsec +=1000000000 ; + diff = pair->a.tv_nsec - pair->b.tv_nsec; - avg =tst_exp_moving_avg (pair->avg_alpha, diff, avg); + avg =TST_EXP_MOVING_AVG (pair->avg_alpha, diff, avg); + pair->avg_dev =TST_EXP_MOVING_AVG (pair->avg_alpha, +fabs (diff - avg), + pair->avg_dev);if (!(loop_index & pair->update_gap)) {if (avg > target)
@@ -179,5 +212,87 @@ static void tst_fzsync_pair_update(int loop_index, struct tst_fzsync_pair *pair) pair->delay += inc; } +if (!(loop_index & pair->info_gap)) +tst_fzsync_pair_info (pair); + pair->avg_diff = avg; } + +/** +* tst_fzsync_pair_wait - Wait for the other thread +* @our_cntr: The counter for the thread we are on +* @other_cntr: The counter for the thread we are synchronising with +* +* Use this (through tst_fzsync_pair_wait_a() and tst_fzsync_pair_wait_b()) if +* you need an additional synchronisation point in a thread or you do not want +* to use the delay facility (not recommended). See +* tst_fzsync_pair_wait_update(). +* +* Returns a non-zero value if the thread should continue otherwise the +* calling thread should exit. +*/ +static inline int tst_fzsync_pair_wait (struct tst_fzsync_pair *pair, +int *our_cntr,int *other_cntr) +{ +tst_atomic_inc (other_cntr); +while (tst_atomic_load (our_cntr) <tst_atomic_load (other_cntr) + && !tst_atomic_load (&pair->exit)) + ; + +return !tst_atomic_load (&pair->exit); +} + +static inline int tst_fzsync_wait_a (struct tst_fzsync_pair *pair) +{ +return tst_fzsync_pair_wait (pair, &pair->a_cntr, &pair->b_cntr); +} + +static inline int tst_fzsync_wait_b (struct tst_fzsync_pair *pair) +{ +return tst_fzsync_pair_wait (pair, &pair->b_cntr, &pair->a_cntr); +} + +/** +* tst_fzsync_pair_wait_update_{a,b} - Wait and then recalculate +* +* This allows you to have two long running threads which wait for each other +* every iteration. So each thread will exit this function at approximately +* the same time. It also updates the delay values in a thread safe manner. +* +* You must call this function in both threads the same number of times each +* iteration. So a call in one thread must match with a call in the +* other. Make sure that calls to tst_fzsync_pair_wait() and +* tst_fzsync_pair_wait_update() happen in the same order in each thread. That +* is, make sure that a call to tst_fzsync_pair_wait_update_a() in one thread +* corresponds to a call to tst_fzsync_pair_wait_update_b() in the other. +* +* Returns a non-zero value if the calling thread should continue to loop. If +* it returns zero then tst_fzsync_exit() has been called and you must exit +* the thread. +*/ +static inline int tst_fzsync_wait_update_a (struct tst_fzsync_pair *pair) +{ +static int loop_index; + +tst_fzsync_pair_wait (pair, &pair->a_cntr, &pair->b_cntr); + loop_index++; +tst_fzsync_pair_update (loop_index, pair); +return tst_fzsync_pair_wait (pair, &pair->a_cntr, &pair->b_cntr); +} + +static inline int tst_fzsync_wait_update_b (struct tst_fzsync_pair *pair) +{ +tst_fzsync_pair_wait (pair, &pair->b_cntr, &pair->a_cntr); +return tst_fzsync_pair_wait (pair, &pair->b_cntr, &pair->a_cntr); +} + +/** +* tst_fzsync_pair_exit - Signal that the other thread should exit +* +* Causes tst_fzsync_pair_wait() and tst_fzsync_pair_wait_update() to return +* 0. +*/ +static inline void tst_fzsync_pair_exit (struct tst_fzsync_pair *pair) +{ +tst_atomic_store (1 , &pair->exit); +}
Signature · 1 line
2.14.1
PATCH v3 3/7 fzsync: Add long running thread support and deviation stats
Hi!
Quoted 22 lines
include/tst_fuzzy_sync.h +7
-3+/** +* tst_fzsync_pair_info - Print some synchronisation statistics +*/ static void tst_fzsync_pair_info (struct tst_fzsync_pair *pair) { -tst_res (TINFO ,"avg_diff = %.5gns, delay = %05ld loops" , - pair->avg_diff, pair->delay); +tst_res (TINFO , +"avg_diff = %.0fns, avg_dev = %.0fns, delay = %05ld loops" , + pair->avg_diff, pair->avg_dev, pair->delay); }/** @@ -133,18 +161,15 @@ static inline void tst_fzsync_time_b(struct tst_fzsync_pair *pair) }/** -* tst_exp_moving_avg - Exponential moving average +* TST_EXP_MOVING_AVG - Exponential moving average * @alpha: The preference for recent samples over old ones. * @sample: The current sample * @prev_avg: The average of the all the previous samples
include/tst_fuzzy_sync.h static inline void tst_fzsync_time_b(struct tst_fzsync_pair *pair) · line 172
Quoted 10 lines
include/tst_fuzzy_sync.h +2
-5* * Returns average including the current sample. */ -static inline double tst_exp_moving_avg (double alpha,long sample, -double prev_avg) -{ -return alpha * sample + (1.0 - alpha) * prev_avg; -} +#define TST_EXP_MOVING_AVG (alpha, sample, prev_avg)\ + (alpha * sample + (1.0 - alpha) * prev_avg)
Why do we define this as a macro instead of static inline function? As far as I can tell the only difference is that we loose type checks and introduce possible side effects.
include/tst_fuzzy_sync.h static void tst_fzsync_pair_update(int loop_index, struct tst_fzsync_pair *pair) · line 200
Quoted 10 lines
include/tst_fuzzy_sync.h +4
-0/** * tst_fzsync_pair_update - Recalculate the delay @@ -169,8 +194,16 @@ static void tst_fzsync_pair_update(int loop_index, struct tst_fzsync_pair *pair)double target = pair->avg_diff_trgt;double avg = pair->avg_diff; +if (pair->a.tv_sec > pair->b.tv_sec) + pair->a.tv_nsec +=1000000000 ; +else if (pair->a.tv_sec < pair->b.tv_sec) + pair->b.tv_nsec +=1000000000 ;
Why the else here? These two cases are mutually exclusive.
Quoted 103 lines
include/tst_fuzzy_sync.h +86
-1diff = pair->a.tv_nsec - pair->b.tv_nsec; - avg =tst_exp_moving_avg (pair->avg_alpha, diff, avg); + avg =TST_EXP_MOVING_AVG (pair->avg_alpha, diff, avg); + pair->avg_dev =TST_EXP_MOVING_AVG (pair->avg_alpha, +fabs (diff - avg), + pair->avg_dev);if (!(loop_index & pair->update_gap)) {if (avg > target)@@ -179,5 +212,87 @@ static void tst_fzsync_pair_update(int loop_index, struct tst_fzsync_pair *pair) pair->delay += inc; } +if (!(loop_index & pair->info_gap)) +tst_fzsync_pair_info (pair); + pair->avg_diff = avg; } + +/** +* tst_fzsync_pair_wait - Wait for the other thread +* @our_cntr: The counter for the thread we are on +* @other_cntr: The counter for the thread we are synchronising with +* +* Use this (through tst_fzsync_pair_wait_a() and tst_fzsync_pair_wait_b()) if +* you need an additional synchronisation point in a thread or you do not want +* to use the delay facility (not recommended). See +* tst_fzsync_pair_wait_update(). +* +* Returns a non-zero value if the thread should continue otherwise the +* calling thread should exit. +*/ +static inline int tst_fzsync_pair_wait (struct tst_fzsync_pair *pair, +int *our_cntr,int *other_cntr) +{ +tst_atomic_inc (other_cntr); +while (tst_atomic_load (our_cntr) <tst_atomic_load (other_cntr) + && !tst_atomic_load (&pair->exit)) + ; + +return !tst_atomic_load (&pair->exit); +} + +static inline int tst_fzsync_wait_a (struct tst_fzsync_pair *pair) +{ +return tst_fzsync_pair_wait (pair, &pair->a_cntr, &pair->b_cntr); +} + +static inline int tst_fzsync_wait_b (struct tst_fzsync_pair *pair) +{ +return tst_fzsync_pair_wait (pair, &pair->b_cntr, &pair->a_cntr); +} + +/** +* tst_fzsync_pair_wait_update_{a,b} - Wait and then recalculate +* +* This allows you to have two long running threads which wait for each other +* every iteration. So each thread will exit this function at approximately +* the same time. It also updates the delay values in a thread safe manner. +* +* You must call this function in both threads the same number of times each +* iteration. So a call in one thread must match with a call in the +* other. Make sure that calls to tst_fzsync_pair_wait() and +* tst_fzsync_pair_wait_update() happen in the same order in each thread. That +* is, make sure that a call to tst_fzsync_pair_wait_update_a() in one thread +* corresponds to a call to tst_fzsync_pair_wait_update_b() in the other. +* +* Returns a non-zero value if the calling thread should continue to loop. If +* it returns zero then tst_fzsync_exit() has been called and you must exit +* the thread. +*/ +static inline int tst_fzsync_wait_update_a (struct tst_fzsync_pair *pair) +{ +static int loop_index; + +tst_fzsync_pair_wait (pair, &pair->a_cntr, &pair->b_cntr); + loop_index++; +tst_fzsync_pair_update (loop_index, pair); +return tst_fzsync_pair_wait (pair, &pair->a_cntr, &pair->b_cntr); +} + +static inline int tst_fzsync_wait_update_b (struct tst_fzsync_pair *pair) +{ +tst_fzsync_pair_wait (pair, &pair->b_cntr, &pair->a_cntr); +return tst_fzsync_pair_wait (pair, &pair->b_cntr, &pair->a_cntr); +} + +/** +* tst_fzsync_pair_exit - Signal that the other thread should exit +* +* Causes tst_fzsync_pair_wait() and tst_fzsync_pair_wait_update() to return +* 0. +*/ +static inline void tst_fzsync_pair_exit (struct tst_fzsync_pair *pair) +{ +tst_atomic_store (1 , &pair->exit); +}Signature · 5 lines
2.14.1 -- Mailing list info: https://lists.linux.it/listinfo/ltp
Signature · 2 lines
Cyril Hrubis [email protected]
PATCH v3 3/7 fzsync: Add long running thread support and deviation stats
Hello,
Quoted 11 lines · Cyril Hrubis
Quoted 11 lines
- *
tst_exp_moving_avg- Exponential moving average
+ *TST_EXP_MOVING_AVG- Exponential moving average
* @alpha: The preference for recent samples over old ones.
* @sample: The current sample
* @prev_avg: The average of the all the previous samples *
* Returns average including the current sample. */include/tst_fuzzy_sync.h +0
-3-static inline double tst_exp_moving_avg (double alpha,long sample, -double prev_avg) -{
Quoted 8 lines
include/tst_fuzzy_sync.h static inline void tst_fzsync_time_b(struct tst_fzsync_pair *pair) · line 172
Quoted 4 lines
include/tst_fuzzy_sync.h +2
-2-return alpha * sample + (1.0 - alpha) * prev_avg; -} +#define TST_EXP_MOVING_AVG (alpha, sample, prev_avg)\ + (alpha * sample + (1.0 - alpha) * prev_avg)Why do we define this as a macro instead of static inline function? As far as I can tell the only difference is that we loose type checks and introduce possible side effects.
Ah, I needed to drop type checking at some point, can't remember why, but it is not necessary now, so I will change it back.
Quoted 7 lines
Quoted 7 lines
/**
*tst_fzsync_pair_update- Recalculate the delayinclude/tst_fuzzy_sync.h +0
-0@@ -169,8 +194,16 @@ static void tst_fzsync_pair_update(int loop_index, struct tst_fzsync_pair *pair)double target = pair->avg_diff_trgt;double avg = pair->avg_diff;
Quoted 6 lines
include/tst_fuzzy_sync.h static void tst_fzsync_pair_update(int loop_index, struct tst_fzsync_pair *pair) · line 200
Quoted 4 lines
include/tst_fuzzy_sync.h +4
-0+if (pair->a.tv_sec > pair->b.tv_sec) + pair->a.tv_nsec +=1000000000 ; +else if (pair->a.tv_sec < pair->b.tv_sec) + pair->b.tv_nsec +=1000000000 ;Why the else here? These two cases are mutually exclusive.
Because if (pair->a.tv_sec == pair->b.tv_sec) then we want to do nothing.
Signature · 2 lines
Thank you, Richard.
PATCH v3 3/7 fzsync: Add long running thread support and deviation stats
Hi!
Quoted 13 lines
Quoted 6 lines
Quoted 6 lines
/**
*tst_fzsync_pair_update- Recalculate the delayinclude/tst_fuzzy_sync.h +0
-0@@ -169,8 +194,16 @@ static void tst_fzsync_pair_update(int loop_index, struct tst_fzsync_pair *pair)double target = pair->avg_diff_trgt;double avg = pair->avg_diff;Quoted 6 lines
include/tst_fuzzy_sync.h static void tst_fzsync_pair_update(int loop_index, struct tst_fzsync_pair *pair) · line 200
Quoted 4 lines
include/tst_fuzzy_sync.h +4
-0+if (pair->a.tv_sec > pair->b.tv_sec) + pair->a.tv_nsec +=1000000000 ; +else if (pair->a.tv_sec < pair->b.tv_sec) + pair->b.tv_nsec +=1000000000 ;Why the else here? These two cases are mutually exclusive.
Quoted 2 lines
Because if (
pair->a.tv_sec==pair->b.tv_sec) then we want to do nothing.
Ah right, missed that. I wonder if can compute the diff directly as:
pair->a.tv_nsec - pair->b.tv_nsec + 1000000000 * (pair->a.tv_sec - pair->b.tv_sec)
instead.
Or do we care not to overflow here?
Signature · 2 lines
Cyril Hrubis [email protected]
PATCH v3 3/7 fzsync: Add long running thread support and deviation stats
In reply to Richard PalethorpeHi!
Quoted 13 lines
include/tst_fuzzy_sync.h +13
-0+/** +* tst_fzsync_pair_wait - Wait for the other thread +* @our_cntr: The counter for the thread we are on +* @other_cntr: The counter for the thread we are synchronising with +* +* Use this (through tst_fzsync_pair_wait_a() and tst_fzsync_pair_wait_b()) if +* you need an additional synchronisation point in a thread or you do not want +* to use the delay facility (not recommended). See +* tst_fzsync_pair_wait_update(). +* +* Returns a non-zero value if the thread should continue otherwise the +* calling thread should exit. +*/
include/tst_fuzzy_sync.h static void tst_fzsync_pair_update(int loop_index, struct tst_fzsync_pair *pair) · line 243
Quoted 10 lines
include/tst_fuzzy_sync.h +10
-0+static inline int tst_fzsync_pair_wait (struct tst_fzsync_pair *pair, +int *our_cntr,int *other_cntr) +{ +tst_atomic_inc (other_cntr); +while (tst_atomic_load (our_cntr) <tst_atomic_load (other_cntr) + && !tst_atomic_load (&pair->exit)) + ; + +return !tst_atomic_load (&pair->exit); +}
Also what happens if the counters overflow?
I guess that this will happen if we call the test with large enough -i paramter, right?
Signature · 2 lines
Cyril Hrubis [email protected]
PATCH v3 3/7 fzsync: Add long running thread support and deviation stats
Hello,
Quoted 27 lines · Cyril Hrubis
Hi!
Quoted 13 lines
include/tst_fuzzy_sync.h +13
-0+/** +* tst_fzsync_pair_wait - Wait for the other thread +* @our_cntr: The counter for the thread we are on +* @other_cntr: The counter for the thread we are synchronising with +* +* Use this (through tst_fzsync_pair_wait_a() and tst_fzsync_pair_wait_b()) if +* you need an additional synchronisation point in a thread or you do not want +* to use the delay facility (not recommended). See +* tst_fzsync_pair_wait_update(). +* +* Returns a non-zero value if the thread should continue otherwise the +* calling thread should exit. +*/ include/tst_fuzzy_sync.h static void tst_fzsync_pair_update(int loop_index, struct tst_fzsync_pair *pair) · line 243
Quoted 10 lines
include/tst_fuzzy_sync.h +10
-0+static inline int tst_fzsync_pair_wait (struct tst_fzsync_pair *pair, +int *our_cntr,int *other_cntr) +{ +tst_atomic_inc (other_cntr); +while (tst_atomic_load (our_cntr) <tst_atomic_load (other_cntr) + && !tst_atomic_load (&pair->exit)) + ; + +return !tst_atomic_load (&pair->exit); +}Also what happens if the counters overflow?
Quoted 2 lines
I guess that this will happen if we call the test with large enough -i paramter, right?
Bad things will happen :-). It can probably be fixed with the following though:
static inline int tst_fzsync_pair_wait(struct tst_fzsync_pair *pair,
int *our_cntr, int *other_cntr)
{
- tst_atomic_inc(other_cntr);
+ if (tst_atomic_inc(other_cntr) < 0) {
+ tst_atomic_store(0, other_cntr);
+ while (tst_atomic_load(our_cntr) > tst_atomic_load(other_cntr)
+ && !tst_atomic_load(&pair->exit))
+ ;
+ }
+
while (tst_atomic_load(our_cntr) < tst_atomic_load(other_cntr)
&& !tst_atomic_load(&pair->exit))
Seems to work and any perf penality is insignificant; with or without branch prediction.
Signature · 2 lines
Thank you, Richard.
PATCH v3 3/7 fzsync: Add long running thread support and deviation stats
Quoted 51 lines · Richard Palethorpe
Hello,
Quoted 27 lines · Cyril Hrubis
Hi!
Quoted 13 lines
include/tst_fuzzy_sync.h +13
-0+/** +* tst_fzsync_pair_wait - Wait for the other thread +* @our_cntr: The counter for the thread we are on +* @other_cntr: The counter for the thread we are synchronising with +* +* Use this (through tst_fzsync_pair_wait_a() and tst_fzsync_pair_wait_b()) if +* you need an additional synchronisation point in a thread or you do not want +* to use the delay facility (not recommended). See +* tst_fzsync_pair_wait_update(). +* +* Returns a non-zero value if the thread should continue otherwise the +* calling thread should exit. +*/ include/tst_fuzzy_sync.h static void tst_fzsync_pair_update(int loop_index, struct tst_fzsync_pair *pair) · line 243
Quoted 10 lines
include/tst_fuzzy_sync.h +10
-0+static inline int tst_fzsync_pair_wait (struct tst_fzsync_pair *pair, +int *our_cntr,int *other_cntr) +{ +tst_atomic_inc (other_cntr); +while (tst_atomic_load (our_cntr) <tst_atomic_load (other_cntr) + && !tst_atomic_load (&pair->exit)) + ; + +return !tst_atomic_load (&pair->exit); +}Also what happens if the counters overflow?
Quoted 2 lines
I guess that this will happen if we call the test with large enough -i paramter, right?
Bad things will happen :-). It can probably be fixed with the following though:
static inline int tst_fzsync_pair_wait(struct
tst_fzsync_pair*pair,(quoted diff) +7
-1int *our_cntr, int *other_cntr) { - tst_atomic_inc(other_cntr); + if (tst_atomic_inc(other_cntr) < 0) { + tst_atomic_store(0, other_cntr); + while (tst_atomic_load(our_cntr) > tst_atomic_load(other_cntr) + && !tst_atomic_load(&pair->exit)) + ; + } + while (tst_atomic_load(our_cntr) < tst_atomic_load(other_cntr) && !tst_atomic_load(&pair->exit))
Quoted 2 lines
Seems to work and any perf penality is insignificant; with or without branch prediction.
Actually this has a race condition if one thread is interrupted after setting other_cntr to zero, but before it enters the while loop.
Signature · 2 lines
Thank you, Richard.
PATCH v3 3/7 fzsync: Add long running thread support and deviation stats
Hi!
Quoted 2 lines
Actually this has a race condition if one thread is interrupted after setting
other_cntrto zero, but before it enters the while loop.
Can't we simply reset these counters between test iterations?
Signature · 2 lines
Cyril Hrubis [email protected]
PATCH v3 4/7 fzsync: Add functionality test for library
In reply to Richard Palethorpe--- lib/newlib_tests/.gitignore | 1 + lib/newlib_tests/Makefile | 2 + lib/newlib_tests/test16.c | 106 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 109 insertions(+) create mode 100644 lib/newlib_tests/test16.c
lib/newlib_tests/.gitignore +1 -0
@@ -13,6 +13,7 @@ test12 test13 test14 test15 +test16 tst_device tst_safe_fileops tst_res_hexd
lib/newlib_tests/Makefile +2 -0
@@ -8,6 +8,8 @@ LDLIBS += -lltp test08: CFLAGS+=-pthread test09: CFLAGS+=-pthread test15: CFLAGS+=-pthread +test16: CFLAGS+=-pthread +test16: LDLIBS+=-lrt ifeq ($(ANDROID),1) FILTER_OUT_MAKE_TARGETS += test08
lib/newlib_tests/test16.c new file +106 -0
106 lines not on this page · raw message
Signature · 1 line
2.14.1
PATCH v3 4/7 fzsync: Add functionality test for library
Hi!
Quoted 12 lines
lib/newlib_tests/test16.c new file +12
-0+/* +* Copyright (c) 2017 Richard Palethorpe <[email protected]> +* +* This program is free software: you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation, either version 2 of the License, or +* (at your option) any later version. +* +* This program 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.
lib/newlib_tests/test16.c line 22
Quoted 10 lines
lib/newlib_tests/test16.c new file +10
-0+* +* You should have received a copy of the GNU General Public License +* along with this program. If not, see <http://www.gnu.org/licenses/>. +*/ +/* Basic functionality test for tst_fuzzy_sync.h similar to the atomic tests +* (test15.c). One thread writes to the odd indexes of an array while the +* other writes to the even. If the threads are not synchronised then they +* will probably write to the wrong indexes as they share an index variable +* which they should take it in turns to update. +*/
I'm just wondering, wouldn't be a better test for the sync library if one of the threads does short usleep(); and we would check if the delay more or less corresponds to this value after some iterations?
Signature · 2 lines
Cyril Hrubis [email protected]
PATCH v3 4/7 fzsync: Add functionality test for library
Quoted 28 lines · Cyril Hrubis
Hi!
Quoted 12 lines
lib/newlib_tests/test16.c new file +12
-0+/* +* Copyright (c) 2017 Richard Palethorpe <[email protected]> +* +* This program is free software: you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation, either version 2 of the License, or +* (at your option) any later version. +* +* This program 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. lib/newlib_tests/test16.c line 22
Quoted 10 lines
lib/newlib_tests/test16.c new file +10
-0+* +* You should have received a copy of the GNU General Public License +* along with this program. If not, see <http://www.gnu.org/licenses/>. +*/ +/* Basic functionality test for tst_fuzzy_sync.h similar to the atomic tests +* (test15.c). One thread writes to the odd indexes of an array while the +* other writes to the even. If the threads are not synchronised then they +* will probably write to the wrong indexes as they share an index variable +* which they should take it in turns to update. +*/ I'm just wondering, wouldn't be a better test for the sync library if one of the threads does short
usleep(); and we would check if the delay more or less corresponds to this value after some iterations?
Quoted 3 lines
--
Cyril Hrubis
[email protected]
This test is currently an OK test for checking race conditions and what happens during a counter overflow. Sleeping pretty much ruins that, so I would add another test. Such a test would go well with the sliding window feature I want to add at some later point. I quickly tried adding a usleep(tv_nsec = 500) and the deviation was way over 500, so I'm not sure usleep is actually accurate enough to test this algorithm, but it needs some analysis. It could be the algorithm is getting a bit chaotic and the averages are smoothing it over (although I don't think so). I atleast need to start collecting and plotting the results.
--
Thank you,
Richard.
PATCH v3 5/7 Convert cve-2016-7117 test to use long running threads
In reply to Richard Palethorpe--- testcases/cve/cve-2016-7117.c | 47 +++++++++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 20 deletions(-)
testcases/cve/cve-2016-7117.c +27 -20
@@ -71,7 +71,7 @@ struct mmsghdr { };#endif -static int socket_fds[2 ]; +static volatile int socket_fds[2 ];static struct mmsghdr msghdrs[2 ] = { { .msg_hdr = {
@@ -94,40 +94,53 @@ static struct mmsghdr msghdrs[2] = {static char rbuf[sizeof (MSG )];static struct timespec timeout = { .tv_sec =RECV_TIMEOUT };static struct tst_fzsync_pair fzsync_pair =TST_FZSYNC_PAIR_INIT ; +static pthread_t pt_send; +static void *send_and_close (void *); + +static void setup (void ) +{ +SAFE_PTHREAD_CREATE (&pt_send,0 , send_and_close,0 ); +}static void cleanup (void ) {close (socket_fds[0 ]);close (socket_fds[1 ]); + +if (pt_send) { +tst_fzsync_pair_exit (&fzsync_pair); +SAFE_PTHREAD_JOIN (pt_send,0 ); + } }static void *send_and_close (void *arg) { -send (socket_fds[0 ],MSG ,sizeof (MSG ),0 ); -send (socket_fds[0 ],MSG ,sizeof (MSG ),0 ); - -tst_fzsync_delay_b (&fzsync_pair); +while (tst_fzsync_wait_update_b (&fzsync_pair)) { +send (socket_fds[0 ],MSG ,sizeof (MSG ),0 ); +send (socket_fds[0 ],MSG ,sizeof (MSG ),0 ); -close (socket_fds[0 ]); -close (socket_fds[1 ]); -tst_fzsync_time_b (&fzsync_pair); +tst_fzsync_delay_b (&fzsync_pair); +close (socket_fds[0 ]); +close (socket_fds[1 ]); +tst_fzsync_time_b (&fzsync_pair); +if (!tst_fzsync_wait_b (&fzsync_pair)) +break ; + }return arg; }static void run (void ) { -pthread_t pt_send;int i, stat, too_early_count =0 ; msghdrs[0 ].msg_hdr.msg_iov->iov_base = (void *)&rbuf;for (i =1 ; i <ATTEMPTS ; i++) { -if (socketpair (AF_LOCAL ,SOCK_DGRAM ,0 , socket_fds)) +if (socketpair (AF_LOCAL ,SOCK_DGRAM ,0 , (int *)socket_fds))tst_brk (TBROK |TERRNO ,"Socket creation failed" ); -SAFE_PTHREAD_CREATE (&pt_send,0 , send_and_close,0 ); - +tst_fzsync_wait_update_a (&fzsync_pair);tst_fzsync_delay_a (&fzsync_pair); stat = tst_syscall(__NR_recvmmsg,
@@ -140,20 +153,14 @@ static void run(void)else if (stat <0 )tst_res (TWARN |TERRNO ,"recvmmsg failed unexpectedly" ); -SAFE_PTHREAD_JOIN (pt_send,0 ); - -tst_fzsync_pair_update (i, &fzsync_pair); -if (!(i &0x7FFFF )) { -tst_res (TINFO ,"Too early: %.1f%%" , -100 * too_early_count / (float )i); -tst_fzsync_pair_info (&fzsync_pair); - } +tst_fzsync_wait_a (&fzsync_pair); }tst_res (TPASS ,"Nothing happened after %d attempts" ,ATTEMPTS ); }static struct tst_test test = { + .setup = setup, .test_all = run, .cleanup = cleanup, .min_kver ="2.6.33" ,
Signature · 1 line
2.14.1
PATCH v3 6/7 Convert cve-2014-0196 to use long running threads
In reply to Richard Palethorpe--- testcases/cve/cve-2014-0196.c | 50 ++++++++++++++++++++++++------------------- 1 file changed, 28 insertions(+), 22 deletions(-)
testcases/cve/cve-2014-0196.c +28 -22
@@ -51,11 +51,13 @@#define ATTEMPTS 0x7000#define BUFLEN 512 -static int master_fd, slave_fd; +static volatile int master_fd, slave_fd;static int filler_ptys[ONEOFF_ALLOCS *2 ];static int target_ptys[RUN_ALLOCS *2 ];static char buf[BUFLEN ]; +static pthread_t overwrite_thread; +static void *overwrite_thread_fn (void *);static struct tst_fzsync_pair fzsync_pair =TST_FZSYNC_PAIR_INIT ;static void create_pty (int *amaster,int *aslave)
@@ -68,35 +70,40 @@ static void setup(void) {int i; - fzsync_pair.delay_inc =100 ;for (i =0 ; i <ONEOFF_ALLOCS ; i++) {create_pty (&filler_ptys[i], &filler_ptys[i +ONEOFF_ALLOCS ]); } + + fzsync_pair.info_gap =0xFFF ; +SAFE_PTHREAD_CREATE (&overwrite_thread,NULL , + overwrite_thread_fn,NULL ); } -static void *overwrite_thread_fn (void *p) +static void *overwrite_thread_fn(void *p LTP_ATTRIBUTE_UNUSED) { -tst_fzsync_delay_b (&fzsync_pair); -tst_fzsync_time_b (&fzsync_pair); - -SAFE_WRITE (0 , slave_fd, buf,BUFLEN -1 ); -SAFE_WRITE (0 , slave_fd, buf,BUFLEN -1 ); -SAFE_WRITE (0 , slave_fd, buf,BUFLEN ); - -return p; +while (tst_fzsync_wait_update_b (&fzsync_pair)) { +tst_fzsync_delay_b (&fzsync_pair); +tst_fzsync_time_b (&fzsync_pair); + +SAFE_WRITE (0 , slave_fd, buf,BUFLEN -1 ); +SAFE_WRITE (0 , slave_fd, buf,BUFLEN -1 ); +SAFE_WRITE (0 , slave_fd, buf,BUFLEN ); +if (!tst_fzsync_wait_b (&fzsync_pair)) +break ; + } +return 0 ; }static void run (void ) {struct termios t; -pthread_t overwrite_thread;int i, j;tst_res (TINFO ,"Attempting to overflow into a tty_struct..." );for (i =0 ; i <ATTEMPTS ; i++) { -create_pty (&master_fd, &slave_fd); +create_pty ((int *)&master_fd, (int *)&slave_fd);for (j =0 ; j <RUN_ALLOCS ; j++) create_pty(&target_ptys[j],
@@ -111,20 +118,12 @@ static void run(void) t.c_lflag |=ECHO ;tcsetattr (master_fd,TCSANOW , &t); -SAFE_PTHREAD_CREATE (&overwrite_thread,NULL , - overwrite_thread_fn,NULL ); +tst_fzsync_wait_update_a (&fzsync_pair);tst_fzsync_delay_a (&fzsync_pair);tst_fzsync_time_a (&fzsync_pair);SAFE_WRITE (0 , master_fd,"A" ,1 ); -SAFE_PTHREAD_JOIN (overwrite_thread,NULL ); - -tst_fzsync_pair_update (i, &fzsync_pair); - -if (!(i &0x1FFF )) -tst_fzsync_pair_info (&fzsync_pair); -for (j =0 ; j <RUN_ALLOCS ; j++) {if (j ==RUN_ALLOCS /2 )continue ;
@@ -139,6 +138,8 @@ static void run(void)ioctl (slave_fd,0xdeadbeef );SAFE_CLOSE (master_fd);SAFE_CLOSE (slave_fd); + +tst_fzsync_wait_a (&fzsync_pair); }tst_res (TPASS ,"Nothing bad happened, probably." );
@@ -148,6 +149,11 @@ static void cleanup(void) {int i; +if (overwrite_thread) { +tst_fzsync_pair_exit (&fzsync_pair); +SAFE_PTHREAD_JOIN (overwrite_thread,NULL ); + } +for (i =0 ; i <ONEOFF_ALLOCS *2 ; i++)close (filler_ptys[i]);close (master_fd);
Signature · 1 line
2.14.1
PATCH v3 7/7 Convert cve-2017-2671 to use long running threads
In reply to Richard Palethorpe--- testcases/cve/cve-2017-2671.c | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-)
testcases/cve/cve-2017-2671.c +19 -11
@@ -49,13 +49,15 @@#include "tst_fuzzy_sync.h" -#define ATTEMPTS 0xFFFF +#define ATTEMPTS 0x80000#define PING_SYSCTL_PATH "/proc/sys/net/ipv4/ping_group_range"static int sockfd;static unsigned int ping_min_grp, ping_max_grp;static struct tst_fzsync_pair fzsync_pair =TST_FZSYNC_PAIR_INIT ;static struct sockaddr_in iaddr, uaddr; +static pthread_t thrd; +static void *connect_b (void *);static void setup (void ) {
@@ -73,13 +75,20 @@ static void setup(void)SAFE_FILE_PRINTF (PING_SYSCTL_PATH ,"0 0" ); sockfd =SAFE_SOCKET (AF_INET ,SOCK_DGRAM ,IPPROTO_ICMP ); +SAFE_PTHREAD_CREATE (&thrd,0 , connect_b,0 );tst_res (TINFO ,"Created ping socket, attempting to race..." ); }static void cleanup (void ) { +if (thrd) { +tst_fzsync_pair_exit (&fzsync_pair); +SAFE_PTHREAD_JOIN (thrd,NULL ); + } +if (sockfd >0 )SAFE_CLOSE (sockfd); +if (ping_min_grp | ping_max_grp)SAFE_FILE_PRINTF (PING_SYSCTL_PATH ,"%u %u" , ping_min_grp, ping_max_grp);
@@ -87,32 +96,31 @@ static void cleanup(void) static void *connect_b(void * param LTP_ATTRIBUTE_UNUSED) { -tst_fzsync_delay_b (&fzsync_pair); -connect (sockfd, (struct sockaddr *)&uaddr,sizeof (uaddr)); -tst_fzsync_time_b (&fzsync_pair); +while (tst_fzsync_wait_update_b (&fzsync_pair)) { +tst_fzsync_delay_b (&fzsync_pair); +connect (sockfd, (struct sockaddr *)&uaddr,sizeof (uaddr)); +tst_fzsync_time_b (&fzsync_pair); +if (!tst_fzsync_wait_b (&fzsync_pair)) +break ; + }return 0 ; }static void run (void ) { -pthread_t thrd;int i;for (i =0 ; i <ATTEMPTS ; i++) {SAFE_CONNECT (sockfd, (struct sockaddr *)&iaddr,sizeof (iaddr)); -SAFE_PTHREAD_CREATE (&thrd,0 , connect_b,0 ); +tst_fzsync_wait_update_a (&fzsync_pair);tst_fzsync_delay_a (&fzsync_pair);connect (sockfd, (struct sockaddr *)&uaddr,sizeof (uaddr));tst_fzsync_time_a (&fzsync_pair); -SAFE_PTHREAD_JOIN (thrd,0 ); -tst_fzsync_pair_update (i, &fzsync_pair); - -if (!(i &0x7FFF )) -tst_fzsync_pair_info (&fzsync_pair); +tst_fzsync_wait_a (&fzsync_pair); }tst_res (TPASS ,"We didn't crash" );
Signature · 1 line
2.14.1
PATCH v3 1/7 tst_atomic: Add load, store and use __atomic builtins
In reply to Richard PalethorpeHi!
Quoted 7 lines
include/tst_atomic.h +7
-0+static inline int tst_atomic_load (int *v) +{ +int ret; + + asmvolatile ("" : : :"memory" ); + ret = *v; + asmvolatile ("" : : :"memory" );
include/tst_atomic.h static inline int tst_atomic_add_return(int i, int *v) · line 135
Quoted 10 lines
include/tst_atomic.h +10
-0+ +return ret; +} + +static inline void tst_atomic_store (int i,int *v) +{ + asmvolatile ("" : : :"memory" ); + *v = i; + asmvolatile ("" : : :"memory" ); +}
These two functions are defined several times here, maybe we should just define something as NEEDS_GENERIC_ASM_LOAD_STORE and do
#ifdef NEEDS_GENERIC_ASM_LOAD_STORE
...
#endif
Where these are defined at the end of the file.
Quoted 193 lines
#elif
defined(__powerpc__)||defined(__powerpc64__)static inline inttst_atomic_add_return(int i, int *v){include/tst_atomic.h +140
-4@@ -83,7 +154,26 @@ static inline int tst_atomic_add_return(int i, int *v)return t; } +static inline int tst_atomic_load (int *v) +{ +int ret; + + asmvolatile ("sync\n" : : :"memory" ); + ret = *v; + asmvolatile ("sync\n" : : :"memory" ); + +return ret; +} + +static inline void tst_atomic_store (int i,int *v) +{ + asmvolatile ("sync\n" : : :"memory" ); + *v = i; + asmvolatile ("sync\n" : : :"memory" ); +} +#elif defined(__s390__) || defined(__s390x__) +static inline int tst_atomic_add_return (int i,int *v) {int old_val, new_val;@@ -102,11 +192,29 @@ static inline int tst_atomic_add_return(int i, int *v) return old_val + i; } +static inline int tst_atomic_load (int *v) +{ +int ret; + + asmvolatile ("" : : :"memory" ); + ret = *v; + asmvolatile ("" : : :"memory" ); + +return ret; +} + +static inline void tst_atomic_store (int i,int *v) +{ + asmvolatile ("" : : :"memory" ); + *v = i; + asmvolatile ("" : : :"memory" ); +} +#elif defined(__arc__)/*ARCv2 defines the smp barriers */ #ifdef __ARC700__ -#define smp_mb () +#define smp_mb () asm volatile("" : : : "memory")#else #define smp_mb () asm volatile("dmb 3\n" : : : "memory")#endif @@ -132,6 +240,24 @@ static inline int tst_atomic_add_return(int i, int *v) return val; } +static inline int tst_atomic_load (int *v) +{ +int ret; + +smp_mb (); + ret = *v; +smp_mb (); + +return ret; +} + +static inline void tst_atomic_store (int i,int *v) +{ +smp_mb (); + *v = i; +smp_mb (); +} +#elif defined (__aarch64__)static inline int tst_atomic_add_return (int i,int *v) {@@ -140,7 +266,7 @@ static inline int tst_atomic_add_return(int i, int *v) __asm__ __volatile__(" prfm pstl1strm, %2 \n" -"1: ldxr %w0, %2 \n" +"1: ldaxr %w0, %2 \n" " add %w0, %w0, %w3 \n" " stlxr %w1, %w0, %2 \n" " cbnz %w1, 1b \n"@@ -152,9 +278,90 @@ static inline int tst_atomic_add_return(int i, int *v) return result; } +/* We are using load and store exclusive (ldaxr & stlxr) instructions to try +* and help prevent the tst_atomic_load and, more likely, tst_atomic_store +* functions from interfering with tst_atomic_add_return which takes advantage +* of exclusivity. It is not clear if this is a good idea or not, but does +* mean that all three functions are very similar. +*/ +static inline int tst_atomic_load (int *v) +{ +int ret; +unsigned long tmp; + + asmvolatile ("//atomic_load \n" +" prfm pstl1strm, %[v] \n" +"1: ldaxr %w[ret], %[v] \n" +" stlxr %w[tmp], %w[ret], %[v] \n" +" cbnz %w[tmp], 1b \n" +" dmb ish \n" + : [tmp]"=&r" (tmp), [ret]"=&r" (ret), [v]"+Q" (*v) + : :"memory" ); + +return ret; +} + +static inline void tst_atomic_store (int i,int *v) +{ +unsigned long tmp; + + asmvolatile ("//atomic_store \n" +" prfm pstl1strm, %[v] \n" +"1: ldaxr %w[tmp], %[v] \n" +" stlxr %w[tmp], %w[i], %[v] \n" +" cbnz %w[tmp], 1b \n" +" dmb ish \n" + : [tmp]"=&r" (tmp), [v]"+Q" (*v) + : [i]"r" (i) + :"memory" ); +} + +#elif defined(__sparc__) && defined(__arch64__) +static inline int tst_atomic_add_return (int i,int *v) +{ +int ret, tmp; + +/* Based on arch/sparc/lib/atomic_64.S with the exponential backoff +* function removed because we are unlikely to have a large (>= 16?) +* number of cores continuously trying to update one variable. +*/ + asmvolatile ("/*atomic_add_return*/ \n" +"1: ldsw [%[v]], %[ret]; \n" +" add %[ret], %[i], %[tmp]; \n" +" cas [%[v]], %[ret], %[tmp]; \n" +" cmp %[ret], %[tmp]; \n" +" bne,pn %%icc, 1b; \n" +" nop; \n" +" add %[ret], %[i], %[ret]; \n" + : [ret]"=r&" (ret), [tmp]"=r&" (tmp) + : [i]"r" (i), [v]"r" (v) + :"memory" ,"cc" ); + +return ret; +} + +static inline int tst_atomic_load (int *v) +{ +int ret; + +/* See arch/sparc/include/asm/barrier_64.h */ + asmvolatile ("" : : :"memory" ); + ret = *v; + asmvolatile ("" : : :"memory" ); + +return ret; +} + +static inline void tst_atomic_store (int i,int *v) +{ + asmvolatile ("" : : :"memory" ); + *v = i; + asmvolatile ("" : : :"memory" ); +} +#else /* HAVE_SYNC_ADD_AND_FETCH == 1 */ -# error Your compiler does not provide __sync_add_and_fetch and LTP\ - implementation is missing for your architecture. +# error Your compiler does not provide __atomic_add_fetch, __sync_add_and_fetch \ + and an LTP implementation is missing for your architecture.#endif static inline int tst_atomic_inc (int *v)include/tst_fuzzy_sync.h +0
-0
include/tst_fuzzy_sync.h line 35
Quoted 4 lines
include/tst_fuzzy_sync.h +1
-0@@ -32,6 +32,7 @@#include <sys/time.h> #include <time.h> +#include "tst_atomic.h"
Hmm, isn't this added out-of-order here?
Quoted 20 lines
include/tst_fuzzy_sync.h +0
-0#ifndef CLOCK_MONOTONIC_RAW # define CLOCK_MONOTONIC_RAW CLOCK_MONOTONICm4/ltp-atomic.m4 new file +12
-0@@ -0,0 +1,34 @@ +dnl +dnl Copyright (c) Linux Test Project, 2016 +dnl +dnl This program is free software; you can redistribute it and/or modify +dnl it under the terms of the GNU General Public License as published by +dnl the Free Software Foundation; either version 2 of the License, or +dnl (at your option) any later version. +dnl +dnl This program is distributed in the hope that it will be useful, +dnl but WITHOUT ANY WARRANTY; without even the implied warranty of +dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See +dnl the GNU General Public License for more details.
m4/ltp-atomic.m4 line 22
Quoted 10 lines
m4/ltp-atomic.m4 new file +10
-0+dnl + +AC_DEFUN([LTP_CHECK_ATOMIC_MEMORY_MODEL],[dnl + AC_MSG_CHECKING([for __atomic_* compiler builtins]) + AC_LINK_IFELSE([AC_LANG_SOURCE([ +int main(void) { + int i = 0, j = 0; + __atomic_add_fetch(&i, 1, __ATOMIC_ACQ_REL); + __atomic_load_n(&i, __ATOMIC_SEQ_CST); + __atomic_compare_exchange_n(&i, &j, 0, 0, __ATOMIC_RELEASE, __ATOMIC_ACQUIRE);
We can drop the exchange function here now.
Quoted 18 lines
m4/ltp-atomic.m4 new file +12
-0+ __atomic_store_n(&i, 0, __ATOMIC_RELAXED); + return i; +}])],[has_atomic_mm="yes"]) + +if test "x$has_atomic_mm" = xyes; then + AC_DEFINE(HAVE_ATOMIC_MEMORY_MODEL,1, + [Define to 1 if you have the __atomic_* compiler builtins]) + AC_MSG_RESULT(yes) +else + AC_MSG_RESULT(no) +fi +])Signature · 5 lines
2.14.1 -- Mailing list info: https://lists.linux.it/listinfo/ltp
Signature · 2 lines
Cyril Hrubis [email protected]