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
<[email protected]> wrote:
> Reviewed-by: Honnappa Nagarahalli <[email protected]>
>
> On 14 June 2017 at 14:34, Brian Brooks <[email protected]> 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 <[email protected]>
>> Reviewed-by: Ola Liljedahl <[email protected]>
>> Reviewed-by: Honnappa Nagarahalli <[email protected]>
>> ---
>>
>> ** 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
>>
