[dpdk-dev] [PATCH 0/2] timer bugs fixes

[dpdk-dev] [PATCH 0/2] timer bugs fixes

[Vadim Suraev]

Vadim Suraev (2):
Bug: when a periodic timer's callback is running, if another      
    timer is manipulated, the periodic timer is not reloaded.    
Solution: set the update flag only if the modified timer is      
    in RUNNING state
Bug: When a timer is running       - if rte_timer_stop is called, the
    pending decrement is       skipped (decremented only if the timer
    is pending) and due       to the update flag the future processing
    is skipped so the       timer is counted as pending while it is
    stopped. - the same       applies when rte_timer_reset is called
    but then the pending       statistics is additionally incremented
    so the timer is       counted pending twice.     
Solution:
    decrement the pending       statistics after returning from the
    callback. If       rte_timer_stop was called, it skipped
    decrementing the       pending statistics. If rte_time_reset was
    called, the       pending statistics was incremented. If neither
    was called       and the timer is periodic, the pending statistics
    is       incremented when it is reloaded

 lib/librte_timer/rte_timer.c |   12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

-- 
1.7.9.5

[dpdk-dev] [PATCH 1/2] timer bug fix

[Vadim Suraev]

Bug: when a periodic timer's callback is running, if
 another   timer is manipulated, the periodic timer is
 not reloaded. 
Solution: set the update flag only if the
 modified timer is   in RUNNING state

Signed-off-by: Vadim Suraev <vadim.suraev@gmail.com>
---
 lib/librte_timer/rte_timer.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c
index 884ee0e..d07232b 100755
--- a/lib/librte_timer/rte_timer.c
+++ b/lib/librte_timer/rte_timer.c
@@ -378,7 +378,9 @@ __rte_timer_reset(struct rte_timer *tim, uint64_t expire,
 		return -1;
 
 	__TIMER_STAT_ADD(reset, 1);
-	priv_timer[lcore_id].updated = 1;
+	if (prev_status.state == RTE_TIMER_RUNNING) {
+		priv_timer[lcore_id].updated = 1;
+	}
 
 	/* remove it from list */
 	if (prev_status.state == RTE_TIMER_PENDING) {
@@ -453,7 +455,9 @@ rte_timer_stop(struct rte_timer *tim)
 		return -1;
 
 	__TIMER_STAT_ADD(stop, 1);
-	priv_timer[lcore_id].updated = 1;
+	if (prev_status.state == RTE_TIMER_RUNNING) {
+		priv_timer[lcore_id].updated = 1;
+	}
 
 	/* remove it from list */
 	if (prev_status.state == RTE_TIMER_PENDING) {
-- 
1.7.9.5

[dpdk-dev] [PATCH 2/2] timer bug fix

[Vadim Suraev]

Bug: When a timer is running   - if rte_timer_stop is
 called, the pending decrement is   skipped (decremented
 only if the timer is pending) and due   to the update
 flag the future processing is skipped so the   timer is
 counted as pending while it is stopped. - the same  
 applies when rte_timer_reset is called but then the
 pending   statistics is additionally incremented so the
 timer is   counted pending twice. 
Solution: decrement
 the pending   statistics after returning from the
 callback. If   rte_timer_stop was called, it skipped
 decrementing the   pending statistics. If
 rte_time_reset was called, the   pending statistics was
 incremented. If neither was called   and the timer is
 periodic, the pending statistics is   incremented when
 it is reloaded

Signed-off-by: Vadim Suraev <vadim.suraev@gmail.com>
---
 lib/librte_timer/rte_timer.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c
index d07232b..f0f0c2f 100755
--- a/lib/librte_timer/rte_timer.c
+++ b/lib/librte_timer/rte_timer.c
@@ -551,7 +551,7 @@ void rte_timer_manage(void)
 		tim->f(tim, tim->arg);
 
 		rte_spinlock_lock(&priv_timer[lcore_id].list_lock);
-
+		__TIMER_STAT_ADD(pending, -1);
 		/* the timer was stopped or reloaded by the callback
 		 * function, we have nothing to do here */
 		if (priv_timer[lcore_id].updated == 1)
@@ -559,7 +559,6 @@ void rte_timer_manage(void)
 
 		if (tim->period == 0) {
 			/* remove from done list and mark timer as stopped */
-			__TIMER_STAT_ADD(pending, -1);
 			status.state = RTE_TIMER_STOP;
 			status.owner = RTE_TIMER_NO_OWNER;
 			rte_wmb();
@@ -568,6 +567,7 @@ void rte_timer_manage(void)
 		else {
 			/* keep it in list and mark timer as pending */
 			status.state = RTE_TIMER_PENDING;
+			__TIMER_STAT_ADD(pending, 1);
 			status.owner = (int16_t)lcore_id;
 			rte_wmb();
 			tim->status.u32 = status.u32;
-- 
1.7.9.5

[dpdk-dev] [PATCH 0/2]

[Vadim Suraev]

two timer bugs fixed

 lib/librte_timer/rte_timer.c |   21 ++++++++++-----------
 lib/librte_timer/rte_timer.h |    7 ++++++-
 2 files changed, 16 insertions(+), 12 deletions(-)

-- 
1.7.9.5

[dpdk-dev] [PATCH 1/2] timer bug fix

[Vadim Suraev]

Description: when going running, the
 pending count is not incremented. 
Fix: decremend
 pending when going running. Then, if periodic,
 increment, if one shot - do nothing

Signed-off-by: Vadim Suraev <vadim.suraev@gmail.com>

---
 lib/librte_timer/rte_timer.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c
index a3d5cca..f98e904 100755
--- a/lib/librte_timer/rte_timer.c
+++ b/lib/librte_timer/rte_timer.c
@@ -173,7 +173,7 @@ timer_set_running_state(struct rte_timer *tim)
 		/* timer is not pending anymore */
 		if (prev_status.state != RTE_TIMER_PENDING)
 			return -1;
-
+		__TIMER_STAT_ADD(pending, -1);
 		/* here, we know that timer is stopped or pending,
 		 * mark it atomically as beeing configured */
 		status.state = RTE_TIMER_RUNNING;
@@ -555,7 +555,6 @@ void rte_timer_manage(void)
 
 		if (tim->period == 0) {
 			/* remove from done list and mark timer as stopped */
-			__TIMER_STAT_ADD(pending, -1);
 			status.state = RTE_TIMER_STOP;
 			status.owner = RTE_TIMER_NO_OWNER;
 			rte_wmb();
@@ -564,6 +563,7 @@ void rte_timer_manage(void)
 		else {
 			/* keep it in list and mark timer as pending */
 			status.state = RTE_TIMER_PENDING;
+			__TIMER_STAT_ADD(pending, 1);
 			status.owner = (int16_t)lcore_id;
 			rte_wmb();
 			tim->status.u32 = status.u32;
-- 
1.7.9.5