ping On 21 April 2016 at 15:32, Christophe Milard <christophe.mil...@linaro.org> wrote:
> fixing: https://bugs.linaro.org/show_bug.cgi?id=2108 > > The no_lock_functional_test does not really tests the ODP functionality: > Instead, it actually checks that race conditions can be created between > concurrent running threads (by making these threads writing shared > variables without lock, and later noting the value written was changed by > some of the concurrent threads). > This test therefore validates other tests: if this test passes -i.e. if > we do have race condition- then if the following tests suppress these > race conditions by using some synchronization mechanism, these > synchronization mechanisms can be said to be efficient. > If, on the other hand, the no_lock_functional_test "fails", it says > that the following tests are really inconclusive as the effect of the > tested synchronization mechanism is not proven. > > When running with valgrind, no_lock_functional_test failed, probably > because the extra execution time introduced by valgrind itself made > the chance to run the critical section of the different threads "at > the same time" much less probable. > > The simple solution is to increase the critical section running time > (by largely increasing the number of iterations performed). > The solution taken here is actually to tune the critical section running > time (currentely to ITER_MPLY_FACTOR=3 times the time needed to note > the first race condition). > This means that the test will take longer to run with valgrind, > but will remain short without valgrind. > > Signed-off-by: Christophe Milard <christophe.mil...@linaro.org> > --- > test/validation/lock/lock.c | 71 > ++++++++++++++++++++++++++++++++++++++++----- > 1 file changed, 64 insertions(+), 7 deletions(-) > > diff --git a/test/validation/lock/lock.c b/test/validation/lock/lock.c > index f1f6d69..515bc77 100644 > --- a/test/validation/lock/lock.c > +++ b/test/validation/lock/lock.c > @@ -12,7 +12,10 @@ > #include "lock.h" > > #define VERBOSE 0 > -#define MAX_ITERATIONS 1000 > + > +#define MIN_ITERATIONS 1000 > +#define MAX_ITERATIONS 30000 > +#define ITER_MPLY_FACTOR 3 > > #define SLOW_BARRIER_DELAY 400 > #define BASE_DELAY 6 > @@ -325,6 +328,12 @@ static void *rwlock_recursive_api_tests(void *arg > UNUSED) > return NULL; > } > > +/* > + * Tests that we do have contention between threads when running. > + * Also adjust the number of iterations to be done (by other tests) > + * so we have a fair chance to see that the tested synchronizer > + * does avoid the race condition. > + */ > static void *no_lock_functional_test(void *arg UNUSED) > { > global_shared_mem_t *global_mem; > @@ -335,17 +344,36 @@ static void *no_lock_functional_test(void *arg > UNUSED) > thread_num = odp_cpu_id() + 1; > per_thread_mem = thread_init(); > global_mem = per_thread_mem->global_mem; > - iterations = global_mem->g_iterations; > + iterations = 0; > > odp_barrier_wait(&global_mem->global_barrier); > > sync_failures = 0; > current_errs = 0; > rs_idx = 0; > - resync_cnt = iterations / NUM_RESYNC_BARRIERS; > + resync_cnt = MAX_ITERATIONS / NUM_RESYNC_BARRIERS; > lock_owner_delay = BASE_DELAY; > > - for (cnt = 1; cnt <= iterations; cnt++) { > + /* > + * Tunning the iteration number: > + * Here, we search for an iteration number that guarantees to show > + * race conditions between the odp threads. > + * Iterations is set to ITER_MPLY_FACTOR * cnt where cnt is when > + * the threads start to see "errors" (i.e. effect of other threads > + * running concurrentely without any synchronisation mechanism). > + * In other words, "iterations" is set to ITER_MPLY_FACTOR times the > + * minimum loop count necessary to see a need for synchronisation > + * mechanism. > + * If, later, these "errors" disappear when running other tests up > to > + * "iterations" with synchro, the effect of the tested synchro > mechanism > + * is likely proven. > + * If we reach "MAX_ITERATIONS", and "iteration" remains zero, > + * it means that we cannot see any race condition between the > different > + * running theads (e.g. the OS is not preemptive) and all other > tests > + * being passed won't tell much about the functionality of the > + * tested synchro mechanism. > + */ > + for (cnt = 1; cnt <= MAX_ITERATIONS; cnt++) { > global_mem->global_lock_owner = thread_num; > odp_mb_full(); > thread_delay(per_thread_mem, lock_owner_delay); > @@ -353,6 +381,8 @@ static void *no_lock_functional_test(void *arg UNUSED) > if (global_mem->global_lock_owner != thread_num) { > current_errs++; > sync_failures++; > + if (!iterations) > + iterations = cnt; > } > > global_mem->global_lock_owner = 0; > @@ -362,6 +392,8 @@ static void *no_lock_functional_test(void *arg UNUSED) > if (global_mem->global_lock_owner == thread_num) { > current_errs++; > sync_failures++; > + if (!iterations) > + iterations = cnt; > } > > if (current_errs == 0) > @@ -392,6 +424,31 @@ static void *no_lock_functional_test(void *arg UNUSED) > */ > CU_ASSERT(sync_failures != 0 || global_mem->g_num_threads == 1); > > + /* > + * set the iterration for the future tests to be far above the > + * contention level > + */ > + iterations *= ITER_MPLY_FACTOR; > + > + if (iterations > MAX_ITERATIONS) > + iterations = MAX_ITERATIONS; > + if (iterations < MIN_ITERATIONS) > + iterations = MIN_ITERATIONS; > + > + /* > + * Note that the following statement has race conditions: > + * global_mem->g_iterations should really be an atomic and a TAS > + * function be used. But this would mean that we would be testing > + * synchronisers assuming synchronisers works... > + * If we do not use atomic TAS, we may not get the grand max for > + * all threads, but we are guaranteed to have passed the error > + * threshold, for at least some threads, which is good enough > + */ > + if (iterations > global_mem->g_iterations) > + global_mem->g_iterations = iterations; > + > + odp_mb_full(); > + > thread_finalize(per_thread_mem); > > return NULL; > @@ -910,7 +967,7 @@ void lock_test_no_lock_functional(void) > } > > odp_testinfo_t lock_suite_no_locking[] = { > - ODP_TEST_INFO(lock_test_no_lock_functional), > + ODP_TEST_INFO(lock_test_no_lock_functional), /* must be first */ > ODP_TEST_INFO_NULL > }; > > @@ -1082,7 +1139,7 @@ int lock_init(void) > memset(global_mem, 0, sizeof(global_shared_mem_t)); > > global_mem->g_num_threads = MAX_WORKERS; > - global_mem->g_iterations = MAX_ITERATIONS; > + global_mem->g_iterations = 0; /* tuned by first test */ > global_mem->g_verbose = VERBOSE; > > workers_count = odp_cpumask_default_worker(&mask, 0); > @@ -1106,7 +1163,7 @@ int lock_init(void) > > odp_suiteinfo_t lock_suites[] = { > {"nolocking", lock_suite_init, NULL, > - lock_suite_no_locking}, > + lock_suite_no_locking}, /* must be first */ > {"spinlock", lock_suite_init, NULL, > lock_suite_spinlock}, > {"spinlock_recursive", lock_suite_init, NULL, > -- > 2.1.4 > >
_______________________________________________ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
