linux-kernel - Re: [PATCH v2 2/7] rtc: alarmtimer: Use maximum alarm time offset

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANDhNCqd5VdwGaqHdeFK=Sui+fX_s7SxXnto9jdF_+0c-cuYSw@mail.gmail.com>
Date:   Thu, 24 Aug 2023 20:52:44 -0700
From:   John Stultz <jstultz@...gle.com>
To:     Guenter Roeck <linux@...ck-us.net>
Cc:     Alexandre Belloni <alexandre.belloni@...tlin.com>,
        Alessandro Zummo <a.zummo@...ertech.it>,
        Benson Leung <bleung@...omium.org>,
        Miquel Raynal <miquel.raynal@...tlin.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Stephen Boyd <sboyd@...nel.org>, linux-rtc@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Brian Norris <briannorris@...omium.org>
Subject: Re: [PATCH v2 2/7] rtc: alarmtimer: Use maximum alarm time offset

On Thu, Aug 17, 2023 at 3:55 PM Guenter Roeck <linux@...ck-us.net> 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 will result in brief
> spurious wakeups from suspend, it is still much better than not suspending
> at all.
>
> Cc: Brian Norris <briannorris@...omium.org>
> Signed-off-by: Guenter Roeck <linux@...ck-us.net>
> ---
> v2: Rename range_max_offset -> alarm_offset_max
>
>  kernel/time/alarmtimer.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
> index 8d9f13d847f0..895e3a6d6444 100644
> --- a/kernel/time/alarmtimer.c
> +++ b/kernel/time/alarmtimer.c
> @@ -290,6 +290,19 @@ 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 will wake up earlier than necessary and is expected
> +        * to go back to sleep if it has nothing to do.
> +        * It would be desirable to handle such early wakeups without fully
> +        * waking up the system, but it is unknown if this is even possible.
> +        */
> +       if (rtc->alarm_offset_max &&
> +           rtc->alarm_offset_max * MSEC_PER_SEC < ktime_to_ms(min))
> +               min = ms_to_ktime(rtc->alarm_offset_max * MSEC_PER_SEC);

I don't really have an objection here, but I wonder if this would be
better abstracted by a rtc_ function?

ktime_t rtc_bound_ktime_interval(ktime interval)
{
  if (!rtc->alarm_offset_max)
      return interval;
  return ms_to_ktime(min(rtc->alarm_offset_max, ktime_to_ms(interval)));
}

(simple enough to throw into rtc.h maybe as an inline function?)

Then the above would be tweaked to:
min = rtc_bound_interval(min);

thanks
-john