On 30 June 2015 at 12:16, Ivan Khoronzhuk <[email protected]>
wrote:

> It's more generic implementation simplify reusing timer API
> for other platforms.
>
> Signed-off-by: Ivan Khoronzhuk <[email protected]>
>
Reviewed-by: Ola Liljedahl <[email protected]>


> -


I got a (spurious) segmentation fault from the timer validation program but
this was after the actual timer tests had been performed. Running on a
quad-threaded Haswell Core i7 running Ubuntu 15.04.

odp_timer.c:484:test_odp_timer_all():#timers..: 2000
odp_timer.c:486:test_odp_timer_all():Tmo range: 2000 ms (600 ticks)
odp_timer.c:393:worker_entrypoint():Thread 1: 2512 timers set
odp_timer.c:394:worker_entrypoint():Thread 1: 442 timers reset
odp_timer.c:395:worker_entrypoint():Thread 1: 446 timers cancelled
odp_timer.c:397:worker_entrypoint():Thread 1: 0 timers reset/cancelled too
late
odp_timer.c:398:worker_entrypoint():Thread 1: 1359 timeouts received
odp_timer.c:400:worker_entrypoint():Thread 1: 0 stale timeout(s) after
odp_timer_free()
odp_timer.c:432:worker_entrypoint():Thread 1: exiting
odp_timer.c:393:worker_entrypoint():Thread 3: 2537 timers set
odp_timer.c:394:worker_entrypoint():Thread 3: 424 timers reset
odp_timer.c:395:worker_entrypoint():Thread 3: 439 timers cancelled
odp_timer.c:397:worker_entrypoint():Thread 3: 0 timers reset/cancelled too
late
odp_timer.c:398:worker_entrypoint():Thread 3: 1394 timeouts received
odp_timer.c:400:worker_entrypoint():Thread 3: 1 stale timeout(s) after
odp_timer_free()
odp_timer.c:393:worker_entrypoint():Thread 2: 2535 timers set
odp_timer.c:394:worker_entrypoint():Thread 2: 442 timers reset
odp_timer.c:395:worker_entrypoint():Thread 2: 423 timers cancelled
odp_timer.c:397:worker_entrypoint():Thread 2: 0 timers reset/cancelled too
late
odp_timer.c:398:worker_entrypoint():Thread 2: 1373 timeouts received
odp_timer.c:400:worker_entrypoint():Thread 2: 0 stale timeout(s) after
odp_timer_free()
odp_timer.c:432:worker_entrypoint():Thread 3: exiting
odp_timer.c:432:worker_entrypoint():Thread 2: exiting
odp_timer.c:511:test_odp_timer_all():Number of timeouts delivered/received
too late: 0
passedSegmentation fault (core dumped)

I assume this "passed" is printed by the following call which is last in
the test_odp_timer_all test case. Something wrong in Cunit or the ODP
test/validation framework?
        CU_PASS("ODP timer test");
}

Only happened once... If you can't reproduce it, you can't fix it. Maybe it
was radiation from outer space.



> --
>
> I've checked this patch with Keystone implementation.
>
>  .../linux-generic/include/odp_timer_internal.h     |  9 -------
>  platform/linux-generic/odp_timer.c                 | 31
> ++++++++++++----------
>  2 files changed, 17 insertions(+), 23 deletions(-)
>
> diff --git a/platform/linux-generic/include/odp_timer_internal.h
> b/platform/linux-generic/include/odp_timer_internal.h
> index 90af62c..8b0e93d 100644
> --- a/platform/linux-generic/include/odp_timer_internal.h
> +++ b/platform/linux-generic/include/odp_timer_internal.h
> @@ -39,13 +39,4 @@ typedef struct odp_timeout_hdr_stride {
>         uint8_t
> pad[ODP_CACHE_LINE_SIZE_ROUNDUP(sizeof(odp_timeout_hdr_t))];
>  } odp_timeout_hdr_stride;
>
> -
> -/**
> - * Return the timeout header
> - */
> -static inline odp_timeout_hdr_t *odp_timeout_hdr(odp_buffer_t buf)
> -{
> -       return (odp_timeout_hdr_t *)odp_buf_to_hdr(buf);
> -}
> -
>  #endif
> diff --git a/platform/linux-generic/odp_timer.c
> b/platform/linux-generic/odp_timer.c
> index bd1b778..68d7b11 100644
> --- a/platform/linux-generic/odp_timer.c
> +++ b/platform/linux-generic/odp_timer.c
> @@ -78,7 +78,13 @@ static _odp_atomic_flag_t locks[NUM_LOCKS]; /* Multiple
> locks per cache line! */
>
>  static odp_timeout_hdr_t *timeout_hdr_from_buf(odp_buffer_t buf)
>  {
> -       return (odp_timeout_hdr_t *)odp_buf_to_hdr(buf);
> +       return (odp_timeout_hdr_t *)(void *)odp_buf_to_hdr(buf);
> +}
> +
> +static odp_timeout_hdr_t *timeout_hdr(odp_timeout_t tmo)
> +{
> +       odp_buffer_t buf =
> odp_buffer_from_event(odp_timeout_to_event(tmo));
> +       return timeout_hdr_from_buf(buf);
>  }
>
>
>  
> /******************************************************************************
> @@ -421,7 +427,8 @@ static bool timer_reset(uint32_t idx,
>         } else {
>                 /* We have a new timeout buffer which replaces any old one
> */
>                 /* Fill in some (constant) header fields for timeout
> events */
> -               if (_odp_buffer_type(*tmo_buf) == ODP_EVENT_TIMEOUT) {
> +               if (odp_event_type(odp_buffer_to_event(*tmo_buf)) ==
> +                   ODP_EVENT_TIMEOUT) {
>                         /* Convert from buffer to timeout hdr */
>                         odp_timeout_hdr_t *tmo_hdr =
>                                 timeout_hdr_from_buf(*tmo_buf);
> @@ -566,7 +573,8 @@ static unsigned timer_expire(odp_timer_pool *tp,
> uint32_t idx, uint64_t tick)
>  #endif
>         if (odp_likely(tmo_buf != ODP_BUFFER_INVALID)) {
>                 /* Fill in expiration tick for timeout events */
> -               if (_odp_buffer_type(tmo_buf) == ODP_EVENT_TIMEOUT) {
> +               if (odp_event_type(odp_buffer_to_event(tmo_buf)) ==
> +                   ODP_EVENT_TIMEOUT) {
>                         /* Convert from buffer to timeout hdr */
>                         odp_timeout_hdr_t *tmo_hdr =
>                                 timeout_hdr_from_buf(tmo_buf);
> @@ -815,19 +823,17 @@ odp_timeout_t odp_timeout_from_event(odp_event_t ev)
>         /* This check not mandated by the API specification */
>         if (odp_event_type(ev) != ODP_EVENT_TIMEOUT)
>                 ODP_ABORT("Event not a timeout");
> -       return
> (odp_timeout_t)timeout_hdr_from_buf(odp_buffer_from_event(ev));
> +       return (odp_timeout_t)ev;
>  }
>
>  odp_event_t odp_timeout_to_event(odp_timeout_t tmo)
>  {
> -       odp_timeout_hdr_t *tmo_hdr = (odp_timeout_hdr_t *)tmo;
> -       odp_buffer_t buf = odp_hdr_to_buf(&tmo_hdr->buf_hdr);
> -       return odp_buffer_to_event(buf);
> +       return (odp_event_t)tmo;
>  }
>
>  int odp_timeout_fresh(odp_timeout_t tmo)
>  {
> -       const odp_timeout_hdr_t *hdr = (odp_timeout_hdr_t *)tmo;
> +       const odp_timeout_hdr_t *hdr = timeout_hdr(tmo);
>         odp_timer_t hdl = hdr->timer;
>         odp_timer_pool *tp = handle_to_tp(hdl);
>         uint32_t idx = handle_to_idx(hdl, tp);
> @@ -840,20 +846,17 @@ int odp_timeout_fresh(odp_timeout_t tmo)
>
>  odp_timer_t odp_timeout_timer(odp_timeout_t tmo)
>  {
> -       const odp_timeout_hdr_t *hdr = (odp_timeout_hdr_t *)tmo;
> -       return hdr->timer;
> +       return timeout_hdr(tmo)->timer;
>  }
>
>  uint64_t odp_timeout_tick(odp_timeout_t tmo)
>  {
> -       const odp_timeout_hdr_t *hdr = (odp_timeout_hdr_t *)tmo;
> -       return hdr->expiration;
> +       return timeout_hdr(tmo)->expiration;
>  }
>
>  void *odp_timeout_user_ptr(odp_timeout_t tmo)
>  {
> -       const odp_timeout_hdr_t *hdr = (odp_timeout_hdr_t *)tmo;
> -       return hdr->user_ptr;
> +       return timeout_hdr(tmo)->user_ptr;
>  }
>
>  odp_timeout_t odp_timeout_alloc(odp_pool_t pool)
> --
> 1.9.1
>
>
_______________________________________________
lng-odp mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to