On Mon, Jun 19, 2017 at 4:19 PM, Honnappa Nagarahalli <honnappa.nagaraha...@linaro.org> wrote: > Hi Bill/Maxim, > I do not see any further comments, can we merge this to api-next? > > Thanks, > Honnappa
Now that v1.15 is tagged we can unblock some of this pending work. Next step is to merge the v1.15 changes back into api-next so that we can continue with the merge of new APIs and features in preparation for v1.16. > > On 16 June 2017 at 10:59, Honnappa Nagarahalli > <honnappa.nagaraha...@linaro.org> wrote: >> Reviewed-by: Honnappa Nagarahalli <honnappa.nagaraha...@arm.com> >> >> On 14 June 2017 at 14:34, Brian Brooks <brian.bro...@arm.com> wrote: >>> Run timer pool processing on worker cores if the application hints >>> that the scheduler will be used. This reduces the latency and jitter >>> of the point at which timer pool processing begins. See [1] for details. >>> >>> [1] >>> https://docs.google.com/document/d/1sY7rOxqCNu-bMqjBiT5_keAIohrX1ZW-eL0oGLAQ4OM/edit?usp=sharing >>> >>> Signed-off-by: Brian Brooks <brian.bro...@arm.com> >>> Reviewed-by: Ola Liljedahl <ola.liljed...@arm.com> >>> Reviewed-by: Honnappa Nagarahalli <honnappa.nagaraha...@arm.com> >>> --- >>> >>> ** There is a false positive checkpatch.pl warning ** >>> >>> v4: >>> - Rebase against Bill's feature init patch >>> >>> v3: >>> - Add rate limiting by scheduling rounds >>> >>> v2: >>> - Reword 'worker_timers' to 'use_scheduler' >>> - Use time instead of ticks >>> >>> platform/linux-generic/include/odp_internal.h | 2 +- >>> .../linux-generic/include/odp_timer_internal.h | 24 +++++ >>> platform/linux-generic/odp_init.c | 2 +- >>> platform/linux-generic/odp_schedule.c | 3 + >>> platform/linux-generic/odp_schedule_iquery.c | 3 + >>> platform/linux-generic/odp_schedule_sp.c | 3 + >>> platform/linux-generic/odp_timer.c | 112 >>> +++++++++++++++++++-- >>> 7 files changed, 138 insertions(+), 11 deletions(-) >>> >>> diff --git a/platform/linux-generic/include/odp_internal.h >>> b/platform/linux-generic/include/odp_internal.h >>> index 90e2a629..404792cf 100644 >>> --- a/platform/linux-generic/include/odp_internal.h >>> +++ b/platform/linux-generic/include/odp_internal.h >>> @@ -108,7 +108,7 @@ int odp_queue_term_global(void); >>> int odp_crypto_init_global(void); >>> int odp_crypto_term_global(void); >>> >>> -int odp_timer_init_global(void); >>> +int odp_timer_init_global(const odp_init_t *params); >>> int odp_timer_term_global(void); >>> int odp_timer_disarm_all(void); >>> >>> diff --git a/platform/linux-generic/include/odp_timer_internal.h >>> b/platform/linux-generic/include/odp_timer_internal.h >>> index 91b12c54..67ee9fef 100644 >>> --- a/platform/linux-generic/include/odp_timer_internal.h >>> +++ b/platform/linux-generic/include/odp_timer_internal.h >>> @@ -20,6 +20,12 @@ >>> #include <odp_pool_internal.h> >>> #include <odp/api/timer.h> >>> >>> +/* Minimum number of nanoseconds between checking timer pools. */ >>> +#define CONFIG_TIMER_RUN_RATELIMIT_NS 100 >>> + >>> +/* Minimum number of scheduling rounds between checking timer pools. */ >>> +#define CONFIG_TIMER_RUN_RATELIMIT_ROUNDS 1 >>> + >>> /** >>> * Internal Timeout header >>> */ >>> @@ -35,4 +41,22 @@ typedef struct { >>> odp_timer_t timer; >>> } odp_timeout_hdr_t; >>> >>> +/* >>> + * Whether to run timer pool processing 'inline' (on worker cores) or in >>> + * background threads (thread-per-timerpool). >>> + * >>> + * If the application will use both scheduler and timer this flag is set >>> + * to true, otherwise false. This application conveys this information via >>> + * the 'not_used' bits in odp_init_t which are passed to odp_global_init(). >>> + */ >>> +extern odp_bool_t inline_timers; >>> + >>> +unsigned _timer_run(void); >>> + >>> +/* Static inline wrapper to minimize modification of schedulers. */ >>> +static inline unsigned timer_run(void) >>> +{ >>> + return inline_timers ? _timer_run() : 0; >>> +} >>> + >>> #endif >>> diff --git a/platform/linux-generic/odp_init.c >>> b/platform/linux-generic/odp_init.c >>> index 62a1fbc2..8c17cbb0 100644 >>> --- a/platform/linux-generic/odp_init.c >>> +++ b/platform/linux-generic/odp_init.c >>> @@ -241,7 +241,7 @@ int odp_init_global(odp_instance_t *instance, >>> } >>> stage = PKTIO_INIT; >>> >>> - if (odp_timer_init_global()) { >>> + if (odp_timer_init_global(params)) { >>> ODP_ERR("ODP timer init failed.\n"); >>> goto init_failed; >>> } >>> diff --git a/platform/linux-generic/odp_schedule.c >>> b/platform/linux-generic/odp_schedule.c >>> index 011d4dc4..04d09981 100644 >>> --- a/platform/linux-generic/odp_schedule.c >>> +++ b/platform/linux-generic/odp_schedule.c >>> @@ -23,6 +23,7 @@ >>> #include <odp/api/packet_io.h> >>> #include <odp_ring_internal.h> >>> #include <odp_queue_internal.h> >>> +#include <odp_timer_internal.h> >>> >>> /* Number of priority levels */ >>> #define NUM_PRIO 8 >>> @@ -998,6 +999,8 @@ static int schedule_loop(odp_queue_t *out_queue, >>> uint64_t wait, >>> int ret; >>> >>> while (1) { >>> + timer_run(); >>> + >>> ret = do_schedule(out_queue, out_ev, max_num); >>> >>> if (ret) >>> diff --git a/platform/linux-generic/odp_schedule_iquery.c >>> b/platform/linux-generic/odp_schedule_iquery.c >>> index bdf1a460..f7c411f6 100644 >>> --- a/platform/linux-generic/odp_schedule_iquery.c >>> +++ b/platform/linux-generic/odp_schedule_iquery.c >>> @@ -23,6 +23,7 @@ >>> #include <odp/api/thrmask.h> >>> #include <odp/api/packet_io.h> >>> #include <odp_config_internal.h> >>> +#include <odp_timer_internal.h> >>> >>> /* Number of priority levels */ >>> #define NUM_SCHED_PRIO 8 >>> @@ -719,6 +720,8 @@ static int schedule_loop(odp_queue_t *out_queue, >>> uint64_t wait, >>> odp_time_t next, wtime; >>> >>> while (1) { >>> + timer_run(); >>> + >>> count = do_schedule(out_queue, out_ev, max_num); >>> >>> if (count) >>> diff --git a/platform/linux-generic/odp_schedule_sp.c >>> b/platform/linux-generic/odp_schedule_sp.c >>> index 91d70e3a..252d128d 100644 >>> --- a/platform/linux-generic/odp_schedule_sp.c >>> +++ b/platform/linux-generic/odp_schedule_sp.c >>> @@ -15,6 +15,7 @@ >>> #include <odp_align_internal.h> >>> #include <odp_config_internal.h> >>> #include <odp_ring_internal.h> >>> +#include <odp_timer_internal.h> >>> >>> #define NUM_THREAD ODP_THREAD_COUNT_MAX >>> #define NUM_QUEUE ODP_CONFIG_QUEUES >>> @@ -517,6 +518,8 @@ static int schedule_multi(odp_queue_t *from, uint64_t >>> wait, >>> uint32_t qi; >>> int num; >>> >>> + timer_run(); >>> + >>> cmd = sched_cmd(); >>> >>> if (cmd && cmd->s.type == CMD_PKTIO) { >>> diff --git a/platform/linux-generic/odp_timer.c >>> b/platform/linux-generic/odp_timer.c >>> index cf610bfa..bf7f1acd 100644 >>> --- a/platform/linux-generic/odp_timer.c >>> +++ b/platform/linux-generic/odp_timer.c >>> @@ -52,6 +52,7 @@ >>> #include <odp/api/sync.h> >>> #include <odp/api/time.h> >>> #include <odp/api/timer.h> >>> +#include <odp_time_internal.h> >>> #include <odp_timer_internal.h> >>> >>> #define TMO_UNUSED ((uint64_t)0xFFFFFFFFFFFFFFFF) >>> @@ -60,6 +61,8 @@ >>> * for checking the freshness of received timeouts */ >>> #define TMO_INACTIVE ((uint64_t)0x8000000000000000) >>> >>> +odp_bool_t inline_timers = false; >>> + >>> >>> /****************************************************************************** >>> * Mutual exclusion in the absence of CAS16 >>> >>> *****************************************************************************/ >>> @@ -165,7 +168,8 @@ static inline void set_next_free(odp_timer *tim, >>> uint32_t nf) >>> >>> *****************************************************************************/ >>> >>> typedef struct odp_timer_pool_s { >>> -/* Put frequently accessed fields in the first cache line */ >>> + odp_time_t prev_scan; /* Time when previous scan started */ >>> + odp_time_t time_per_tick; /* Time per timer pool tick */ >>> odp_atomic_u64_t cur_tick;/* Current tick value */ >>> uint64_t min_rel_tck; >>> uint64_t max_rel_tck; >>> @@ -242,6 +246,8 @@ static odp_timer_pool_t odp_timer_pool_new(const char >>> *name, >>> ODP_ABORT("%s: timer pool shm-alloc(%zuKB) failed\n", >>> name, (sz0 + sz1 + sz2) / 1024); >>> odp_timer_pool *tp = (odp_timer_pool *)odp_shm_addr(shm); >>> + tp->prev_scan = odp_time_global(); >>> + tp->time_per_tick = odp_time_global_from_ns(param->res_ns); >>> odp_atomic_init_u64(&tp->cur_tick, 0); >>> >>> if (name == NULL) { >>> @@ -276,8 +282,10 @@ static odp_timer_pool_t odp_timer_pool_new(const char >>> *name, >>> tp->tp_idx = tp_idx; >>> odp_spinlock_init(&tp->lock); >>> timer_pool[tp_idx] = tp; >>> - if (tp->param.clk_src == ODP_CLOCK_CPU) >>> - itimer_init(tp); >>> + if (!inline_timers) { >>> + if (tp->param.clk_src == ODP_CLOCK_CPU) >>> + itimer_init(tp); >>> + } >>> return tp; >>> } >>> >>> @@ -306,11 +314,13 @@ static void odp_timer_pool_del(odp_timer_pool *tp) >>> odp_spinlock_lock(&tp->lock); >>> timer_pool[tp->tp_idx] = NULL; >>> >>> - /* Stop timer triggering */ >>> - if (tp->param.clk_src == ODP_CLOCK_CPU) >>> - itimer_fini(tp); >>> + if (!inline_timers) { >>> + /* Stop POSIX itimer signals */ >>> + if (tp->param.clk_src == ODP_CLOCK_CPU) >>> + itimer_fini(tp); >>> >>> - stop_timer_thread(tp); >>> + stop_timer_thread(tp); >>> + } >>> >>> if (tp->num_alloc != 0) { >>> /* It's a programming error to attempt to destroy a */ >>> @@ -671,6 +681,81 @@ static unsigned odp_timer_pool_expire(odp_timer_pool_t >>> tpid, uint64_t tick) >>> } >>> >>> >>> /****************************************************************************** >>> + * Inline timer processing >>> + >>> *****************************************************************************/ >>> + >>> +static unsigned process_timer_pools(void) >>> +{ >>> + odp_timer_pool *tp; >>> + odp_time_t prev_scan, now; >>> + uint64_t nticks; >>> + unsigned nexp = 0; >>> + >>> + for (size_t i = 0; i < MAX_TIMER_POOLS; i++) { >>> + tp = timer_pool[i]; >>> + >>> + if (tp == NULL) >>> + continue; >>> + >>> + /* >>> + * Check the last time this timer pool was expired. If one >>> + * or more periods have passed, attempt to expire it. >>> + */ >>> + prev_scan = tp->prev_scan; >>> + now = odp_time_global(); >>> + >>> + nticks = (now.u64 - prev_scan.u64) / tp->time_per_tick.u64; >>> + >>> + if (nticks < 1) >>> + continue; >>> + >>> + if (__atomic_compare_exchange_n( >>> + &tp->prev_scan.u64, &prev_scan.u64, >>> + prev_scan.u64 + (tp->time_per_tick.u64 * >>> nticks), >>> + false, __ATOMIC_RELAXED, __ATOMIC_RELAXED)) { >>> + uint64_t tp_tick = _odp_atomic_u64_fetch_add_mm( >>> + &tp->cur_tick, nticks, _ODP_MEMMODEL_RLX); >>> + >>> + if (tp->notify_overrun && nticks > 1) { >>> + ODP_ERR("\n\t%d ticks overrun on timer pool >>> " >>> + "\"%s\", timer resolution too >>> high\n", >>> + nticks - 1, tp->name); >>> + tp->notify_overrun = 0; >>> + } >>> + nexp += odp_timer_pool_expire(tp, tp_tick + nticks); >>> + } >>> + } >>> + return nexp; >>> +} >>> + >>> +static odp_time_t time_per_ratelimit_period; >>> + >>> +unsigned _timer_run(void) >>> +{ >>> + static __thread odp_time_t last_timer_run; >>> + static __thread unsigned timer_run_cnt = >>> + CONFIG_TIMER_RUN_RATELIMIT_ROUNDS; >>> + odp_time_t now; >>> + >>> + /* Rate limit how often this thread checks the timer pools. */ >>> + >>> + if (CONFIG_TIMER_RUN_RATELIMIT_ROUNDS > 1) { >>> + if (--timer_run_cnt) >>> + return 0; >>> + timer_run_cnt = CONFIG_TIMER_RUN_RATELIMIT_ROUNDS; >>> + } >>> + >>> + now = odp_time_global(); >>> + if (odp_time_cmp(odp_time_diff(now, last_timer_run), >>> + time_per_ratelimit_period) == -1) >>> + return 0; >>> + last_timer_run = now; >>> + >>> + /* Check the timer pools. */ >>> + return process_timer_pools(); >>> +} >>> + >>> +/****************************************************************************** >>> * POSIX timer support >>> * Functions that use Linux/POSIX per-process timers and related facilities >>> >>> *****************************************************************************/ >>> @@ -989,7 +1074,7 @@ void odp_timeout_free(odp_timeout_t tmo) >>> odp_buffer_free(odp_buffer_from_event(ev)); >>> } >>> >>> -int odp_timer_init_global(void) >>> +int odp_timer_init_global(const odp_init_t *params) >>> { >>> #ifndef ODP_ATOMIC_U128 >>> uint32_t i; >>> @@ -1000,7 +1085,16 @@ int odp_timer_init_global(void) >>> #endif >>> odp_atomic_init_u32(&num_timer_pools, 0); >>> >>> - block_sigalarm(); >>> + if (params) >>> + inline_timers = >>> + !params->not_used.feat.schedule && >>> + !params->not_used.feat.timer; >>> + >>> + time_per_ratelimit_period = >>> + odp_time_global_from_ns(CONFIG_TIMER_RUN_RATELIMIT_NS); >>> + >>> + if (!inline_timers) >>> + block_sigalarm(); >>> >>> return 0; >>> } >>> -- >>> 2.13.1 >>>
