timer: Stop calling list_top() racily

Message ID 1512581776.2224.129.camel@kernel.crashing.org
State Accepted
Headers show
Series
  • timer: Stop calling list_top() racily
Related show

Commit Message

Benjamin Herrenschmidt Dec. 6, 2017, 5:36 p.m.
This will trip the debug checks in debug builds under some circumstances
and is actually a rather bad idea as we might look at a timer that is
concurrently being removed and modified, and thus incorrectly assume
there is no work to do.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 core/timer.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

Comments

Stewart Smith Dec. 13, 2017, 3:07 a.m. | #1
Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:
> This will trip the debug checks in debug builds under some circumstances
> and is actually a rather bad idea as we might look at a timer that is
> concurrently being removed and modified, and thus incorrectly assume
> there is no work to do.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
>  core/timer.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)

THanks,

merged to master as of b504f2737e9ba3b6935b4ea1c015b3643aefaf51
and 5.9.x as of 6e975e1d0fdafdf6cf534da7742b57a538c4fa28

Patch

diff --git a/core/timer.c b/core/timer.c
index 4b5925fe..b10a5646 100644
--- a/core/timer.c
+++ b/core/timer.c
@@ -223,7 +223,6 @@  static void __check_timers(uint64_t now)
 
 void check_timers(bool from_interrupt)
 {
-	struct timer *t;
 	uint64_t now = mftb();
 
 	/* This is the polling variant, the SLW interrupt path, when it
@@ -231,9 +230,11 @@  void check_timers(bool from_interrupt)
 	 * the pollers
 	 */
 
-	/* Lockless "peek", a bit racy but shouldn't be a problem */
-	t = list_top(&timer_list, struct timer, link);
-	if (list_empty_nocheck(&timer_poll_list) && (!t || t->target > now))
+	/* Lockless "peek", a bit racy but shouldn't be a problem as
+	 * we are only looking at whether the list is empty
+	 */
+	if (list_empty_nocheck(&timer_poll_list) &&
+	    list_empty_nocheck(&timer_list))
 		return;
 
 	/* Take lock and try again */