[API-NEXT,v3,2/2] timer: allow timer processing to run on worker cores

Message ID 20170608201658.33027-2-brian.brooks@arm.com
State New
Headers show
Series
  • [API-NEXT,v3,1/2] timer: organize #include
Related show

Commit Message

Brian Brooks June 8, 2017, 8:16 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>

---

v3:
 - Add rate limiting by scheduling rounds

v2:
 - Reword 'worker_timers' to 'use_scheduler'
 - Use ODP Time instead of ticks


include/odp/api/spec/init.h                        |  5 ++
 platform/linux-generic/include/odp_internal.h      |  1 +
 .../linux-generic/include/odp_timer_internal.h     | 11 +++
 platform/linux-generic/odp_init.c                  |  8 +-
 platform/linux-generic/odp_schedule.c              |  4 +-
 platform/linux-generic/odp_schedule_iquery.c       |  3 +
 platform/linux-generic/odp_schedule_sp.c           |  4 +
 platform/linux-generic/odp_timer.c                 | 98 ++++++++++++++++++++--
 8 files changed, 122 insertions(+), 12 deletions(-)

-- 
2.13.0

Comments

Savolainen, Petri (Nokia - FI/Espoo) June 9, 2017, 11:56 a.m. | #1
> -----Original Message-----

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

> Brooks

> Sent: Thursday, June 08, 2017 11:17 PM

> To: lng-odp@lists.linaro.org

> Subject: [lng-odp] [API-NEXT v3 2/2] timer: allow timer processing to run

> on worker cores

> 

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

> ---

> 

> v3:

>  - Add rate limiting by scheduling rounds

> 

> v2:

>  - Reword 'worker_timers' to 'use_scheduler'

>  - Use ODP Time instead of ticks

> 

> 

> include/odp/api/spec/init.h                        |  5 ++

>  platform/linux-generic/include/odp_internal.h      |  1 +

>  .../linux-generic/include/odp_timer_internal.h     | 11 +++

>  platform/linux-generic/odp_init.c                  |  8 +-

>  platform/linux-generic/odp_schedule.c              |  4 +-

>  platform/linux-generic/odp_schedule_iquery.c       |  3 +

>  platform/linux-generic/odp_schedule_sp.c           |  4 +

>  platform/linux-generic/odp_timer.c                 | 98

> ++++++++++++++++++++--

>  8 files changed, 122 insertions(+), 12 deletions(-)

> 

> diff --git a/include/odp/api/spec/init.h b/include/odp/api/spec/init.h

> index 154cdf8f..44950893 100644

> --- a/include/odp/api/spec/init.h

> +++ b/include/odp/api/spec/init.h

> @@ -153,6 +153,11 @@ typedef struct odp_init_t {

>  	odp_log_func_t log_fn;

>  	/** Replacement for the default abort fn */

>  	odp_abort_func_t abort_fn;

> +	/** Whether the application will ever call odp_schedule() or

> not.

> +

> +	    Default: true

> +	*/

> +	odp_bool_t use_scheduler;

>  } odp_init_t;



This will not be merged, not even temporarily. We should avoid unnecessary (API) changes back and forth. Instead of this, we'll add odp_feature_t bit field as discussed before.

-Petri
Honnappa Nagarahalli June 9, 2017, 8:44 p.m. | #2
On 9 June 2017 at 06:56, Savolainen, Petri (Nokia - FI/Espoo)
<petri.savolainen@nokia.com> wrote:
>

>

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

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

>> Brooks

>> Sent: Thursday, June 08, 2017 11:17 PM

>> To: lng-odp@lists.linaro.org

>> Subject: [lng-odp] [API-NEXT v3 2/2] timer: allow timer processing to run

>> on worker cores

>>

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

>> ---

>>

>> v3:

>>  - Add rate limiting by scheduling rounds

>>

>> v2:

>>  - Reword 'worker_timers' to 'use_scheduler'

>>  - Use ODP Time instead of ticks

>>

>>

>> include/odp/api/spec/init.h                        |  5 ++

>>  platform/linux-generic/include/odp_internal.h      |  1 +

>>  .../linux-generic/include/odp_timer_internal.h     | 11 +++

>>  platform/linux-generic/odp_init.c                  |  8 +-

>>  platform/linux-generic/odp_schedule.c              |  4 +-

>>  platform/linux-generic/odp_schedule_iquery.c       |  3 +

>>  platform/linux-generic/odp_schedule_sp.c           |  4 +

>>  platform/linux-generic/odp_timer.c                 | 98

>> ++++++++++++++++++++--

>>  8 files changed, 122 insertions(+), 12 deletions(-)

>>

>> diff --git a/include/odp/api/spec/init.h b/include/odp/api/spec/init.h

>> index 154cdf8f..44950893 100644

>> --- a/include/odp/api/spec/init.h

>> +++ b/include/odp/api/spec/init.h

>> @@ -153,6 +153,11 @@ typedef struct odp_init_t {

>>       odp_log_func_t log_fn;

>>       /** Replacement for the default abort fn */

>>       odp_abort_func_t abort_fn;

>> +     /** Whether the application will ever call odp_schedule() or

>> not.

>> +

>> +         Default: true

>> +     */

>> +     odp_bool_t use_scheduler;

>>  } odp_init_t;

>

>

> This will not be merged, not even temporarily. We should avoid unnecessary (API) changes back and forth. Instead of this, we'll add odp_feature_t bit field as discussed before.

>


As agreed on the call, review other portions of the code.

> -Petri

>

Patch hide | download patch | download mbox

diff --git a/include/odp/api/spec/init.h b/include/odp/api/spec/init.h
index 154cdf8f..44950893 100644
--- a/include/odp/api/spec/init.h
+++ b/include/odp/api/spec/init.h
@@ -153,6 +153,11 @@  typedef struct odp_init_t {
 	odp_log_func_t log_fn;
 	/** Replacement for the default abort fn */
 	odp_abort_func_t abort_fn;
+	/** Whether the application will ever call odp_schedule() or not.
+
+	    Default: true
+	*/
+	odp_bool_t use_scheduler;
 } odp_init_t;
 
 /**
diff --git a/platform/linux-generic/include/odp_internal.h b/platform/linux-generic/include/odp_internal.h
index 90e2a629..26a5ffd3 100644
--- a/platform/linux-generic/include/odp_internal.h
+++ b/platform/linux-generic/include/odp_internal.h
@@ -51,6 +51,7 @@  struct odp_global_data_s {
 	odp_cpumask_t worker_cpus;
 	int num_cpus_installed;
 	config_t configuration;
+	odp_bool_t use_scheduler;
 };
 
 enum init_stage {
diff --git a/platform/linux-generic/include/odp_timer_internal.h b/platform/linux-generic/include/odp_timer_internal.h
index 91b12c54..cd09a0fe 100644
--- a/platform/linux-generic/include/odp_timer_internal.h
+++ b/platform/linux-generic/include/odp_timer_internal.h
@@ -20,6 +20,15 @@ 
 #include <odp_pool_internal.h>
 #include <odp/api/timer.h>
 
+/* Minimum number of nanoseconds between calls to _timer_run() per thread. */
+#define CONFIG_TIMER_RUN_RATELIMIT_NS 100
+
+/*
+ * Minimum number of scheduling rounds between calls to _timer_run()
+ * per thread.
+ */
+#define CONFIG_TIMER_RUN_RATELIMIT_ROUNDS 1
+
 /**
  * Internal Timeout header
  */
@@ -35,4 +44,6 @@  typedef struct {
 	odp_timer_t timer;
 } odp_timeout_hdr_t;
 
+unsigned timer_run(void);
+
 #endif
diff --git a/platform/linux-generic/odp_init.c b/platform/linux-generic/odp_init.c
index 685e02fa..cbec83c3 100644
--- a/platform/linux-generic/odp_init.c
+++ b/platform/linux-generic/odp_init.c
@@ -4,11 +4,13 @@ 
  * SPDX-License-Identifier:     BSD-3-Clause
  */
 #include <odp/api/init.h>
-#include <odp_debug_internal.h>
 #include <odp/api/debug.h>
-#include <unistd.h>
+
 #include <odp_internal.h>
+#include <odp_debug_internal.h>
 #include <odp_schedule_if.h>
+
+#include <unistd.h>
 #include <string.h>
 #include <libconfig.h>
 #include <stdlib.h>
@@ -159,12 +161,14 @@  int odp_init_global(odp_instance_t *instance,
 	enum init_stage stage = NO_INIT;
 	odp_global_data.log_fn = odp_override_log;
 	odp_global_data.abort_fn = odp_override_abort;
+	odp_global_data.use_scheduler = true;
 
 	if (params != NULL) {
 		if (params->log_fn != NULL)
 			odp_global_data.log_fn = params->log_fn;
 		if (params->abort_fn != NULL)
 			odp_global_data.abort_fn = params->abort_fn;
+		odp_global_data.use_scheduler = params->use_scheduler;
 	}
 
 	cleanup_files(_ODP_TMPDIR, odp_global_data.main_pid);
diff --git a/platform/linux-generic/odp_schedule.c b/platform/linux-generic/odp_schedule.c
index c4567d81..f8ff315c 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
@@ -988,7 +989,6 @@  static inline int do_schedule(odp_queue_t *out_queue, odp_event_t out_ev[],
 	return 0;
 }
 
-
 static int schedule_loop(odp_queue_t *out_queue, uint64_t wait,
 			 odp_event_t out_ev[],
 			 unsigned int max_num)
@@ -998,6 +998,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 75470aff..e865650a 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 0fd4d87d..9cb5f3e3 100644
--- a/platform/linux-generic/odp_schedule_sp.c
+++ b/platform/linux-generic/odp_schedule_sp.c
@@ -11,10 +11,12 @@ 
 #include <odp/api/schedule.h>
 #include <odp/api/shared_memory.h>
 #include <odp_schedule_if.h>
+#include <odp_internal.h>
 #include <odp_debug_internal.h>
 #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
@@ -501,6 +503,8 @@  static int schedule_multi(odp_queue_t *from, uint64_t wait,
 	odp_time_t t1;
 	int update_t1 = 1;
 
+	timer_run();
+
 	if (sched_local.cmd) {
 		/* Continue scheduling if queue is not empty */
 		if (sched_cb_queue_empty(sched_local.cmd->s.index) == 0)
diff --git a/platform/linux-generic/odp_timer.c b/platform/linux-generic/odp_timer.c
index 4539ea48..8a434fd9 100644
--- a/platform/linux-generic/odp_timer.c
+++ b/platform/linux-generic/odp_timer.c
@@ -53,6 +53,7 @@ 
 #include <odp_debug_internal.h>
 #include <odp_internal.h>
 #include <odp_pool_internal.h>
+#include <odp_time_internal.h>
 #include <odp_timer_internal.h>
 
 #define TMO_UNUSED   ((uint64_t)0xFFFFFFFFFFFFFFFF)
@@ -166,7 +167,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; /* Amount of time in a timer pool tick */
 	odp_atomic_u64_t cur_tick;/* Current tick value */
 	uint64_t min_rel_tck;
 	uint64_t max_rel_tck;
@@ -220,7 +222,6 @@  static inline odp_timer_t tp_idx_to_handle(struct odp_timer_pool_s *tp,
 	return _odp_cast_scalar(odp_timer_t, (tp->tp_idx << INDEX_BITS) | idx);
 }
 
-/* Forward declarations */
 static void itimer_init(odp_timer_pool *tp);
 static void itimer_fini(odp_timer_pool *tp);
 
@@ -243,6 +244,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) {
@@ -277,8 +280,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 (!odp_global_data.use_scheduler) {
+		if (tp->param.clk_src == ODP_CLOCK_CPU)
+			itimer_init(tp);
+	}
 	return tp;
 }
 
@@ -307,11 +312,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 (!odp_global_data.use_scheduler) {
+		/* Stop timer triggering */
+		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 +678,75 @@  static unsigned odp_timer_pool_expire(odp_timer_pool_t tpid, uint64_t tick)
 	return nexp;
 }
 
+static unsigned _timer_run(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, 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 _timer_run();
+}
+
 /******************************************************************************
  * POSIX timer support
  * Functions that use Linux/POSIX per-process timers and related facilities
@@ -1001,7 +1077,11 @@  int odp_timer_init_global(void)
 #endif
 	odp_atomic_init_u32(&num_timer_pools, 0);
 
-	block_sigalarm();
+	time_per_ratelimit_period =
+		odp_time_global_from_ns(CONFIG_TIMER_RUN_RATELIMIT_NS);
+
+	if (!odp_global_data.use_scheduler)
+		block_sigalarm();
 
 	return 0;
 }