[dpdk-dev] [RFC PATCH] rte_timer: Fix rte_timer_reset return value

[rsanford2@gmail.com]

From: Robert Sanford <rsanford2@gmail.com>

- API rte_timer_reset() should return -1 when the timer is in the
RUNNING or CONFIG state. Instead, it ignores the return value of
internal function __rte_timer_reset() and always returns 0.
We change rte_timer_reset() to return the value returned by
__rte_timer_reset().

- Change API rte_timer_reset_sync() to invoke rte_pause() while
spin-waiting for rte_timer_reset() to succeed.

- Enhance timer stress test 2 to report how many timer reset
collisions occur, i.e., how many times rte_timer_reset() fails
due to a timer being in the CONFIG state.

Signed-off-by: Robert Sanford <rsanford2@gmail.com>

---
 app/test/test_timer.c        |   25 ++++++++++++++++++++++---
 lib/librte_timer/rte_timer.c |    7 +++----
 2 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/app/test/test_timer.c b/app/test/test_timer.c
index 4b4800b..2f27f84 100644
--- a/app/test/test_timer.c
+++ b/app/test/test_timer.c
@@ -247,12 +247,15 @@ static int
 timer_stress2_main_loop(__attribute__((unused)) void *arg)
 {
 	static struct rte_timer *timers;
-	int i;
+	int i, ret;
 	static volatile int ready = 0;
 	uint64_t delay = rte_get_timer_hz() / 4;
 	unsigned lcore_id = rte_lcore_id();
+	int32_t my_collisions = 0;
+	static rte_atomic32_t collisions = RTE_ATOMIC32_INIT(0);
 
 	if (lcore_id == rte_get_master_lcore()) {
+		cb_count = 0;
 		timers = rte_malloc(NULL, sizeof(*timers) * NB_STRESS2_TIMERS, 0);
 		if (timers == NULL) {
 			printf("Test Failed\n");
@@ -268,15 +271,24 @@ timer_stress2_main_loop(__attribute__((unused)) void *arg)
 	}
 
 	/* have all cores schedule all timers on master lcore */
-	for (i = 0; i < NB_STRESS2_TIMERS; i++)
-		rte_timer_reset(&timers[i], delay, SINGLE, rte_get_master_lcore(),
+	for (i = 0; i < NB_STRESS2_TIMERS; i++) {
+		ret = rte_timer_reset(&timers[i], delay, SINGLE, rte_get_master_lcore(),
 				timer_stress2_cb, NULL);
+		/* there will be collisions when multiple cores simultaneously
+		 * configure the same timers */
+		if (ret != 0)
+			my_collisions++;
+	}
+	if (my_collisions != 0)
+		rte_atomic32_add(&collisions, my_collisions);
 
 	ready = 0;
 	rte_delay_ms(500);
 
 	/* now check that we get the right number of callbacks */
 	if (lcore_id == rte_get_master_lcore()) {
+		if ((my_collisions = rte_atomic32_read(&collisions)) != 0)
+			printf("- %d timer reset collisions (OK)\n", my_collisions);
 		rte_timer_manage();
 		if (cb_count != NB_STRESS2_TIMERS) {
 			printf("Test Failed\n");
@@ -311,6 +323,13 @@ timer_stress2_main_loop(__attribute__((unused)) void *arg)
 	/* now check that we get the right number of callbacks */
 	if (lcore_id == rte_get_master_lcore()) {
 		rte_timer_manage();
+
+		/* clean up statics, in case we run again */
+		rte_free(timers);
+		timers = 0;
+		ready = 0;
+		rte_atomic32_set(&collisions, 0);
+
 		if (cb_count != NB_STRESS2_TIMERS) {
 			printf("Test Failed\n");
 			printf("- Stress test 2, part 2 failed\n");
diff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c
index 269a992..d18abf5 100644
--- a/lib/librte_timer/rte_timer.c
+++ b/lib/librte_timer/rte_timer.c
@@ -424,10 +424,8 @@ rte_timer_reset(struct rte_timer *tim, uint64_t ticks,
 	else
 		period = 0;
 
-	__rte_timer_reset(tim,  cur_time + ticks, period, tim_lcore,
+	return __rte_timer_reset(tim,  cur_time + ticks, period, tim_lcore,
 			  fct, arg, 0);
-
-	return 0;
 }
 
 /* loop until rte_timer_reset() succeed */
@@ -437,7 +435,8 @@ rte_timer_reset_sync(struct rte_timer *tim, uint64_t ticks,
 		     rte_timer_cb_t fct, void *arg)
 {
 	while (rte_timer_reset(tim, ticks, type, tim_lcore,
-			       fct, arg) != 0);
+			       fct, arg) != 0)
+		rte_pause();
 }
 
 /* Stop the timer associated with the timer handle tim */
-- 
1.7.1