| Message ID | 1409662044-23337-1-git-send-email-petri.savolainen@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 2014-09-02 15:47, Petri Savolainen wrote: > Use resolution and min/max tmo values from timer create call. > Use common nanosec time defines. > > Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org> > --- > platform/linux-generic/include/api/odp_time.h | 5 ++ > platform/linux-generic/odp_timer.c | 67 ++++++++++++++++++++------- > 2 files changed, 55 insertions(+), 17 deletions(-) > > diff --git a/platform/linux-generic/include/api/odp_time.h b/platform/linux-generic/include/api/odp_time.h > index 188d1fe..1770223 100644 > --- a/platform/linux-generic/include/api/odp_time.h > +++ b/platform/linux-generic/include/api/odp_time.h > @@ -21,6 +21,11 @@ extern "C" { > > #include <odp_std_types.h> > > +/* Time in nanoseconds */ > +#define ODP_TIME_USEC 1000UL /*< Microsecond in nsec */ > +#define ODP_TIME_MSEC 1000000UL /*< Millisecond in nsec */ > +#define ODP_TIME_SEC 1000000000UL /*< Second in nsec */ you need to add "/**<" and not "/*<" > + > > /** > * Current time in CPU cycles > diff --git a/platform/linux-generic/odp_timer.c b/platform/linux-generic/odp_timer.c > index 1bf37f9..6e89f5c 100644 > --- a/platform/linux-generic/odp_timer.c > +++ b/platform/linux-generic/odp_timer.c > @@ -6,6 +6,7 @@ > > #include <odp_timer.h> > #include <odp_timer_internal.h> > +#include <odp_time.h> > #include <odp_buffer_pool_internal.h> > #include <odp_internal.h> > #include <odp_atomic.h> > @@ -18,9 +19,14 @@ > > #include <string.h> > > +#define USEC ODP_TIME_USEC > +#define MSEC ODP_TIME_MSEC > +#define SEC ODP_TIME_SEC no, why duplicate it? Cheers, Anders > + > #define NUM_TIMERS 1 > #define MAX_TICKS 1024 > -#define RESOLUTION_NS 1000000 > +#define MAX_RES SEC > +#define MIN_RES (100*USEC) > > > typedef struct { > @@ -112,13 +118,13 @@ static int find_and_del_tmo(timeout_t **tmo, odp_timer_tmo_t handle) > int odp_timer_cancel_tmo(odp_timer_t timer_hdl, odp_timer_tmo_t tmo) > { > int id; > - uint64_t tick_idx; > + int tick_idx; > timeout_t *cancel_tmo; > odp_timeout_hdr_t *tmo_hdr; > tick_t *tick; > > /* get id */ > - id = timer_hdl - 1; > + id = (int)timer_hdl - 1; > > tmo_hdr = odp_timeout_hdr((odp_timeout_t) tmo); > /* get tmo_buf to cancel */ > @@ -179,6 +185,7 @@ static void timer_start(timer_ring_t *timer) > { > struct sigevent sigev; > struct itimerspec ispec; > + uint64_t res, sec, nsec; > > ODP_DBG("\nTimer (%u) starts\n", timer->timer_hdl); > > @@ -194,10 +201,14 @@ static void timer_start(timer_ring_t *timer) > return; > } > > - ispec.it_interval.tv_sec = 0; > - ispec.it_interval.tv_nsec = RESOLUTION_NS; > - ispec.it_value.tv_sec = 0; > - ispec.it_value.tv_nsec = RESOLUTION_NS; > + res = timer->resolution_ns; > + sec = res / SEC; > + nsec = res - sec*SEC; > + > + ispec.it_interval.tv_sec = (time_t)sec; > + ispec.it_interval.tv_nsec = (long)nsec; > + ispec.it_value.tv_sec = (time_t)sec; > + ispec.it_value.tv_nsec = (long)nsec; > > if (timer_settime(timer->timerid, 0, &ispec, NULL)) { > ODP_DBG("Timer set failed\n"); > @@ -250,19 +261,41 @@ int odp_timer_disarm_all(void) > } > > 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) > + uint64_t resolution_ns, uint64_t min_ns, > + uint64_t max_ns) > { > uint32_t id; > timer_ring_t *timer; > odp_timer_t timer_hdl; > int i; > - (void) name; (void) resolution; (void) min_tmo; (void) max_tmo; > + uint64_t max_ticks; > + (void) name; > + > + if (resolution_ns < MIN_RES) > + resolution_ns = MIN_RES; > + > + if (resolution_ns > MAX_RES) > + resolution_ns = MAX_RES; > + > + max_ticks = max_ns / resolution_ns; > + > + if (max_ticks > MAX_TICKS) { > + ODP_DBG("Maximum timeout too long: %"PRIu64" ticks\n", > + max_ticks); > + return ODP_TIMER_INVALID; > + } > + > + if (min_ns < resolution_ns) { > + ODP_DBG("Min timeout %"PRIu64" ns < resolution %"PRIu64" ns\n", > + min_ns, resolution_ns); > + return ODP_TIMER_INVALID; > + } > > odp_spinlock_lock(&odp_timer.lock); > > if (odp_timer.num_timers >= NUM_TIMERS) { > odp_spinlock_unlock(&odp_timer.lock); > + ODP_DBG("All timers allocated\n"); > return ODP_TIMER_INVALID; > } > > @@ -281,7 +314,7 @@ odp_timer_t odp_timer_create(const char *name, odp_buffer_pool_t pool, > > timer->timer_hdl = timer_hdl; > timer->pool = pool; > - timer->resolution_ns = RESOLUTION_NS; > + timer->resolution_ns = resolution_ns; > timer->max_ticks = MAX_TICKS; > > for (i = 0; i < MAX_TICKS; i++) { > @@ -308,7 +341,7 @@ odp_timer_tmo_t odp_timer_absolute_tmo(odp_timer_t timer_hdl, uint64_t tmo_tick, > odp_timeout_hdr_t *tmo_hdr; > timer_ring_t *timer; > > - id = timer_hdl - 1; > + id = (int)timer_hdl - 1; > timer = &odp_timer.timer[id]; > > cur_tick = timer->cur_tick; > @@ -317,17 +350,17 @@ odp_timer_tmo_t odp_timer_absolute_tmo(odp_timer_t timer_hdl, uint64_t tmo_tick, > return ODP_TIMER_TMO_INVALID; > } > > - tick = tmo_tick - cur_tick; > - if (tick > MAX_TICKS) { > - ODP_DBG("timeout too far\n"); > + if ((tmo_tick - cur_tick) > MAX_TICKS) { > + ODP_DBG("timeout too far: cur %"PRIu64" tmo %"PRIu64"\n", > + cur_tick, tmo_tick); > return ODP_TIMER_TMO_INVALID; > } > > - tick = (cur_tick + tick) % MAX_TICKS; > + tick = tmo_tick % MAX_TICKS; > > tmo_buf = odp_buffer_alloc(timer->pool); > if (tmo_buf == ODP_BUFFER_INVALID) { > - ODP_DBG("alloc failed\n"); > + ODP_DBG("tmo buffer alloc failed\n"); > return ODP_TIMER_TMO_INVALID; > } > > -- > 2.1.0 > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp
> -----Original Message----- > From: ext Anders Roxell [mailto:anders.roxell@linaro.org] > Sent: Tuesday, September 02, 2014 10:07 PM > To: Petri Savolainen > Cc: lng-odp@lists.linaro.org > Subject: Re: [lng-odp] [PATCH v2 1/2] Use timer resolution > > On 2014-09-02 15:47, Petri Savolainen wrote: > > Use resolution and min/max tmo values from timer create call. > > Use common nanosec time defines. > > > > Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org> > > --- > > platform/linux-generic/include/api/odp_time.h | 5 ++ > > platform/linux-generic/odp_timer.c | 67 > ++++++++++++++++++++------- > > 2 files changed, 55 insertions(+), 17 deletions(-) > > > > diff --git a/platform/linux-generic/include/api/odp_time.h > b/platform/linux-generic/include/api/odp_time.h > > index 188d1fe..1770223 100644 > > --- a/platform/linux-generic/include/api/odp_time.h > > +++ b/platform/linux-generic/include/api/odp_time.h > > @@ -21,6 +21,11 @@ extern "C" { > > > > #include <odp_std_types.h> > > > > +/* Time in nanoseconds */ > > +#define ODP_TIME_USEC 1000UL /*< Microsecond in nsec */ > > +#define ODP_TIME_MSEC 1000000UL /*< Millisecond in nsec */ > > +#define ODP_TIME_SEC 1000000000UL /*< Second in nsec */ > > you need to add "/**<" and not "/*<" OK. > > > + > > > > /** > > * Current time in CPU cycles > > diff --git a/platform/linux-generic/odp_timer.c b/platform/linux- > generic/odp_timer.c > > index 1bf37f9..6e89f5c 100644 > > --- a/platform/linux-generic/odp_timer.c > > +++ b/platform/linux-generic/odp_timer.c > > @@ -6,6 +6,7 @@ > > > > #include <odp_timer.h> > > #include <odp_timer_internal.h> > > +#include <odp_time.h> > > #include <odp_buffer_pool_internal.h> > > #include <odp_internal.h> > > #include <odp_atomic.h> > > @@ -18,9 +19,14 @@ > > > > #include <string.h> > > > > +#define USEC ODP_TIME_USEC > > +#define MSEC ODP_TIME_MSEC > > +#define SEC ODP_TIME_SEC > > no, why duplicate it? What's the problem? For example this is shorter and more readable... sec = res / SEC; nsec = res - sec*SEC; than this. sec = res / ODP_TIME_SEC; nsec = res - sec*ODP_TIME_SEC; SEC is too generic definition for ODP API, so ODP_TIME_SEC was used (for nanoseconds, time bases could be added also but nanoseconds is the current default). -Petri
On Sep 3, 2014, at 2:07 AM, Savolainen, Petri (NSN - FI/Espoo) <petri.savolainen@nsn.com> wrote: > > >> -----Original Message----- >> From: ext Anders Roxell [mailto:anders.roxell@linaro.org] >> Sent: Tuesday, September 02, 2014 10:07 PM >> To: Petri Savolainen >> Cc: lng-odp@lists.linaro.org >> Subject: Re: [lng-odp] [PATCH v2 1/2] Use timer resolution >> >> On 2014-09-02 15:47, Petri Savolainen wrote: >>> Use resolution and min/max tmo values from timer create call. >>> Use common nanosec time defines. >>> >>> Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org> >>> --- >>> platform/linux-generic/include/api/odp_time.h | 5 ++ >>> platform/linux-generic/odp_timer.c | 67 >> ++++++++++++++++++++------- >>> 2 files changed, 55 insertions(+), 17 deletions(-) >>> >>> diff --git a/platform/linux-generic/include/api/odp_time.h >> b/platform/linux-generic/include/api/odp_time.h >>> index 188d1fe..1770223 100644 >>> --- a/platform/linux-generic/include/api/odp_time.h >>> +++ b/platform/linux-generic/include/api/odp_time.h >>> @@ -21,6 +21,11 @@ extern "C" { >>> >>> #include <odp_std_types.h> >>> >>> +/* Time in nanoseconds */ >>> +#define ODP_TIME_USEC 1000UL /*< Microsecond in nsec */ >>> +#define ODP_TIME_MSEC 1000000UL /*< Millisecond in nsec */ >>> +#define ODP_TIME_SEC 1000000000UL /*< Second in nsec */ >> >> you need to add "/**<" and not "/*<" > > OK. > >> >>> + >>> >>> /** >>> * Current time in CPU cycles >>> diff --git a/platform/linux-generic/odp_timer.c b/platform/linux- >> generic/odp_timer.c >>> index 1bf37f9..6e89f5c 100644 >>> --- a/platform/linux-generic/odp_timer.c >>> +++ b/platform/linux-generic/odp_timer.c >>> @@ -6,6 +6,7 @@ >>> >>> #include <odp_timer.h> >>> #include <odp_timer_internal.h> >>> +#include <odp_time.h> >>> #include <odp_buffer_pool_internal.h> >>> #include <odp_internal.h> >>> #include <odp_atomic.h> >>> @@ -18,9 +19,14 @@ >>> >>> #include <string.h> >>> >>> +#define USEC ODP_TIME_USEC >>> +#define MSEC ODP_TIME_MSEC >>> +#define SEC ODP_TIME_SEC >> >> no, why duplicate it? > > What's the problem? The only problem I see with using the shorting names is possible name space collision with pre-existing code. As much as I like the shorter names the ODP_TIME_XXX prefix is safer and the one I would have picked. If we already have ODP_TIME_XXX defines then using them instead of creating new defines just to save a few keystrokes is not worth it IMO. As for readability they both are readable and when I see ODP_TIME_XXX I know for a fact it is an ODP define. > > For example this is shorter and more readable... > sec = res / SEC; > nsec = res - sec*SEC; > > than this. > sec = res / ODP_TIME_SEC; > nsec = res - sec*ODP_TIME_SEC; > > > SEC is too generic definition for ODP API, so ODP_TIME_SEC was used (for nanoseconds, time bases could be added also but nanoseconds is the current default). > > -Petri > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533
> -----Original Message----- > From: ext Wiles, Roger Keith [mailto:keith.wiles@windriver.com] > Sent: Wednesday, September 03, 2014 10:31 AM > To: Savolainen, Petri (NSN - FI/Espoo) > Cc: ext Anders Roxell; Petri Savolainen; lng-odp@lists.linaro.org > Subject: Re: [lng-odp] [PATCH v2 1/2] Use timer resolution > > > On Sep 3, 2014, at 2:07 AM, Savolainen, Petri (NSN - FI/Espoo) > <petri.savolainen@nsn.com> wrote: > > > > > > >> -----Original Message----- > >> From: ext Anders Roxell [mailto:anders.roxell@linaro.org] > >> Sent: Tuesday, September 02, 2014 10:07 PM > >> To: Petri Savolainen > >> Cc: lng-odp@lists.linaro.org > >> Subject: Re: [lng-odp] [PATCH v2 1/2] Use timer resolution > >> > >> On 2014-09-02 15:47, Petri Savolainen wrote: > >>> Use resolution and min/max tmo values from timer create call. > >>> Use common nanosec time defines. > >>> > >>> Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org> > >>> --- > >>> platform/linux-generic/include/api/odp_time.h | 5 ++ > >>> platform/linux-generic/odp_timer.c | 67 > >> ++++++++++++++++++++------- > >>> 2 files changed, 55 insertions(+), 17 deletions(-) > >>> > >>> diff --git a/platform/linux-generic/include/api/odp_time.h > >> b/platform/linux-generic/include/api/odp_time.h > >>> index 188d1fe..1770223 100644 > >>> --- a/platform/linux-generic/include/api/odp_time.h > >>> +++ b/platform/linux-generic/include/api/odp_time.h > >>> @@ -21,6 +21,11 @@ extern "C" { > >>> > >>> #include <odp_std_types.h> > >>> > >>> +/* Time in nanoseconds */ > >>> +#define ODP_TIME_USEC 1000UL /*< Microsecond in nsec */ > >>> +#define ODP_TIME_MSEC 1000000UL /*< Millisecond in nsec */ > >>> +#define ODP_TIME_SEC 1000000000UL /*< Second in nsec */ > >> > >> you need to add "/**<" and not "/*<" > > > > OK. > > > >> > >>> + > >>> > >>> /** > >>> * Current time in CPU cycles > >>> diff --git a/platform/linux-generic/odp_timer.c b/platform/linux- > >> generic/odp_timer.c > >>> index 1bf37f9..6e89f5c 100644 > >>> --- a/platform/linux-generic/odp_timer.c > >>> +++ b/platform/linux-generic/odp_timer.c > >>> @@ -6,6 +6,7 @@ > >>> > >>> #include <odp_timer.h> > >>> #include <odp_timer_internal.h> > >>> +#include <odp_time.h> > >>> #include <odp_buffer_pool_internal.h> > >>> #include <odp_internal.h> > >>> #include <odp_atomic.h> > >>> @@ -18,9 +19,14 @@ > >>> > >>> #include <string.h> > >>> > >>> +#define USEC ODP_TIME_USEC > >>> +#define MSEC ODP_TIME_MSEC > >>> +#define SEC ODP_TIME_SEC > >> > >> no, why duplicate it? > > > > What's the problem? > > The only problem I see with using the shorting names is possible name > space collision with pre-existing code. As much as I like the shorter > names the ODP_TIME_XXX prefix is safer and the one I would have picked. If > we already have ODP_TIME_XXX defines then using them instead of creating > new defines just to save a few keystrokes is not worth it IMO. As for > readability they both are readable and when I see ODP_TIME_XXX I know for > a fact it is an ODP define. Compiler would catch it, if I'd include some file that already defined SEC. This is internal to implementation (.c files) and according to coding rules (== checkpatch). So it's more matter of taste than a technical issue. -Petri
diff --git a/platform/linux-generic/include/api/odp_time.h b/platform/linux-generic/include/api/odp_time.h index 188d1fe..1770223 100644 --- a/platform/linux-generic/include/api/odp_time.h +++ b/platform/linux-generic/include/api/odp_time.h @@ -21,6 +21,11 @@ extern "C" { #include <odp_std_types.h> +/* Time in nanoseconds */ +#define ODP_TIME_USEC 1000UL /*< Microsecond in nsec */ +#define ODP_TIME_MSEC 1000000UL /*< Millisecond in nsec */ +#define ODP_TIME_SEC 1000000000UL /*< Second in nsec */ + /** * Current time in CPU cycles diff --git a/platform/linux-generic/odp_timer.c b/platform/linux-generic/odp_timer.c index 1bf37f9..6e89f5c 100644 --- a/platform/linux-generic/odp_timer.c +++ b/platform/linux-generic/odp_timer.c @@ -6,6 +6,7 @@ #include <odp_timer.h> #include <odp_timer_internal.h> +#include <odp_time.h> #include <odp_buffer_pool_internal.h> #include <odp_internal.h> #include <odp_atomic.h> @@ -18,9 +19,14 @@ #include <string.h> +#define USEC ODP_TIME_USEC +#define MSEC ODP_TIME_MSEC +#define SEC ODP_TIME_SEC + #define NUM_TIMERS 1 #define MAX_TICKS 1024 -#define RESOLUTION_NS 1000000 +#define MAX_RES SEC +#define MIN_RES (100*USEC) typedef struct { @@ -112,13 +118,13 @@ static int find_and_del_tmo(timeout_t **tmo, odp_timer_tmo_t handle) int odp_timer_cancel_tmo(odp_timer_t timer_hdl, odp_timer_tmo_t tmo) { int id; - uint64_t tick_idx; + int tick_idx; timeout_t *cancel_tmo; odp_timeout_hdr_t *tmo_hdr; tick_t *tick; /* get id */ - id = timer_hdl - 1; + id = (int)timer_hdl - 1; tmo_hdr = odp_timeout_hdr((odp_timeout_t) tmo); /* get tmo_buf to cancel */ @@ -179,6 +185,7 @@ static void timer_start(timer_ring_t *timer) { struct sigevent sigev; struct itimerspec ispec; + uint64_t res, sec, nsec; ODP_DBG("\nTimer (%u) starts\n", timer->timer_hdl); @@ -194,10 +201,14 @@ static void timer_start(timer_ring_t *timer) return; } - ispec.it_interval.tv_sec = 0; - ispec.it_interval.tv_nsec = RESOLUTION_NS; - ispec.it_value.tv_sec = 0; - ispec.it_value.tv_nsec = RESOLUTION_NS; + res = timer->resolution_ns; + sec = res / SEC; + nsec = res - sec*SEC; + + ispec.it_interval.tv_sec = (time_t)sec; + ispec.it_interval.tv_nsec = (long)nsec; + ispec.it_value.tv_sec = (time_t)sec; + ispec.it_value.tv_nsec = (long)nsec; if (timer_settime(timer->timerid, 0, &ispec, NULL)) { ODP_DBG("Timer set failed\n"); @@ -250,19 +261,41 @@ int odp_timer_disarm_all(void) } 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) + uint64_t resolution_ns, uint64_t min_ns, + uint64_t max_ns) { uint32_t id; timer_ring_t *timer; odp_timer_t timer_hdl; int i; - (void) name; (void) resolution; (void) min_tmo; (void) max_tmo; + uint64_t max_ticks; + (void) name; + + if (resolution_ns < MIN_RES) + resolution_ns = MIN_RES; + + if (resolution_ns > MAX_RES) + resolution_ns = MAX_RES; + + max_ticks = max_ns / resolution_ns; + + if (max_ticks > MAX_TICKS) { + ODP_DBG("Maximum timeout too long: %"PRIu64" ticks\n", + max_ticks); + return ODP_TIMER_INVALID; + } + + if (min_ns < resolution_ns) { + ODP_DBG("Min timeout %"PRIu64" ns < resolution %"PRIu64" ns\n", + min_ns, resolution_ns); + return ODP_TIMER_INVALID; + } odp_spinlock_lock(&odp_timer.lock); if (odp_timer.num_timers >= NUM_TIMERS) { odp_spinlock_unlock(&odp_timer.lock); + ODP_DBG("All timers allocated\n"); return ODP_TIMER_INVALID; } @@ -281,7 +314,7 @@ odp_timer_t odp_timer_create(const char *name, odp_buffer_pool_t pool, timer->timer_hdl = timer_hdl; timer->pool = pool; - timer->resolution_ns = RESOLUTION_NS; + timer->resolution_ns = resolution_ns; timer->max_ticks = MAX_TICKS; for (i = 0; i < MAX_TICKS; i++) { @@ -308,7 +341,7 @@ odp_timer_tmo_t odp_timer_absolute_tmo(odp_timer_t timer_hdl, uint64_t tmo_tick, odp_timeout_hdr_t *tmo_hdr; timer_ring_t *timer; - id = timer_hdl - 1; + id = (int)timer_hdl - 1; timer = &odp_timer.timer[id]; cur_tick = timer->cur_tick; @@ -317,17 +350,17 @@ odp_timer_tmo_t odp_timer_absolute_tmo(odp_timer_t timer_hdl, uint64_t tmo_tick, return ODP_TIMER_TMO_INVALID; } - tick = tmo_tick - cur_tick; - if (tick > MAX_TICKS) { - ODP_DBG("timeout too far\n"); + if ((tmo_tick - cur_tick) > MAX_TICKS) { + ODP_DBG("timeout too far: cur %"PRIu64" tmo %"PRIu64"\n", + cur_tick, tmo_tick); return ODP_TIMER_TMO_INVALID; } - tick = (cur_tick + tick) % MAX_TICKS; + tick = tmo_tick % MAX_TICKS; tmo_buf = odp_buffer_alloc(timer->pool); if (tmo_buf == ODP_BUFFER_INVALID) { - ODP_DBG("alloc failed\n"); + ODP_DBG("tmo buffer alloc failed\n"); return ODP_TIMER_TMO_INVALID; }
Use resolution and min/max tmo values from timer create call. Use common nanosec time defines. Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org> --- platform/linux-generic/include/api/odp_time.h | 5 ++ platform/linux-generic/odp_timer.c | 67 ++++++++++++++++++++------- 2 files changed, 55 insertions(+), 17 deletions(-)