| Message ID | 1435659415-16841-1-git-send-email-ivan.khoronzhuk@linaro.org |
|---|---|
| State | Accepted |
| Commit | cb82aeb6f0cb99767625e0897a7f867a3fe76199 |
| Headers | show |
Ola, I need this change. Could you please comment on this if you have some objections. or maybe send your version. On 30.06.15 13:16, Ivan Khoronzhuk wrote: > It's more generic implementation simplify reusing timer API > for other platforms. > > Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> > --- > > 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) >
On 30 June 2015 at 12:16, Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote: > It's more generic implementation simplify reusing timer API > for other platforms. > > Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> > Reviewed-by: Ola Liljedahl <ola.liljedahl@linaro.org> > - 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 > >
odp_timer fails periodically with segfaults in CI https://bugs.linaro.org/show_bug.cgi?id=1615 You can replicate on my machine by running a load and then running this a few times until it pops, I assume sometimes the CI build machines are also loaded and it surfaces as an issue. This is seen on ARM and X86 HW On 1 July 2015 at 12:03, Ola Liljedahl <ola.liljedahl@linaro.org> wrote: > On 30 June 2015 at 12:16, Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> > wrote: > >> It's more generic implementation simplify reusing timer API >> for other platforms. >> >> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> >> > Reviewed-by: Ola Liljedahl <ola.liljedahl@linaro.org> > > >> - > > > 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 > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp > >
On 07/01/15 20:17, Mike Holmes wrote: > odp_timer fails periodically with segfaults in CI > > https://bugs.linaro.org/show_bug.cgi?id=1615 > > You can replicate on my machine by running a load and then running > this a few times until it pops, I assume sometimes the CI build > machines are also loaded and it surfaces as an issue. This is seen on > ARM and X86 HW > Can you get core dump for that? Maxim. > On 1 July 2015 at 12:03, Ola Liljedahl <ola.liljedahl@linaro.org > <mailto:ola.liljedahl@linaro.org>> wrote: > > On 30 June 2015 at 12:16, Ivan Khoronzhuk > <ivan.khoronzhuk@linaro.org <mailto:ivan.khoronzhuk@linaro.org>> > wrote: > > It's more generic implementation simplify reusing timer API > for other platforms. > > Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org > <mailto:ivan.khoronzhuk@linaro.org>> > > Reviewed-by: Ola Liljedahl <ola.liljedahl@linaro.org > <mailto:ola.liljedahl@linaro.org>> > > - > > > 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 > lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> > https://lists.linaro.org/mailman/listinfo/lng-odp > > > > > -- > Mike Holmes > Technical Manager - Linaro Networking Group > Linaro.org <http://www.linaro.org/>***│ *Open source software for ARM SoCs > > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp
I will try, I did get one ages ago and some people took a look, the short time I spent with it did not help. On 2 July 2015 at 05:05, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > On 07/01/15 20:17, Mike Holmes wrote: > >> odp_timer fails periodically with segfaults in CI >> >> https://bugs.linaro.org/show_bug.cgi?id=1615 >> >> You can replicate on my machine by running a load and then running this a >> few times until it pops, I assume sometimes the CI build machines are also >> loaded and it surfaces as an issue. This is seen on ARM and X86 HW >> >> > Can you get core dump for that? > > Maxim. > > On 1 July 2015 at 12:03, Ola Liljedahl <ola.liljedahl@linaro.org <mailto: >> ola.liljedahl@linaro.org>> wrote: >> >> On 30 June 2015 at 12:16, Ivan Khoronzhuk >> <ivan.khoronzhuk@linaro.org <mailto:ivan.khoronzhuk@linaro.org>> >> wrote: >> >> It's more generic implementation simplify reusing timer API >> for other platforms. >> >> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org >> <mailto:ivan.khoronzhuk@linaro.org>> >> >> Reviewed-by: Ola Liljedahl <ola.liljedahl@linaro.org >> <mailto:ola.liljedahl@linaro.org>> >> >> >> - >> >> >> 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 >> lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> >> https://lists.linaro.org/mailman/listinfo/lng-odp >> >> >> >> >> -- >> Mike Holmes >> Technical Manager - Linaro Networking Group >> Linaro.org <http://www.linaro.org/>***│ *Open source software for ARM >> SoCs >> >> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> https://lists.linaro.org/mailman/listinfo/lng-odp >> > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp >
What about to pick up this patch? I still need it. I'm not sure, but seems this issue was before this patch. On 01.07.15 19:03, Ola Liljedahl wrote: > On 30 June 2015 at 12:16, Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org > <mailto:ivan.khoronzhuk@linaro.org>> wrote: > > It's more generic implementation simplify reusing timer API > for other platforms. > > Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org > <mailto:ivan.khoronzhuk@linaro.org>> > > Reviewed-by: Ola Liljedahl <ola.liljedahl@linaro.org > <mailto:ola.liljedahl@linaro.org>> > > - > > > 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 > >
Merged. Maxim. On 07/20/15 12:46, Ivan Khoronzhuk wrote: > What about to pick up this patch? > I still need it. I'm not sure, but seems this issue was before this > patch. > > On 01.07.15 19:03, Ola Liljedahl wrote: >> On 30 June 2015 at 12:16, Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org >> <mailto:ivan.khoronzhuk@linaro.org>> wrote: >> >> It's more generic implementation simplify reusing timer API >> for other platforms. >> >> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org >> <mailto:ivan.khoronzhuk@linaro.org>> >> >> Reviewed-by: Ola Liljedahl <ola.liljedahl@linaro.org >> <mailto:ola.liljedahl@linaro.org>> >> >> - >> >> >> 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 > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp
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)
It's more generic implementation simplify reusing timer API for other platforms. Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> --- 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(-)