Hi Bill/Maxim, I do not see any further comments, can we merge this to api-next?
Thanks, Honnappa 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 >>