[1/2] alarmtimer: Create alarmtimer sysfs to make duration of kernel suspend check configurable

Message ID 20240207100619.3947442-2-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
  Currently, the alarmtimer_suspend does not allow the kernel
to suspend if the next alarm is within 2 seconds.
Create alarmtimer sysfs to make the value of 2 seconds configurable.
This allows flexibility to provide a different value based on the
type of device running the Linux kernel. As a data point, about 40% of
kernel suspend failures in a subset of Android devices were due to
this check. A differently configured value can avoid these suspend
failures which performs a lot of additional work affecting the
power consumption of these Android devices.

Signed-off-by: Pranav Prasad <pranavpp@google.com>
Signed-off-by: Kelly Rossmoyer <krossmo@google.com>
---
 kernel/time/alarmtimer.c | 61 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 58 insertions(+), 3 deletions(-)
  

Comments

Pranav Prasad Feb. 7, 2024, 4:46 p.m. UTC | #1
Please ignore this patch, submitting v2 with some more suggested fixes.

Pranav

On Wed, Feb 7, 2024 at 2:06 AM Pranav Prasad <pranavpp@google.com> wrote:
>
> Currently, the alarmtimer_suspend does not allow the kernel
> to suspend if the next alarm is within 2 seconds.
> Create alarmtimer sysfs to make the value of 2 seconds configurable.
> This allows flexibility to provide a different value based on the
> type of device running the Linux kernel. As a data point, about 40% of
> kernel suspend failures in a subset of Android devices were due to
> this check. A differently configured value can avoid these suspend
> failures which performs a lot of additional work affecting the
> power consumption of these Android devices.
>
> Signed-off-by: Pranav Prasad <pranavpp@google.com>
> Signed-off-by: Kelly Rossmoyer <krossmo@google.com>
> ---
>  kernel/time/alarmtimer.c | 61 ++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 58 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
> index 4657cb8e8b1f..e5d2e560b4c1 100644
> --- a/kernel/time/alarmtimer.c
> +++ b/kernel/time/alarmtimer.c
> @@ -33,6 +33,8 @@
>  #define CREATE_TRACE_POINTS
>  #include <trace/events/alarmtimer.h>
>
> +static const char alarmtimer_group_name[] = "alarmtimer";
> +
>  /**
>   * struct alarm_base - Alarm timer bases
>   * @lock:              Lock for syncrhonized access to the base
> @@ -63,6 +65,56 @@ static struct rtc_timer              rtctimer;
>  static struct rtc_device       *rtcdev;
>  static DEFINE_SPINLOCK(rtcdev_lock);
>
> +/* Duration to check for soonest alarm during kernel suspend */
> +static u64 suspend_check_duration_ns = 2 * NSEC_PER_SEC;
> +
> +static ssize_t suspend_check_duration_show(struct kobject *kobj,
> +                       struct kobj_attribute *attr, char *buf)
> +{
> +       return sysfs_emit(buf, "%llu\n", suspend_check_duration_ns);
> +}
> +
> +static ssize_t suspend_check_duration_store(struct kobject *kobj,
> +                       struct kobj_attribute *attr, const char *buf, size_t n)
> +{
> +       u64 val;
> +
> +       if (kstrtou64(buf, 10, &val))
> +               return -EINVAL;
> +
> +       suspend_check_duration_ns = val;
> +
> +       return n;
> +}
> +
> +static struct kobj_attribute suspend_check_duration =
> +                       __ATTR_RW(suspend_check_duration);
> +
> +static struct attribute *alarmtimer_attrs[] = {
> +       &suspend_check_duration.attr,
> +       NULL,
> +};
> +
> +static const struct attribute_group alarmtimer_attr_group = {
> +       .name   = alarmtimer_group_name,
> +       .attrs  = alarmtimer_attrs,
> +};
> +
> +/**
> + * alarmtimer_sysfs_add - Adds sysfs attributes for alarmtimer
> + *
> + * Returns 0 if successful, non-zero value for error.
> + */
> +static int alarmtimer_sysfs_add(void)
> +{
> +       int ret = sysfs_create_group(kernel_kobj, &alarmtimer_attr_group);
> +
> +       if (ret)
> +               pr_warn("[%s] failed to create a sysfs group\n", __func__);
> +
> +       return ret;
> +}
> +
>  /**
>   * alarmtimer_get_rtcdev - Return selected rtcdevice
>   *
> @@ -98,8 +150,11 @@ static int alarmtimer_rtc_add_device(struct device *dev)
>
>         pdev = platform_device_register_data(dev, "alarmtimer",
>                                              PLATFORM_DEVID_AUTO, NULL, 0);
> -       if (!IS_ERR(pdev))
> +       if (!IS_ERR(pdev)) {
>                 device_init_wakeup(&pdev->dev, true);
> +               if (alarmtimer_sysfs_add())
> +                       pr_warn("[%s] Failed to add alarmtimer sysfs attributes\n", __func__);
> +       }
>
>         spin_lock_irqsave(&rtcdev_lock, flags);
>         if (!IS_ERR(pdev) && !rtcdev) {
> @@ -279,8 +334,8 @@ static int alarmtimer_suspend(struct device *dev)
>         if (min == 0)
>                 return 0;
>
> -       if (ktime_to_ns(min) < 2 * NSEC_PER_SEC) {
> -               pm_wakeup_event(dev, 2 * MSEC_PER_SEC);
> +       if (ktime_to_ns(min) < suspend_check_duration_ns) {
> +               pm_wakeup_event(dev, suspend_check_duration_ns/NSEC_PER_MSEC);
>                 return -EBUSY;
>         }
>
> --
> 2.43.0.594.gd9cf4e227d-goog
>
  

Patch

diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
index 4657cb8e8b1f..e5d2e560b4c1 100644
--- a/kernel/time/alarmtimer.c
+++ b/kernel/time/alarmtimer.c
@@ -33,6 +33,8 @@ 
 #define CREATE_TRACE_POINTS
 #include <trace/events/alarmtimer.h>
 
+static const char alarmtimer_group_name[] = "alarmtimer";
+
 /**
  * struct alarm_base - Alarm timer bases
  * @lock:		Lock for syncrhonized access to the base
@@ -63,6 +65,56 @@  static struct rtc_timer		rtctimer;
 static struct rtc_device	*rtcdev;
 static DEFINE_SPINLOCK(rtcdev_lock);
 
+/* Duration to check for soonest alarm during kernel suspend */
+static u64 suspend_check_duration_ns = 2 * NSEC_PER_SEC;
+
+static ssize_t suspend_check_duration_show(struct kobject *kobj,
+			struct kobj_attribute *attr, char *buf)
+{
+	return sysfs_emit(buf, "%llu\n", suspend_check_duration_ns);
+}
+
+static ssize_t suspend_check_duration_store(struct kobject *kobj,
+			struct kobj_attribute *attr, const char *buf, size_t n)
+{
+	u64 val;
+
+	if (kstrtou64(buf, 10, &val))
+		return -EINVAL;
+
+	suspend_check_duration_ns = val;
+
+	return n;
+}
+
+static struct kobj_attribute suspend_check_duration =
+			__ATTR_RW(suspend_check_duration);
+
+static struct attribute *alarmtimer_attrs[] = {
+	&suspend_check_duration.attr,
+	NULL,
+};
+
+static const struct attribute_group alarmtimer_attr_group = {
+	.name	= alarmtimer_group_name,
+	.attrs	= alarmtimer_attrs,
+};
+
+/**
+ * alarmtimer_sysfs_add - Adds sysfs attributes for alarmtimer
+ *
+ * Returns 0 if successful, non-zero value for error.
+ */
+static int alarmtimer_sysfs_add(void)
+{
+	int ret = sysfs_create_group(kernel_kobj, &alarmtimer_attr_group);
+
+	if (ret)
+		pr_warn("[%s] failed to create a sysfs group\n", __func__);
+
+	return ret;
+}
+
 /**
  * alarmtimer_get_rtcdev - Return selected rtcdevice
  *
@@ -98,8 +150,11 @@  static int alarmtimer_rtc_add_device(struct device *dev)
 
 	pdev = platform_device_register_data(dev, "alarmtimer",
 					     PLATFORM_DEVID_AUTO, NULL, 0);
-	if (!IS_ERR(pdev))
+	if (!IS_ERR(pdev)) {
 		device_init_wakeup(&pdev->dev, true);
+		if (alarmtimer_sysfs_add())
+			pr_warn("[%s] Failed to add alarmtimer sysfs attributes\n", __func__);
+	}
 
 	spin_lock_irqsave(&rtcdev_lock, flags);
 	if (!IS_ERR(pdev) && !rtcdev) {
@@ -279,8 +334,8 @@  static int alarmtimer_suspend(struct device *dev)
 	if (min == 0)
 		return 0;
 
-	if (ktime_to_ns(min) < 2 * NSEC_PER_SEC) {
-		pm_wakeup_event(dev, 2 * MSEC_PER_SEC);
+	if (ktime_to_ns(min) < suspend_check_duration_ns) {
+		pm_wakeup_event(dev, suspend_check_duration_ns/NSEC_PER_MSEC);
 		return -EBUSY;
 	}