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

Pranav Prasad
2024-02-07T17:29:36Z

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

Pranav

On Wed, Feb 7, 2024 at 8:39 AM 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 AM 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, 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@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 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
>>