[API-NEXT,v4] timer: allow timer processing to run on worker cores

Message ID 20170614193450.78477-1-brian.brooks@arm.com
State New
Headers show

Commit Message

Brian Brooks June 14, 2017, 7:34 p.m.
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.brooks@arm.com>

Reviewed-by: Ola Liljedahl <ola.liljedahl@arm.com>

Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@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(-)

-- 
2.13.1

Comments

Honnappa Nagarahalli June 16, 2017, 3:59 p.m. | #1
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>


On 14 June 2017 at 14:34, Brian Brooks <brian.brooks@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.brooks@arm.com>

> Reviewed-by: Ola Liljedahl <ola.liljedahl@arm.com>

> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@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

>
Honnappa Nagarahalli June 19, 2017, 9:19 p.m. | #2
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.nagarahalli@linaro.org> wrote:
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>

>

> On 14 June 2017 at 14:34, Brian Brooks <brian.brooks@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.brooks@arm.com>

>> Reviewed-by: Ola Liljedahl <ola.liljedahl@arm.com>

>> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@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

>>
Bill Fischofer June 19, 2017, 10:42 p.m. | #3
On Mon, Jun 19, 2017 at 4:19 PM, Honnappa Nagarahalli
<honnappa.nagarahalli@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.nagarahalli@linaro.org> wrote:

>> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>

>>

>> On 14 June 2017 at 14:34, Brian Brooks <brian.brooks@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.brooks@arm.com>

>>> Reviewed-by: Ola Liljedahl <ola.liljedahl@arm.com>

>>> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@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

>>>
Honnappa Nagarahalli June 20, 2017, 4:06 a.m. | #4
Are you saying we should be good to merge this now?

On 19 June 2017 at 17:42, Bill Fischofer <bill.fischofer@linaro.org> wrote:
> On Mon, Jun 19, 2017 at 4:19 PM, Honnappa Nagarahalli

> <honnappa.nagarahalli@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.nagarahalli@linaro.org> wrote:

>>> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>

>>>

>>> On 14 June 2017 at 14:34, Brian Brooks <brian.brooks@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.brooks@arm.com>

>>>> Reviewed-by: Ola Liljedahl <ola.liljedahl@arm.com>

>>>> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@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

>>>>
Savolainen, Petri (Nokia - FI/Espoo) June 20, 2017, 7:34 a.m. | #5
Do you have some performance numbers? E.g. how much this slows down an application which does not use timers (e.g. l2fwd), or an application that uses only few, non-frequent timeouts?

Additionally, init.h/feature.h is not yet in api-next - so this would not build yet.


-Petri


> -----Original Message-----

> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of

> Honnappa Nagarahalli

> Sent: Tuesday, June 20, 2017 7:07 AM

> To: Bill Fischofer <bill.fischofer@linaro.org>

> Cc: lng-odp-forward <lng-odp@lists.linaro.org>

> Subject: Re: [lng-odp] [API-NEXT PATCH v4] timer: allow timer processing

> to run on worker cores

> 

> Are you saying we should be good to merge this now?

> 

> On 19 June 2017 at 17:42, Bill Fischofer <bill.fischofer@linaro.org>

> wrote:

> > On Mon, Jun 19, 2017 at 4:19 PM, Honnappa Nagarahalli

> > <honnappa.nagarahalli@linaro.org> wrote:

> >> Hi Bill/Maxim,

> >>     I do not see any further comments, can we merge this to api-next?

> >>

> >> Thanks,

> >> Honnappa
Honnappa Nagarahalli June 21, 2017, 6:52 p.m. | #6
We have not run any performance application. In our Linaro connect
meeting, we presented numbers on how it improves the timer resolution.
At this point, there is enough configuration options to control the
effect of calling timer in the scheduler. For applications that do not
want to use the timer, there should not be any change. For
applications that use timers non-frequently, the check frequency can
be controlled via the provided configuration options.

On 20 June 2017 at 02:34, Savolainen, Petri (Nokia - FI/Espoo)
<petri.savolainen@nokia.com> wrote:
> Do you have some performance numbers? E.g. how much this slows down an application which does not use timers (e.g. l2fwd), or an application that uses only few, non-frequent timeouts?

>

> Additionally, init.h/feature.h is not yet in api-next - so this would not build yet.

>

>

> -Petri

>

>

>> -----Original Message-----

>> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of

>> Honnappa Nagarahalli

>> Sent: Tuesday, June 20, 2017 7:07 AM

>> To: Bill Fischofer <bill.fischofer@linaro.org>

>> Cc: lng-odp-forward <lng-odp@lists.linaro.org>

>> Subject: Re: [lng-odp] [API-NEXT PATCH v4] timer: allow timer processing

>> to run on worker cores

>>

>> Are you saying we should be good to merge this now?

>>

>> On 19 June 2017 at 17:42, Bill Fischofer <bill.fischofer@linaro.org>

>> wrote:

>> > On Mon, Jun 19, 2017 at 4:19 PM, Honnappa Nagarahalli

>> > <honnappa.nagarahalli@linaro.org> wrote:

>> >> Hi Bill/Maxim,

>> >>     I do not see any further comments, can we merge this to api-next?

>> >>

>> >> Thanks,

>> >> Honnappa

>

>
Maxim Uvarov June 22, 2017, 8:22 a.m. | #7
Petri, do you want to test performance before patch inclusion?

Maxim.

On 21 June 2017 at 21:52, Honnappa Nagarahalli <
honnappa.nagarahalli@linaro.org> wrote:

> We have not run any performance application. In our Linaro connect

> meeting, we presented numbers on how it improves the timer resolution.

> At this point, there is enough configuration options to control the

> effect of calling timer in the scheduler. For applications that do not

> want to use the timer, there should not be any change. For

> applications that use timers non-frequently, the check frequency can

> be controlled via the provided configuration options.

>

> On 20 June 2017 at 02:34, Savolainen, Petri (Nokia - FI/Espoo)

> <petri.savolainen@nokia.com> wrote:

> > Do you have some performance numbers? E.g. how much this slows down an

> application which does not use timers (e.g. l2fwd), or an application that

> uses only few, non-frequent timeouts?

> >

> > Additionally, init.h/feature.h is not yet in api-next - so this would

> not build yet.

> >

> >

> > -Petri

> >

> >

> >> -----Original Message-----

> >> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of

> >> Honnappa Nagarahalli

> >> Sent: Tuesday, June 20, 2017 7:07 AM

> >> To: Bill Fischofer <bill.fischofer@linaro.org>

> >> Cc: lng-odp-forward <lng-odp@lists.linaro.org>

> >> Subject: Re: [lng-odp] [API-NEXT PATCH v4] timer: allow timer processing

> >> to run on worker cores

> >>

> >> Are you saying we should be good to merge this now?

> >>

> >> On 19 June 2017 at 17:42, Bill Fischofer <bill.fischofer@linaro.org>

> >> wrote:

> >> > On Mon, Jun 19, 2017 at 4:19 PM, Honnappa Nagarahalli

> >> > <honnappa.nagarahalli@linaro.org> wrote:

> >> >> Hi Bill/Maxim,

> >> >>     I do not see any further comments, can we merge this to api-next?

> >> >>

> >> >> Thanks,

> >> >> Honnappa

> >

> >

>
Savolainen, Petri (Nokia - FI/Espoo) June 22, 2017, 10:27 a.m. | #8
I was asking to make sure that performance impact has been checked also when timers are not used, e.g. l2fwd performance before and after the change. It would be also appropriate to test impact in the worst case: l2fwd type application + a periodic 1sec timeout. Timer is on, but timeouts come very unfrequently (compared to packets).

It seems that no performance tests were run, although the change affects performance of many applications (e.g. OFP has high packet rate with timers). Configuration options should be set with  defaults that are acceptable trade-off between packet processing performance and timeout accuracy.


-Petri


From: Maxim Uvarov [mailto:maxim.uvarov@linaro.org] 

Sent: Thursday, June 22, 2017 11:22 AM
To: Honnappa Nagarahalli <honnappa.nagarahalli@linaro.org>
Cc: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com>; lng-odp-forward <lng-odp@lists.linaro.org>
Subject: Re: [lng-odp] [API-NEXT PATCH v4] timer: allow timer processing to run on worker cores

Petri, do you want to test performance before patch inclusion?
Maxim.

On 21 June 2017 at 21:52, Honnappa Nagarahalli <mailto:honnappa.nagarahalli@linaro.org> wrote:
We have not run any performance application. In our Linaro connect
meeting, we presented numbers on how it improves the timer resolution.
At this point, there is enough configuration options to control the
effect of calling timer in the scheduler. For applications that do not
want to use the timer, there should not be any change. For
applications that use timers non-frequently, the check frequency can
be controlled via the provided configuration options.

On 20 June 2017 at 02:34, Savolainen, Petri (Nokia - FI/Espoo)
<mailto:petri.savolainen@nokia.com> wrote:
> Do you have some performance numbers? E.g. how much this slows down an application which does not use timers (e.g. l2fwd), or an application that uses only few, non-frequent timeouts?

>

> Additionally, init.h/feature.h is not yet in api-next - so this would not build yet.

>

>

> -Petri

>

>

>> -----Original Message-----

>> From: lng-odp [mailto:mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of

>> Honnappa Nagarahalli

>> Sent: Tuesday, June 20, 2017 7:07 AM

>> To: Bill Fischofer <mailto:bill.fischofer@linaro.org>

>> Cc: lng-odp-forward <mailto:lng-odp@lists.linaro.org>

>> Subject: Re: [lng-odp] [API-NEXT PATCH v4] timer: allow timer processing

>> to run on worker cores

>>

>> Are you saying we should be good to merge this now?

>>

>> On 19 June 2017 at 17:42, Bill Fischofer <mailto:bill.fischofer@linaro.org>

>> wrote:

>> > On Mon, Jun 19, 2017 at 4:19 PM, Honnappa Nagarahalli

>> > <mailto:honnappa.nagarahalli@linaro.org> wrote:

>> >> Hi Bill/Maxim,

>> >>     I do not see any further comments, can we merge this to api-next?

>> >>

>> >> Thanks,

>> >> Honnappa

>

>
Brian Brooks June 22, 2017, 2:55 p.m. | #9
On 06/22 10:27:01, Savolainen, Petri (Nokia - FI/Espoo) wrote:
> I was asking to make sure that performance impact has been checked also when timers are not used, e.g. l2fwd performance before and after the change. It would be also appropriate to test impact in the worst case: l2fwd type application + a periodic 1sec timeout. Timer is on, but timeouts come very unfrequently (compared to packets).

> 

> It seems that no performance tests were run, although the change affects performance of many applications (e.g. OFP has high packet rate with timers). Configuration options should be set with  defaults that are acceptable trade-off between packet processing performance and timeout accuracy.


If timers are not used, the overhead is just checking a RO variable
(post global init). If timers are used, CONFIG_ parameters have been
provided. The defaults for these parameters came from the work to
drastically reduce jitter of timer processing which is documented
here [1] and presented at Linaro Connect here [2].

If you speculate that these defaults might need to be changed, e.g.
l2fwd, we welcome collaboration and data. But, this is not a blocking
issue for this patch right now.

[1] https://docs.google.com/document/d/1sY7rOxqCNu-bMqjBiT5_keAIohrX1ZW-eL0oGLAQ4OM/edit?usp=sharing
[2] http://connect.linaro.org/resource/bud17/bud17-320/

> -Petri

> 

> 

> From: Maxim Uvarov [mailto:maxim.uvarov@linaro.org] 

> Sent: Thursday, June 22, 2017 11:22 AM

> To: Honnappa Nagarahalli <honnappa.nagarahalli@linaro.org>

> Cc: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com>; lng-odp-forward <lng-odp@lists.linaro.org>

> Subject: Re: [lng-odp] [API-NEXT PATCH v4] timer: allow timer processing to run on worker cores

> 

> Petri, do you want to test performance before patch inclusion?

> Maxim.

> 

> On 21 June 2017 at 21:52, Honnappa Nagarahalli <mailto:honnappa.nagarahalli@linaro.org> wrote:

> We have not run any performance application. In our Linaro connect

> meeting, we presented numbers on how it improves the timer resolution.

> At this point, there is enough configuration options to control the

> effect of calling timer in the scheduler. For applications that do not

> want to use the timer, there should not be any change. For

> applications that use timers non-frequently, the check frequency can

> be controlled via the provided configuration options.

> 

> On 20 June 2017 at 02:34, Savolainen, Petri (Nokia - FI/Espoo)

> <mailto:petri.savolainen@nokia.com> wrote:

> > Do you have some performance numbers? E.g. how much this slows down an application which does not use timers (e.g. l2fwd), or an application that uses only few, non-frequent timeouts?

> >

> > Additionally, init.h/feature.h is not yet in api-next - so this would not build yet.

> >

> >

> > -Petri

> >

> >

> >> -----Original Message-----

> >> From: lng-odp [mailto:mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of

> >> Honnappa Nagarahalli

> >> Sent: Tuesday, June 20, 2017 7:07 AM

> >> To: Bill Fischofer <mailto:bill.fischofer@linaro.org>

> >> Cc: lng-odp-forward <mailto:lng-odp@lists.linaro.org>

> >> Subject: Re: [lng-odp] [API-NEXT PATCH v4] timer: allow timer processing

> >> to run on worker cores

> >>

> >> Are you saying we should be good to merge this now?

> >>

> >> On 19 June 2017 at 17:42, Bill Fischofer <mailto:bill.fischofer@linaro.org>

> >> wrote:

> >> > On Mon, Jun 19, 2017 at 4:19 PM, Honnappa Nagarahalli

> >> > <mailto:honnappa.nagarahalli@linaro.org> wrote:

> >> >> Hi Bill/Maxim,

> >> >>     I do not see any further comments, can we merge this to api-next?

> >> >>

> >> >> Thanks,

> >> >> Honnappa

> >

> >

>
Maxim Uvarov June 22, 2017, 3:30 p.m. | #10
On 06/22/17 17:55, Brian Brooks wrote:
> On 06/22 10:27:01, Savolainen, Petri (Nokia - FI/Espoo) wrote:

>> I was asking to make sure that performance impact has been checked also when timers are not used, e.g. l2fwd performance before and after the change. It would be also appropriate to test impact in the worst case: l2fwd type application + a periodic 1sec timeout. Timer is on, but timeouts come very unfrequently (compared to packets).

>>

>> It seems that no performance tests were run, although the change affects performance of many applications (e.g. OFP has high packet rate with timers). Configuration options should be set with  defaults that are acceptable trade-off between packet processing performance and timeout accuracy.

> 

> If timers are not used, the overhead is just checking a RO variable

> (post global init). If timers are used, CONFIG_ parameters have been

> provided. The defaults for these parameters came from the work to

> drastically reduce jitter of timer processing which is documented

> here [1] and presented at Linaro Connect here [2].

> 

> If you speculate that these defaults might need to be changed, e.g.

> l2fwd, we welcome collaboration and data. But, this is not a blocking

> issue for this patch right now.

> 

> [1] https://docs.google.com/document/d/1sY7rOxqCNu-bMqjBiT5_keAIohrX1ZW-eL0oGLAQ4OM/edit?usp=sharing

> [2] http://connect.linaro.org/resource/bud17/bud17-320/

> 


1) we have all adjustable configs here
./platform/linux-generic/include/odp_config_internal.h
that might be also needs to be there.


2) Do we need something special in CI to check different config values?

3) Why it's compile time config values and not run time?

Maxim.


>> -Petri

>>

>>

>> From: Maxim Uvarov [mailto:maxim.uvarov@linaro.org] 

>> Sent: Thursday, June 22, 2017 11:22 AM

>> To: Honnappa Nagarahalli <honnappa.nagarahalli@linaro.org>

>> Cc: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com>; lng-odp-forward <lng-odp@lists.linaro.org>

>> Subject: Re: [lng-odp] [API-NEXT PATCH v4] timer: allow timer processing to run on worker cores

>>

>> Petri, do you want to test performance before patch inclusion?

>> Maxim.

>>

>> On 21 June 2017 at 21:52, Honnappa Nagarahalli <mailto:honnappa.nagarahalli@linaro.org> wrote:

>> We have not run any performance application. In our Linaro connect

>> meeting, we presented numbers on how it improves the timer resolution.

>> At this point, there is enough configuration options to control the

>> effect of calling timer in the scheduler. For applications that do not

>> want to use the timer, there should not be any change. For

>> applications that use timers non-frequently, the check frequency can

>> be controlled via the provided configuration options.

>>

>> On 20 June 2017 at 02:34, Savolainen, Petri (Nokia - FI/Espoo)

>> <mailto:petri.savolainen@nokia.com> wrote:

>>> Do you have some performance numbers? E.g. how much this slows down an application which does not use timers (e.g. l2fwd), or an application that uses only few, non-frequent timeouts?

>>>

>>> Additionally, init.h/feature.h is not yet in api-next - so this would not build yet.

>>>

>>>

>>> -Petri

>>>

>>>

>>>> -----Original Message-----

>>>> From: lng-odp [mailto:mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of

>>>> Honnappa Nagarahalli

>>>> Sent: Tuesday, June 20, 2017 7:07 AM

>>>> To: Bill Fischofer <mailto:bill.fischofer@linaro.org>

>>>> Cc: lng-odp-forward <mailto:lng-odp@lists.linaro.org>

>>>> Subject: Re: [lng-odp] [API-NEXT PATCH v4] timer: allow timer processing

>>>> to run on worker cores

>>>>

>>>> Are you saying we should be good to merge this now?

>>>>

>>>> On 19 June 2017 at 17:42, Bill Fischofer <mailto:bill.fischofer@linaro.org>

>>>> wrote:

>>>>> On Mon, Jun 19, 2017 at 4:19 PM, Honnappa Nagarahalli

>>>>> <mailto:honnappa.nagarahalli@linaro.org> wrote:

>>>>>> Hi Bill/Maxim,

>>>>>>      I do not see any further comments, can we merge this to api-next?

>>>>>>

>>>>>> Thanks,

>>>>>> Honnappa

>>>

>>>

>>
Brian Brooks June 22, 2017, 4:11 p.m. | #11
On 06/22 18:30:47, Maxim Uvarov wrote:
> On 06/22/17 17:55, Brian Brooks wrote:

> > On 06/22 10:27:01, Savolainen, Petri (Nokia - FI/Espoo) wrote:

> >> I was asking to make sure that performance impact has been checked also when timers are not used, e.g. l2fwd performance before and after the change. It would be also appropriate to test impact in the worst case: l2fwd type application + a periodic 1sec timeout. Timer is on, but timeouts come very unfrequently (compared to packets).

> >>

> >> It seems that no performance tests were run, although the change affects performance of many applications (e.g. OFP has high packet rate with timers). Configuration options should be set with  defaults that are acceptable trade-off between packet processing performance and timeout accuracy.

> > 

> > If timers are not used, the overhead is just checking a RO variable

> > (post global init). If timers are used, CONFIG_ parameters have been

> > provided. The defaults for these parameters came from the work to

> > drastically reduce jitter of timer processing which is documented

> > here [1] and presented at Linaro Connect here [2].

> > 

> > If you speculate that these defaults might need to be changed, e.g.

> > l2fwd, we welcome collaboration and data. But, this is not a blocking

> > issue for this patch right now.

> > 

> > [1] https://docs.google.com/document/d/1sY7rOxqCNu-bMqjBiT5_keAIohrX1ZW-eL0oGLAQ4OM/edit?usp=sharing

> > [2] http://connect.linaro.org/resource/bud17/bud17-320/

> > 

> 

> 1) we have all adjustable configs here

> ./platform/linux-generic/include/odp_config_internal.h

> that might be also needs to be there.


We had to move scalable scheduler CONFIG_ into include/odp_schedule_scalable_config.h
because it was decided that placing component-specific CONFIG_ in odp_config_internal.h
was not allowed.

> 2) Do we need something special in CI to check different config values?


No, the two CONFIG_ in this patch are related to timing. So, they do not
affect things like conditional compilation or enable/disable functionality.

> 3) Why it's compile time config values and not run time?


It is simpler.

> Maxim.

> 

> 

> >> -Petri

> >>

> >>

> >> From: Maxim Uvarov [mailto:maxim.uvarov@linaro.org] 

> >> Sent: Thursday, June 22, 2017 11:22 AM

> >> To: Honnappa Nagarahalli <honnappa.nagarahalli@linaro.org>

> >> Cc: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com>; lng-odp-forward <lng-odp@lists.linaro.org>

> >> Subject: Re: [lng-odp] [API-NEXT PATCH v4] timer: allow timer processing to run on worker cores

> >>

> >> Petri, do you want to test performance before patch inclusion?

> >> Maxim.

> >>

> >> On 21 June 2017 at 21:52, Honnappa Nagarahalli <mailto:honnappa.nagarahalli@linaro.org> wrote:

> >> We have not run any performance application. In our Linaro connect

> >> meeting, we presented numbers on how it improves the timer resolution.

> >> At this point, there is enough configuration options to control the

> >> effect of calling timer in the scheduler. For applications that do not

> >> want to use the timer, there should not be any change. For

> >> applications that use timers non-frequently, the check frequency can

> >> be controlled via the provided configuration options.

> >>

> >> On 20 June 2017 at 02:34, Savolainen, Petri (Nokia - FI/Espoo)

> >> <mailto:petri.savolainen@nokia.com> wrote:

> >>> Do you have some performance numbers? E.g. how much this slows down an application which does not use timers (e.g. l2fwd), or an application that uses only few, non-frequent timeouts?

> >>>

> >>> Additionally, init.h/feature.h is not yet in api-next - so this would not build yet.

> >>>

> >>>

> >>> -Petri

> >>>

> >>>

> >>>> -----Original Message-----

> >>>> From: lng-odp [mailto:mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of

> >>>> Honnappa Nagarahalli

> >>>> Sent: Tuesday, June 20, 2017 7:07 AM

> >>>> To: Bill Fischofer <mailto:bill.fischofer@linaro.org>

> >>>> Cc: lng-odp-forward <mailto:lng-odp@lists.linaro.org>

> >>>> Subject: Re: [lng-odp] [API-NEXT PATCH v4] timer: allow timer processing

> >>>> to run on worker cores

> >>>>

> >>>> Are you saying we should be good to merge this now?

> >>>>

> >>>> On 19 June 2017 at 17:42, Bill Fischofer <mailto:bill.fischofer@linaro.org>

> >>>> wrote:

> >>>>> On Mon, Jun 19, 2017 at 4:19 PM, Honnappa Nagarahalli

> >>>>> <mailto:honnappa.nagarahalli@linaro.org> wrote:

> >>>>>> Hi Bill/Maxim,

> >>>>>>      I do not see any further comments, can we merge this to api-next?

> >>>>>>

> >>>>>> Thanks,

> >>>>>> Honnappa

> >>>

> >>>

> >>

>
Honnappa Nagarahalli June 22, 2017, 6:48 p.m. | #12
On 22 June 2017 at 10:30, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> On 06/22/17 17:55, Brian Brooks wrote:

>> On 06/22 10:27:01, Savolainen, Petri (Nokia - FI/Espoo) wrote:

>>> I was asking to make sure that performance impact has been checked also when timers are not used, e.g. l2fwd performance before and after the change. It would be also appropriate to test impact in the worst case: l2fwd type application + a periodic 1sec timeout. Timer is on, but timeouts come very unfrequently (compared to packets).

>>>

>>> It seems that no performance tests were run, although the change affects performance of many applications (e.g. OFP has high packet rate with timers). Configuration options should be set with  defaults that are acceptable trade-off between packet processing performance and timeout accuracy.

>>

>> If timers are not used, the overhead is just checking a RO variable

>> (post global init). If timers are used, CONFIG_ parameters have been

>> provided. The defaults for these parameters came from the work to

>> drastically reduce jitter of timer processing which is documented

>> here [1] and presented at Linaro Connect here [2].

>>

>> If you speculate that these defaults might need to be changed, e.g.

>> l2fwd, we welcome collaboration and data. But, this is not a blocking

>> issue for this patch right now.

>>

>> [1] https://docs.google.com/document/d/1sY7rOxqCNu-bMqjBiT5_keAIohrX1ZW-eL0oGLAQ4OM/edit?usp=sharing

>> [2] http://connect.linaro.org/resource/bud17/bud17-320/

>>

>

> 1) we have all adjustable configs here

> ./platform/linux-generic/include/odp_config_internal.h

> that might be also needs to be there.

>

That file has all the global config values. These are internal to this
timer implementation, hence they do not need to be moved.
>

> 2) Do we need something special in CI to check different config values?


Nope.
>

> 3) Why it's compile time config values and not run time?


These config values are particular to this timer implementation.
Similar to config values in
./platform/linux-generic/include/odp_config_internal.h, these also
will be compile time constants.

>

> Maxim.

>

>

>>> -Petri

>>>

>>>

>>> From: Maxim Uvarov [mailto:maxim.uvarov@linaro.org]

>>> Sent: Thursday, June 22, 2017 11:22 AM

>>> To: Honnappa Nagarahalli <honnappa.nagarahalli@linaro.org>

>>> Cc: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com>; lng-odp-forward <lng-odp@lists.linaro.org>

>>> Subject: Re: [lng-odp] [API-NEXT PATCH v4] timer: allow timer processing to run on worker cores

>>>

>>> Petri, do you want to test performance before patch inclusion?

>>> Maxim.

>>>

>>> On 21 June 2017 at 21:52, Honnappa Nagarahalli <mailto:honnappa.nagarahalli@linaro.org> wrote:

>>> We have not run any performance application. In our Linaro connect

>>> meeting, we presented numbers on how it improves the timer resolution.

>>> At this point, there is enough configuration options to control the

>>> effect of calling timer in the scheduler. For applications that do not

>>> want to use the timer, there should not be any change. For

>>> applications that use timers non-frequently, the check frequency can

>>> be controlled via the provided configuration options.

>>>

>>> On 20 June 2017 at 02:34, Savolainen, Petri (Nokia - FI/Espoo)

>>> <mailto:petri.savolainen@nokia.com> wrote:

>>>> Do you have some performance numbers? E.g. how much this slows down an application which does not use timers (e.g. l2fwd), or an application that uses only few, non-frequent timeouts?

>>>>

>>>> Additionally, init.h/feature.h is not yet in api-next - so this would not build yet.

>>>>

>>>>

>>>> -Petri

>>>>

>>>>

>>>>> -----Original Message-----

>>>>> From: lng-odp [mailto:mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of

>>>>> Honnappa Nagarahalli

>>>>> Sent: Tuesday, June 20, 2017 7:07 AM

>>>>> To: Bill Fischofer <mailto:bill.fischofer@linaro.org>

>>>>> Cc: lng-odp-forward <mailto:lng-odp@lists.linaro.org>

>>>>> Subject: Re: [lng-odp] [API-NEXT PATCH v4] timer: allow timer processing

>>>>> to run on worker cores

>>>>>

>>>>> Are you saying we should be good to merge this now?

>>>>>

>>>>> On 19 June 2017 at 17:42, Bill Fischofer <mailto:bill.fischofer@linaro.org>

>>>>> wrote:

>>>>>> On Mon, Jun 19, 2017 at 4:19 PM, Honnappa Nagarahalli

>>>>>> <mailto:honnappa.nagarahalli@linaro.org> wrote:

>>>>>>> Hi Bill/Maxim,

>>>>>>>      I do not see any further comments, can we merge this to api-next?

>>>>>>>

>>>>>>> Thanks,

>>>>>>> Honnappa

>>>>

>>>>

>>>

>
Maxim Uvarov June 23, 2017, 2:22 p.m. | #13
Merged to api-next.

I think this patch is clean and we can do further improvements / tuning
later.

Regards,
Maxim.

On 22 June 2017 at 21:48, Honnappa Nagarahalli <
honnappa.nagarahalli@linaro.org> wrote:

> On 22 June 2017 at 10:30, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:

> > On 06/22/17 17:55, Brian Brooks wrote:

> >> On 06/22 10:27:01, Savolainen, Petri (Nokia - FI/Espoo) wrote:

> >>> I was asking to make sure that performance impact has been checked

> also when timers are not used, e.g. l2fwd performance before and after the

> change. It would be also appropriate to test impact in the worst case:

> l2fwd type application + a periodic 1sec timeout. Timer is on, but timeouts

> come very unfrequently (compared to packets).

> >>>

> >>> It seems that no performance tests were run, although the change

> affects performance of many applications (e.g. OFP has high packet rate

> with timers). Configuration options should be set with  defaults that are

> acceptable trade-off between packet processing performance and timeout

> accuracy.

> >>

> >> If timers are not used, the overhead is just checking a RO variable

> >> (post global init). If timers are used, CONFIG_ parameters have been

> >> provided. The defaults for these parameters came from the work to

> >> drastically reduce jitter of timer processing which is documented

> >> here [1] and presented at Linaro Connect here [2].

> >>

> >> If you speculate that these defaults might need to be changed, e.g.

> >> l2fwd, we welcome collaboration and data. But, this is not a blocking

> >> issue for this patch right now.

> >>

> >> [1] https://docs.google.com/document/d/1sY7rOxqCNu-

> bMqjBiT5_keAIohrX1ZW-eL0oGLAQ4OM/edit?usp=sharing

> >> [2] http://connect.linaro.org/resource/bud17/bud17-320/

> >>

> >

> > 1) we have all adjustable configs here

> > ./platform/linux-generic/include/odp_config_internal.h

> > that might be also needs to be there.

> >

> That file has all the global config values. These are internal to this

> timer implementation, hence they do not need to be moved.

> >

> > 2) Do we need something special in CI to check different config values?

>

> Nope.

> >

> > 3) Why it's compile time config values and not run time?

>

> These config values are particular to this timer implementation.

> Similar to config values in

> ./platform/linux-generic/include/odp_config_internal.h, these also

> will be compile time constants.

>

> >

> > Maxim.

> >

> >

> >>> -Petri

> >>>

> >>>

> >>> From: Maxim Uvarov [mailto:maxim.uvarov@linaro.org]

> >>> Sent: Thursday, June 22, 2017 11:22 AM

> >>> To: Honnappa Nagarahalli <honnappa.nagarahalli@linaro.org>

> >>> Cc: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com>;

> lng-odp-forward <lng-odp@lists.linaro.org>

> >>> Subject: Re: [lng-odp] [API-NEXT PATCH v4] timer: allow timer

> processing to run on worker cores

> >>>

> >>> Petri, do you want to test performance before patch inclusion?

> >>> Maxim.

> >>>

> >>> On 21 June 2017 at 21:52, Honnappa Nagarahalli <mailto:

> honnappa.nagarahalli@linaro.org> wrote:

> >>> We have not run any performance application. In our Linaro connect

> >>> meeting, we presented numbers on how it improves the timer resolution.

> >>> At this point, there is enough configuration options to control the

> >>> effect of calling timer in the scheduler. For applications that do not

> >>> want to use the timer, there should not be any change. For

> >>> applications that use timers non-frequently, the check frequency can

> >>> be controlled via the provided configuration options.

> >>>

> >>> On 20 June 2017 at 02:34, Savolainen, Petri (Nokia - FI/Espoo)

> >>> <mailto:petri.savolainen@nokia.com> wrote:

> >>>> Do you have some performance numbers? E.g. how much this slows down

> an application which does not use timers (e.g. l2fwd), or an application

> that uses only few, non-frequent timeouts?

> >>>>

> >>>> Additionally, init.h/feature.h is not yet in api-next - so this would

> not build yet.

> >>>>

> >>>>

> >>>> -Petri

> >>>>

> >>>>

> >>>>> -----Original Message-----

> >>>>> From: lng-odp [mailto:mailto:lng-odp-bounces@lists.linaro.org] On

> Behalf Of

> >>>>> Honnappa Nagarahalli

> >>>>> Sent: Tuesday, June 20, 2017 7:07 AM

> >>>>> To: Bill Fischofer <mailto:bill.fischofer@linaro.org>

> >>>>> Cc: lng-odp-forward <mailto:lng-odp@lists.linaro.org>

> >>>>> Subject: Re: [lng-odp] [API-NEXT PATCH v4] timer: allow timer

> processing

> >>>>> to run on worker cores

> >>>>>

> >>>>> Are you saying we should be good to merge this now?

> >>>>>

> >>>>> On 19 June 2017 at 17:42, Bill Fischofer <mailto:

> bill.fischofer@linaro.org>

> >>>>> wrote:

> >>>>>> On Mon, Jun 19, 2017 at 4:19 PM, Honnappa Nagarahalli

> >>>>>> <mailto:honnappa.nagarahalli@linaro.org> wrote:

> >>>>>>> Hi Bill/Maxim,

> >>>>>>>      I do not see any further comments, can we merge this to

> api-next?

> >>>>>>>

> >>>>>>> Thanks,

> >>>>>>> Honnappa

> >>>>

> >>>>

> >>>

> >

>
Savolainen, Petri (Nokia - FI/Espoo) June 26, 2017, 10:50 a.m. | #14
When init params are set to defaults...

	odp_init_param_init(&init);

	odp_init_global(&instance, &init, NULL);


... this patch causes -15% (about 1Mpps per cpu) packet rate degradation with l2fwd. The timer poll function shows up as the second highest CPU cycle consumer on perf. The defaults needs to be less aggressive: a tradeoff between somewhat improved timer accuracy for some timer using applications vs. performance degradation for all application using the scheduler (also with the default inits)...

The patch actually disables the polling when passing NULL as init param. API defines that NULL and odp_init_param_init(&init) result the same config ("all defaults"). This implementation mismatch helps now when most applications pass NULL, and thus timer polling is kept disabled by default.

When contributing to a central piece of data plane code, it would be really important to measure packet rate before and after the change...


-Petri

 



From: Maxim Uvarov [mailto:maxim.uvarov@linaro.org] 

Sent: Friday, June 23, 2017 5:23 PM
To: Honnappa Nagarahalli <honnappa.nagarahalli@linaro.org>
Cc: Brian Brooks <brian.brooks@arm.com>; Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com>; lng-odp-forward <lng-odp@lists.linaro.org>
Subject: Re: [lng-odp] [API-NEXT PATCH v4] timer: allow timer processing to run on worker cores

Merged to api-next.
I think this patch is clean and we can do further improvements / tuning later.
Regards,
Maxim.

On 22 June 2017 at 21:48, Honnappa Nagarahalli <mailto:honnappa.nagarahalli@linaro.org> wrote:
On 22 June 2017 at 10:30, Maxim Uvarov <mailto:maxim.uvarov@linaro.org> wrote:
> On 06/22/17 17:55, Brian Brooks wrote:

>> On 06/22 10:27:01, Savolainen, Petri (Nokia - FI/Espoo) wrote:

>>> I was asking to make sure that performance impact has been checked also when timers are not used, e.g. l2fwd performance before and after the change. It would be also appropriate to test impact in the worst case: l2fwd type application + a periodic 1sec timeout. Timer is on, but timeouts come very unfrequently (compared to packets).

>>>

>>> It seems that no performance tests were run, although the change affects performance of many applications (e.g. OFP has high packet rate with timers). Configuration options should be set with  defaults that are acceptable trade-off between packet processing performance and timeout accuracy.

>>

>> If timers are not used, the overhead is just checking a RO variable

>> (post global init). If timers are used, CONFIG_ parameters have been

>> provided. The defaults for these parameters came from the work to

>> drastically reduce jitter of timer processing which is documented

>> here [1] and presented at Linaro Connect here [2].

>>

>> If you speculate that these defaults might need to be changed, e.g.

>> l2fwd, we welcome collaboration and data. But, this is not a blocking

>> issue for this patch right now.

>>

>> [1] https://docs.google.com/document/d/1sY7rOxqCNu-bMqjBiT5_keAIohrX1ZW-eL0oGLAQ4OM/edit?usp=sharing

>> [2] http://connect.linaro.org/resource/bud17/bud17-320/

>>

>

> 1) we have all adjustable configs here

> ./platform/linux-generic/include/odp_config_internal.h

> that might be also needs to be there.

>

That file has all the global config values. These are internal to this
timer implementation, hence they do not need to be moved.
>

> 2) Do we need something special in CI to check different config values?


Nope.
>

> 3) Why it's compile time config values and not run time?


These config values are particular to this timer implementation.
Similar to config values in
./platform/linux-generic/include/odp_config_internal.h, these also
will be compile time constants.

>

> Maxim.

>

>

>>> -Petri

>>>

>>>

>>> From: Maxim Uvarov [mailto:mailto:maxim.uvarov@linaro.org]

>>> Sent: Thursday, June 22, 2017 11:22 AM

>>> To: Honnappa Nagarahalli <mailto:honnappa.nagarahalli@linaro.org>

>>> Cc: Savolainen, Petri (Nokia - FI/Espoo) <mailto:petri.savolainen@nokia.com>; lng-odp-forward <mailto:lng-odp@lists.linaro.org>

>>> Subject: Re: [lng-odp] [API-NEXT PATCH v4] timer: allow timer processing to run on worker cores

>>>

>>> Petri, do you want to test performance before patch inclusion?

>>> Maxim.

>>>

>>> On 21 June 2017 at 21:52, Honnappa Nagarahalli <mailto:mailto:honnappa.nagarahalli@linaro.org> wrote:

>>> We have not run any performance application. In our Linaro connect

>>> meeting, we presented numbers on how it improves the timer resolution.

>>> At this point, there is enough configuration options to control the

>>> effect of calling timer in the scheduler. For applications that do not

>>> want to use the timer, there should not be any change. For

>>> applications that use timers non-frequently, the check frequency can

>>> be controlled via the provided configuration options.

>>>

>>> On 20 June 2017 at 02:34, Savolainen, Petri (Nokia - FI/Espoo)

>>> <mailto:mailto:petri.savolainen@nokia.com> wrote:

>>>> Do you have some performance numbers? E.g. how much this slows down an application which does not use timers (e.g. l2fwd), or an application that uses only few, non-frequent timeouts?

>>>>

>>>> Additionally, init.h/feature.h is not yet in api-next - so this would not build yet.

>>>>

>>>>

>>>> -Petri

>>>>

>>>>

>>>>> -----Original Message-----

>>>>> From: lng-odp [mailto:mailto:mailto:mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of

>>>>> Honnappa Nagarahalli

>>>>> Sent: Tuesday, June 20, 2017 7:07 AM

>>>>> To: Bill Fischofer <mailto:mailto:bill.fischofer@linaro.org>

>>>>> Cc: lng-odp-forward <mailto:mailto:lng-odp@lists.linaro.org>

>>>>> Subject: Re: [lng-odp] [API-NEXT PATCH v4] timer: allow timer processing

>>>>> to run on worker cores

>>>>>

>>>>> Are you saying we should be good to merge this now?

>>>>>

>>>>> On 19 June 2017 at 17:42, Bill Fischofer <mailto:mailto:bill.fischofer@linaro.org>

>>>>> wrote:

>>>>>> On Mon, Jun 19, 2017 at 4:19 PM, Honnappa Nagarahalli

>>>>>> <mailto:mailto:honnappa.nagarahalli@linaro.org> wrote:

>>>>>>> Hi Bill/Maxim,

>>>>>>>      I do not see any further comments, can we merge this to api-next?

>>>>>>>

>>>>>>> Thanks,

>>>>>>> Honnappa

>>>>

>>>>

>>>

>
Maxim Uvarov June 26, 2017, 12:34 p.m. | #15
On 06/26/17 13:50, Savolainen, Petri (Nokia - FI/Espoo) wrote:
> When init params are set to defaults...

> 

> 	odp_init_param_init(&init);

> 

> 	odp_init_global(&instance, &init, NULL);

> 

> 

> ... this patch causes -15% (about 1Mpps per cpu) packet rate degradation with l2fwd. The timer poll function shows up as the second highest CPU cycle consumer on perf. The defaults needs to be less aggressive: a tradeoff between somewhat improved timer accuracy for some timer using applications vs. performance degradation for all application using the scheduler (also with the default inits)...

> 

> The patch actually disables the polling when passing NULL as init param. API defines that NULL and odp_init_param_init(&init) result the same config ("all defaults"). This implementation mismatch helps now when most applications pass NULL, and thus timer polling is kept disabled by default.

> 

> When contributing to a central piece of data plane code, it would be really important to measure packet rate before and after the change...

> 

> 

> -Petri

> 


Thank Petri,

I opened bug to track this:
https://bugs.linaro.org/show_bug.cgi?id=3078

Maxim.


>  

> 

> 

> 

> From: Maxim Uvarov [mailto:maxim.uvarov@linaro.org] 

> Sent: Friday, June 23, 2017 5:23 PM

> To: Honnappa Nagarahalli <honnappa.nagarahalli@linaro.org>

> Cc: Brian Brooks <brian.brooks@arm.com>; Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com>; lng-odp-forward <lng-odp@lists.linaro.org>

> Subject: Re: [lng-odp] [API-NEXT PATCH v4] timer: allow timer processing to run on worker cores

> 

> Merged to api-next.

> I think this patch is clean and we can do further improvements / tuning later.

> Regards,

> Maxim.

> 

> On 22 June 2017 at 21:48, Honnappa Nagarahalli <mailto:honnappa.nagarahalli@linaro.org> wrote:

> On 22 June 2017 at 10:30, Maxim Uvarov <mailto:maxim.uvarov@linaro.org> wrote:

>> On 06/22/17 17:55, Brian Brooks wrote:

>>> On 06/22 10:27:01, Savolainen, Petri (Nokia - FI/Espoo) wrote:

>>>> I was asking to make sure that performance impact has been checked also when timers are not used, e.g. l2fwd performance before and after the change. It would be also appropriate to test impact in the worst case: l2fwd type application + a periodic 1sec timeout. Timer is on, but timeouts come very unfrequently (compared to packets).

>>>>

>>>> It seems that no performance tests were run, although the change affects performance of many applications (e.g. OFP has high packet rate with timers). Configuration options should be set with  defaults that are acceptable trade-off between packet processing performance and timeout accuracy.

>>>

>>> If timers are not used, the overhead is just checking a RO variable

>>> (post global init). If timers are used, CONFIG_ parameters have been

>>> provided. The defaults for these parameters came from the work to

>>> drastically reduce jitter of timer processing which is documented

>>> here [1] and presented at Linaro Connect here [2].

>>>

>>> If you speculate that these defaults might need to be changed, e.g.

>>> l2fwd, we welcome collaboration and data. But, this is not a blocking

>>> issue for this patch right now.

>>>

>>> [1] https://docs.google.com/document/d/1sY7rOxqCNu-bMqjBiT5_keAIohrX1ZW-eL0oGLAQ4OM/edit?usp=sharing

>>> [2] http://connect.linaro.org/resource/bud17/bud17-320/

>>>

>>

>> 1) we have all adjustable configs here

>> ./platform/linux-generic/include/odp_config_internal.h

>> that might be also needs to be there.

>>

> That file has all the global config values. These are internal to this

> timer implementation, hence they do not need to be moved.

>>

>> 2) Do we need something special in CI to check different config values?

> 

> Nope.

>>

>> 3) Why it's compile time config values and not run time?

> 

> These config values are particular to this timer implementation.

> Similar to config values in

> ./platform/linux-generic/include/odp_config_internal.h, these also

> will be compile time constants.

> 

>>

>> Maxim.

>>

>>

>>>> -Petri

>>>>

>>>>

>>>> From: Maxim Uvarov [mailto:mailto:maxim.uvarov@linaro.org]

>>>> Sent: Thursday, June 22, 2017 11:22 AM

>>>> To: Honnappa Nagarahalli <mailto:honnappa.nagarahalli@linaro.org>

>>>> Cc: Savolainen, Petri (Nokia - FI/Espoo) <mailto:petri.savolainen@nokia.com>; lng-odp-forward <mailto:lng-odp@lists.linaro.org>

>>>> Subject: Re: [lng-odp] [API-NEXT PATCH v4] timer: allow timer processing to run on worker cores

>>>>

>>>> Petri, do you want to test performance before patch inclusion?

>>>> Maxim.

>>>>

>>>> On 21 June 2017 at 21:52, Honnappa Nagarahalli <mailto:mailto:honnappa.nagarahalli@linaro.org> wrote:

>>>> We have not run any performance application. In our Linaro connect

>>>> meeting, we presented numbers on how it improves the timer resolution.

>>>> At this point, there is enough configuration options to control the

>>>> effect of calling timer in the scheduler. For applications that do not

>>>> want to use the timer, there should not be any change. For

>>>> applications that use timers non-frequently, the check frequency can

>>>> be controlled via the provided configuration options.

>>>>

>>>> On 20 June 2017 at 02:34, Savolainen, Petri (Nokia - FI/Espoo)

>>>> <mailto:mailto:petri.savolainen@nokia.com> wrote:

>>>>> Do you have some performance numbers? E.g. how much this slows down an application which does not use timers (e.g. l2fwd), or an application that uses only few, non-frequent timeouts?

>>>>>

>>>>> Additionally, init.h/feature.h is not yet in api-next - so this would not build yet.

>>>>>

>>>>>

>>>>> -Petri

>>>>>

>>>>>

>>>>>> -----Original Message-----

>>>>>> From: lng-odp [mailto:mailto:mailto:mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of

>>>>>> Honnappa Nagarahalli

>>>>>> Sent: Tuesday, June 20, 2017 7:07 AM

>>>>>> To: Bill Fischofer <mailto:mailto:bill.fischofer@linaro.org>

>>>>>> Cc: lng-odp-forward <mailto:mailto:lng-odp@lists.linaro.org>

>>>>>> Subject: Re: [lng-odp] [API-NEXT PATCH v4] timer: allow timer processing

>>>>>> to run on worker cores

>>>>>>

>>>>>> Are you saying we should be good to merge this now?

>>>>>>

>>>>>> On 19 June 2017 at 17:42, Bill Fischofer <mailto:mailto:bill.fischofer@linaro.org>

>>>>>> wrote:

>>>>>>> On Mon, Jun 19, 2017 at 4:19 PM, Honnappa Nagarahalli

>>>>>>> <mailto:mailto:honnappa.nagarahalli@linaro.org> wrote:

>>>>>>>> Hi Bill/Maxim,

>>>>>>>>       I do not see any further comments, can we merge this to api-next?

>>>>>>>>

>>>>>>>> Thanks,

>>>>>>>> Honnappa

>>>>>

>>>>>

>>>>

>>

>
Kevin Wang June 28, 2017, 3:18 a.m. | #16
Hi Petri,
Could you let me know which platform are you using and what's the command
line of your l2_fwd test?

​Kevin​

2017-06-26 20:34 GMT+08:00 Maxim Uvarov <maxim.uvarov@linaro.org>:

> On 06/26/17 13:50, Savolainen, Petri (Nokia - FI/Espoo) wrote:

> > When init params are set to defaults...

> >

> >       odp_init_param_init(&init);

> >

> >       odp_init_global(&instance, &init, NULL);

> >

> >

> > ... this patch causes -15% (about 1Mpps per cpu) packet rate degradation

> with l2fwd. The timer poll function shows up as the second highest CPU

> cycle consumer on perf. The defaults needs to be less aggressive: a

> tradeoff between somewhat improved timer accuracy for some timer using

> applications vs. performance degradation for all application using the

> scheduler (also with the default inits)...

> >

> > The patch actually disables the polling when passing NULL as init param.

> API defines that NULL and odp_init_param_init(&init) result the same config

> ("all defaults"). This implementation mismatch helps now when most

> applications pass NULL, and thus timer polling is kept disabled by default.

> >

> > When contributing to a central piece of data plane code, it would be

> really important to measure packet rate before and after the change...

> >

> >

> > -Petri

> >

>

> Thank Petri,

>

> I opened bug to track this:

> https://bugs.linaro.org/show_bug.cgi?id=3078

>

> Maxim.

>

>

> >

> >

> >

> >

> > From: Maxim Uvarov [mailto:maxim.uvarov@linaro.org]

> > Sent: Friday, June 23, 2017 5:23 PM

> > To: Honnappa Nagarahalli <honnappa.nagarahalli@linaro.org>

> > Cc: Brian Brooks <brian.brooks@arm.com>; Savolainen, Petri (Nokia -

> FI/Espoo) <petri.savolainen@nokia.com>; lng-odp-forward <

> lng-odp@lists.linaro.org>

> > Subject: Re: [lng-odp] [API-NEXT PATCH v4] timer: allow timer processing

> to run on worker cores

> >

> > Merged to api-next.

> > I think this patch is clean and we can do further improvements / tuning

> later.

> > Regards,

> > Maxim.

> >

> > On 22 June 2017 at 21:48, Honnappa Nagarahalli <mailto:

> honnappa.nagarahalli@linaro.org> wrote:

> > On 22 June 2017 at 10:30, Maxim Uvarov <mailto:maxim.uvarov@linaro.org>

> wrote:

> >> On 06/22/17 17:55, Brian Brooks wrote:

> >>> On 06/22 10:27:01, Savolainen, Petri (Nokia - FI/Espoo) wrote:

> >>>> I was asking to make sure that performance impact has been checked

> also when timers are not used, e.g. l2fwd performance before and after the

> change. It would be also appropriate to test impact in the worst case:

> l2fwd type application + a periodic 1sec timeout. Timer is on, but timeouts

> come very unfrequently (compared to packets).

> >>>>

> >>>> It seems that no performance tests were run, although the change

> affects performance of many applications (e.g. OFP has high packet rate

> with timers). Configuration options should be set with  defaults that are

> acceptable trade-off between packet processing performance and timeout

> accuracy.

> >>>

> >>> If timers are not used, the overhead is just checking a RO variable

> >>> (post global init). If timers are used, CONFIG_ parameters have been

> >>> provided. The defaults for these parameters came from the work to

> >>> drastically reduce jitter of timer processing which is documented

> >>> here [1] and presented at Linaro Connect here [2].

> >>>

> >>> If you speculate that these defaults might need to be changed, e.g.

> >>> l2fwd, we welcome collaboration and data. But, this is not a blocking

> >>> issue for this patch right now.

> >>>

> >>> [1] https://docs.google.com/document/d/1sY7rOxqCNu-

> bMqjBiT5_keAIohrX1ZW-eL0oGLAQ4OM/edit?usp=sharing

> >>> [2] http://connect.linaro.org/resource/bud17/bud17-320/

> >>>

> >>

> >> 1) we have all adjustable configs here

> >> ./platform/linux-generic/include/odp_config_internal.h

> >> that might be also needs to be there.

> >>

> > That file has all the global config values. These are internal to this

> > timer implementation, hence they do not need to be moved.

> >>

> >> 2) Do we need something special in CI to check different config values?

> >

> > Nope.

> >>

> >> 3) Why it's compile time config values and not run time?

> >

> > These config values are particular to this timer implementation.

> > Similar to config values in

> > ./platform/linux-generic/include/odp_config_internal.h, these also

> > will be compile time constants.

> >

> >>

> >> Maxim.

> >>

> >>

> >>>> -Petri

> >>>>

> >>>>

> >>>> From: Maxim Uvarov [mailto:mailto:maxim.uvarov@linaro.org]

> >>>> Sent: Thursday, June 22, 2017 11:22 AM

> >>>> To: Honnappa Nagarahalli <mailto:honnappa.nagarahalli@linaro.org>

> >>>> Cc: Savolainen, Petri (Nokia - FI/Espoo) <mailto:petri.savolainen@

> nokia.com>; lng-odp-forward <mailto:lng-odp@lists.linaro.org>

> >>>> Subject: Re: [lng-odp] [API-NEXT PATCH v4] timer: allow timer

> processing to run on worker cores

> >>>>

> >>>> Petri, do you want to test performance before patch inclusion?

> >>>> Maxim.

> >>>>

> >>>> On 21 June 2017 at 21:52, Honnappa Nagarahalli <mailto:mailto:

> honnappa.nagarahalli@linaro.org> wrote:

> >>>> We have not run any performance application. In our Linaro connect

> >>>> meeting, we presented numbers on how it improves the timer resolution.

> >>>> At this point, there is enough configuration options to control the

> >>>> effect of calling timer in the scheduler. For applications that do not

> >>>> want to use the timer, there should not be any change. For

> >>>> applications that use timers non-frequently, the check frequency can

> >>>> be controlled via the provided configuration options.

> >>>>

> >>>> On 20 June 2017 at 02:34, Savolainen, Petri (Nokia - FI/Espoo)

> >>>> <mailto:mailto:petri.savolainen@nokia.com> wrote:

> >>>>> Do you have some performance numbers? E.g. how much this slows down

> an application which does not use timers (e.g. l2fwd), or an application

> that uses only few, non-frequent timeouts?

> >>>>>

> >>>>> Additionally, init.h/feature.h is not yet in api-next - so this

> would not build yet.

> >>>>>

> >>>>>

> >>>>> -Petri

> >>>>>

> >>>>>

> >>>>>> -----Original Message-----

> >>>>>> From: lng-odp [mailto:mailto:mailto:mailto:l

> ng-odp-bounces@lists.linaro.org] On Behalf Of

> >>>>>> Honnappa Nagarahalli

> >>>>>> Sent: Tuesday, June 20, 2017 7:07 AM

> >>>>>> To: Bill Fischofer <mailto:mailto:bill.fischofer@linaro.org>

> >>>>>> Cc: lng-odp-forward <mailto:mailto:lng-odp@lists.linaro.org>

> >>>>>> Subject: Re: [lng-odp] [API-NEXT PATCH v4] timer: allow timer

> processing

> >>>>>> to run on worker cores

> >>>>>>

> >>>>>> Are you saying we should be good to merge this now?

> >>>>>>

> >>>>>> On 19 June 2017 at 17:42, Bill Fischofer <mailto:mailto:

> bill.fischofer@linaro.org>

> >>>>>> wrote:

> >>>>>>> On Mon, Jun 19, 2017 at 4:19 PM, Honnappa Nagarahalli

> >>>>>>> <mailto:mailto:honnappa.nagarahalli@linaro.org> wrote:

> >>>>>>>> Hi Bill/Maxim,

> >>>>>>>>       I do not see any further comments, can we merge this to

> api-next?

> >>>>>>>>

> >>>>>>>> Thanks,

> >>>>>>>> Honnappa

> >>>>>

> >>>>>

> >>>>

> >>

> >

>

>



-- 
Thanks,
Kevin

Patch hide | download patch | download mbox

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;
 }