Date:   Sat, 11 Feb 2023 06:45:26 +0000
Mime-Version: 1.0
Message-ID: <20230211064527.3481754-1-jstultz@google.com>
Subject: [RFC][PATCH 1/2] time: alarmtimer: Fix erroneous case of using 0 as
 an "invalid" initialization value
From:   John Stultz <jstultz@google.com>
To:     LKML <linux-kernel@vger.kernel.org>
Cc:     John Stultz <jstultz@google.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Stephen Boyd <sboyd@kernel.org>, Arnd Bergmann <arnd@arndb.de>,
        Michael <michael@mipisi.de>,
        Michael Trimarchi <michael@amarulasolutions.com>,
        kernel-team@android.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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