Re: [PATCH 2/2] alarmtimer: Modify alarmtimer suspend callback to

[Pranav Prasad]

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

Pranav


On Wed, Feb 7, 2024 at 8:39 AM Pranav Prasad <pranavpp@xxxxxxxxxx> wrote:
>
> Please ignore this patch, submitting v2 with some more suggested fixes.
>
> Pranav
>
> On Wed, Feb 7, 2024, 2:06 AM Pranav Prasad <pranavpp@xxxxxxxxxx> wrote:
>>
>> The alarmtimer driver currently fails suspend attempts when there is an
>> alarm pending within the next suspend_check_duration_ns nanoseconds, since
>> the system is expected to wake up soon anyway. The entire suspend process
>> is initiated even though the system will immediately awaken. This process
>> 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, it
>> may be advantageous to be able to fail the suspend earlier in the suspend
>> flow to avoid power consumption instead of unnecessarily doing extra work.
>> As one data point, an analysis of a subset of Android devices showed that
>> imminent alarms account for roughly 40% of all suspend failures on average
>> 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 to
>> make it easily diagnosable.
>>
>> Signed-off-by: Pranav Prasad <pranavpp@xxxxxxxxxx>
>> Signed-off-by: Kelly Rossmoyer <krossmo@xxxxxxxxxx>
>> ---
>>  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 alarm.
>> + * @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 = freezer_delta;
>> +       *expires = freezer_expires;
>> +       *type = freezer_alarmtype;
>> +       freezer_delta = 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 = 0; i < ALARM_NUMTYPE; i++) {
>> +               struct alarm_base *base = &alarm_bases[i];
>> +               struct timerqueue_node *next;
>> +               ktime_t delta;
>> +
>> +               spin_lock_irqsave(&base->lock, flags);
>> +               next = timerqueue_getnext(&base->timerqueue);
>> +               spin_unlock_irqrestore(&base->lock, flags);
>> +               if (!next)
>> +                       continue;
>> +               delta = ktime_sub(next->expires, base->get_ktime());
>> +               if (!(*min) || (delta < *min)) {
>> +                       *expires = next->expires;
>> +                       *min = delta;
>> +                       *type = 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 == 0)
>> +                       return NOTIFY_DONE;
>> +
>> +               if (ktime_to_ns(min) < suspend_check_duration_ns) {
>> +                       pr_warn("[%s] Suspend abort due to imminent alarm\n", __func__);
>> +                       return notifier_from_errno(-ETIME);
>> +               }
>> +       }
>> +
>> +       return NOTIFY_DONE;
>> +}
>> +
>> +static struct notifier_block alarmtimer_pm_notifier = {
>> +       .notifier_call = 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 = {
>> @@ -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 = freezer_delta;
>> -       expires = freezer_expires;
>> -       type = freezer_alarmtype;
>> -       freezer_delta = 0;
>> -       spin_unlock_irqrestore(&freezer_delta_lock, flags);
>> +       /* Initialize parameters to find soonest timer */
>> +       alarmtimer_init_soonest(&min, &expires, &type);
>>
>>         rtc = alarmtimer_get_rtcdev();
>>         /* If we have no rtcdev, just return */
>>         if (!rtc)
>>                 return 0;
>>
>> -       /* Find the soonest timer to expire*/
>> -       for (i = 0; i < ALARM_NUMTYPE; i++) {
>> -               struct alarm_base *base = &alarm_bases[i];
>> -               struct timerqueue_node *next;
>> -               ktime_t delta;
>> -
>> -               spin_lock_irqsave(&base->lock, flags);
>> -               next = timerqueue_getnext(&base->timerqueue);
>> -               spin_unlock_irqrestore(&base->lock, flags);
>> -               if (!next)
>> -                       continue;
>> -               delta = ktime_sub(next->expires, base->get_ktime());
>> -               if (!min || (delta < min)) {
>> -                       expires = next->expires;
>> -                       min = delta;
>> -                       type = i;
>> -               }
>> -       }
>> -       if (min == 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
>>