diff mbox series

[v4,3/4] timer: fix function to stop all timers

Message ID 20220812160748.287176-1-s.v.naga.harish.k@intel.com (mailing list archive)
State Changes Requested, archived
Delegated to: Jerin Jacob
Headers
Series [v4,1/4] eventdev/timer: add periodic event timer support |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation success Compilation OK
ci/github-robot: build success github build: passed
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/intel-Testing success Testing PASS

Commit Message

Naga Harish K, S V Aug. 12, 2022, 4:07 p.m. UTC 
  There is a possibility of deadlock in this API,
as same spinlock is tried to be acquired in nested manner.

If the lcore that is stopping the timer is different from the lcore
that owns the timer, the timer list lock is acquired in timer_del(),
even if local_is_locked is true. Because the same lock was already
acquired in rte_timer_stop_all(), the thread will hang.

This patch removes the acquisition of nested lock.

Fixes: 821c51267bcd63a ("timer: add function to stop all timers in a list")
Cc: stable@dpdk.org

Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
---
 lib/timer/rte_timer.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

Comments

Carrillo, Erik G Aug. 18, 2022, 1:12 p.m. UTC | #1 
> -----Original Message-----
> From: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> Sent: Friday, August 12, 2022 11:08 AM
> To: Carrillo, Erik G <erik.g.carrillo@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: [PATCH v4 3/4] timer: fix function to stop all timers
> 
> There is a possibility of deadlock in this API, as same spinlock is tried to be
> acquired in nested manner.
> 
> If the lcore that is stopping the timer is different from the lcore that owns the
> timer, the timer list lock is acquired in timer_del(), even if local_is_locked is
> true. Because the same lock was already acquired in rte_timer_stop_all(),
> the thread will hang.
> 
> This patch removes the acquisition of nested lock.
> 
> Fixes: 821c51267bcd63a ("timer: add function to stop all timers in a list")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
Acked-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>
diff mbox series

Patch

diff --git a/lib/timer/rte_timer.c b/lib/timer/rte_timer.c
index 9994813d0d..85d67573eb 100644
--- a/lib/timer/rte_timer.c
+++ b/lib/timer/rte_timer.c
@@ -580,7 +580,7 @@  rte_timer_reset_sync(struct rte_timer *tim, uint64_t ticks,
 }
 
 static int
-__rte_timer_stop(struct rte_timer *tim, int local_is_locked,
+__rte_timer_stop(struct rte_timer *tim,
 		 struct rte_timer_data *timer_data)
 {
 	union rte_timer_status prev_status, status;
@@ -602,7 +602,7 @@  __rte_timer_stop(struct rte_timer *tim, int local_is_locked,
 
 	/* remove it from list */
 	if (prev_status.state == RTE_TIMER_PENDING) {
-		timer_del(tim, prev_status, local_is_locked, priv_timer);
+		timer_del(tim, prev_status, 0, priv_timer);
 		__TIMER_STAT_ADD(priv_timer, pending, -1);
 	}
 
@@ -631,7 +631,7 @@  rte_timer_alt_stop(uint32_t timer_data_id, struct rte_timer *tim)
 
 	TIMER_DATA_VALID_GET_OR_ERR_RET(timer_data_id, timer_data, -EINVAL);
 
-	return __rte_timer_stop(tim, 0, timer_data);
+	return __rte_timer_stop(tim, timer_data);
 }
 
 /* loop until rte_timer_stop() succeed */
@@ -987,21 +987,16 @@  rte_timer_stop_all(uint32_t timer_data_id, unsigned int *walk_lcores,
 		walk_lcore = walk_lcores[i];
 		priv_timer = &timer_data->priv_timer[walk_lcore];
 
-		rte_spinlock_lock(&priv_timer->list_lock);
-
 		for (tim = priv_timer->pending_head.sl_next[0];
 		     tim != NULL;
 		     tim = next_tim) {
 			next_tim = tim->sl_next[0];
 
-			/* Call timer_stop with lock held */
-			__rte_timer_stop(tim, 1, timer_data);
+			__rte_timer_stop(tim, timer_data);
 
 			if (f)
 				f(tim, f_arg);
 		}
-
-		rte_spinlock_unlock(&priv_timer->list_lock);
 	}
 
 	return 0;