[Skiboot] [PATCH 04/11] core/timer: Always update hardware timer

[Reza Arbab]

On Sat, Nov 16, 2024 at 02:07:20PM +1000, Nicholas Piggin wrote:
>--- a/core/timer.c
>+++ b/core/timer.c
>@@ -119,15 +119,18 @@ static void __schedule_timer_at(struct timer *t, uint64_t when)
> 			if (when >= lt->target)
> 				continue;
> 			list_add_before(&timer_list, &lt->link, &t->link);
>-			goto bail;
>+			goto added;
> 		}
> 		list_add_tail(&timer_list, &t->link);
>-	}
>- bail:
>-	/* Pick up the next timer and upddate the SBE HW timer */
>-	lt = list_top(&timer_list, struct timer, link);
>-	if (lt) {
>-		update_timer_expiry(lt->target);
>+ added:
>+		/* Timer running code will update expiry at the end */
>+		if (!this_cpu()->running_timer) {
>+			/* Pick the next timer and upddate the SBE HW timer */
>+			lt = list_top(&timer_list, struct timer, link);
>+			if (lt && (lt == t || when < lt->target)) {
>+				update_timer_expiry(lt->target);
>+			}
>+		}
> 	}
> }
>
>@@ -190,6 +193,7 @@ static void __check_poll_timers(uint64_t now)
> 		/* Allright, first remove it and mark it running */
> 		__remove_timer(t);
> 		t->running = this_cpu();
>+		this_cpu()->running_timer = true;
>
> 		/* Now we can unlock and call it's expiry */
> 		unlock(&timer_lock);
>@@ -197,6 +201,7 @@ static void __check_poll_timers(uint64_t now)
>
> 		/* Re-lock and mark not running */
> 		lock(&timer_lock);
>+		this_cpu()->running_timer = false;
> 		t->running = NULL;
> 	}
> 	timer_in_poll = false;
>@@ -210,8 +215,12 @@ static void __check_timers(uint64_t now)
> 		t = list_top(&timer_list, struct timer, link);
>
> 		/* Top of list not expired ? that's it ... */
>-		if (!t || t->target > now)
>+		if (!t)
> 			break;
>+		if (t->target > now) {
>+			update_timer_expiry(t->target);
>+			break;
>+		}
>
> 		/* Top of list still running, we have to delay handling it,
> 		 * let's reprogram the SLW/SBE with a small delay. We chose
>@@ -225,6 +234,7 @@ static void __check_timers(uint64_t now)
> 		/* Allright, first remove it and mark it running */
> 		__remove_timer(t);
> 		t->running = this_cpu();
>+		this_cpu()->running_timer = true;
>
> 		/* Now we can unlock and call it's expiry */
> 		unlock(&timer_lock);
>@@ -232,6 +242,7 @@ static void __check_timers(uint64_t now)
>
> 		/* Re-lock and mark not running */
> 		lock(&timer_lock);
>+		this_cpu()->running_timer = false;
> 		t->running = NULL;
>
> 		/* Update time stamp */

At the top of the file we have:

   #ifdef __TEST__
   #define this_cpu()      ((void *)-1)
   #define cpu_relax()
   #else
   #include <cpu.h>
   #endif

So this patch breaks the build of one of the tests.

   $ make core/test/run-timer
	  [ HOSTCC ]  core/test/run-timer.c
   In file included from core/test/run-timer.c:32:
   core/test/../timer.c: In function ‘__schedule_timer_at’:
   core/test/../timer.c:127:18: error: dereferencing ‘void *’ pointer [-Werror]
      if (!this_cpu()->running_timer) {
		    ^~
   core/test/../timer.c:127:18: error: request for member ‘running_timer’ in something not a structure or union
   core/test/../timer.c: In function ‘__check_poll_timers’:
   core/test/../timer.c:197:13: error: dereferencing ‘void *’ pointer [-Werror]
      this_cpu()->running_timer = true;
	       ^~
   core/test/../timer.c:197:13: error: request for member ‘running_timer’ in something not a structure or union
   core/test/../timer.c:205:13: error: dereferencing ‘void *’ pointer [-Werror]
      this_cpu()->running_timer = false;
	       ^~
   core/test/../timer.c:205:13: error: request for member ‘running_timer’ in something not a structure or union
   core/test/../timer.c: In function ‘__check_timers’:
   core/test/../timer.c:237:13: error: dereferencing ‘void *’ pointer [-Werror]
      this_cpu()->running_timer = true;
	       ^~
   core/test/../timer.c:237:13: error: request for member ‘running_timer’ in something not a structure or union
   core/test/../timer.c:245:13: error: dereferencing ‘void *’ pointer [-Werror]
      this_cpu()->running_timer = false;
	       ^~
   core/test/../timer.c:245:13: error: request for member ‘running_timer’ in something not a structure or union

-- 
Reza Arbab