Messages in this thread | | | From | Michael Nazzareno Trimarchi <> | Date | Sat, 18 Feb 2023 15:51:40 +0100 | Subject | Re: [RFC][PATCH 1/2] time: alarmtimer: Fix erroneous case of using 0 as an "invalid" initialization value |
| |
On Sat, Feb 11, 2023 at 7:45 AM John Stultz <jstultz@google.com> wrote: > > Michael reported seeing an error where alarmtimers would > occasionally not wake the system up. > > It was found that in alarmtimer_suspend() it was exiting via > the: > if (min == 0) > return 0; > check. This logic was from one of the early versions of the > original alarmtimer patch, where we initialized min to 0, and > then this check would exit early if we found no timers to expire > (leaving min still at 0). > > However, its possible for an alarmtimer to expire as we are > checking it, leaving the calculated delta to be zero, and thus > setting min to zero. > > This is the result of my using 0 as an invalid time value which > is clearly erroneous. Instead KTIME_MAX should have been used. > > This patch, split out from a change originally suggested by > Thomas Gleixner, changes the logic to instead use KTIME_MAX. > > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Stephen Boyd <sboyd@kernel.org> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Michael <michael@mipisi.de> > Cc: Michael Trimarchi <michael@amarulasolutions.com> > Cc: kernel-team@android.com > Reported-by: Michael <michael@mipisi.de> > Reported-by: Michael Trimarchi <michael@amarulasolutions.com> > Fixes: ff3ead96d17f ("timers: Introduce in-kernel alarm-timer interface") > Originally-by: Thomas Gleixner <tglx@linutronix.de> > Link: https://lore.kernel.org/lkml/alpine.DEB.2.21.1909021247250.3955@nanos.tec.linutronix.de/ > [jstultz: Forward ported to 6.2-rc, and split out just the > KTIME_MAX change] > Signed-off-by: John Stultz <jstultz@google.com> > --- > kernel/time/alarmtimer.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c > index 5897828b9d7e..f7b2128f64e2 100644 > --- a/kernel/time/alarmtimer.c > +++ b/kernel/time/alarmtimer.c > @@ -251,7 +251,7 @@ static int alarmtimer_suspend(struct device *dev) > min = freezer_delta; > expires = freezer_expires; > type = freezer_alarmtype; > - freezer_delta = 0; > + freezer_delta = KTIME_MAX; > spin_unlock_irqrestore(&freezer_delta_lock, flags); > > rtc = alarmtimer_get_rtcdev(); > @@ -271,13 +271,14 @@ static int alarmtimer_suspend(struct device *dev) > if (!next) > continue; > delta = ktime_sub(next->expires, base->get_ktime()); > - if (!min || (delta < min)) { > + if (delta < min) { > expires = next->expires; > min = delta; > type = i; > } > } > - if (min == 0) > + /* No timers to expire */ > + if (min == KTIME_MAX) > return 0; > > if (ktime_to_ns(min) < 2 * NSEC_PER_SEC) { > @@ -503,7 +504,7 @@ static void alarmtimer_freezerset(ktime_t absexp, enum alarmtimer_type type) > delta = ktime_sub(absexp, base->get_ktime()); > > spin_lock_irqsave(&freezer_delta_lock, flags); > - if (!freezer_delta || (delta < freezer_delta)) { > + if (delta < freezer_delta) { > freezer_delta = delta; > freezer_expires = absexp; > freezer_alarmtype = type; > -- > 2.39.1.581.gbfd45094c4-goog >
Tested-by: Michael Trimarchi <michael@amarulasolutions.com>
I don't find regression on this
Michael -- Michael Nazzareno Trimarchi Co-Founder & Chief Executive Officer M. +39 347 913 2170 michael@amarulasolutions.com __________________________________
Amarula Solutions BV Joop Geesinkweg 125, 1114 AB, Amsterdam, NL T. +31 (0)85 111 9172 info@amarulasolutions.com www.amarulasolutions.com
| |