| Message ID | 20180503132126.6830-2-hegdevasant@linux.vnet.ibm.com |
|---|---|
| State | Deferred |
| Headers | show |
| Series | [1/2] timer: Move update_timer_expiry call to separate function | expand |
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 ? I'd be keen on only doing that from an actual timer interrupt 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). > Suggested-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> > --- > core/timer.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/core/timer.c b/core/timer.c > index 1c5395178..5890b203c 100644 > --- a/core/timer.c > +++ b/core/timer.c > @@ -203,10 +203,15 @@ static void __check_timers(uint64_t now) > 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) { > + 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
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@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@kernel.crashing.org> Signed-off-by: Vasant Hegde <hegdevasant@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
diff --git a/core/timer.c b/core/timer.c index 1c5395178..5890b203c 100644 --- a/core/timer.c +++ b/core/timer.c @@ -203,10 +203,15 @@ static void __check_timers(uint64_t now) 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) { + 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
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@kernel.crashing.org> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> --- core/timer.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)