Received-SPF: pass (google.com: domain of linux-kernel+bounces-56829-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240207100619.3947442-1-pranavpp@google.com> <20240207100619.3947442-3-pranavpp@google.com>
 <CACkwYU22HOuB=8qpTTUoxZUm7kpnWhSeH2ui3vsHg8AP__xd9Q@mail.gmail.com>
In-Reply-To: <CACkwYU22HOuB=8qpTTUoxZUm7kpnWhSeH2ui3vsHg8AP__xd9Q@mail.gmail.com>
From: Pranav Prasad <pranavpp@google.com>
Date: Wed, 7 Feb 2024 09:29:23 -0800
Message-ID: <CACkwYU31qGLYSNZRyZerddNRxSWS3UL=X19Wevx2cATXKOWDoQ@mail.gmail.com>
Subject: Re: [PATCH 2/2] alarmtimer: Modify alarmtimer suspend callback to
To: Thomas Gleixner <tglx@linutronix.de>, John Stultz <jstultz@google.com>, sboyd@kernel.org
Cc: linux-kernel@vger.kernel.org, Kelly Rossmoyer <krossmo@google.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Please ignore this patch, submitting v2 with some more suggested fixes.

Pranav


On Wed, Feb 7, 2024 at 8:39=E2=80=AFAM Pranav Prasad <pranavpp@google.com> =
wrote:
>
> Please ignore this patch, submitting v2 with some more suggested fixes.
>
> Pranav
>
> On Wed, Feb 7, 2024, 2:06=E2=80=AFAM Pranav Prasad <pranavpp@google.com> =
wrote:
>>
>> The alarmtimer driver currently fails suspend attempts when there is an
>> alarm pending within the next suspend_check_duration_ns nanoseconds, sin=
ce
>> the system is expected to wake up soon anyway. The entire suspend proces=
s
>> is initiated even though the system will immediately awaken. This proces=
s
>> includes substantial work before the suspend fails and additional work
>> afterwards to undo the failed suspend that was attempted. Therefore on
>> battery-powered devices that initiate suspend attempts from userspace, i=
t
>> may be advantageous to be able to fail the suspend earlier in the suspen=
d
>> flow to avoid power consumption instead of unnecessarily doing extra wor=
k.
>> As one data point, an analysis of a subset of Android devices showed tha=
t
>> imminent alarms account for roughly 40% of all suspend failures on avera=
ge
>> leading to unnecessary power wastage.
>>
>> To facilitate this, register a PM notifier in the alarmtimer subsystem
>> that checks if an alarm is imminent during the prepare stage of kernel
>> suspend denoted by the event PM_SUSPEND_PREPARE. If an alarm is imminent=
,
>> it returns the errno code ETIME instead of EBUSY to userspace in order t=
o
>> make it easily diagnosable.
>>
>> Signed-off-by: Pranav Prasad <pranavpp@google.com>
>> Signed-off-by: Kelly Rossmoyer <krossmo@google.com>
>> ---
>>  kernel/time/alarmtimer.c | 121 ++++++++++++++++++++++++++++-----------
>>  1 file changed, 88 insertions(+), 33 deletions(-)
>>
>> diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
>> index e5d2e560b4c1..229de937c266 100644
>> --- a/kernel/time/alarmtimer.c
>> +++ b/kernel/time/alarmtimer.c
>> @@ -27,6 +27,7 @@
>>  #include <linux/compat.h>
>>  #include <linux/module.h>
>>  #include <linux/time_namespace.h>
>> +#include <linux/suspend.h>
>>
>>  #include "posix-timers.h"
>>
>> @@ -115,6 +116,87 @@ static int alarmtimer_sysfs_add(void)
>>         return ret;
>>  }
>>
>> +/**
>> + * alarmtimer_init_soonest - Initializes parameters to find soonest ala=
rm.
>> + * @min: ptr to relative time to the soonest alarm to expire
>> + * @expires: ptr to absolute time of the soonest alarm to expire
>> + * @type: ptr to alarm type
>> + *
>> + */
>> +static void alarmtimer_init_soonest(ktime_t *min, ktime_t *expires, int=
 *type)
>> +{
>> +       unsigned long flags;
>> +
>> +       spin_lock_irqsave(&freezer_delta_lock, flags);
>> +       *min =3D freezer_delta;
>> +       *expires =3D freezer_expires;
>> +       *type =3D freezer_alarmtype;
>> +       freezer_delta =3D 0;
>> +       spin_unlock_irqrestore(&freezer_delta_lock, flags);
>> +}
>> +
>> +/**
>> + * alarmtimer_get_soonest - Finds the soonest alarm to expire among the=
 alarm bases.
>> + * @min: ptr to relative time to the soonest alarm to expire
>> + * @expires: ptr to absolute time of the soonest alarm to expire
>> + * @type: ptr to alarm type
>> + *
>> + */
>> +static void alarmtimer_get_soonest(ktime_t *min, ktime_t *expires, int =
*type)
>> +{
>> +       int i;
>> +       unsigned long flags;
>> +
>> +       /* Find the soonest timer to expire */
>> +       for (i =3D 0; i < ALARM_NUMTYPE; i++) {
>> +               struct alarm_base *base =3D &alarm_bases[i];
>> +               struct timerqueue_node *next;
>> +               ktime_t delta;
>> +
>> +               spin_lock_irqsave(&base->lock, flags);
>> +               next =3D timerqueue_getnext(&base->timerqueue);
>> +               spin_unlock_irqrestore(&base->lock, flags);
>> +               if (!next)
>> +                       continue;
>> +               delta =3D ktime_sub(next->expires, base->get_ktime());
>> +               if (!(*min) || (delta < *min)) {
>> +                       *expires =3D next->expires;
>> +                       *min =3D delta;
>> +                       *type =3D i;
>> +               }
>> +       }
>> +}
>> +
>> +static int alarmtimer_pm_callback(struct notifier_block *nb,
>> +                           unsigned long mode, void *_unused)
>> +{
>> +       ktime_t min, expires;
>> +       int type;
>> +
>> +       switch (mode) {
>> +       case PM_SUSPEND_PREPARE:
>> +               /* Initialize parameters to find soonest timer */
>> +               alarmtimer_init_soonest(&min, &expires, &type);
>> +
>> +               /* Find the soonest timer to expire */
>> +               alarmtimer_get_soonest(&min, &expires, &type);
>> +
>> +               if (min =3D=3D 0)
>> +                       return NOTIFY_DONE;
>> +
>> +               if (ktime_to_ns(min) < suspend_check_duration_ns) {
>> +                       pr_warn("[%s] Suspend abort due to imminent alar=
m\n", __func__);
>> +                       return notifier_from_errno(-ETIME);
>> +               }
>> +       }
>> +
>> +       return NOTIFY_DONE;
>> +}
>> +
>> +static struct notifier_block alarmtimer_pm_notifier =3D {
>> +       .notifier_call =3D alarmtimer_pm_callback,
>> +};
>> +
>>  /**
>>   * alarmtimer_get_rtcdev - Return selected rtcdevice
>>   *
>> @@ -181,6 +263,7 @@ static int alarmtimer_rtc_add_device(struct device *=
dev)
>>  static inline void alarmtimer_rtc_timer_init(void)
>>  {
>>         rtc_timer_init(&rtctimer, NULL, NULL);
>> +       register_pm_notifier(&alarmtimer_pm_notifier);
>>  }
>>
>>  static struct class_interface alarmtimer_rtc_interface =3D {
>> @@ -296,48 +379,20 @@ EXPORT_SYMBOL_GPL(alarm_expires_remaining);
>>  static int alarmtimer_suspend(struct device *dev)
>>  {
>>         ktime_t min, now, expires;
>> -       int i, ret, type;
>> +       int ret, type;
>>         struct rtc_device *rtc;
>> -       unsigned long flags;
>>         struct rtc_time tm;
>>
>> -       spin_lock_irqsave(&freezer_delta_lock, flags);
>> -       min =3D freezer_delta;
>> -       expires =3D freezer_expires;
>> -       type =3D freezer_alarmtype;
>> -       freezer_delta =3D 0;
>> -       spin_unlock_irqrestore(&freezer_delta_lock, flags);
>> +       /* Initialize parameters to find soonest timer */
>> +       alarmtimer_init_soonest(&min, &expires, &type);
>>
>>         rtc =3D alarmtimer_get_rtcdev();
>>         /* If we have no rtcdev, just return */
>>         if (!rtc)
>>                 return 0;
>>
>> -       /* Find the soonest timer to expire*/
>> -       for (i =3D 0; i < ALARM_NUMTYPE; i++) {
>> -               struct alarm_base *base =3D &alarm_bases[i];
>> -               struct timerqueue_node *next;
>> -               ktime_t delta;
>> -
>> -               spin_lock_irqsave(&base->lock, flags);
>> -               next =3D timerqueue_getnext(&base->timerqueue);
>> -               spin_unlock_irqrestore(&base->lock, flags);
>> -               if (!next)
>> -                       continue;
>> -               delta =3D ktime_sub(next->expires, base->get_ktime());
>> -               if (!min || (delta < min)) {
>> -                       expires =3D next->expires;
>> -                       min =3D delta;
>> -                       type =3D i;
>> -               }
>> -       }
>> -       if (min =3D=3D 0)
>> -               return 0;
>> -
>> -       if (ktime_to_ns(min) < suspend_check_duration_ns) {
>> -               pm_wakeup_event(dev, suspend_check_duration_ns/NSEC_PER_=
MSEC);
>> -               return -EBUSY;
>> -       }
>> +       /* Find the soonest timer to expire */
>> +       alarmtimer_get_soonest(&min, &expires, &type);
>>
>>         trace_alarmtimer_suspend(expires, type);
>>
>> --
>> 2.43.0.594.gd9cf4e227d-goog
>>