| Message ID | 1461245551-9954-1-git-send-email-christophe.milard@linaro.org |
|---|---|
| State | Accepted |
| Commit | 19180324c00fd94417603b9c8d96002f748bb843 |
| Headers | show |
ping On 21 April 2016 at 15:32, Christophe Milard <christophe.milard@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.milard@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 > >
On Thu, Apr 21, 2016 at 8:32 AM, Christophe Milard < christophe.milard@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.milard@linaro.org> > Reviewed-and-tested-by: Bill Fischofer <bill.fischofer@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 > >
merged On 7 May 2016 at 19:16, Bill Fischofer <bill.fischofer@linaro.org> wrote: > > > On Thu, Apr 21, 2016 at 8:32 AM, Christophe Milard < > christophe.milard@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.milard@linaro.org> >> > > Reviewed-and-tested-by: Bill Fischofer <bill.fischofer@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 >> >> > -- Mike Holmes Technical Manager - Linaro Networking Group Linaro.org <http://www.linaro.org/> *│ *Open source software for ARM SoCs "Work should be fun and collaborative, the rest follows"
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,
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.milard@linaro.org> --- test/validation/lock/lock.c | 71 ++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 64 insertions(+), 7 deletions(-)