[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAOf5uwn4NPsXp91aUAwa0RFP9jFRyaNDzOQr6FzUHTC26Y6+bg@mail.gmail.com>
Date: Wed, 15 Feb 2023 09:22:57 +0100
From: Michael Nazzareno Trimarchi <michael@...rulasolutions.com>
To: John Stultz <jstultz@...gle.com>
Cc: LKML <linux-kernel@...r.kernel.org>,
Thomas Gleixner <tglx@...utronix.de>,
Stephen Boyd <sboyd@...nel.org>, Arnd Bergmann <arnd@...db.de>,
Michael <michael@...isi.de>, kernel-team@...roid.com
Subject: Re: [RFC][PATCH 2/2] time: alarmtimer: Use TASK_FREEZABLE to cleanup
freezer handling
Hi John
On Sat, Feb 11, 2023 at 7:45 AM John Stultz <jstultz@...gle.com> wrote:
>
> Instead of trying to handle the freezer waking up tasks from
> schedule() in nanosleep on alarmtimers explicitly, use
> TASK_FREEZABLE which marks the task freezable when it goes
> to schedule, which prevents the signal wakeup.
>
> This allows for the freezer handling to be removed, simplifying
> the code.
>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: Stephen Boyd <sboyd@...nel.org>
> Cc: Arnd Bergmann <arnd@...db.de>
> Cc: Michael <michael@...isi.de>
> Cc: Michael Trimarchi <michael@...rulasolutions.com>
> Cc: kernel-team@...roid.com
> Originally-by: Thomas Gleixner <tglx@...utronix.de>
> Link: https://lore.kernel.org/lkml/alpine.DEB.2.21.1909021247250.3955@nanos.tec.linutronix.de/
> [jstultz: Forward ported to 6.2-rc and split out from a separate
> fix.]
> Signed-off-by: John Stultz <jstultz@...gle.com>
> ---
> kernel/time/alarmtimer.c | 53 ++--------------------------------------
> 1 file changed, 2 insertions(+), 51 deletions(-)
>
> diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
> index f7b2128f64e2..15ecde8fcc1b 100644
> --- a/kernel/time/alarmtimer.c
> +++ b/kernel/time/alarmtimer.c
> @@ -49,14 +49,6 @@ static struct alarm_base {
> clockid_t base_clockid;
> } alarm_bases[ALARM_NUMTYPE];
>
> -#if defined(CONFIG_POSIX_TIMERS) || defined(CONFIG_RTC_CLASS)
> -/* freezer information to handle clock_nanosleep triggered wakeups */
> -static enum alarmtimer_type freezer_alarmtype;
> -static ktime_t freezer_expires;
> -static ktime_t freezer_delta;
> -static DEFINE_SPINLOCK(freezer_delta_lock);
> -#endif
> -
> #ifdef CONFIG_RTC_CLASS
> /* rtc timer and device for setting alarm wakeups at suspend */
> static struct rtc_timer rtctimer;
> @@ -241,19 +233,12 @@ EXPORT_SYMBOL_GPL(alarm_expires_remaining);
> */
> static int alarmtimer_suspend(struct device *dev)
> {
> - ktime_t min, now, expires;
> + ktime_t now, expires, min = KTIME_MAX;
> int i, 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 = KTIME_MAX;
> - spin_unlock_irqrestore(&freezer_delta_lock, flags);
> -
> rtc = alarmtimer_get_rtcdev();
> /* If we have no rtcdev, just return */
> if (!rtc)
> @@ -480,38 +465,6 @@ u64 alarm_forward_now(struct alarm *alarm, ktime_t interval)
> EXPORT_SYMBOL_GPL(alarm_forward_now);
>
> #ifdef CONFIG_POSIX_TIMERS
> -
> -static void alarmtimer_freezerset(ktime_t absexp, enum alarmtimer_type type)
> -{
> - struct alarm_base *base;
> - unsigned long flags;
> - ktime_t delta;
> -
> - switch(type) {
> - case ALARM_REALTIME:
> - base = &alarm_bases[ALARM_REALTIME];
> - type = ALARM_REALTIME_FREEZER;
> - break;
> - case ALARM_BOOTTIME:
> - base = &alarm_bases[ALARM_BOOTTIME];
> - type = ALARM_BOOTTIME_FREEZER;
> - break;
> - default:
> - WARN_ONCE(1, "Invalid alarm type: %d\n", type);
> - return;
> - }
> -
> - delta = ktime_sub(absexp, base->get_ktime());
> -
> - spin_lock_irqsave(&freezer_delta_lock, flags);
> - if (delta < freezer_delta) {
> - freezer_delta = delta;
> - freezer_expires = absexp;
> - freezer_alarmtype = type;
> - }
> - spin_unlock_irqrestore(&freezer_delta_lock, flags);
> -}
> -
> /**
> * clock2alarm - helper that converts from clockid to alarmtypes
> * @clockid: clockid.
> @@ -750,7 +703,7 @@ static int alarmtimer_do_nsleep(struct alarm *alarm, ktime_t absexp,
> struct restart_block *restart;
> alarm->data = (void *)current;
> do {
> - set_current_state(TASK_INTERRUPTIBLE);
> + set_current_state(TASK_INTERRUPTIBLE | TASK_FREEZABLE);
For kernel 5.10.x and lts is possible to use freezable_schedule and
let this set_current_state as was before.
I have seen patch that introduce the new state but I suppose that in
order to be compatible to stable this should be the
change. Am I right?
Michael
> alarm_start(alarm, absexp);
> if (likely(alarm->data))
> schedule();
> @@ -765,8 +718,6 @@ static int alarmtimer_do_nsleep(struct alarm *alarm, ktime_t absexp,
> if (!alarm->data)
> return 0;
>
> - if (freezing(current))
> - alarmtimer_freezerset(absexp, type);
> restart = ¤t->restart_block;
> if (restart->nanosleep.type != TT_NONE) {
> struct timespec64 rmt;
> --
> 2.39.1.581.gbfd45094c4-goog
>
--
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
michael@...rulasolutions.com
__________________________________
Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
info@...rulasolutions.com
www.amarulasolutions.com
Powered by blists - more mailing lists