api: timer: only inactive timers can be freed

Remove possibility to free a timer that is running. When timer
free call returns, the timer handle cannot be referenced any
more. Expiration and event delivery of an already destroyed
timer is an error prone corner case.

Signed-off-by: Petri Savolainen <petri.savolainen@nokia.com>
Reviewed-by: Jere Leppänen <jere.leppanen@nokia.com>
Acked-by: Pavan Nikhilesh <pbhagavatula@marvell.com>

3 files changed, 50 insertions, 31 deletions

diff --git a/test/performance/odp_sched_pktio.c b/test/performance/odp_sched_pktio.c
index 927d35cbd..ceaef0898 100644
--- a/test/performance/odp_sched_pktio.c
+++ b/test/performance/odp_sched_pktio.c
@@ -1081,10 +1081,10 @@ static int stop_pktios(test_global_t *test_global)
 	return ret;
 }
 
-static void empty_queues(void)
+static void empty_queues(uint64_t wait_ns)
 {
 	odp_event_t ev;
-	uint64_t wait_time = odp_schedule_wait_time(ODP_TIME_SEC_IN_NS / 2);
+	uint64_t wait_time = odp_schedule_wait_time(wait_ns);
 
 	/* Drop all events from all queues */
 	while (1) {
@@ -1365,7 +1365,6 @@ static int start_timers(test_global_t *test_global)
 static void destroy_timers(test_global_t *test_global)
 {
 	int i, j;
-	odp_event_t event;
 	odp_timer_t timer;
 	int num_pktio = test_global->opt.num_pktio;
 	int num_queue = test_global->opt.num_pktio_queue;
@@ -1375,6 +1374,9 @@ static void destroy_timers(test_global_t *test_global)
 	if (timer_pool == ODP_TIMER_POOL_INVALID)
 		return;
 
+	/* Wait any remaining timers to expire */
+	empty_queues(2000 * test_global->opt.timeout_us);
+
 	for (i = 0; i < num_pktio; i++) {
 		for (j = 0; j < num_queue; j++) {
 			timer = test_global->timer.timer[i][j];
@@ -1382,10 +1384,8 @@ static void destroy_timers(test_global_t *test_global)
 			if (timer == ODP_TIMER_INVALID)
 				break;
 
-			event = odp_timer_free(timer);
-
-			if (event != ODP_EVENT_INVALID)
-				odp_event_free(event);
+			if (odp_timer_free(timer))
+				printf("Timer free failed: %i, %i\n", i, j);
 		}
 	}
 
@@ -1552,7 +1552,7 @@ int main(int argc, char *argv[])
 
 quit:
 	stop_pktios(test_global);
-	empty_queues();
+	empty_queues(ODP_TIME_SEC_IN_NS / 2);
 	close_pktios(test_global);
 	destroy_pipeline_queues(test_global);
 	destroy_timers(test_global);
diff --git a/test/performance/odp_stress.c b/test/performance/odp_stress.c
index d5e3142f6..f18d85c89 100644
--- a/test/performance/odp_stress.c
+++ b/test/performance/odp_stress.c
@@ -61,6 +61,7 @@ typedef struct test_global_t {
 	test_stat_t stat[ODP_THREAD_COUNT_MAX];
 	thread_arg_t thread_arg[ODP_THREAD_COUNT_MAX];
 	test_stat_sum_t stat_sum;
+	odp_atomic_u64_t tot_rounds;
 
 } test_global_t;
 
@@ -219,19 +220,21 @@ static int worker_thread(void *arg)
 	test_global_t *global = thread_arg->global;
 	test_options_t *test_options = &global->test_options;
 	int mode = test_options->mode;
+	int group_mode = test_options->group_mode;
 	uint64_t mem_size = test_options->mem_size;
 	uint64_t copy_size = mem_size / 2;
 	uint64_t rounds = 0;
 	int ret = 0;
 	uint32_t done = 0;
 	uint64_t wait = ODP_SCHED_WAIT;
+	uint64_t tot_rounds = test_options->rounds * test_options->num_cpu;
 
 	thr = odp_thread_id();
 	max_nsec = 2 * test_options->rounds * test_options->period_ns;
 	max_time = odp_time_local_from_ns(max_nsec);
 	printf("Thread %i starting on CPU %i\n", thr, odp_cpu_id());
 
-	if (test_options->group_mode == 0) {
+	if (group_mode == 0) {
 		/* Timeout events are load balanced. Using this
 		 * period to poll exit status. */
 		wait = odp_schedule_wait_time(100 * ODP_TIME_MSEC_IN_NS);
@@ -280,7 +283,15 @@ static int worker_thread(void *arg)
 
 		rounds++;
 
-		if (rounds < test_options->rounds) {
+		if (group_mode) {
+			if (rounds >= test_options->rounds)
+				done = 1;
+		} else {
+			if (odp_atomic_fetch_inc_u64(&global->tot_rounds) >= (tot_rounds - 1))
+				done = 1;
+		}
+
+		if (done == 0) {
 			tmo = odp_timeout_from_event(ev);
 			timer = odp_timeout_timer(tmo);
 			start_param.tmo_ev = ev;
@@ -291,8 +302,6 @@ static int worker_thread(void *arg)
 				ODPH_ERR("Timer start failed (%" PRIu64 ")\n", rounds);
 				done = 1;
 			}
-		} else {
-			done = 1;
 		}
 
 		/* Do work */
@@ -530,7 +539,6 @@ static int start_timers(test_global_t *global)
 static void destroy_timers(test_global_t *global)
 {
 	uint32_t i;
-	odp_event_t ev;
 	test_options_t *test_options = &global->test_options;
 	uint32_t num_cpu = test_options->num_cpu;
 
@@ -540,9 +548,8 @@ static void destroy_timers(test_global_t *global)
 		if (timer == ODP_TIMER_INVALID)
 			continue;
 
-		ev = odp_timer_free(timer);
-		if (ev != ODP_EVENT_INVALID)
-			odp_event_free(ev);
+		if (odp_timer_free(timer))
+			ODPH_ERR("Timer free failed (%u)\n", i);
 	}
 
 	if (global->timer_pool != ODP_TIMER_POOL_INVALID)
@@ -744,6 +751,7 @@ int main(int argc, char **argv)
 
 	memset(global, 0, sizeof(test_global_t));
 	odp_atomic_init_u32(&global->exit_test, 0);
+	odp_atomic_init_u64(&global->tot_rounds, 0);
 
 	global->timer_pool = ODP_TIMER_POOL_INVALID;
 	global->tmo_pool   = ODP_POOL_INVALID;
diff --git a/test/performance/odp_timer_perf.c b/test/performance/odp_timer_perf.c
index 5abee1988..d5b2a6e03 100644
--- a/test/performance/odp_timer_perf.c
+++ b/test/performance/odp_timer_perf.c
@@ -514,7 +514,6 @@ static int destroy_timer_pool(test_global_t *global)
 	odp_pool_t pool;
 	odp_queue_t queue;
 	odp_timer_t timer;
-	odp_event_t ev;
 	uint32_t i, j;
 	test_options_t *test_options = &global->test_options;
 	uint32_t num_timer = test_options->num_timer;
@@ -527,13 +526,8 @@ static int destroy_timer_pool(test_global_t *global)
 			if (timer == ODP_TIMER_INVALID)
 				break;
 
-			ev = odp_timer_free(timer);
-
-			if (ev != ODP_EVENT_INVALID) {
-				if (test_options->mode == MODE_SCHED_OVERH)
-					printf("Event from timer free %i/%i\n", i, j);
-				odp_event_free(ev);
-			}
+			if (odp_timer_free(timer))
+				printf("Timer free failed: %i/%i\n", i, j);
 		}
 
 		queue = global->queue[i];
@@ -649,15 +643,17 @@ static int sched_mode_worker(void *arg)
 	return ret;
 }
 
-static void cancel_timers(test_global_t *global, uint32_t worker_idx)
+static int cancel_timers(test_global_t *global, uint32_t worker_idx)
 {
 	uint32_t i, j;
+	int r;
 	odp_timer_t timer;
 	odp_event_t ev;
 	test_options_t *test_options = &global->test_options;
 	uint32_t num_tp = test_options->num_tp;
 	uint32_t num_timer = test_options->num_timer;
 	uint32_t num_worker = test_options->num_cpu;
+	int ret = 0;
 
 	for (i = 0; i < num_tp; i++) {
 		for (j = 0; j < num_timer; j++) {
@@ -668,10 +664,20 @@ static void cancel_timers(test_global_t *global, uint32_t worker_idx)
 			if (timer == ODP_TIMER_INVALID)
 				continue;
 
-			if (odp_timer_cancel(timer, &ev) == ODP_TIMER_SUCCESS)
+			r = odp_timer_cancel(timer, &ev);
+
+			if (r == ODP_TIMER_SUCCESS) {
 				odp_event_free(ev);
+			} else if (r == ODP_TIMER_TOO_NEAR) {
+				ret = 1;
+			} else {
+				ret = -1;
+				break;
+			}
 		}
 	}
+
+	return ret;
 }
 
 static int set_cancel_mode_worker(void *arg)
@@ -815,7 +821,8 @@ static int set_cancel_mode_worker(void *arg)
 	diff = odp_cpu_cycles_diff(c2, c1);
 
 	/* Cancel all timers that belong to this thread */
-	cancel_timers(global, worker_idx);
+	if (cancel_timers(global, worker_idx))
+		ODPH_ERR("Timer cancel failed\n");
 
 	/* Update stats */
 	global->stat[thr].events = num_tmo;
@@ -835,7 +842,7 @@ static int set_expire_mode_worker(void *arg)
 	uint32_t i, j, exit_test;
 	odp_event_t ev;
 	odp_timeout_t tmo;
-	uint64_t c2, c3, c4, diff, nsec, time_ns, target_ns, period_tick;
+	uint64_t c2, c3, c4, diff, nsec, time_ns, target_ns, period_tick, wait;
 	odp_timer_t timer;
 	odp_timer_start_t start_param;
 	odp_time_t t1, t2;
@@ -937,11 +944,15 @@ static int set_expire_mode_worker(void *arg)
 	nsec = odp_time_diff_ns(t2, t1);
 
 	/* Cancel all timers that belong to this thread */
-	cancel_timers(global, thread_arg->worker_idx);
+	status = cancel_timers(global, thread_arg->worker_idx);
+
+	wait = ODP_SCHED_NO_WAIT;
+	if (status > 0)
+		wait = odp_schedule_wait_time(opt->period_ns);
 
-	/* Free already scheduled events */
+	/* Wait and free remaining events */
 	while (1) {
-		ev = odp_schedule(NULL, ODP_SCHED_NO_WAIT);
+		ev = odp_schedule(NULL, wait);
 		if (ev == ODP_EVENT_INVALID)
 			break;
 		odp_event_free(ev);