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

[Nicholas Piggin]

On Sat Jan 11, 2025 at 6:02 AM AEST, Reza Arbab wrote:
> 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) {
> 		    ^~

Sorry I always seem to do that, I'll try to fix.

Thanks,
Nick