[Skiboot] [PATCH 2/2] timer: Reschedule timer if OPAL receives early timer interrupt

[Vasant Hegde]

On 05/04/2018 02:33 AM, Benjamin Herrenschmidt wrote:
> On Thu, 2018-05-03 at 18:51 +0530, Vasant Hegde wrote:
>> If we receive timer interrupt before timer expiry, we are not rescheduling
>> timer again. Lets fix this by rescheduling timer.
> 
> Doesn't that mean we'll reschedule every time we call check timer in a
> loop ?

update_timer_expiry() compares with previously scheduled timer value. I thought
that's enough.

> I'd be keen on only doing that from an actual timer interrupt

Ok. I can add that check.

> and only if we haven't already rescheduled (we could cache the
> scheduled time, and clear it on interrupt entry, so we reschedule at
> most once per interrupt).

Sorry. I don't understand why we should cache schedule time.

- I will add check to make sure we reschedule only in interrupt path.
- So we will be rescheduling only when we get early response from SBE.
   And once we reschedule we will not enter this path until we get next 
interrupt from SBE.

  something like below works ?

commit dbf2f5e2fa465433b0ee54162d69179bd1148032
Author: Vasant Hegde <hegdevasant at linux.vnet.ibm.com>
Date:   Thu Apr 19 11:32:01 2018 +0530

     timer: Reschedule timer if OPAL receives early timer interrupt

     If we receive timer interrupt before timer expiry, we are not rescheduling
     timer again. Lets fix this by rescheduling timer.

     Suggested-by: Benjamin Herrenschmidt <benh at kernel.crashing.org>
     Signed-off-by: Vasant Hegde <hegdevasant at linux.vnet.ibm.com>

diff --git a/core/timer.c b/core/timer.c
index 1c5395178..d7f9dae00 100644
--- a/core/timer.c
+++ b/core/timer.c
@@ -196,17 +196,23 @@ static void __check_poll_timers(uint64_t now)
  	timer_in_poll = false;
  }

-static void __check_timers(uint64_t now)
+static void __check_timers(bool from_interrupt, uint64_t now)
  {
  	struct timer *t;

  	for (;;) {
  		t = list_top(&timer_list, struct timer, link);

-		/* Top of list not expired ? that's it ... */
-		if (!t || t->target > now)
+		if (!t)
  			break;

+		/* Top of list not expired ? re-schedule timer */
+		if (t->target > now) {
+			if (from_interrupt)
+				update_timer_expiry(t->target);
+			break;
+		}
+
  		/* Top of list still running, we have to delay handling
  		 * it. For now just skip until the next poll, when we have
  		 * SLW interrupts, we'll probably want to trip another one
@@ -252,7 +258,7 @@ void check_timers(bool from_interrupt)
  	lock(&timer_lock);
  	if (!from_interrupt)
  		__check_poll_timers(now);
-	__check_timers(now);
+	__check_timers(from_interrupt, now);
  	unlock(&timer_lock);
  }


-Vasant