Re: [RFC][PATCH 2/2] time: alarmtimer: Use TASK_FREEZABLE to cleanup freezer handling

[Michael Nazzareno Trimarchi]

Hi

On Mon, Feb 20, 2023 at 9:23 AM Michael Nazzareno Trimarchi
<michael@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> Hi Thomas
>
> On Mon, Feb 20, 2023 at 8:23 AM Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:
> >
> > On Sat, Feb 18 2023 at 15:56, Michael Nazzareno Trimarchi wrote:
> > >
> > > I have changed the alarm test to check some corner case
> >
> > Could you tell us please which test did you change and what the change is?
> >
>

There are no changes in the kernel apart pr_info on enqueue dequeue
and fired call
in alarmtimer.c. linux  master branch sha
38f8ccde04a3fa317b51b05e63c3cb57e1641931
and both patches applied

time: alarmtimer: Use TASK_FREEZABLE to cleanup freezer handling
time: alarmtimer: Fix erroneous case of using 0 as an "invalid"
initialization value

Michael

>   if (timer_create(CLOCK_REALTIME_ALARM, &se, &tm1) == -1) {
>        printf("timer_create failed, %s unsupported?\n",
>        clockstring(alarm_clock_id));
>        exit(1);
>   }
>
>   clock_gettime(alarm_clock_id, &start_time);
>   printf("Start time (%s): %ld:%ld\n", clockstring(alarm_clock_id),
>                                 start_time.tv_sec, start_time.tv_nsec);
>   printf("Setting alarm for every %i seconds\n", SUSPEND_SECS);
>   its1.it_value = start_time;
>   its1.it_value.tv_sec += 4;
>   /* Empiric value for get in between a freeze task and fire of the timer */
>   its1.it_value.tv_nsec += 132079666;
>   its1.it_interval.tv_sec = 4;
>   its1.it_interval.tv_nsec = 0;
>
>   timer_settime(tm1, TIMER_ABSTIME, &its1, &its2);
>
>   printf("Starting suspend loops\n");
>   while (1) {
>       int ret;
>       sleep(4);
>       system("echo mem > /sys/power/state");
>   }
>
> > > periodic_alarm
> > > Start time (CLOCK_REALTIME_ALARM)[   85.624819] alarmtimer_enqueue: called
> > > : 94:865096467
> > > Setting alarm for every 4 seconds
> > > Starting suspend loops
> > > [   89.674127] PM: suspend entry (deep)
> > > [   89.714916] Filesystems sync: 0.037 seconds
> > > [   89.733594] Freezing user space processes
> > > [   89.740680] Freezing user space processes completed (elapsed 0.002 seconds)
> > > [   89.748593] OOM killer disabled.
> > > [   89.752257] Freezing remaining freezable tasks
> > > [   89.756807] alarmtimer_fired: called
> > > [   89.756831] alarmtimer_dequeue: called <---- HERE
> > >
> > > I have the dequeue but not an enquee of the periodic alarm. I was
> > > thinking that create a periodic time of 4 seconds
> > > and have the first alarm on suspend will always guarantee the re-arm
> > > it but it's not working as I expect
> >
> > Again. You are not telling what you expect. It depends on how the timer
> > is set up whether the timer is self rearmed or not.
> >
>
> Posted the pseudo code. As far as I understand, the timer periodic is
> re-armed in get_signal
> do_work_pending->do_signal()->get_signal(), then in the posix timer
> code the enqueue_alarm is called. All the timers
> used from suspend are coming from the expiration list that contains
> only the enqueue alarm
>
> My test case is a single core, arm and with only one REAL_TIME_ALARM
> periodic timer created.
>
> Michael
>
> > Thanks,
> >
> >         tglx



-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
michael@xxxxxxxxxxxxxxxxxxxx
__________________________________

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
info@xxxxxxxxxxxxxxxxxxxx
www.amarulasolutions.com