Commenting mainly on the API...
> -----Original Message-----
> From: [email protected] [mailto:lng-odp-
> [email protected]] On Behalf Of ext Ola Liljedahl
> Sent: Thursday, September 11, 2014 3:38 PM
> To: [email protected]
> Subject: [lng-odp] [PATCHv2] Timer API and and priority queue-based
> implementation
>
> Signed-off-by: Ola Liljedahl <[email protected]>
> ---
> (This document/code contribution attached is provided under the terms of
> agreement LES-LTM-21309)
> New timer API and corresponding SW implementation for linux-generic.
> Read more about use cases and usage here:
> https://docs.google.com/a/linaro.org/document/d/1bfY_J8ecLJPsFTmYftb0NVmGn
> B9qkEc_NpcJ87yfaD8
> Updated after review:
> Renamed and moved files (see git summary below).
> Removed/rephrased some TODO texts.
> Removed unused assignment of "val".
> Modified "#if 1" expression.
> Enhanced a comment in the odp_timer_ping test program.
> Incorporated some later updates to odp_timer.c.
>
> example/timer/odp_timer_test.c | 99 +--
> platform/linux-generic/Makefile.am | 1 +
> platform/linux-generic/include/api/odp_timer.h | 478 ++++++++++--
> .../include/odp_priority_queue_internal.h | 108 +++
> .../linux-generic/include/odp_timer_internal.h | 71 +-
> platform/linux-generic/odp_priority_queue.c | 284 +++++++
> platform/linux-generic/odp_timer.c | 847 +++++++++++++---
> -----
> test/api_test/odp_timer_ping.c | 60 +-
> 8 files changed, 1452 insertions(+), 496 deletions(-)
> create mode 100644 platform/linux-
> generic/include/odp_priority_queue_internal.h
> create mode 100644 platform/linux-generic/odp_priority_queue.c
>
> diff --git a/example/timer/odp_timer_test.c
> b/example/timer/odp_timer_test.c
> index 1061190..87112fb 100644
> --- a/example/timer/odp_timer_test.c
> +++ b/example/timer/odp_timer_test.c
> @@ -31,7 +31,6 @@
> typedef struct {
> int core_count; /**< Core count*/
> int resolution_us; /**< Timeout resolution in usec*/
> - int min_us; /**< Minimum timeout in usec*/
> int max_us; /**< Maximum timeout in usec*/
> int period_us; /**< Timeout period in usec*/
> int tmo_count; /**< Timeout count*/
> @@ -41,18 +40,16 @@ typedef struct {
> /** @private Barrier for test synchronisation */
> static odp_barrier_t test_barrier;
>
> -/** @private Timer handle*/
> -static odp_timer_t test_timer;
> +/** @private Timer pool handle*/
> +static odp_timer_pool_t tp;
>
>
> /** @private test timeout */
> static void test_abs_timeouts(int thr, test_args_t *args)
> {
> - uint64_t tick;
> - uint64_t period;
> + odp_timer_tick_t period;
> uint64_t period_ns;
> odp_queue_t queue;
> - odp_buffer_t buf;
> int num;
>
> ODP_DBG(" [%i] test_timeouts\n", thr);
> @@ -60,37 +57,51 @@ static void test_abs_timeouts(int thr, test_args_t
> *args)
> queue = odp_queue_lookup("timer_queue");
>
> period_ns = args->period_us*ODP_TIME_USEC;
> - period = odp_timer_ns_to_tick(test_timer, period_ns);
> + period = odp_timer_ns_to_tick(tp, period_ns);
>
> ODP_DBG(" [%i] period %"PRIu64" ticks, %"PRIu64" ns\n", thr,
> period, period_ns);
>
> - tick = odp_timer_current_tick(test_timer);
> + ODP_DBG(" [%i] current tick %"PRIu64"\n", thr,
> + odp_timer_current_tick(tp));
>
> - ODP_DBG(" [%i] current tick %"PRIu64"\n", thr, tick);
> -
> - tick += period;
> -
> - if (odp_timer_absolute_tmo(test_timer, tick, queue,
> ODP_BUFFER_INVALID)
> - == ODP_TIMER_TMO_INVALID){
> - ODP_DBG("Timeout request failed\n");
> + odp_timer_t test_timer;
> + test_timer = odp_timer_alloc(tp, queue, NULL);
> + if (test_timer == ODP_TIMER_INVALID) {
> + ODP_ERR("Failed to allocate timer\n");
> return;
> }
> + odp_timer_set_rel(test_timer, period);
>
> num = args->tmo_count;
>
> while (1) {
> - odp_timeout_t tmo;
> -
> - buf = odp_schedule_one(&queue, ODP_SCHED_WAIT);
> + /* Local variables because received timeouts may not
> + originate from timer we created above */
> + odp_timer_tmo_t tmo;
> + odp_timer_tick_t tick;
> + odp_timer_t hdl;
> +
> + /* Get the next ready buffer/timeout */
> + tmo = odp_schedule_one(&queue, ODP_SCHED_WAIT);
This is not portable. Scheduler returns odp_buffer_t. odp_timer_tmo_t type is
not always odp_buffer_t.
> + switch (odp_timer_tmo_status(tmo)) {
> + case ODP_TMO_FRESH:
> + break;
> + case ODP_TMO_STALE:
> + ODP_DBG("[%i] Stale timeout received\n", thr);
> + break;
> + case ODP_TMO_ORPHAN:
> + ODP_DBG("[%i] Orphaned timeout received\n",
> + thr);
> + odp_buffer_free(tmo);
odp_timer_tmo_t != odp_buffer_t
> + continue;
> + }
Could we optimize for the common case. Minimize C lines/cycles for freshness
check and ack.
if (odp_unlikely(odp_timer_tmo_check(tmo))) {
/* handle errors */
status = odp_timer_tmo_status(tmo);
...
}
>
> - tmo = odp_timeout_from_buffer(buf);
> - tick = odp_timeout_tick(tmo);
> + tick = odp_timer_get_expiry(tmo);
> + hdl = odp_timer_get_handle(tmo);
>
> ODP_DBG(" [%i] timeout, tick %"PRIu64"\n", thr, tick);
>
> - odp_buffer_free(buf);
> -
> num--;
>
> if (num == 0)
> @@ -98,10 +109,13 @@ static void test_abs_timeouts(int thr, test_args_t
> *args)
>
> tick += period;
>
> - odp_timer_absolute_tmo(test_timer, tick,
> - queue, ODP_BUFFER_INVALID);
> + if (hdl != ODP_TIMER_INVALID)
> + odp_timer_set_abs(hdl, tick);
> }
>
> + odp_timer_cancel(test_timer);
> + odp_timer_free(test_timer);
> +
> if (odp_queue_sched_type(queue) == ODP_SCHED_SYNC_ATOMIC)
> odp_schedule_release_atomic();
> }
> @@ -155,7 +169,6 @@ static void print_usage(void)
> printf("Options:\n");
> printf(" -c, --count <number> core count, core IDs start from
> 1\n");
> printf(" -r, --resolution <us> timeout resolution in usec\n");
> - printf(" -m, --min <us> minimum timeout in usec\n");
> printf(" -x, --max <us> maximum timeout in usec\n");
> printf(" -p, --period <us> timeout period in usec\n");
> printf(" -t, --timeouts <count> timeout repeat count\n");
> @@ -179,7 +192,6 @@ static void parse_args(int argc, char *argv[],
> test_args_t *args)
> static struct option longopts[] = {
> {"count", required_argument, NULL, 'c'},
> {"resolution", required_argument, NULL, 'r'},
> - {"min", required_argument, NULL, 'm'},
> {"max", required_argument, NULL, 'x'},
> {"period", required_argument, NULL, 'p'},
> {"timeouts", required_argument, NULL, 't'},
> @@ -190,14 +202,13 @@ static void parse_args(int argc, char *argv[],
> test_args_t *args)
> /* defaults */
> args->core_count = 0; /* all cores */
> args->resolution_us = 10000;
> - args->min_us = args->resolution_us;
> args->max_us = 10000000;
> args->period_us = 1000000;
> args->tmo_count = 30;
>
> while (1) {
> opt = getopt_long(argc, argv, "+c:r:m:x:p:t:h",
> - longopts, &long_index);
> + longopts, &long_index);
>
> if (opt == -1)
> break; /* No more options */
> @@ -209,9 +220,6 @@ static void parse_args(int argc, char *argv[],
> test_args_t *args)
> case 'r':
> args->resolution_us = atoi(optarg);
> break;
> - case 'm':
> - args->min_us = atoi(optarg);
> - break;
> case 'x':
> args->max_us = atoi(optarg);
> break;
> @@ -295,7 +303,6 @@ int main(int argc, char *argv[])
>
> printf("first core: %i\n", first_core);
> printf("resolution: %i usec\n", args.resolution_us);
> - printf("min timeout: %i usec\n", args.min_us);
> printf("max timeout: %i usec\n", args.max_us);
> printf("period: %i usec\n", args.period_us);
> printf("timeouts: %i\n", args.tmo_count);
> @@ -319,10 +326,23 @@ int main(int argc, char *argv[])
> ODP_BUFFER_TYPE_TIMEOUT);
>
> if (pool == ODP_BUFFER_POOL_INVALID) {
> - ODP_ERR("Pool create failed.\n");
> + ODP_ERR("Buffer pool create failed.\n");
> return -1;
> }
>
> + tp = odp_timer_pool_create("timer_pool", pool,
> + args.resolution_us*ODP_TIME_USEC,
> + args.max_us*ODP_TIME_USEC,
> + num_workers, /* One timer per worker */
> + true,
> + ODP_CLOCK_DEFAULT);
> + if (tp == ODP_TIMER_POOL_INVALID) {
> + ODP_ERR("Timer pool create failed.\n");
> + return -1;
> + }
> +
> + odp_shm_print_all();
> +
> /*
> * Create a queue for timer test
> */
> @@ -338,19 +358,6 @@ int main(int argc, char *argv[])
> return -1;
> }
>
> - test_timer = odp_timer_create("test_timer", pool,
> - args.resolution_us*ODP_TIME_USEC,
> - args.min_us*ODP_TIME_USEC,
> - args.max_us*ODP_TIME_USEC);
> -
> - if (test_timer == ODP_TIMER_INVALID) {
> - ODP_ERR("Timer create failed.\n");
> - return -1;
> - }
> -
> -
> - odp_shm_print_all();
> -
> printf("CPU freq %"PRIu64" hz\n", odp_sys_cpu_hz());
> printf("Cycles vs nanoseconds:\n");
> ns = 0;
> diff --git a/platform/linux-generic/Makefile.am b/platform/linux-
> generic/Makefile.am
> index 25c82ea..26964d8 100644
> --- a/platform/linux-generic/Makefile.am
> +++ b/platform/linux-generic/Makefile.am
> @@ -62,6 +62,7 @@ __LIB__libodp_la_SOURCES = \
> odp_packet_flags.c \
> odp_packet_io.c \
> odp_packet_socket.c \
> + odp_priority_queue.c \
> odp_queue.c \
> odp_ring.c \
> odp_rwlock.c \
> diff --git a/platform/linux-generic/include/api/odp_timer.h
> b/platform/linux-generic/include/api/odp_timer.h
> index 01db839..b05283a 100644
> --- a/platform/linux-generic/include/api/odp_timer.h
> +++ b/platform/linux-generic/include/api/odp_timer.h
> @@ -1,4 +1,4 @@
> -/* Copyright (c) 2013, Linaro Limited
> +/* Copyright (c) 2014, Linaro Limited
> * All rights reserved.
> *
> * SPDX-License-Identifier: BSD-3-Clause
> @@ -8,7 +8,175 @@
> /**
> * @file
> *
> - * ODP timer
> + * ODP timer service
> + *
> +
> +//Example #1 Retransmission timer (e.g. for reliable connections)
> +
> +//Create timer pool for reliable connections
> +#define SEC 1000000000ULL //1s expressed in nanoseconds
> +odp_timer_pool_t tcp_tpid =
> + odp_timer_pool_create("TCP",
> + buffer_pool,
> + 1000000,//resolution 1ms
> + 7200 * SEC,//max tmo length 2hours
> + 40000,//num_timers
> + true,//shared
> + ODP_CLOCK_DEFAULT
> + );
> +if (tcp_tpid == ODP_TIMER_POOL_INVALID)
> +{
> + //Failed to create timer pool => fatal error
> +}
> +
> +
> +//Setting up a new connection
> +//Allocate retransmission timeout (identical for supervision timeout)
> +//The user pointer points back to the connection context
> +conn->ret_tim = odp_timer_alloc(tcp_tpid, queue, conn);
> +//Check if all resources were successfully allocated
> +if (conn->ret_tim == ODP_TIMER_INVALID)
> +{
> + //Failed to allocate all resources for connection => tear down
> + //Destroy timeout
> + odp_timer_free(conn->ret_tim);
> + //Tear down connection
> + ...
> + return false;
> +}
> +//All necessary resources successfully allocated
> +//Compute initial retransmission length in timer ticks
> +conn->ret_len = odp_timer_ns_to_tick(tcp_tpid, 3 * SEC);//Per RFC1122
> +//Arm the timer
> +odp_timer_set_rel(conn->ret_tim, conn->ret_len);
> +return true;
> +
> +
> +//A packet for the connection has just been transmitted
> +//Reset the retransmission timer
> +odp_timer_set_rel(conn->ret_tim, conn->ret_len);
> +
> +
> +//A retransmission timeout for the connection has been received
> +//Check if timeout is fresh or stale, for stale timeouts we need to reset
> the
> +//timer
> +switch (odp_timer_tmo_status(tmo))
> +{
> + case ODP_TMO_FRESH :
> + //Fresh timeout, last transmitted packet not acked in time =>
> + retransmit
> + //Get connection from timeout event
> + conn = odp_timer_get_userptr(tmo);
> + //Retransmit last packet (e.g. TCP segment)
> + ...
> + //Re-arm timer using original delta value
> + odp_timer_set_rel(conn->ret_tim, conn->ret_len);
> + break;
> + case ODP_TMO_STALE :
> + break;//Do nothing
> + case ODP_TMO_ORPHAN :
> + odp_free_buffer(tmo);
> + return;//Get out of here
> +}
> +
> +
> +//Example #2 Periodic tick
> +
> +//Create timer pool for periodic ticks
> +odp_timer_pool_t per_tpid =
> + odp_timer_pool_create("periodic-tick",
> + buffer_pool,
> + 1,//resolution 1ns
> + 1000000000,//maximum timeout length 1s
> + 10,//num_timers
> + false,//not shared
> + ODP_CLOCK_DEFAULT
> + );
> +if (per_tpid == ODP_TIMER_POOL_INVALID)
> +{
> + //Failed to create timer pool => fatal error
> +}
> +
> +
> +//Allocate periodic timer
> +tim_1733 = odp_timer_alloc(per_tpid, queue, NULL);
> +//Check if all resources were successfully allocated
> +if (tim_1733 == ODP_TIMER_INVALID)
> +{
> + //Failed to allocate all resources => tear down
> + //Destroy timeout
> + odp_timer_free(tim_1733);
> + //Tear down other state
> + ...
> + return false;
> +}
> +//All necessary resources successfully allocated
> +//Compute tick period in timer ticks
> +period_1733 = odp_timer_ns_to_tick(per_tpid, 1000000000U /
> 1733U);//1733Hz
> +//Compute when next tick should expire
> +next_1733 = odp_timer_current_tick(per_tpid) + period_1733;
> +//Arm the periodic timer
> +odp_timer_set_abs(tim_1733, next_1733);
> +return true;
> +
> +
> +
> +//A periodic timer timeout has been received
> +//Must call odp_timer_tmo_status() on timeout!
> +ret = odp_timer_tmo_status(tmo);
> +//We expect the timeout is fresh since we are not calling set or cancel
> on
> +//active or expired timers in this example
> +assert(ret == ODP_TMO_FRESH);
> +//Do processing driven by timeout *before*
> +...
> +do {
> + //Compute when the timer should expire next
> + next_1733 += period_1733;
> + //Check that this is in the future
> + if (likely(next_1733 > odp_timer_current_tick(per_tpid))
> + break;//Yes, done
> + //Else we missed a timeout
> + //Optionally attempt some recovery and/or logging of the problem
> + ...
> +} while (0);
> +//Re-arm periodic timer
> +odp_timer_set_abs(tim_1733, next_1733);
> +//Or do processing driven by timeout *after*
> +...
> +return;
> +
> +//Example #3 Tear down of flow
> +//ctx points to flow context data structure owned by application
> +//Free the timer, cancelling any timeout
> +odp_timer_free(ctx->timer);//Any enqueued timeout will be made invalid
> +//Continue tearing down and eventually freeing context
> +...
> +return;
> +
> +//A timeout has been received, check status
> +switch (odp_timer_tmo_status(tmo))
> +{
> + case ODP_TMO_FRESH :
> + //A flow has timed out, tear it down
> + //Find flow context from timeout
> + ctx = (context *)odp_timer_get_userptr(tmo);
> + //Free the supervision timer, any enqueued timeout will remain
> + odp_timer_free(ctx->tim);
> + //Free other flow related resources
> + ...
> + //Flow torn down
> + break;
> + case ODP_TMO_STALE :
> + //A stale timeout was received, timer automatically reset
> + break;
> + case ODP_TMO_ORPHAN :
> + //Orphaned timeout (from previously torn down flow)
> + //No corresponding timer or flow context
> + //Free the timeout
> + odp_buffer_free(tmo);
> + break;
> +}
> +
> */
>
> #ifndef ODP_TIMER_H_
> @@ -23,139 +191,325 @@ extern "C" {
> #include <odp_buffer_pool.h>
> #include <odp_queue.h>
>
> +/**
> +* ODP timer pool handle (platform dependent)
> +*/
> +struct odp_timer_pool;
struct odp_timer_pool_t (or struct odp_timer_pool_s or struct odp_timer_pool_)
> +typedef struct odp_timer_pool *odp_timer_pool_t;
>
> /**
> - * ODP timer handle
> + * Invalid timer pool handle (platform dependent)
> */
> -typedef uint32_t odp_timer_t;
> +#define ODP_TIMER_POOL_INVALID NULL
>
> -/** Invalid timer */
> -#define ODP_TIMER_INVALID 0
> +typedef enum odp_timer_pool_clock_source_e {
> + ODP_CLOCK_DEFAULT = 0,
> + /* Platform dependent which clock sources exist beyond
> + ODP_CLOCK_DEFAULT */
Maybe this comment can be removed. We may be able standardize some. E.g.
_CPU_CLOCK (chip internal) and _TIMER_INPUT (chip external, connected to timer
input pin)
> + ODP_CLOCK_NONE = 1
When none is used?
> +} odp_timer_pool_clock_source_t;
Pretty long type name, could we use odp_timer_clock_source_t instead?
>
> +/**
> +* ODP timer handle (platform dependent)
> +*/
> +struct odp_timer;
> +typedef struct odp_timer *odp_timer_t;
struct odp_timer_t
>
> /**
> - * ODP timeout handle
> + * Invalid timer handle (platform dependent)
> */
> -typedef odp_buffer_t odp_timer_tmo_t;
> -
> -/** Invalid timeout */
> -#define ODP_TIMER_TMO_INVALID 0
> +#define ODP_TIMER_INVALID NULL
>
> +/**
> + * ODP timeout event handle
> + */
> +typedef odp_buffer_t odp_timer_tmo_t;
>
> /**
> - * Timeout notification
> + * ODP timeout status
> */
> -typedef odp_buffer_t odp_timeout_t;
> +typedef enum odp_timer_tmo_status_e {
> + ODP_TMO_FRESH, /* Timeout is fresh, process it */
> + ODP_TMO_STALE, /* Timer reset or cancelled, do nothing */
Do nothing? Did the status function free the buffer? The buffer has to be freed
or reused, otherwise it's a memory leak.
> + ODP_TMO_ORPHAN,/* Timer deleted, free timeout */
> +} odp_timer_tmo_status_t;
> +
> +/**
> +* ODP tick value
> +*/
> +typedef uint64_t odp_timer_tick_t;
Why tick could not be fixed to uint64_t? It's simpler for the user to perform
(portable) tick calculations if the type is fixed. Now it could be signed vs.
unsigned, 16/32/64 bits...
>
>
> /**
> - * Create a timer
> + * Create a timer pool
> *
> - * Creates a new timer with requested properties.
> + * Create a new timer pool.
> + * odp_timer_pool_create() is typically called once or a couple of times
> during
> + * application initialisation.
I think the speculation above is not needed. There's also timer pool destroy
function, and user may destroy/create pools also after init.
> *
> * @param name Name
> - * @param pool Buffer pool for allocating timeout notifications
> + * @param buf_pool Buffer pool for allocating timers
odp_timer_t or odp_timer_tmo_t or both? I'm thinking timeouts.
> * @param resolution Timeout resolution in nanoseconds
> - * @param min_tmo Minimum timeout duration in nanoseconds
Min_tmo could be useful still. Some implementations may appreciate to have
more information about the intended usage, e.g. what kind of (cascading)
timer wheels to create, etc. Zero could be the default (== resolution).
> - * @param max_tmo Maximum timeout duration in nanoseconds
> + * @param max_tmo Maximum relative timeout in nanoseconds
> + * @param num_timers Number of supported timers (minimum)
> + * @param shared Shared or private timer pool.
> + * Operations on shared timers will include the necessary
> + * mutual exclusion, operations on private timers may not
> + * (mutual exclusion is the responsibility of the caller).
> + * @param clk_src Clock source to use
> + *
> + * @return Timer pool handle if successful, otherwise
> ODP_TIMER_POOL_INVALID
> + * and errno set
> + */
> +odp_timer_pool_t
> +odp_timer_pool_create(const char *name,
> + odp_buffer_pool_t buf_pool,
> + uint64_t resolution,
> + uint64_t max_tmo,
> + uint32_t num_timers,
> + bool shared,
> + odp_timer_pool_clock_source_t clk_src);
> +
> +/**
> + * Start a timer pool
> + *
> + * Start all created timer pools, enabling the allocation of timers.
> + * The purpose of this call is to coordinate the creation of multiple
> timer
> + * pools that may use the same underlying HW resources.
> + * This function may be called multiple times.
> + */
> +void odp_timer_pool_start(void);
> +
> +/**
> + * Destroy a timer pool
> *
> - * @return Timer handle if successful, otherwise ODP_TIMER_INVALID
> + * Destroy a timer pool, freeing all resources.
> + * All timers must have been freed.
When user have called odp_timer_free() for all timers, there may be still
timeouts in timer HW and in queues. Is it OK to delete the timer pool
if all remaining timeouts have not been received yet? E.g. user could not
delete the "timer buffer pool" after this call returns, since some timeouts
may be still on the way and need to be freed. How user knows when the buffer
pool can be reused or freed?
> + *
> + * @param tpid Timer pool identifier
> */
> -odp_timer_t odp_timer_create(const char *name, odp_buffer_pool_t pool,
> - uint64_t resolution, uint64_t min_tmo,
> - uint64_t max_tmo);
> +void odp_timer_pool_destroy(odp_timer_pool_t tpid);
>
> /**
> * Convert timer ticks to nanoseconds
> *
> - * @param timer Timer
> + * @param tpid Timer pool identifier
> * @param ticks Timer ticks
> *
> * @return Nanoseconds
> */
> -uint64_t odp_timer_tick_to_ns(odp_timer_t timer, uint64_t ticks);
> +uint64_t odp_timer_tick_to_ns(odp_timer_pool_t tpid, odp_timer_tick_t
> ticks);
>
> /**
> * Convert nanoseconds to timer ticks
> *
> - * @param timer Timer
> + * @param tpid Timer pool identifier
> * @param ns Nanoseconds
> *
> * @return Timer ticks
> */
> -uint64_t odp_timer_ns_to_tick(odp_timer_t timer, uint64_t ns);
> +odp_timer_tick_t odp_timer_ns_to_tick(odp_timer_pool_t tpid, uint64_t
> ns);
>
> /**
> - * Timer resolution in nanoseconds
> + * Current tick value
> *
> - * @param timer Timer
> + * @param tpid Timer pool identifier
> *
> - * @return Resolution in nanoseconds
> + * @return Current time in timer ticks
> + */
> +odp_timer_tick_t odp_timer_current_tick(odp_timer_pool_t tpid);
> +
> +/**
> + * ODP timer configurations
> */
> -uint64_t odp_timer_resolution(odp_timer_t timer);
> +
> +typedef enum odp_timer_pool_conf_e {
> + ODP_TIMER_NAME, /* Return name of timer pool */
> + ODP_TIMER_RESOLUTION,/* Return the timer resolution (in ns) */
> + ODP_TIMER_MAX_TMO, /* Return the maximum supported timeout (in ns)
> */
> + ODP_TIMER_NUM_TIMERS,/* Return number of supported timers */
> + ODP_TIMER_SHARED /* Return shared flag */
> +} odp_timer_pool_conf_t;
>
> /**
> - * Maximum timeout in timer ticks
> + * Query different timer pool configurations, e.g.
> + * Timer resolution in nanoseconds
> + * Maximum timeout in timer ticks
> + * Number of supported timers
> + * Shared or private timer pool
> *
> - * @param timer Timer
> + * @param tpid Timer pool identifier
> + * @param item Configuration item being queried
> *
> - * @return Maximum timeout in timer ticks
> + * @return the requested piece of information or 0 for unknown item.
> */
> -uint64_t odp_timer_maximum_tmo(odp_timer_t timer);
> +uintptr_t odp_timer_pool_query_conf(odp_timer_pool_t tpid,
> + odp_timer_pool_conf_t item);
It would be simpler to just query all information at once.
typedef struct odp_timer_pool_info_s {
const char *name;
uint64_t resolution;
uint64_t max_tmo;
uint32_t num_timers;
bool shared
} odp_timer_pool_info_t;
int odp_timer_pool_info(odp_timer_pool_t tpid, odp_timer_pool_info_t *info);
>
> /**
> - * Current timer tick
> + * Allocate a timer
> *
> - * @param timer Timer
> + * Create a timer (allocating all necessary resources e.g. timeout event)
> from
> + * the timer pool.
> *
> - * @return Current time in timer ticks
> + * @param tpid Timer pool identifier
> + * @param queue Destination queue for timeout notifications
> + * @param user_ptr User defined pointer or NULL (copied to timeouts)
> + *
> + * @return Timer handle if successful, otherwise ODP_TIMER_INVALID and
> + * errno set.
> + */
> +odp_timer_t odp_timer_alloc(odp_timer_pool_t tpid,
> + odp_queue_t queue,
> + void *user_ptr);
> +
> +/**
> + * Free a timer
> + *
> + * Free (destroy) a timer, freeing all associated resources (e.g. default
> + * timeout event). An expired and enqueued timeout event will not be
> freed.
User should see from a return code if free was successful or not. Now user
does not know if it has to wait for timeout or not.
> + * It is the responsibility of the application to free this timeout when
> it
> + * is received.
> + *
> + * @param tim Timer handle
> */
> -uint64_t odp_timer_current_tick(odp_timer_t timer);
> +void odp_timer_free(odp_timer_t tim);
>
> /**
> - * Request timeout with an absolute timer tick
> + * Set a timer (absolute time) with a user-defined timeout buffer
> *
> - * When tick reaches tmo_tick, the timer enqueues the timeout
> notification into
> - * the destination queue.
> + * Set (arm) the timer to expire at specific time. The user-defined
> + * buffer will be enqueued when the timer expires.
> + * Arming may fail (if the timer is in state EXPIRED), an earlier timeout
> + * will then be received. odp_timer_tmo_status() must be used to check if
> + * the received timeout is valid.
> *
> - * @param timer Timer
> - * @param tmo_tick Absolute timer tick value which triggers the timeout
> - * @param queue Destination queue for the timeout notification
> - * @param buf User defined timeout notification buffer. When
> - * ODP_BUFFER_INVALID, default timeout notification is
> used.
> + * Note: any invalid parameters will be treated as programming errors and
> will
> + * cause the application to abort.
> + * Note: a timeout too near in time may be delivered immediately.
> + * Note: a timeout too far away in time (beyond max_timeout) might be
> + * delivered early.
In the two cases above, application must be in control what happens next. ODP
does not know what
application was trying to do with the timeout (tmo priority or severity of the
failure) or
what is the right corrective action (if any is needed). E.g. in "too near" case
it may be better
for application to skip that tmo and just continue from the next cycle, etc.
Speculation on
"too far" is also bad. For example, if user was supposed to ask a 1 sec, but
miss
calculated and asked a 500 year timeout, now ODP implementation would silently
round
that to the max e.g. 3 days. How do you debug it after 3 days? With a "too far"
return code
application could log/report error and take the corrective action where the bug
was noticed the first time.
> *
> - * @return Timeout handle if successful, otherwise ODP_TIMER_TMO_INVALID
> + * @param tim Timer
> + * @param abs_tck Expiration time in absolute timer ticks
> + * @param user_buf The buffer to use as timeout event
> */
> -odp_timer_tmo_t odp_timer_absolute_tmo(odp_timer_t timer, uint64_t
> tmo_tick,
> - odp_queue_t queue, odp_buffer_t buf);
> +void odp_timer_set_abs_w_buf(odp_timer_t tim,
> + odp_timer_tick_t abs_tck,
> + odp_buffer_t user_buf);
>
> /**
> - * Cancel a timeout
> + * Set a timer with an absolute expiration time
> + *
> + * Set (arm) the timer to expire at a specific time.
> + * Arming may fail (if the timer is in state EXPIRED), an earlier timeout
> + * will then be received. odp_timer_tmo_status() must be used to check if
> + * the received timeout is valid.
> *
> - * @param timer Timer
> - * @param tmo Timeout to cancel
> + * Note: any invalid parameters will be treated as programming errors and
> will
> + * cause the application to abort.
> + * Note: a timeout too near in time may be delivered immediately.
> + * Note: a timeout too far away in time (beyond max_timeout) might be
> delivered
> + * early, it will automatically be reset by odp_timer_tmo_status().
> *
> - * @return 0 if successful
> + * @param tim Timer
> + * @param abs_tck Expiration time in absolute timer ticks
> */
> -int odp_timer_cancel_tmo(odp_timer_t timer, odp_timer_tmo_t tmo);
> +void odp_timer_set_abs(odp_timer_t tim, odp_timer_tick_t abs_tck);
>
> /**
> - * Convert buffer handle to timeout handle
> + * Set a timer with a relative expiration time
> *
> - * @param buf Buffer handle
> + * Set (arm) the timer to expire at a relative future time.
> + * Arming may fail (if the timer is in state EXPIRED),
> + * an earlier timeout will then be received. odp_timer_tmo_status() must
> + * be used to check if the received timeout is valid.
> *
> - * @return Timeout buffer handle
> + * Note: any invalid parameters will be treated as programming errors and
> will
> + * cause the application to abort.
> + * Note: a timeout too near in time may be delivered immediately.
> + * Note: a timeout too far away in time (beyond max_timeout) might be
> delivered
> + * early, it will automatically be reset by odp_timer_tmo_status().
> + *
> + * @param tim Timer
> + * @param rel_tck Expiration time in timer ticks relative to current time
> of
> + * the timer pool the timer belongs to
> */
> -odp_timeout_t odp_timeout_from_buffer(odp_buffer_t buf);
> +void odp_timer_set_rel(odp_timer_t tim, odp_timer_tick_t rel_tck);
odp_timer_set_rel_w_buf() missing?
>
> /**
> - * Return absolute timeout tick
> + * Cancel a timer
> + *
> + * Cancel a timer, preventing future expiration and delivery.
> + *
> + * A timer that has already expired and been enqueued for delivery may be
> + * impossible to cancel and will instead be delivered to the destination
> queue.
How user knows if it has to wait for a pending timeout still, or can it e.g.
remove the destination queue? Function should return a success code.
> + * Use odp_timer_tmo_status() the check whether a received timeout is
> fresh or
> + * stale (cancelled). Stale timeouts will automatically be recycled.
> *
> - * @param tmo Timeout buffer handle
> + * Note: any invalid parameters will be treated as programming errors and
> will
> + * cause the application to abort.
> *
> - * @return Absolute timeout tick
> + * @param tim Timer handle
> */
> -uint64_t odp_timeout_tick(odp_timeout_t tmo);
> +void odp_timer_cancel(odp_timer_t tim);
> +
> +/**
> + * Return fresh/stale/orphan status of timeout.
> + *
> + * Check a received timeout for orphaness (i.e. parent timer freed) and
> + * staleness (i.e. parent timer has been reset or cancelled after timeout
> + * was enqueued).
It's not very well documented how a timer is reset. E.g. another
odp_timer_set_abs_w_buf() call on an already active timer?
Can user mix timer set calls when resetting e.g. odp_timer_set_abs_w_buf(timer,
tick, buf) -> odp_timer_set_abs(timer, tick)?
What happens if timer is reset too late? Does user receive the original, the
new or a stale tmo?
> + * If the timeout is fresh, it should be processed.
> + * If the timeout is stale, the timer will automatically be reset unless
> it
> + * was cancelled.
> + * If the timeout is orphaned, it should be freed (by the caller).
> + *
> + * Note: odp_timer_tmo_status() must be called on all received (not
> + * user-defined) timeouts!
> + *
> + * @param tmo Timeout
> + *
> + * @return ODP_TMO_FRESH, ODP_TMO_STALE, ODP_TMO_ORPHAN
> + */
> +odp_timer_tmo_status_t odp_timer_tmo_status(odp_timer_tmo_t tmo);
> +
> +/**
> + * Get timer handle
> + *
> + * Return Handle of parent timer.
> + *
> + * @param tmo Timeout
> + *
> + * @return Timer handle or ODP_TIMER_INVALID for orphaned timeouts
> + */
> +odp_timer_t odp_timer_get_handle(odp_timer_tmo_t tmo);
Should we drop _get_ to be consistent with other ODP APIs.
> +
> +/**
> + * Get expiration time
> + *
> + * Return (actual) expiration time of timeout.
> + *
> + * @param tmo Timeout
> + *
> + * @return Expiration time
> + */
> +odp_timer_tick_t odp_timer_get_expiry(odp_timer_tmo_t tmo);
Should we drop _get_ to be consistent with other ODP APIs.
> +
> +/**
> + * Get user pointer
> + *
> + * Return User pointer of timer associated with timeout.
> + * The user pointer is often used to point to some associated context.
> + *
> + * @param tmo Timeout
> + *
> + * @return User pointer
> + */
> +void *odp_timer_get_userptr(odp_timer_tmo_t tmo);
Should we drop _get_ to be consistent with other ODP APIs.
> +
> +/* Helper functions */
> +unsigned odp_timer_pool_expire(odp_timer_pool_t tpid, odp_timer_tick_t
> tick);
Is this function part of the API?
-Petri
_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp
