2023-09-15 16:45:14

by Guenter Roeck

[permalink] [raw]
Subject: [PATCH 2/2] rtc: alarmtimer: Use maximum alarm time offset

Some userspace applications use timerfd_create() to request wakeups after
a long period of time. For example, a backup application may request a
wakeup once per week. This is perfectly fine as long as the system does
not try to suspend. However, if the system tries to suspend and the
system's RTC does not support the required alarm timeout, the suspend
operation will fail with an error such as

rtc_cmos 00:01: Alarms can be up to one day in the future
PM: dpm_run_callback(): platform_pm_suspend+0x0/0x4a returns -22
alarmtimer alarmtimer.4.auto: platform_pm_suspend+0x0/0x4a returned -22 after 117 usecs
PM: Device alarmtimer.4.auto failed to suspend: error -22

This results in a refusal to suspend the system, causing substantial
battery drain on affected systems.

To fix the problem, use the maximum alarm time offset as reported by rtc
drivers to set the maximum alarm time. While this may result in early
wakeups from suspend, it is still much better than not suspending at all.

This patch standardizes system behavior if the requested alarm timeout
is larger than the alarm timeout supported by the rtc chip. Currently,
in this situation, the rtc driver will do one of the following.
- It may return an error.
- It may limit the alarm timeout to the maximum supported by the rtc chip.
- It may mask the timeout by the maximum alarm timeout supported by the RTC
chip (i.e. a requested timeout of 1 day + 1 minute may result in a 1
minute timeout).

With this patch in place, if the rtc driver reports the maximum alarm
timeout supported by the rtc chip, the system will always limit the alarm
timeout to the maximum supported by the rtc chip.

Cc: Brian Norris <[email protected]>
Signed-off-by: Guenter Roeck <[email protected]>
---
v2: Use API function instead of accessing rtc core internals directly.
Modify comment in code per feedback received.

kernel/time/alarmtimer.c | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
index 8d9f13d847f0..0dc68372efd0 100644
--- a/kernel/time/alarmtimer.c
+++ b/kernel/time/alarmtimer.c
@@ -290,6 +290,17 @@ static int alarmtimer_suspend(struct device *dev)
rtc_timer_cancel(rtc, &rtctimer);
rtc_read_time(rtc, &tm);
now = rtc_tm_to_ktime(tm);
+
+ /*
+ * If the RTC alarm timer only supports a limited time offset, set the
+ * alarm time to the maximum supported value.
+ * The system may wake up earlier (possibly much earlier) than expected
+ * when the alarmtimer runs. This is the best the kernel can do if
+ * the alarmtimer exceeds the time that the rtc device can be programmed
+ * for.
+ */
+ min = rtc_bound_alarmtime(rtc, min);
+
now = ktime_add(now, min);

/* Set alarm, if in the past reject suspend briefly to handle */
--
2.39.2


2023-09-15 17:52:57

by John Stultz

[permalink] [raw]
Subject: Re: [PATCH 2/2] rtc: alarmtimer: Use maximum alarm time offset

On Fri, Sep 15, 2023 at 8:22 AM Guenter Roeck <[email protected]> wrote:
>
> Some userspace applications use timerfd_create() to request wakeups after
> a long period of time. For example, a backup application may request a
> wakeup once per week. This is perfectly fine as long as the system does
> not try to suspend. However, if the system tries to suspend and the
> system's RTC does not support the required alarm timeout, the suspend
> operation will fail with an error such as
>
> rtc_cmos 00:01: Alarms can be up to one day in the future
> PM: dpm_run_callback(): platform_pm_suspend+0x0/0x4a returns -22
> alarmtimer alarmtimer.4.auto: platform_pm_suspend+0x0/0x4a returned -22 after 117 usecs
> PM: Device alarmtimer.4.auto failed to suspend: error -22
>
> This results in a refusal to suspend the system, causing substantial
> battery drain on affected systems.
>
> To fix the problem, use the maximum alarm time offset as reported by rtc
> drivers to set the maximum alarm time. While this may result in early
> wakeups from suspend, it is still much better than not suspending at all.
>
> This patch standardizes system behavior if the requested alarm timeout
> is larger than the alarm timeout supported by the rtc chip. Currently,
> in this situation, the rtc driver will do one of the following.
> - It may return an error.
> - It may limit the alarm timeout to the maximum supported by the rtc chip.
> - It may mask the timeout by the maximum alarm timeout supported by the RTC
> chip (i.e. a requested timeout of 1 day + 1 minute may result in a 1
> minute timeout).
>
> With this patch in place, if the rtc driver reports the maximum alarm
> timeout supported by the rtc chip, the system will always limit the alarm
> timeout to the maximum supported by the rtc chip.
>
> Cc: Brian Norris <[email protected]>
> Signed-off-by: Guenter Roeck <[email protected]>
> ---
> v2: Use API function instead of accessing rtc core internals directly.
> Modify comment in code per feedback received.
>
> kernel/time/alarmtimer.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
> index 8d9f13d847f0..0dc68372efd0 100644
> --- a/kernel/time/alarmtimer.c
> +++ b/kernel/time/alarmtimer.c
> @@ -290,6 +290,17 @@ static int alarmtimer_suspend(struct device *dev)
> rtc_timer_cancel(rtc, &rtctimer);
> rtc_read_time(rtc, &tm);
> now = rtc_tm_to_ktime(tm);
> +
> + /*
> + * If the RTC alarm timer only supports a limited time offset, set the
> + * alarm time to the maximum supported value.
> + * The system may wake up earlier (possibly much earlier) than expected
> + * when the alarmtimer runs. This is the best the kernel can do if
> + * the alarmtimer exceeds the time that the rtc device can be programmed
> + * for.
> + */
> + min = rtc_bound_alarmtime(rtc, min);
> +

Thanks for all the iterations on this Guenter! This looks nice.

Acked-by: John Stultz <[email protected]>

thanks
-john

Subject: [tip: timers/core] alarmtimer: Use maximum alarm time for suspend

The following commit has been merged into the timers/core branch of tip:

Commit-ID: 8ceea12d183cf29f28072dede218a04eda2a789c
Gitweb: https://git.kernel.org/tip/8ceea12d183cf29f28072dede218a04eda2a789c
Author: Guenter Roeck <[email protected]>
AuthorDate: Fri, 15 Sep 2023 08:22:38 -07:00
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Mon, 09 Oct 2023 15:03:28 +02:00

alarmtimer: Use maximum alarm time for suspend

Some userspace applications use timerfd_create() to request wakeups after
a long period of time. For example, a backup application may request a
wakeup once per week. This is perfectly fine as long as the system does
not try to suspend. However, if the system tries to suspend and the
system's RTC does not support the required alarm timeout, the suspend
operation will fail with an error such as

rtc_cmos 00:01: Alarms can be up to one day in the future
PM: dpm_run_callback(): platform_pm_suspend+0x0/0x4a returns -22
alarmtimer alarmtimer.4.auto: platform_pm_suspend+0x0/0x4a returned -22 after 117 usecs
PM: Device alarmtimer.4.auto failed to suspend: error -22

This results in a refusal to suspend the system, causing substantial
battery drain on affected systems.

To fix the problem, use the maximum alarm time offset as reported by RTC
drivers to set the maximum alarm time. While this may result in early
wakeups from suspend, it is still much better than not suspending at all.

Standardize system behavior if the requested alarm timeout is larger than
the alarm timeout supported by the rtc chip. Currently, in this situation,
the RTC driver will do one of the following:

- It may return an error.
- It may limit the alarm timeout to the maximum supported by the rtc chip.
- It may mask the timeout by the maximum alarm timeout supported by the RTC
chip (i.e. a requested timeout of 1 day + 1 minute may result in a 1
minute timeout).

With this in place, if the RTC driver reports the maximum alarm timeout
supported by the RTC chip, the system will always limit the alarm timeout
to the maximum supported by the RTC chip.

Signed-off-by: Guenter Roeck <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Acked-by: John Stultz <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
kernel/time/alarmtimer.c | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
index 8d9f13d..4657cb8 100644
--- a/kernel/time/alarmtimer.c
+++ b/kernel/time/alarmtimer.c
@@ -290,6 +290,17 @@ static int alarmtimer_suspend(struct device *dev)
rtc_timer_cancel(rtc, &rtctimer);
rtc_read_time(rtc, &tm);
now = rtc_tm_to_ktime(tm);
+
+ /*
+ * If the RTC alarm timer only supports a limited time offset, set the
+ * alarm time to the maximum supported value.
+ * The system may wake up earlier (possibly much earlier) than expected
+ * when the alarmtimer runs. This is the best the kernel can do if
+ * the alarmtimer exceeds the time that the rtc device can be programmed
+ * for.
+ */
+ min = rtc_bound_alarmtime(rtc, min);
+
now = ktime_add(now, min);

/* Set alarm, if in the past reject suspend briefly to handle */