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

Message ID 20240207100619.3947442-3-pranavpp@google.com
State New
Headers
Series alarmtimer: Rework the suspend flow in alarmtimer |

Commit Message

Pranav Prasad Feb. 7, 2024, 10:06 a.m. UTC
  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(-)
  

Comments

Pranav Prasad Feb. 7, 2024, 5:29 p.m. UTC | #1
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
>>
  

Patch

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);