Received-SPF: pass (google.com: domain of linux-kernel+bounces-135460-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240405083410.4896-1-anna-maria@linutronix.de>
 <87r0fg5ocg.fsf@somnus> <CAPDyKFo7KT4V8Nvn58N3mNfeW6ai=-5hampjN7N19kYaR7zdVA@mail.gmail.com>
In-Reply-To: <CAPDyKFo7KT4V8Nvn58N3mNfeW6ai=-5hampjN7N19kYaR7zdVA@mail.gmail.com>
From: "Rafael J. Wysocki" <rafael@kernel.org>
Date: Mon, 8 Apr 2024 15:39:12 +0200
Message-ID: <CAJZ5v0gUciunPSMsytW_voxx5F=DJURsLQb4TMLepMD6YPUZ-A@mail.gmail.com>
Subject: Re: [PATCH v2] PM: s2idle: Make sure CPUs will wakeup directly on resume
To: Ulf Hansson <ulf.hansson@linaro.org>, Anna-Maria Behnsen <anna-maria@linutronix.de>
Cc: linux-kernel@vger.kernel.org, "Rafael J . Wysocki" <rafael@kernel.org>, 
	Pavel Machek <pavel@ucw.cz>, Len Brown <len.brown@intel.com>, linux-pm@vger.kernel.org, 
	Frederic Weisbecker <frederic@kernel.org>, Thomas Gleixner <tglx@linutronix.de>, x86@kernel.org, 
	Mario Limonciello <mario.limonciello@amd.com>, stable@kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Mon, Apr 8, 2024 at 2:43=E2=80=AFPM Ulf Hansson <ulf.hansson@linaro.org>=
 wrote:
>
> On Mon, 8 Apr 2024 at 09:02, Anna-Maria Behnsen
> <anna-maria@linutronix.de> wrote:
> >
> > s2idle works like a regular suspend with freezing processes and freezin=
g
> > devices. All CPUs except the control CPU go into idle. Once this is
> > completed the control CPU kicks all other CPUs out of idle, so that the=
y
> > reenter the idle loop and then enter s2idle state. The control CPU then
> > issues an swait() on the suspend state and therefore enters the idle lo=
op
> > as well.
> >
> > Due to being kicked out of idle, the other CPUs leave their NOHZ states=
,
> > which means the tick is active and the corresponding hrtimer is program=
med
> > to the next jiffie.
> >
> > On entering s2idle the CPUs shut down their local clockevent device to
> > prevent wakeups. The last CPU which enters s2idle shuts down its local
> > clockevent and freezes timekeeping.
> >
> > On resume, one of the CPUs receives the wakeup interrupt, unfreezes
> > timekeeping and its local clockevent and starts the resume process. At =
that
> > point all other CPUs are still in s2idle with their clockevents switche=
d
> > off. They only resume when they are kicked by another CPU or after resu=
ming
> > devices and then receiving a device interrupt.
> >
> > That means there is no guarantee that all CPUs will wakeup directly on
> > resume. As a consequence there is no guarantee that timers which are qu=
eued
> > on those CPUs and should expire directly after resume, are handled. Als=
o
> > timer list timers which are remotely queued to one of those CPUs after
> > resume will not result in a reprogramming IPI as the tick is
> > active. Queueing a hrtimer will also not result in a reprogramming IPI
> > because the first hrtimer event is already in the past.
> >
> > The recent introduction of the timer pull model (7ee988770326 ("timers:
> > Implement the hierarchical pull model")) amplifies this problem, if the
> > current migrator is one of the non woken up CPUs. When a non pinned tim=
er
> > list timer is queued and the queuing CPU goes idle, it relies on the st=
ill
> > suspended migrator CPU to expire the timer which will happen by chance.
> >
> > The problem exists since commit 8d89835b0467 ("PM: suspend: Do not paus=
e
> > cpuidle in the suspend-to-idle path"). There the cpuidle_pause() call w=
hich
> > in turn invoked a wakeup for all idle CPUs was moved to a later point i=
n
> > the resume process. This might not be reached or reached very late beca=
use
> > it waits on a timer of a still suspended CPU.
> >
> > Address this by kicking all CPUs out of idle after the control CPU retu=
rns
> > from swait() so that they resume their timers and restore consistent sy=
stem
> > state.
> >
> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=3D218641
> > Fixes: 8d89835b0467 ("PM: suspend: Do not pause cpuidle in the suspend-=
to-idle path")
> > Signed-off-by: Anna-Maria Behnsen <anna-maria@linutronix.de>
> > Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
> > Tested-by: Mario Limonciello <mario.limonciello@amd.com>
> > Cc: stable@kernel.org
>
> Thanks for the detailed commit message! Please add:
>
> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

Applied as 6.9-rc material, many thanks to everyone involved!