MIME-Version: 1.0
References: <20230211064527.3481754-1-jstultz@google.com> <20230211064527.3481754-2-jstultz@google.com>
 <CAOf5uwnW1u=nfFnj3C8kCVmhgwRaVh6sHZR1RGnXdbrCNpkGVg@mail.gmail.com> <87o7porea9.ffs@tglx>
In-Reply-To: <87o7porea9.ffs@tglx>
From:   Michael Nazzareno Trimarchi <michael@amarulasolutions.com>
Date:   Mon, 20 Feb 2023 09:23:04 +0100
Message-ID: <CAOf5uwmhtQ8GXhMiE-Y3-wgL5=xfjOv0Tpq1vqPB8p=LyZHBmw@mail.gmail.com>
Subject: Re: [RFC][PATCH 2/2] time: alarmtimer: Use TASK_FREEZABLE to cleanup
 freezer handling
To:     Thomas Gleixner <tglx@linutronix.de>
Cc:     John Stultz <jstultz@google.com>,
        LKML <linux-kernel@vger.kernel.org>,
        Stephen Boyd <sboyd@kernel.org>, Arnd Bergmann <arnd@arndb.de>,
        Michael <michael@mipisi.de>, kernel-team@android.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Thomas

On Mon, Feb 20, 2023 at 8:23 AM Thomas Gleixner <tglx@linutronix.de> 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?
>

  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