From:   Thomas Gleixner <tglx@linutronix.de>
To:     Michael Nazzareno Trimarchi <michael@amarulasolutions.com>
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
Subject: Re: [RFC][PATCH 2/2] time: alarmtimer: Use TASK_FREEZABLE to
 cleanup freezer handling
In-Reply-To: <CAOf5uwmpayJwpAFzUS6qsCgdpyek1f-2t2t9YNr76vnRjSC8=w@mail.gmail.com>
References: <20230211064527.3481754-1-jstultz@google.com>
 <20230211064527.3481754-2-jstultz@google.com>
 <CAOf5uwnW1u=nfFnj3C8kCVmhgwRaVh6sHZR1RGnXdbrCNpkGVg@mail.gmail.com>
 <87o7porea9.ffs@tglx>
 <CAOf5uwmhtQ8GXhMiE-Y3-wgL5=xfjOv0Tpq1vqPB8p=LyZHBmw@mail.gmail.com>
 <CAOf5uwmpayJwpAFzUS6qsCgdpyek1f-2t2t9YNr76vnRjSC8=w@mail.gmail.com>
Date:   Mon, 20 Feb 2023 18:48:22 +0100
Message-ID: <87a618qlcp.ffs@tglx>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

Michael!

On Mon, Feb 20 2023 at 12:47, Michael Nazzareno Trimarchi wrote:
> On Mon, Feb 20, 2023 at 9:23 AM Michael Nazzareno Trimarchi
> <michael@amarulasolutions.com> wrote:
>> On Mon, Feb 20, 2023 at 8:23 AM Thomas Gleixner <tglx@linutronix.de> wrote:
>> > > 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.

Let me try to decode the above.

Yes, periodic timers are re-armed when the signal is delivered, but that
happens way later after the system resumed.

Here is what I observe:

[   27.349352] alarmtimer_enqueue()

U: Before SUSPEND

[   31.353228] PM: suspend entry (s2idle)
[   31.388857] Filesystems sync: 0.033 seconds
[   31.418427] Freezing user space processes
[   31.422406] Freezing user space processes completed (elapsed 0.002 seconds)
[   31.425435] OOM killer disabled.
[   31.426833] Freezing remaining freezable tasks
[   31.429838] Freezing remaining freezable tasks completed (elapsed 0.001 seconds)
[   31.432922] printk: Suspending console(s) (use no_console_suspend to debug)
[   31.435912] alarmtimer alarmtimer.0.auto: PM: dpm_run_callback(): platform_pm_suspend+0x0/0x50 returns -16
[   31.435954] alarmtimer alarmtimer.0.auto: PM: failed to suspend: error -16

That means the RTC interrupt was raised before the system was able to suspend.

[   31.436077] PM: Some devices failed to suspend, or early wake event detected
[   31.444270] OOM killer enabled.
[   31.445011] Restarting tasks ... done.
[   31.446820] random: crng reseeded on system resumption
[   31.466019] PM: suspend exit

[   31.480283] alarmtimer_fired()
[   31.481403] alarmtimer_dequeue()     <- Signal queued

[   31.482596] alarmtimer_rearm()_      <- Signal delivery
[   31.483713] alarmtimer_enqueue()

U: ALRM signal received                 <- User space signal handler
U: Post Suspend                         <- system("echo .... >") returns

That's 6.2 + John's patches.

Thanks,

        tglx