On 10 March 2015 at 10:10, Savolainen, Petri (Nokia - FI/Espoo)
<petri.savolai...@nokia.com> wrote:
>
>
>> -----Original Message-----
>> From: ext Ola Liljedahl [mailto:ola.liljed...@linaro.org]
>> Sent: Monday, March 09, 2015 4:44 PM
>> To: Savolainen, Petri (Nokia - FI/Espoo)
>> Cc: LNG ODP Mailman List
>> Subject: Re: [lng-odp] [PATCH 2/2] validation: timer: added cancel test
>>
>> On 6 March 2015 at 15:30, Petri Savolainen <petri.savolai...@nokia.com>
>> wrote:
>> > Added timeout cancel test that simply creates everything
>> > needed for requensting
>> Spelling error.
>
> Maybe Maxim can correct this during merge.
>
>>
>> > one timeout and then cancels that
>> > before it expires.
>> >
>> > The timeout user_ptr bug can be reproduced with this by
>> > uncommenting the user_ptr check.
>> >
>> > Signed-off-by: Petri Savolainen <petri.savolai...@nokia.com>
>> > ---
>> > test/validation/odp_timer.c | 83
>> +++++++++++++++++++++++++++++++++++++++++++++
>> > 1 file changed, 83 insertions(+)
>> >
>> > diff --git a/test/validation/odp_timer.c b/test/validation/odp_timer.c
>> > index 2ef3b39..1def85b 100644
>> > --- a/test/validation/odp_timer.c
>> > +++ b/test/validation/odp_timer.c
>> > @@ -124,6 +124,88 @@ static void test_timeout_pool_free(void)
>> > CU_ASSERT(odp_pool_destroy(pool) == 0);
>> > }
>> >
>> > +static void test_odp_timer_cancel(void)
>> > +{
>> > + odp_pool_t pool;
>> > + odp_pool_param_t params;
>> > + odp_timer_pool_param_t tparam;
>> > + odp_timer_pool_t tp;
>> > + odp_queue_t queue;
>> > + odp_timer_t tim;
>> > + odp_event_t ev;
>> > + odp_timeout_t tmo;
>> > + odp_timer_set_t rc;
>> > + uint64_t tick;
>> > +
>> > + params.tmo.num = 1;
>> > + params.type = ODP_POOL_TIMEOUT;
>> > + pool = odp_pool_create("tmo_pool_for_cancel", ODP_SHM_NULL,
>> ¶ms);
>> > +
>> > + if (pool == ODP_POOL_INVALID)
>> > + CU_FAIL_FATAL("Timeout pool create failed");
>> > +
>> > + tparam.res_ns = 100 * ODP_TIME_MSEC;
>> > + tparam.min_tmo = 1 * ODP_TIME_SEC;
>> > + tparam.max_tmo = 10 * ODP_TIME_SEC;
>> > + tparam.num_timers = 1;
>> > + tparam.priv = 0;
>> > + tparam.clk_src = ODP_CLOCK_CPU;
>> > + tp = odp_timer_pool_create("timer_pool0", &tparam);
>> > + if (tp == ODP_TIMER_POOL_INVALID)
>> > + CU_FAIL_FATAL("Timer pool create failed");
>> > +
>> > + /* Start all created timer pools */
>> > + odp_timer_pool_start();
>> > +
>> > + queue = odp_queue_create("timer_queue", ODP_QUEUE_TYPE_POLL,
>> NULL);
>> > + if (queue == ODP_QUEUE_INVALID)
>> > + CU_FAIL_FATAL("Queue create failed");
>> > +
>> > + #define USER_PTR ((void *)0xdead)
>> > + tim = odp_timer_alloc(tp, queue, USER_PTR);
>> > + if (tim == ODP_TIMER_INVALID)
>> > + CU_FAIL_FATAL("Failed to allocate timer");
>> > +
>> > + ev = odp_timeout_to_event(odp_timeout_alloc(pool));
>> > + if (ev == ODP_EVENT_INVALID)
>> > + CU_FAIL_FATAL("Failed to allocate timeout");
>> > +
>> > + tick = odp_timer_ns_to_tick(tp, 2 * ODP_TIME_SEC);
>> > +
>> > + rc = odp_timer_set_rel(tim, tick, &ev);
>> > + if (rc != ODP_TIMER_SUCCESS)
>> > + CU_FAIL_FATAL("Failed to set timer (relative time)");
>> > +
>> > + ev = ODP_EVENT_INVALID;
>> > + if (odp_timer_cancel(tim, &ev) != 0)
>> > + CU_FAIL_FATAL("Failed to cancel timer (relative time)");
>> > +
>> > + if (ev == ODP_EVENT_INVALID)
>> > + CU_FAIL_FATAL("Cancel did not return event");
>> > +
>> > + tmo = odp_timeout_from_event(ev);
>> > + if (tmo == ODP_TIMEOUT_INVALID)
>> > + CU_FAIL_FATAL("Cancel returned invalid tmo");
>> No need for fatal error? CU_FAIL should be enough.
>
> It's fatal for this test if something else than tmo is coming out from the
> cancel.
True.
>
>
>>
>> > +
>> > +/* Uncomment after user_ptr bug have been corrected
>> > + if (odp_timeout_user_ptr(tmo) != USER_PTR)
>> > + CU_FAIL_FATAL("Cancel corrupted user_ptr");
>> CU_FAIL again.
>
> True. Test can continue to next timeout metadata tests, but it in comment now
> and need to updated (uncommented) any way after implementation fix.
>
>>
>> Tests for odp_timeout_timer() and odp_timeout_tick()?
>
> Yes. Those tests should be added after implementation is fixed.
Actually I think it is a bad idea to check that the timeout expiration
tick is always the latest set tick. That would require the
implementation to update this field *every* time a timer is reset,
forcing an access to a data structure that otherwise would not be
accessed. The expiration tick is set when the timer expires and the
timeout is enqueued.
The timeout timer and user_ptr fields should be updated, this only has
to be done once, when a timeout event is specified in one of the
odp_timer_set_xxx calls.
The expiration should not be of the same importance as e.g. the user pointer.
>
> I suggest to merge this patch as is now, fix the implementation and then
> fix/add these tests.
>
> -Petri
>
>>
>> > +*/
>> > + odp_timeout_free(tmo);
>> > +
>> > + ev = odp_timer_free(tim);
>> > + if (ev != ODP_EVENT_INVALID)
>> > + CU_FAIL_FATAL("Free returned event");
>> > +
>> > + odp_timer_pool_destroy(tp);
>> > +
>> > + if (odp_queue_destroy(queue) != 0)
>> > + CU_FAIL_FATAL("Failed to destroy queue");
>> > +
>> > + if (odp_pool_destroy(pool) != 0)
>> > + CU_FAIL_FATAL("Failed to destroy pool");
>> > +}
>> > +
>> > /* @private Handle a received (timeout) event */
>> > static void handle_tmo(odp_event_t ev, bool stale, uint64_t prev_tick)
>> > {
>> > @@ -432,6 +514,7 @@ static void test_odp_timer_all(void)
>> > CU_TestInfo test_odp_timer[] = {
>> > {"test_timeout_pool_alloc", test_timeout_pool_alloc},
>> > {"test_timeout_pool_free", test_timeout_pool_free},
>> > + {"test_odp_timer_cancel", test_odp_timer_cancel},
>> > {"test_odp_timer_all", test_odp_timer_all},
>> > CU_TEST_INFO_NULL,
>> > };
>> > --
>> > 2.3.1
>> >
>> >
>> > _______________________________________________
>> > lng-odp mailing list
>> > lng-odp@lists.linaro.org
>> > http://lists.linaro.org/mailman/listinfo/lng-odp
_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/lng-odp
