To keep it simple:

1)     Merge with spelling fixed: “requensting” => “requesting”

2)     Send another patch that enables odp_timeout_user_ptr() test and adds 
odp_timeout_timer() test.

-Petri



From: ext Ola Liljedahl [mailto:ola.liljed...@linaro.org]
Sent: Thursday, March 12, 2015 2:58 PM
To: Maxim Uvarov
Cc: Savolainen, Petri (Nokia - FI/Espoo); LNG ODP Mailman List
Subject: Re: [lng-odp] [PATCH 2/2] validation: timer: added cancel test

On 12 March 2015 at 12:21, Maxim Uvarov 
<maxim.uva...@linaro.org<mailto:maxim.uva...@linaro.org>> wrote:
Ola, do you want to add your review-by to these patches?
I think the patch should be reposted (by someone) with the user_ptr test 
enabled and the spelling error corrected. I could do this but then someone else 
needs to review it...

-- Ola



Maxim.


On 03/12/15 13:26, Savolainen, Petri (Nokia - FI/Espoo) wrote:
Ping. I think it's time to merge these two patches. This sets the stage for 
testing odp_timeout_xxx() after timer cancel.

-Petri
-----Original Message-----
From: ext Ola Liljedahl 
[mailto:ola.liljed...@linaro.org<mailto:ola.liljed...@linaro.org>]
Sent: Tuesday, March 10, 2015 4:11 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 10 March 2015 at 10:10, Savolainen, Petri (Nokia - FI/Espoo)
<petri.savolai...@nokia.com<mailto:petri.savolai...@nokia.com>> wrote:

-----Original Message-----
From: ext Ola Liljedahl 
[mailto:ola.liljed...@linaro.org<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<mailto: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<mailto: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,
&params);
+
+       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<mailto:lng-odp@lists.linaro.org>
http://lists.linaro.org/mailman/listinfo/lng-odp


_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org<mailto: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

Reply via email to