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

Pranav Prasad
2024-02-07T16:46:55Z

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
>