| Message ID | 1465332412-30587-1-git-send-email-maxim.uvarov@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On Tue, Jun 7, 2016 at 3:46 PM, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > Example for simple timer usage. > https://bugs.linaro.org/show_bug.cgi?id=2090 > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> > --- > example/timer/.gitignore | 1 + > example/timer/Makefile.am | 12 ++- > example/timer/odp_timer_simple.c | 167 > +++++++++++++++++++++++++++++++++++++++ > 3 files changed, 177 insertions(+), 3 deletions(-) > create mode 100644 example/timer/odp_timer_simple.c > > diff --git a/example/timer/.gitignore b/example/timer/.gitignore > index eb59265..f53a0df 100644 > --- a/example/timer/.gitignore > +++ b/example/timer/.gitignore > @@ -1 +1,2 @@ > odp_timer_test > +odp_timer_simple > diff --git a/example/timer/Makefile.am b/example/timer/Makefile.am > index fcb67a9..1c733d3 100644 > --- a/example/timer/Makefile.am > +++ b/example/timer/Makefile.am > @@ -1,10 +1,16 @@ > include $(top_srcdir)/example/Makefile.inc > > -bin_PROGRAMS = odp_timer_test$(EXEEXT) > +bin_PROGRAMS = odp_timer_test$(EXEEXT) \ > + odp_timer_simple$(EXEEXT) > odp_timer_test_LDFLAGS = $(AM_LDFLAGS) -static > odp_timer_test_CFLAGS = $(AM_CFLAGS) -I${top_srcdir}/example > +dist_odp_timer_test_SOURCES = odp_timer_test.c > + > +odp_timer_simple_LDFLAGS = $(AM_LDFLAGS) -static > +odp_timer_simple_CFLAGS = $(AM_CFLAGS) -I${top_srcdir}/example > +dist_odp_timer_simple_SOURCES = odp_timer_simple.c > + > +TESTS = odp_timer_simple > > noinst_HEADERS = \ > $(top_srcdir)/example/example_debug.h > - > -dist_odp_timer_test_SOURCES = odp_timer_test.c > diff --git a/example/timer/odp_timer_simple.c > b/example/timer/odp_timer_simple.c > new file mode 100644 > index 0000000..d4917c3 > --- /dev/null > +++ b/example/timer/odp_timer_simple.c > @@ -0,0 +1,167 @@ > +/* Copyright (c) 2016, Linaro Limited > + * All rights reserved. > + * > + * SPDX-License-Identifier: BSD-3-Clause > + */ > +/** > + * @file > + * > + * @example odp_timer_simple.c ODP simple example to schedule timer > + * action for 1 second. > + */ > + > +#include <string.h> > +#include <stdlib.h> > +#include <example_debug.h> > + > +/* ODP main header */ > +#include <odp_api.h> > + > +int main(int argc ODP_UNUSED, char *argv[] ODP_UNUSED) > +{ > + odp_instance_t instance; > + odp_pool_param_t params; > + odp_timer_pool_param_t tparams; > + odp_pool_t tmo_pool; > + odp_timer_pool_t timer_pool; > + odp_queue_param_t qparam; > + odp_queue_t queue; > + odp_event_t ev = ODP_EVENT_INVALID; > + odp_timer_t tim; > + uint64_t sched_tmo; > + int i, rc; > + uint64_t period; > + uint64_t tick; > + odp_timeout_t tmo; > + int ret = 0; > + > + /* > + * Init ODP app > + */ > + if (odp_init_global(&instance, NULL, NULL)) > + goto err_global; > + > + if (odp_init_local(instance, ODP_THREAD_CONTROL)) > + goto err_local; > + > + /* > + * Create pool for timeouts > + */ > + odp_pool_param_init(¶ms); > + params.tmo.num = 10; > + params.type = ODP_POOL_TIMEOUT; > + > + tmo_pool = odp_pool_create("msg_pool", ¶ms); > Is "msg_pool" the best choice of name for a timeout pool? "timeout_pool" would seem to make more sense here. > + if (tmo_pool == ODP_POOL_INVALID) { > + ret += 1; > + goto err_tp; > + } > + > + /* > + * Create pool of timeouts > + */ > + tparams.res_ns = 10 * ODP_TIME_USEC_IN_NS; > + tparams.min_tmo = 10 * ODP_TIME_USEC_IN_NS; > + tparams.max_tmo = 1 * ODP_TIME_SEC_IN_NS; > + tparams.num_timers = 1; /* One timer per worker */ > + tparams.priv = 0; /* Shared */ > + tparams.clk_src = ODP_CLOCK_CPU; > + timer_pool = odp_timer_pool_create("timer_pool", &tparams); > + if (timer_pool == ODP_TIMER_POOL_INVALID) { > + ret += 1; > + goto err; > + } > + > + /* > + * Create a queue for timer test > + */ > + odp_queue_param_init(&qparam); > + qparam.type = ODP_QUEUE_TYPE_SCHED; > + qparam.sched.prio = ODP_SCHED_PRIO_DEFAULT; > + qparam.sched.sync = ODP_SCHED_SYNC_PARALLEL; > + qparam.sched.group = ODP_SCHED_GROUP_ALL; > + > + queue = odp_queue_create("timer_queue", &qparam); > + if (queue == ODP_QUEUE_INVALID) { > + ret += 1; > + goto err; > + } > + > + tim = odp_timer_alloc(timer_pool, queue, NULL); > + if (tim == ODP_TIMER_INVALID) { > + EXAMPLE_ERR("Failed to allocate timer\n"); > + ret += 1; > + goto err; > + } > + > + tmo = odp_timeout_alloc(tmo_pool); > + if (tmo == ODP_TIMEOUT_INVALID) { > + EXAMPLE_ERR("Failed to allocate timeout\n"); > + return -1; > + } > + > + ev = odp_timeout_to_event(tmo); > + > + /* Calculate period for timer in uint64_t value, in current case > + * we will schedule timer for 1 second */ > + period = odp_timer_ns_to_tick(timer_pool, 1 * ODP_TIME_SEC_IN_NS); > + > + /* Get current tick uint64_t value */ > + tick = odp_timer_current_tick(timer_pool); > + > + rc = odp_timer_set_abs(tim, tick + period, &ev); > + if (odp_unlikely(rc != ODP_TIMER_SUCCESS)) { > + /* Too early or too late timeout requested */ > + EXAMPLE_ABORT("odp_timer_set_abs() failed: %d\n", > + rc); > + } > + > + /* Wait time to return from odp_schedule() if there are no > + * events > + */ > + sched_tmo = odp_schedule_wait_time(2 * ODP_TIME_SEC_IN_NS); > + > + for (i = 0; i < 5; i++) { > + odp_time_t time; > + > + /* Program timeout action on current tick + period */ > + tick = odp_timer_current_tick(timer_pool); > + rc = odp_timer_set_abs(tim, tick + period, &ev); > This replaces the odp_timer_set_abs() call made prior to entering this loop. Is that intentional? If yes, a comment would seem appropriate to explain why. > + /* Too early or too late timeout requested */ > + if (odp_unlikely(rc != ODP_TIMER_SUCCESS)) > + EXAMPLE_ABORT("odp_timer_set_abs() failed: %d\n", > + rc); > + > + /* Wait for 2 seconds for timeout action to be generated */ > + ev = odp_schedule(&queue, sched_tmo); > + if (ev == ODP_EVENT_INVALID) > + EXAMPLE_ABORT("Invalid event\n"); > Isn't the real error here that the 1 second timer failed to trigger after the scheduler waited for 2 seconds? > + if (odp_event_type(ev) != ODP_EVENT_TIMEOUT) > + EXAMPLE_ABORT("Unexpected event type (%u) > received\n", > + odp_event_type(ev)); > + > + time = odp_time_global(); > + printf("timer tick %d, time ns %" PRIu64 "\n", > + i, odp_time_to_ns(time)); > + > + /* Do not free current event, just go back to loop and > program > + * timeout to next second. > + */ > + } > + > + /* Destroy created resources */ > + odp_timer_cancel(tim, &ev); > + odp_timer_free(tim); > + odp_event_free(ev); > Since examples illustrate best practices, shouldn't RCs be checked for these calls? > + > + ret += odp_queue_destroy(queue); > +err: > + odp_timer_pool_destroy(timer_pool); > Why not ret += for consistency with the rest of these exit calls? Oversight? > +err_tp: > + ret += odp_pool_destroy(tmo_pool); > + ret += odp_term_local(); > +err_local: > + ret += odp_term_global(instance); > +err_global: > + return ret; > +} > -- > 2.7.1.250.gff4ea60 > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp >
On 06/08/16 06:07, Bill Fischofer wrote: > > > On Tue, Jun 7, 2016 at 3:46 PM, Maxim Uvarov <maxim.uvarov@linaro.org > <mailto:maxim.uvarov@linaro.org>> wrote: > > Example for simple timer usage. > https://bugs.linaro.org/show_bug.cgi?id=2090 > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org > <mailto:maxim.uvarov@linaro.org>> > --- > example/timer/.gitignore | 1 + > example/timer/Makefile.am | 12 ++- > example/timer/odp_timer_simple.c | 167 > +++++++++++++++++++++++++++++++++++++++ > 3 files changed, 177 insertions(+), 3 deletions(-) > create mode 100644 example/timer/odp_timer_simple.c > > diff --git a/example/timer/.gitignore b/example/timer/.gitignore > index eb59265..f53a0df 100644 > --- a/example/timer/.gitignore > +++ b/example/timer/.gitignore > @@ -1 +1,2 @@ > odp_timer_test > +odp_timer_simple > diff --git a/example/timer/Makefile.am b/example/timer/Makefile.am > index fcb67a9..1c733d3 100644 > --- a/example/timer/Makefile.am > +++ b/example/timer/Makefile.am > @@ -1,10 +1,16 @@ > include $(top_srcdir)/example/Makefile.inc > > -bin_PROGRAMS = odp_timer_test$(EXEEXT) > +bin_PROGRAMS = odp_timer_test$(EXEEXT) \ > + odp_timer_simple$(EXEEXT) > odp_timer_test_LDFLAGS = $(AM_LDFLAGS) -static > odp_timer_test_CFLAGS = $(AM_CFLAGS) -I${top_srcdir}/example > +dist_odp_timer_test_SOURCES = odp_timer_test.c > + > +odp_timer_simple_LDFLAGS = $(AM_LDFLAGS) -static > +odp_timer_simple_CFLAGS = $(AM_CFLAGS) -I${top_srcdir}/example > +dist_odp_timer_simple_SOURCES = odp_timer_simple.c > + > +TESTS = odp_timer_simple > > noinst_HEADERS = \ > $(top_srcdir)/example/example_debug.h > - > -dist_odp_timer_test_SOURCES = odp_timer_test.c > diff --git a/example/timer/odp_timer_simple.c > b/example/timer/odp_timer_simple.c > new file mode 100644 > index 0000000..d4917c3 > --- /dev/null > +++ b/example/timer/odp_timer_simple.c > @@ -0,0 +1,167 @@ > +/* Copyright (c) 2016, Linaro Limited > + * All rights reserved. > + * > + * SPDX-License-Identifier: BSD-3-Clause > + */ > +/** > + * @file > + * > + * @example odp_timer_simple.c ODP simple example to schedule timer > + * action for 1 second. > + */ > + > +#include <string.h> > +#include <stdlib.h> > +#include <example_debug.h> > + > +/* ODP main header */ > +#include <odp_api.h> > + > +int main(int argc ODP_UNUSED, char *argv[] ODP_UNUSED) > +{ > + odp_instance_t instance; > + odp_pool_param_t params; > + odp_timer_pool_param_t tparams; > + odp_pool_t tmo_pool; > + odp_timer_pool_t timer_pool; > + odp_queue_param_t qparam; > + odp_queue_t queue; > + odp_event_t ev = ODP_EVENT_INVALID; > + odp_timer_t tim; > + uint64_t sched_tmo; > + int i, rc; > + uint64_t period; > + uint64_t tick; > + odp_timeout_t tmo; > + int ret = 0; > + > + /* > + * Init ODP app > + */ > + if (odp_init_global(&instance, NULL, NULL)) > + goto err_global; > + > + if (odp_init_local(instance, ODP_THREAD_CONTROL)) > + goto err_local; > + > + /* > + * Create pool for timeouts > + */ > + odp_pool_param_init(¶ms); > + params.tmo.num = 10; > + params.type = ODP_POOL_TIMEOUT; > + > + tmo_pool = odp_pool_create("msg_pool", ¶ms); > > > Is "msg_pool" the best choice of name for a timeout pool? > "timeout_pool" would seem to make more sense here. agree, will change. > + if (tmo_pool == ODP_POOL_INVALID) { > + ret += 1; > + goto err_tp; > + } > + > + /* > + * Create pool of timeouts > + */ > + tparams.res_ns = 10 * ODP_TIME_USEC_IN_NS; > + tparams.min_tmo = 10 * ODP_TIME_USEC_IN_NS; > + tparams.max_tmo = 1 * ODP_TIME_SEC_IN_NS; > + tparams.num_timers = 1; /* One timer per worker */ > + tparams.priv = 0; /* Shared */ > + tparams.clk_src = ODP_CLOCK_CPU; > + timer_pool = odp_timer_pool_create("timer_pool", &tparams); > + if (timer_pool == ODP_TIMER_POOL_INVALID) { > + ret += 1; > + goto err; > + } > + > + /* > + * Create a queue for timer test > + */ > + odp_queue_param_init(&qparam); > + qparam.type = ODP_QUEUE_TYPE_SCHED; > + qparam.sched.prio = ODP_SCHED_PRIO_DEFAULT; > + qparam.sched.sync = ODP_SCHED_SYNC_PARALLEL; > + qparam.sched.group = ODP_SCHED_GROUP_ALL; > + > + queue = odp_queue_create("timer_queue", &qparam); > + if (queue == ODP_QUEUE_INVALID) { > + ret += 1; > + goto err; > + } > + > + tim = odp_timer_alloc(timer_pool, queue, NULL); > + if (tim == ODP_TIMER_INVALID) { > + EXAMPLE_ERR("Failed to allocate timer\n"); > + ret += 1; > + goto err; > + } > + > + tmo = odp_timeout_alloc(tmo_pool); > + if (tmo == ODP_TIMEOUT_INVALID) { > + EXAMPLE_ERR("Failed to allocate timeout\n"); > + return -1; > + } > + > + ev = odp_timeout_to_event(tmo); > + > + /* Calculate period for timer in uint64_t value, in > current case > + * we will schedule timer for 1 second */ > + period = odp_timer_ns_to_tick(timer_pool, 1 * > ODP_TIME_SEC_IN_NS); > + > + /* Get current tick uint64_t value */ > + tick = odp_timer_current_tick(timer_pool); > + > + rc = odp_timer_set_abs(tim, tick + period, &ev); > + if (odp_unlikely(rc != ODP_TIMER_SUCCESS)) { > + /* Too early or too late timeout requested */ > + EXAMPLE_ABORT("odp_timer_set_abs() failed: %d\n", > + rc); > + } > + > + /* Wait time to return from odp_schedule() if there are no > + * events > + */ > + sched_tmo = odp_schedule_wait_time(2 * ODP_TIME_SEC_IN_NS); > + > + for (i = 0; i < 5; i++) { > + odp_time_t time; > + > + /* Program timeout action on current tick + period */ > + tick = odp_timer_current_tick(timer_pool); > + rc = odp_timer_set_abs(tim, tick + period, &ev); > > > This replaces the odp_timer_set_abs() call made prior to entering this > loop. Is that intentional? If yes, a comment would seem appropriate > to explain why. it has to be only inside this loop. Above loop is not needed, will remove. > + /* Too early or too late timeout requested */ > + if (odp_unlikely(rc != ODP_TIMER_SUCCESS)) > + EXAMPLE_ABORT("odp_timer_set_abs() failed: > %d\n", > + rc); > + > + /* Wait for 2 seconds for timeout action to be > generated */ > + ev = odp_schedule(&queue, sched_tmo); > + if (ev == ODP_EVENT_INVALID) > + EXAMPLE_ABORT("Invalid event\n"); > > > Isn't the real error here that the 1 second timer failed to trigger > after the scheduler waited for 2 seconds? Yes, that is real error if there was no timeout. > + if (odp_event_type(ev) != ODP_EVENT_TIMEOUT) > + EXAMPLE_ABORT("Unexpected event type (%u) > received\n", > + odp_event_type(ev)); > + > + time = odp_time_global(); > + printf("timer tick %d, time ns %" PRIu64 "\n", > + i, odp_time_to_ns(time)); > + > + /* Do not free current event, just go back to loop > and program > + * timeout to next second. > + */ > + } > + > + /* Destroy created resources */ > + odp_timer_cancel(tim, &ev); > + odp_timer_free(tim); > + odp_event_free(ev); > > > Since examples illustrate best practices, shouldn't RCs be checked for > these calls? odp_event_free() is void. For 2 above function I will add ret check. > + > + ret += odp_queue_destroy(queue); > +err: > + odp_timer_pool_destroy(timer_pool); > > > Why not ret += for consistency with the rest of these exit calls? > Oversight? Because it's void function in api. For me it looks also not really consistent to other calls. So I will send v2 with above comments. Maxim. > > +err_tp: > + ret += odp_pool_destroy(tmo_pool); > + ret += odp_term_local(); > +err_local: > + ret += odp_term_global(instance); > +err_global: > + return ret; > +} > -- > 2.7.1.250.gff4ea60 > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> > https://lists.linaro.org/mailman/listinfo/lng-odp > >
On Wed, Jun 8, 2016 at 1:57 AM, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > On 06/08/16 06:07, Bill Fischofer wrote: > >> >> >> On Tue, Jun 7, 2016 at 3:46 PM, Maxim Uvarov <maxim.uvarov@linaro.org >> <mailto:maxim.uvarov@linaro.org>> wrote: >> >> Example for simple timer usage. >> https://bugs.linaro.org/show_bug.cgi?id=2090 >> >> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org >> <mailto:maxim.uvarov@linaro.org>> >> >> --- >> example/timer/.gitignore | 1 + >> example/timer/Makefile.am | 12 ++- >> example/timer/odp_timer_simple.c | 167 >> +++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 177 insertions(+), 3 deletions(-) >> create mode 100644 example/timer/odp_timer_simple.c >> >> diff --git a/example/timer/.gitignore b/example/timer/.gitignore >> index eb59265..f53a0df 100644 >> --- a/example/timer/.gitignore >> +++ b/example/timer/.gitignore >> @@ -1 +1,2 @@ >> odp_timer_test >> +odp_timer_simple >> diff --git a/example/timer/Makefile.am b/example/timer/Makefile.am >> index fcb67a9..1c733d3 100644 >> --- a/example/timer/Makefile.am >> +++ b/example/timer/Makefile.am >> @@ -1,10 +1,16 @@ >> include $(top_srcdir)/example/Makefile.inc >> >> -bin_PROGRAMS = odp_timer_test$(EXEEXT) >> +bin_PROGRAMS = odp_timer_test$(EXEEXT) \ >> + odp_timer_simple$(EXEEXT) >> odp_timer_test_LDFLAGS = $(AM_LDFLAGS) -static >> odp_timer_test_CFLAGS = $(AM_CFLAGS) -I${top_srcdir}/example >> +dist_odp_timer_test_SOURCES = odp_timer_test.c >> + >> +odp_timer_simple_LDFLAGS = $(AM_LDFLAGS) -static >> +odp_timer_simple_CFLAGS = $(AM_CFLAGS) -I${top_srcdir}/example >> +dist_odp_timer_simple_SOURCES = odp_timer_simple.c >> + >> +TESTS = odp_timer_simple >> >> noinst_HEADERS = \ >> $(top_srcdir)/example/example_debug.h >> - >> -dist_odp_timer_test_SOURCES = odp_timer_test.c >> diff --git a/example/timer/odp_timer_simple.c >> b/example/timer/odp_timer_simple.c >> new file mode 100644 >> index 0000000..d4917c3 >> --- /dev/null >> +++ b/example/timer/odp_timer_simple.c >> @@ -0,0 +1,167 @@ >> +/* Copyright (c) 2016, Linaro Limited >> + * All rights reserved. >> + * >> + * SPDX-License-Identifier: BSD-3-Clause >> + */ >> +/** >> + * @file >> + * >> + * @example odp_timer_simple.c ODP simple example to schedule timer >> + * action for 1 second. >> + */ >> + >> +#include <string.h> >> +#include <stdlib.h> >> +#include <example_debug.h> >> + >> +/* ODP main header */ >> +#include <odp_api.h> >> + >> +int main(int argc ODP_UNUSED, char *argv[] ODP_UNUSED) >> +{ >> + odp_instance_t instance; >> + odp_pool_param_t params; >> + odp_timer_pool_param_t tparams; >> + odp_pool_t tmo_pool; >> + odp_timer_pool_t timer_pool; >> + odp_queue_param_t qparam; >> + odp_queue_t queue; >> + odp_event_t ev = ODP_EVENT_INVALID; >> + odp_timer_t tim; >> + uint64_t sched_tmo; >> + int i, rc; >> + uint64_t period; >> + uint64_t tick; >> + odp_timeout_t tmo; >> + int ret = 0; >> + >> + /* >> + * Init ODP app >> + */ >> + if (odp_init_global(&instance, NULL, NULL)) >> + goto err_global; >> + >> + if (odp_init_local(instance, ODP_THREAD_CONTROL)) >> + goto err_local; >> + >> + /* >> + * Create pool for timeouts >> + */ >> + odp_pool_param_init(¶ms); >> + params.tmo.num = 10; >> + params.type = ODP_POOL_TIMEOUT; >> + >> + tmo_pool = odp_pool_create("msg_pool", ¶ms); >> >> >> Is "msg_pool" the best choice of name for a timeout pool? "timeout_pool" >> would seem to make more sense here. >> > > agree, will change. > > > + if (tmo_pool == ODP_POOL_INVALID) { >> + ret += 1; >> + goto err_tp; >> + } >> + >> + /* >> + * Create pool of timeouts >> + */ >> + tparams.res_ns = 10 * ODP_TIME_USEC_IN_NS; >> + tparams.min_tmo = 10 * ODP_TIME_USEC_IN_NS; >> + tparams.max_tmo = 1 * ODP_TIME_SEC_IN_NS; >> + tparams.num_timers = 1; /* One timer per worker */ >> + tparams.priv = 0; /* Shared */ >> + tparams.clk_src = ODP_CLOCK_CPU; >> + timer_pool = odp_timer_pool_create("timer_pool", &tparams); >> + if (timer_pool == ODP_TIMER_POOL_INVALID) { >> + ret += 1; >> + goto err; >> + } >> + >> + /* >> + * Create a queue for timer test >> + */ >> + odp_queue_param_init(&qparam); >> + qparam.type = ODP_QUEUE_TYPE_SCHED; >> + qparam.sched.prio = ODP_SCHED_PRIO_DEFAULT; >> + qparam.sched.sync = ODP_SCHED_SYNC_PARALLEL; >> + qparam.sched.group = ODP_SCHED_GROUP_ALL; >> + >> + queue = odp_queue_create("timer_queue", &qparam); >> + if (queue == ODP_QUEUE_INVALID) { >> + ret += 1; >> + goto err; >> + } >> + >> + tim = odp_timer_alloc(timer_pool, queue, NULL); >> + if (tim == ODP_TIMER_INVALID) { >> + EXAMPLE_ERR("Failed to allocate timer\n"); >> + ret += 1; >> + goto err; >> + } >> + >> + tmo = odp_timeout_alloc(tmo_pool); >> + if (tmo == ODP_TIMEOUT_INVALID) { >> + EXAMPLE_ERR("Failed to allocate timeout\n"); >> + return -1; >> + } >> + >> + ev = odp_timeout_to_event(tmo); >> + >> + /* Calculate period for timer in uint64_t value, in >> current case >> + * we will schedule timer for 1 second */ >> + period = odp_timer_ns_to_tick(timer_pool, 1 * >> ODP_TIME_SEC_IN_NS); >> + >> + /* Get current tick uint64_t value */ >> + tick = odp_timer_current_tick(timer_pool); >> + >> + rc = odp_timer_set_abs(tim, tick + period, &ev); >> + if (odp_unlikely(rc != ODP_TIMER_SUCCESS)) { >> + /* Too early or too late timeout requested */ >> + EXAMPLE_ABORT("odp_timer_set_abs() failed: %d\n", >> + rc); >> + } >> + >> + /* Wait time to return from odp_schedule() if there are no >> + * events >> + */ >> + sched_tmo = odp_schedule_wait_time(2 * ODP_TIME_SEC_IN_NS); >> + >> + for (i = 0; i < 5; i++) { >> + odp_time_t time; >> + >> + /* Program timeout action on current tick + period */ >> + tick = odp_timer_current_tick(timer_pool); >> + rc = odp_timer_set_abs(tim, tick + period, &ev); >> >> >> This replaces the odp_timer_set_abs() call made prior to entering this >> loop. Is that intentional? If yes, a comment would seem appropriate to >> explain why. >> > > it has to be only inside this loop. Above loop is not needed, will remove. > > + /* Too early or too late timeout requested */ >> + if (odp_unlikely(rc != ODP_TIMER_SUCCESS)) >> + EXAMPLE_ABORT("odp_timer_set_abs() failed: >> %d\n", >> + rc); >> + >> + /* Wait for 2 seconds for timeout action to be >> generated */ >> + ev = odp_schedule(&queue, sched_tmo); >> + if (ev == ODP_EVENT_INVALID) >> + EXAMPLE_ABORT("Invalid event\n"); >> >> >> Isn't the real error here that the 1 second timer failed to trigger after >> the scheduler waited for 2 seconds? >> > Yes, that is real error if there was no timeout. > > > > + if (odp_event_type(ev) != ODP_EVENT_TIMEOUT) >> + EXAMPLE_ABORT("Unexpected event type (%u) >> received\n", >> + odp_event_type(ev)); >> + >> + time = odp_time_global(); >> + printf("timer tick %d, time ns %" PRIu64 "\n", >> + i, odp_time_to_ns(time)); >> + >> + /* Do not free current event, just go back to loop >> and program >> + * timeout to next second. >> + */ >> + } >> + >> + /* Destroy created resources */ >> + odp_timer_cancel(tim, &ev); >> + odp_timer_free(tim); >> + odp_event_free(ev); >> >> >> Since examples illustrate best practices, shouldn't RCs be checked for >> these calls? >> > > odp_event_free() is void. For 2 above function I will add ret check. > > > + >> + ret += odp_queue_destroy(queue); >> +err: >> + odp_timer_pool_destroy(timer_pool); >> >> >> Why not ret += for consistency with the rest of these exit calls? >> Oversight? >> > > Because it's void function in api. For me it looks also not really > consistent to other calls. > I should have checked. You're right, that does seem inconsistent. Perhaps something to revisit in Tiger Moth? > > So I will send v2 with above comments. > > Maxim. > >> >> +err_tp: >> + ret += odp_pool_destroy(tmo_pool); >> + ret += odp_term_local(); >> +err_local: >> + ret += odp_term_global(instance); >> +err_global: >> + return ret; >> +} >> -- >> 2.7.1.250.gff4ea60 >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> >> https://lists.linaro.org/mailman/listinfo/lng-odp >> >> >> >
On 06/08/16 14:22, Bill Fischofer wrote: > > > On Wed, Jun 8, 2016 at 1:57 AM, Maxim Uvarov <maxim.uvarov@linaro.org > <mailto:maxim.uvarov@linaro.org>> wrote: > > On 06/08/16 06:07, Bill Fischofer wrote: > > > > On Tue, Jun 7, 2016 at 3:46 PM, Maxim Uvarov > <maxim.uvarov@linaro.org <mailto:maxim.uvarov@linaro.org> > <mailto:maxim.uvarov@linaro.org > <mailto:maxim.uvarov@linaro.org>>> wrote: > > Example for simple timer usage. > https://bugs.linaro.org/show_bug.cgi?id=2090 > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org > <mailto:maxim.uvarov@linaro.org> > <mailto:maxim.uvarov@linaro.org > <mailto:maxim.uvarov@linaro.org>>> > > --- > example/timer/.gitignore | 1 + > example/timer/Makefile.am | 12 ++- > example/timer/odp_timer_simple.c | 167 > +++++++++++++++++++++++++++++++++++++++ > 3 files changed, 177 insertions(+), 3 deletions(-) > create mode 100644 example/timer/odp_timer_simple.c > > diff --git a/example/timer/.gitignore > b/example/timer/.gitignore > index eb59265..f53a0df 100644 > --- a/example/timer/.gitignore > +++ b/example/timer/.gitignore > @@ -1 +1,2 @@ > odp_timer_test > +odp_timer_simple > diff --git a/example/timer/Makefile.am > b/example/timer/Makefile.am > index fcb67a9..1c733d3 100644 > --- a/example/timer/Makefile.am > +++ b/example/timer/Makefile.am > @@ -1,10 +1,16 @@ > include $(top_srcdir)/example/Makefile.inc > > -bin_PROGRAMS = odp_timer_test$(EXEEXT) > +bin_PROGRAMS = odp_timer_test$(EXEEXT) \ > + odp_timer_simple$(EXEEXT) > odp_timer_test_LDFLAGS = $(AM_LDFLAGS) -static > odp_timer_test_CFLAGS = $(AM_CFLAGS) -I${top_srcdir}/example > +dist_odp_timer_test_SOURCES = odp_timer_test.c > + > +odp_timer_simple_LDFLAGS = $(AM_LDFLAGS) -static > +odp_timer_simple_CFLAGS = $(AM_CFLAGS) > -I${top_srcdir}/example > +dist_odp_timer_simple_SOURCES = odp_timer_simple.c > + > +TESTS = odp_timer_simple > > noinst_HEADERS = \ > $(top_srcdir)/example/example_debug.h > - > -dist_odp_timer_test_SOURCES = odp_timer_test.c > diff --git a/example/timer/odp_timer_simple.c > b/example/timer/odp_timer_simple.c > new file mode 100644 > index 0000000..d4917c3 > --- /dev/null > +++ b/example/timer/odp_timer_simple.c > @@ -0,0 +1,167 @@ > +/* Copyright (c) 2016, Linaro Limited > + * All rights reserved. > + * > + * SPDX-License-Identifier: BSD-3-Clause > + */ > +/** > + * @file > + * > + * @example odp_timer_simple.c ODP simple example to > schedule timer > + * action for 1 second. > + */ > + > +#include <string.h> > +#include <stdlib.h> > +#include <example_debug.h> > + > +/* ODP main header */ > +#include <odp_api.h> > + > +int main(int argc ODP_UNUSED, char *argv[] ODP_UNUSED) > +{ > + odp_instance_t instance; > + odp_pool_param_t params; > + odp_timer_pool_param_t tparams; > + odp_pool_t tmo_pool; > + odp_timer_pool_t timer_pool; > + odp_queue_param_t qparam; > + odp_queue_t queue; > + odp_event_t ev = ODP_EVENT_INVALID; > + odp_timer_t tim; > + uint64_t sched_tmo; > + int i, rc; > + uint64_t period; > + uint64_t tick; > + odp_timeout_t tmo; > + int ret = 0; > + > + /* > + * Init ODP app > + */ > + if (odp_init_global(&instance, NULL, NULL)) > + goto err_global; > + > + if (odp_init_local(instance, ODP_THREAD_CONTROL)) > + goto err_local; > + > + /* > + * Create pool for timeouts > + */ > + odp_pool_param_init(¶ms); > + params.tmo.num = 10; > + params.type = ODP_POOL_TIMEOUT; > + > + tmo_pool = odp_pool_create("msg_pool", ¶ms); > > > Is "msg_pool" the best choice of name for a timeout pool? > "timeout_pool" would seem to make more sense here. > > > agree, will change. > > > + if (tmo_pool == ODP_POOL_INVALID) { > + ret += 1; > + goto err_tp; > + } > + > + /* > + * Create pool of timeouts > + */ > + tparams.res_ns = 10 * ODP_TIME_USEC_IN_NS; > + tparams.min_tmo = 10 * ODP_TIME_USEC_IN_NS; > + tparams.max_tmo = 1 * ODP_TIME_SEC_IN_NS; > + tparams.num_timers = 1; /* One timer per worker */ > + tparams.priv = 0; /* Shared */ > + tparams.clk_src = ODP_CLOCK_CPU; > + timer_pool = odp_timer_pool_create("timer_pool", > &tparams); > + if (timer_pool == ODP_TIMER_POOL_INVALID) { > + ret += 1; > + goto err; > + } > + > + /* > + * Create a queue for timer test > + */ > + odp_queue_param_init(&qparam); > + qparam.type = ODP_QUEUE_TYPE_SCHED; > + qparam.sched.prio = ODP_SCHED_PRIO_DEFAULT; > + qparam.sched.sync = ODP_SCHED_SYNC_PARALLEL; > + qparam.sched.group = ODP_SCHED_GROUP_ALL; > + > + queue = odp_queue_create("timer_queue", &qparam); > + if (queue == ODP_QUEUE_INVALID) { > + ret += 1; > + goto err; > + } > + > + tim = odp_timer_alloc(timer_pool, queue, NULL); > + if (tim == ODP_TIMER_INVALID) { > + EXAMPLE_ERR("Failed to allocate timer\n"); > + ret += 1; > + goto err; > + } > + > + tmo = odp_timeout_alloc(tmo_pool); > + if (tmo == ODP_TIMEOUT_INVALID) { > + EXAMPLE_ERR("Failed to allocate timeout\n"); > + return -1; > + } > + > + ev = odp_timeout_to_event(tmo); > + > + /* Calculate period for timer in uint64_t value, in > current case > + * we will schedule timer for 1 second */ > + period = odp_timer_ns_to_tick(timer_pool, 1 * > ODP_TIME_SEC_IN_NS); > + > + /* Get current tick uint64_t value */ > + tick = odp_timer_current_tick(timer_pool); > + > + rc = odp_timer_set_abs(tim, tick + period, &ev); > + if (odp_unlikely(rc != ODP_TIMER_SUCCESS)) { > + /* Too early or too late timeout requested */ > + EXAMPLE_ABORT("odp_timer_set_abs() failed: %d\n", > + rc); > + } > + > + /* Wait time to return from odp_schedule() if > there are no > + * events > + */ > + sched_tmo = odp_schedule_wait_time(2 * > ODP_TIME_SEC_IN_NS); > + > + for (i = 0; i < 5; i++) { > + odp_time_t time; > + > + /* Program timeout action on current tick > + period */ > + tick = odp_timer_current_tick(timer_pool); > + rc = odp_timer_set_abs(tim, tick + period, > &ev); > > > This replaces the odp_timer_set_abs() call made prior to > entering this loop. Is that intentional? If yes, a comment > would seem appropriate to explain why. > > > it has to be only inside this loop. Above loop is not needed, will > remove. > > + /* Too early or too late timeout requested */ > + if (odp_unlikely(rc != ODP_TIMER_SUCCESS)) > + EXAMPLE_ABORT("odp_timer_set_abs() failed: > %d\n", > + rc); > + > + /* Wait for 2 seconds for timeout action to be > generated */ > + ev = odp_schedule(&queue, sched_tmo); > + if (ev == ODP_EVENT_INVALID) > + EXAMPLE_ABORT("Invalid event\n"); > > > Isn't the real error here that the 1 second timer failed to > trigger after the scheduler waited for 2 seconds? > > Yes, that is real error if there was no timeout. > > > > + if (odp_event_type(ev) != ODP_EVENT_TIMEOUT) > + EXAMPLE_ABORT("Unexpected event > type (%u) > received\n", > + odp_event_type(ev)); > + > + time = odp_time_global(); > + printf("timer tick %d, time ns %" PRIu64 "\n", > + i, odp_time_to_ns(time)); > + > + /* Do not free current event, just go back > to loop > and program > + * timeout to next second. > + */ > + } > + > + /* Destroy created resources */ > + odp_timer_cancel(tim, &ev); > + odp_timer_free(tim); > + odp_event_free(ev); > > > Since examples illustrate best practices, shouldn't RCs be > checked for these calls? > > > odp_event_free() is void. For 2 above function I will add ret check. > > > + > + ret += odp_queue_destroy(queue); > +err: > + odp_timer_pool_destroy(timer_pool); > > > Why not ret += for consistency with the rest of these exit > calls? Oversight? > > > Because it's void function in api. For me it looks also not really > consistent to other calls. > > > I should have checked. You're right, that does seem inconsistent. > Perhaps something to revisit in Tiger Moth? right, we should write about it somewhere to not forget. Maxim. > > So I will send v2 with above comments. > > Maxim. > > > +err_tp: > + ret += odp_pool_destroy(tmo_pool); > + ret += odp_term_local(); > +err_local: > + ret += odp_term_global(instance); > +err_global: > + return ret; > +} > -- > 2.7.1.250.gff4ea60 > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> > <mailto:lng-odp@lists.linaro.org > <mailto:lng-odp@lists.linaro.org>> > https://lists.linaro.org/mailman/listinfo/lng-odp > > > >
diff --git a/example/timer/.gitignore b/example/timer/.gitignore index eb59265..f53a0df 100644 --- a/example/timer/.gitignore +++ b/example/timer/.gitignore @@ -1 +1,2 @@ odp_timer_test +odp_timer_simple diff --git a/example/timer/Makefile.am b/example/timer/Makefile.am index fcb67a9..1c733d3 100644 --- a/example/timer/Makefile.am +++ b/example/timer/Makefile.am @@ -1,10 +1,16 @@ include $(top_srcdir)/example/Makefile.inc -bin_PROGRAMS = odp_timer_test$(EXEEXT) +bin_PROGRAMS = odp_timer_test$(EXEEXT) \ + odp_timer_simple$(EXEEXT) odp_timer_test_LDFLAGS = $(AM_LDFLAGS) -static odp_timer_test_CFLAGS = $(AM_CFLAGS) -I${top_srcdir}/example +dist_odp_timer_test_SOURCES = odp_timer_test.c + +odp_timer_simple_LDFLAGS = $(AM_LDFLAGS) -static +odp_timer_simple_CFLAGS = $(AM_CFLAGS) -I${top_srcdir}/example +dist_odp_timer_simple_SOURCES = odp_timer_simple.c + +TESTS = odp_timer_simple noinst_HEADERS = \ $(top_srcdir)/example/example_debug.h - -dist_odp_timer_test_SOURCES = odp_timer_test.c diff --git a/example/timer/odp_timer_simple.c b/example/timer/odp_timer_simple.c new file mode 100644 index 0000000..d4917c3 --- /dev/null +++ b/example/timer/odp_timer_simple.c @@ -0,0 +1,167 @@ +/* Copyright (c) 2016, Linaro Limited + * All rights reserved. + * + * SPDX-License-Identifier: BSD-3-Clause + */ +/** + * @file + * + * @example odp_timer_simple.c ODP simple example to schedule timer + * action for 1 second. + */ + +#include <string.h> +#include <stdlib.h> +#include <example_debug.h> + +/* ODP main header */ +#include <odp_api.h> + +int main(int argc ODP_UNUSED, char *argv[] ODP_UNUSED) +{ + odp_instance_t instance; + odp_pool_param_t params; + odp_timer_pool_param_t tparams; + odp_pool_t tmo_pool; + odp_timer_pool_t timer_pool; + odp_queue_param_t qparam; + odp_queue_t queue; + odp_event_t ev = ODP_EVENT_INVALID; + odp_timer_t tim; + uint64_t sched_tmo; + int i, rc; + uint64_t period; + uint64_t tick; + odp_timeout_t tmo; + int ret = 0; + + /* + * Init ODP app + */ + if (odp_init_global(&instance, NULL, NULL)) + goto err_global; + + if (odp_init_local(instance, ODP_THREAD_CONTROL)) + goto err_local; + + /* + * Create pool for timeouts + */ + odp_pool_param_init(¶ms); + params.tmo.num = 10; + params.type = ODP_POOL_TIMEOUT; + + tmo_pool = odp_pool_create("msg_pool", ¶ms); + if (tmo_pool == ODP_POOL_INVALID) { + ret += 1; + goto err_tp; + } + + /* + * Create pool of timeouts + */ + tparams.res_ns = 10 * ODP_TIME_USEC_IN_NS; + tparams.min_tmo = 10 * ODP_TIME_USEC_IN_NS; + tparams.max_tmo = 1 * ODP_TIME_SEC_IN_NS; + tparams.num_timers = 1; /* One timer per worker */ + tparams.priv = 0; /* Shared */ + tparams.clk_src = ODP_CLOCK_CPU; + timer_pool = odp_timer_pool_create("timer_pool", &tparams); + if (timer_pool == ODP_TIMER_POOL_INVALID) { + ret += 1; + goto err; + } + + /* + * Create a queue for timer test + */ + odp_queue_param_init(&qparam); + qparam.type = ODP_QUEUE_TYPE_SCHED; + qparam.sched.prio = ODP_SCHED_PRIO_DEFAULT; + qparam.sched.sync = ODP_SCHED_SYNC_PARALLEL; + qparam.sched.group = ODP_SCHED_GROUP_ALL; + + queue = odp_queue_create("timer_queue", &qparam); + if (queue == ODP_QUEUE_INVALID) { + ret += 1; + goto err; + } + + tim = odp_timer_alloc(timer_pool, queue, NULL); + if (tim == ODP_TIMER_INVALID) { + EXAMPLE_ERR("Failed to allocate timer\n"); + ret += 1; + goto err; + } + + tmo = odp_timeout_alloc(tmo_pool); + if (tmo == ODP_TIMEOUT_INVALID) { + EXAMPLE_ERR("Failed to allocate timeout\n"); + return -1; + } + + ev = odp_timeout_to_event(tmo); + + /* Calculate period for timer in uint64_t value, in current case + * we will schedule timer for 1 second */ + period = odp_timer_ns_to_tick(timer_pool, 1 * ODP_TIME_SEC_IN_NS); + + /* Get current tick uint64_t value */ + tick = odp_timer_current_tick(timer_pool); + + rc = odp_timer_set_abs(tim, tick + period, &ev); + if (odp_unlikely(rc != ODP_TIMER_SUCCESS)) { + /* Too early or too late timeout requested */ + EXAMPLE_ABORT("odp_timer_set_abs() failed: %d\n", + rc); + } + + /* Wait time to return from odp_schedule() if there are no + * events + */ + sched_tmo = odp_schedule_wait_time(2 * ODP_TIME_SEC_IN_NS); + + for (i = 0; i < 5; i++) { + odp_time_t time; + + /* Program timeout action on current tick + period */ + tick = odp_timer_current_tick(timer_pool); + rc = odp_timer_set_abs(tim, tick + period, &ev); + /* Too early or too late timeout requested */ + if (odp_unlikely(rc != ODP_TIMER_SUCCESS)) + EXAMPLE_ABORT("odp_timer_set_abs() failed: %d\n", + rc); + + /* Wait for 2 seconds for timeout action to be generated */ + ev = odp_schedule(&queue, sched_tmo); + if (ev == ODP_EVENT_INVALID) + EXAMPLE_ABORT("Invalid event\n"); + if (odp_event_type(ev) != ODP_EVENT_TIMEOUT) + EXAMPLE_ABORT("Unexpected event type (%u) received\n", + odp_event_type(ev)); + + time = odp_time_global(); + printf("timer tick %d, time ns %" PRIu64 "\n", + i, odp_time_to_ns(time)); + + /* Do not free current event, just go back to loop and program + * timeout to next second. + */ + } + + /* Destroy created resources */ + odp_timer_cancel(tim, &ev); + odp_timer_free(tim); + odp_event_free(ev); + + ret += odp_queue_destroy(queue); +err: + odp_timer_pool_destroy(timer_pool); +err_tp: + ret += odp_pool_destroy(tmo_pool); + ret += odp_term_local(); +err_local: + ret += odp_term_global(instance); +err_global: + return ret; +}
Example for simple timer usage. https://bugs.linaro.org/show_bug.cgi?id=2090 Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> --- example/timer/.gitignore | 1 + example/timer/Makefile.am | 12 ++- example/timer/odp_timer_simple.c | 167 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 177 insertions(+), 3 deletions(-) create mode 100644 example/timer/odp_timer_simple.c