| Message ID | 1452849247-32266-2-git-send-email-maxim.uvarov@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 15.01.16 11:14, Maxim Uvarov wrote: > Add test case based on previous seg fault code that pool destroy > does not fail if there are some timers in fight. > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> > --- > platform/linux-generic/odp_timer.c | 15 ++++++++++++++- > test/validation/timer/timer.c | 15 +++++++++++++++ > test/validation/timer/timer.h | 1 + > 3 files changed, 30 insertions(+), 1 deletion(-) > > diff --git a/platform/linux-generic/odp_timer.c b/platform/linux-generic/odp_timer.c > index b8f34fb..95fe130 100644 > --- a/platform/linux-generic/odp_timer.c > +++ b/platform/linux-generic/odp_timer.c > @@ -259,6 +259,16 @@ static odp_timer_pool *odp_timer_pool_new( > return tp; > } > > +static void _finalyze_timers_handlers(void) > +{ > + struct timespec ts; > + > + ts.tv_sec = 0; > + ts.tv_nsec = 50 * ODP_TIME_MSEC_IN_NS; > + if (nanosleep(&ts, NULL) < 0) > + ODP_ABORT("nanosleep failed"); > +} > + For linux generic maybe that's OK, but why not odp_time API > static void odp_timer_pool_del(odp_timer_pool *tp) > { > odp_spinlock_lock(&tp->lock); > @@ -270,8 +280,11 @@ static void odp_timer_pool_del(odp_timer_pool *tp) > /* timer pool which is still in use */ > ODP_ABORT("%s: timers in use\n", tp->name); > } > - if (tp->param.clk_src == ODP_CLOCK_CPU) > + if (tp->param.clk_src == ODP_CLOCK_CPU) { > itimer_fini(tp); > + _finalyze_timers_handlers(); > + } > + Why are you sure that it's still receiving events after finish. Did you catch it? > int rc = odp_shm_free(tp->shm); > if (rc != 0) > ODP_ABORT("Failed to free shared memory (%d)\n", rc); > diff --git a/test/validation/timer/timer.c b/test/validation/timer/timer.c > index db0a5ca..879ff67 100644 > --- a/test/validation/timer/timer.c > +++ b/test/validation/timer/timer.c > @@ -22,6 +22,9 @@ > /** @private Number of timers per thread */ > #define NTIMERS 2000 > > +/** @private Dont wait for timers completition */ > +static int dont_wait_for_timer_comp; > + > /** @private Barrier for thread synchronisation */ > static odp_barrier_t test_barrier; > > @@ -277,6 +280,9 @@ static void _wait_for_timers_expiration(void) > { > struct timespec ts; > > + if (dont_wait_for_timer_comp) > + return; > + Pay attention, you've skipped not only "wait for timers expiration". you also deleted delay in polling loop, decreased in the same time timeout for receiving events, as result at the end of the test the number of timeouts in the flight will be much more. That will increase contrast of fail/true tests...presented here. > ts.tv_sec = 0; > ts.tv_nsec = 50 * ODP_TIME_MSEC_IN_NS; > if (nanosleep(&ts, NULL) < 0) > @@ -539,11 +545,20 @@ void timer_test_odp_timer_all(void) > CU_PASS("ODP timer test"); > } > > +/* @private Timer test case entrypoint */ > +void timer_test_destroy_pool_with_scheduled_timers(void) > +{ > + dont_wait_for_timer_comp = 1; > + timer_test_odp_timer_all(); > + dont_wait_for_timer_comp = 0; > +} > + > odp_testinfo_t timer_suite[] = { > ODP_TEST_INFO(timer_test_timeout_pool_alloc), > ODP_TEST_INFO(timer_test_timeout_pool_free), > ODP_TEST_INFO(timer_test_odp_timer_cancel), > ODP_TEST_INFO(timer_test_odp_timer_all), > + ODP_TEST_INFO(timer_test_destroy_pool_with_scheduled_timers), Is it supposed to be before timer_test_odp_timer_all()? Or test can continue after seg fault? :-|. IMHO, no need in it. > ODP_TEST_INFO_NULL, > }; > > diff --git a/test/validation/timer/timer.h b/test/validation/timer/timer.h > index 46ea8d7..4f805b4 100644 > --- a/test/validation/timer/timer.h > +++ b/test/validation/timer/timer.h > @@ -14,6 +14,7 @@ void timer_test_timeout_pool_alloc(void); > void timer_test_timeout_pool_free(void); > void timer_test_odp_timer_cancel(void); > void timer_test_odp_timer_all(void); > +void timer_test_destroy_pool_with_scheduled_timers(void); > > /* test arrays: */ > extern odp_testinfo_t timer_suite[]; >
On 01/19/2016 02:15, Ivan Khoronzhuk wrote: > > > On 15.01.16 11:14, Maxim Uvarov wrote: >> Add test case based on previous seg fault code that pool destroy >> does not fail if there are some timers in fight. >> >> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> >> --- >> platform/linux-generic/odp_timer.c | 15 ++++++++++++++- >> test/validation/timer/timer.c | 15 +++++++++++++++ >> test/validation/timer/timer.h | 1 + >> 3 files changed, 30 insertions(+), 1 deletion(-) >> >> diff --git a/platform/linux-generic/odp_timer.c >> b/platform/linux-generic/odp_timer.c >> index b8f34fb..95fe130 100644 >> --- a/platform/linux-generic/odp_timer.c >> +++ b/platform/linux-generic/odp_timer.c >> @@ -259,6 +259,16 @@ static odp_timer_pool *odp_timer_pool_new( >> return tp; >> } >> >> +static void _finalyze_timers_handlers(void) >> +{ >> + struct timespec ts; >> + >> + ts.tv_sec = 0; >> + ts.tv_nsec = 50 * ODP_TIME_MSEC_IN_NS; >> + if (nanosleep(&ts, NULL) < 0) >> + ODP_ABORT("nanosleep failed"); >> +} >> + > For linux generic maybe that's OK, but why not odp_time API > >> static void odp_timer_pool_del(odp_timer_pool *tp) >> { >> odp_spinlock_lock(&tp->lock); >> @@ -270,8 +280,11 @@ static void odp_timer_pool_del(odp_timer_pool *tp) >> /* timer pool which is still in use */ >> ODP_ABORT("%s: timers in use\n", tp->name); >> } >> - if (tp->param.clk_src == ODP_CLOCK_CPU) >> + if (tp->param.clk_src == ODP_CLOCK_CPU) { >> itimer_fini(tp); >> + _finalyze_timers_handlers(); >> + } >> + > Why are you sure that it's still receiving events after finish. > Did you catch it? > >> int rc = odp_shm_free(tp->shm); >> if (rc != 0) >> ODP_ABORT("Failed to free shared memory (%d)\n", rc); >> diff --git a/test/validation/timer/timer.c >> b/test/validation/timer/timer.c >> index db0a5ca..879ff67 100644 >> --- a/test/validation/timer/timer.c >> +++ b/test/validation/timer/timer.c >> @@ -22,6 +22,9 @@ >> /** @private Number of timers per thread */ >> #define NTIMERS 2000 >> >> +/** @private Dont wait for timers completition */ >> +static int dont_wait_for_timer_comp; >> + >> /** @private Barrier for thread synchronisation */ >> static odp_barrier_t test_barrier; >> >> @@ -277,6 +280,9 @@ static void _wait_for_timers_expiration(void) >> { >> struct timespec ts; >> >> + if (dont_wait_for_timer_comp) >> + return; >> + > Pay attention, you've skipped not only "wait for timers expiration". > you also deleted delay in polling loop, decreased in the same time > timeout for receiving events, as result at the end of the test the > number of timeouts in the flight will be much more. That will increase > contrast of fail/true tests...presented here. that is what this test does. I.e. it test that pool destroy also take care about bunch of in flight handler. I.e. odp_abort() was not called and timer was cancelled, and handlers execution was cancelled also. > >> ts.tv_sec = 0; >> ts.tv_nsec = 50 * ODP_TIME_MSEC_IN_NS; >> if (nanosleep(&ts, NULL) < 0) >> @@ -539,11 +545,20 @@ void timer_test_odp_timer_all(void) >> CU_PASS("ODP timer test"); >> } >> >> +/* @private Timer test case entrypoint */ >> +void timer_test_destroy_pool_with_scheduled_timers(void) >> +{ >> + dont_wait_for_timer_comp = 1; >> + timer_test_odp_timer_all(); >> + dont_wait_for_timer_comp = 0; >> +} >> + >> odp_testinfo_t timer_suite[] = { >> ODP_TEST_INFO(timer_test_timeout_pool_alloc), >> ODP_TEST_INFO(timer_test_timeout_pool_free), >> ODP_TEST_INFO(timer_test_odp_timer_cancel), >> ODP_TEST_INFO(timer_test_odp_timer_all), >> + ODP_TEST_INFO(timer_test_destroy_pool_with_scheduled_timers), > Is it supposed to be before timer_test_odp_timer_all()? > Or test can continue after seg fault? :-|. > IMHO, no need in it. You should not see segfault now after these 2 patches. > >> ODP_TEST_INFO_NULL, >> }; >> >> diff --git a/test/validation/timer/timer.h >> b/test/validation/timer/timer.h >> index 46ea8d7..4f805b4 100644 >> --- a/test/validation/timer/timer.h >> +++ b/test/validation/timer/timer.h >> @@ -14,6 +14,7 @@ void timer_test_timeout_pool_alloc(void); >> void timer_test_timeout_pool_free(void); >> void timer_test_odp_timer_cancel(void); >> void timer_test_odp_timer_all(void); >> +void timer_test_destroy_pool_with_scheduled_timers(void); >> >> /* test arrays: */ >> extern odp_testinfo_t timer_suite[]; >> >
diff --git a/platform/linux-generic/odp_timer.c b/platform/linux-generic/odp_timer.c index b8f34fb..95fe130 100644 --- a/platform/linux-generic/odp_timer.c +++ b/platform/linux-generic/odp_timer.c @@ -259,6 +259,16 @@ static odp_timer_pool *odp_timer_pool_new( return tp; } +static void _finalyze_timers_handlers(void) +{ + struct timespec ts; + + ts.tv_sec = 0; + ts.tv_nsec = 50 * ODP_TIME_MSEC_IN_NS; + if (nanosleep(&ts, NULL) < 0) + ODP_ABORT("nanosleep failed"); +} + static void odp_timer_pool_del(odp_timer_pool *tp) { odp_spinlock_lock(&tp->lock); @@ -270,8 +280,11 @@ static void odp_timer_pool_del(odp_timer_pool *tp) /* timer pool which is still in use */ ODP_ABORT("%s: timers in use\n", tp->name); } - if (tp->param.clk_src == ODP_CLOCK_CPU) + if (tp->param.clk_src == ODP_CLOCK_CPU) { itimer_fini(tp); + _finalyze_timers_handlers(); + } + int rc = odp_shm_free(tp->shm); if (rc != 0) ODP_ABORT("Failed to free shared memory (%d)\n", rc); diff --git a/test/validation/timer/timer.c b/test/validation/timer/timer.c index db0a5ca..879ff67 100644 --- a/test/validation/timer/timer.c +++ b/test/validation/timer/timer.c @@ -22,6 +22,9 @@ /** @private Number of timers per thread */ #define NTIMERS 2000 +/** @private Dont wait for timers completition */ +static int dont_wait_for_timer_comp; + /** @private Barrier for thread synchronisation */ static odp_barrier_t test_barrier; @@ -277,6 +280,9 @@ static void _wait_for_timers_expiration(void) { struct timespec ts; + if (dont_wait_for_timer_comp) + return; + ts.tv_sec = 0; ts.tv_nsec = 50 * ODP_TIME_MSEC_IN_NS; if (nanosleep(&ts, NULL) < 0) @@ -539,11 +545,20 @@ void timer_test_odp_timer_all(void) CU_PASS("ODP timer test"); } +/* @private Timer test case entrypoint */ +void timer_test_destroy_pool_with_scheduled_timers(void) +{ + dont_wait_for_timer_comp = 1; + timer_test_odp_timer_all(); + dont_wait_for_timer_comp = 0; +} + odp_testinfo_t timer_suite[] = { ODP_TEST_INFO(timer_test_timeout_pool_alloc), ODP_TEST_INFO(timer_test_timeout_pool_free), ODP_TEST_INFO(timer_test_odp_timer_cancel), ODP_TEST_INFO(timer_test_odp_timer_all), + ODP_TEST_INFO(timer_test_destroy_pool_with_scheduled_timers), ODP_TEST_INFO_NULL, }; diff --git a/test/validation/timer/timer.h b/test/validation/timer/timer.h index 46ea8d7..4f805b4 100644 --- a/test/validation/timer/timer.h +++ b/test/validation/timer/timer.h @@ -14,6 +14,7 @@ void timer_test_timeout_pool_alloc(void); void timer_test_timeout_pool_free(void); void timer_test_odp_timer_cancel(void); void timer_test_odp_timer_all(void); +void timer_test_destroy_pool_with_scheduled_timers(void); /* test arrays: */ extern odp_testinfo_t timer_suite[];
Add test case based on previous seg fault code that pool destroy does not fail if there are some timers in fight. Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> --- platform/linux-generic/odp_timer.c | 15 ++++++++++++++- test/validation/timer/timer.c | 15 +++++++++++++++ test/validation/timer/timer.h | 1 + 3 files changed, 30 insertions(+), 1 deletion(-)