Re: [PATCH v1 0/2] x86/smp: Fix power regression introduced by commit 96040f7273e2

[Peter Zijlstra <peterz@infradead.org>]

On Wed, May 28, 2025 at 03:20:16PM +0200, Rafael J. Wysocki wrote:
> On Wed, May 28, 2025 at 3:18 PM Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > On Wed, May 28, 2025 at 02:53:13PM +0200, Rafael J. Wysocki wrote:
> > > Hi Everyone,
> > >
> > > Commit 96040f7273e2 ("x86/smp: Eliminate mwait_play_dead_cpuid_hint()")
> > > that shipped in 6.15 introduced a nasty power regression on systems that
> > > start with "nosmt" in the kernel command line which prevents it from entering
> > > deep package idle states (for instance, PC10) later on.  Idle power, including
> > > power in suspend-to-idle, goes up significantly on those systems as a result.
> > >
> > > Address this by reverting commit 96040f7273e2 (patch [1/2]) and using a
> > > different approach, which is to retain mwait_play_dead_cpuid_hint() and
> > > still prefer it to hlt_play_dead() in case it is needed when cpuidle is
> > > not available, but prefer cpuidle_play_dead() to it by default (patch [2/2]).
> >
> > I don't understand. The revert says the reason it regresses is that it
> > goes into play_dead before cpuidle is initialized. The fix is then to
> > call cpuidle first.
> >
> > But if cpuidle isn't initialized yet, how does that fix anything?
> 
> The revert fixes the bug.

This is not what I asked.

> The other patch does what the reverted commit was supposed to be
> doing, but differently.

No, it does not.

The whole point was that mwait_play_dead did not DTRT because hints are
stupid and it could not select the deepest C state in an unambiguous
fashion.

And now you're restoring that -- code you all argued was fundamentally
buggered.

Yes is 'fixes' things on old platforms, but it is equally broken on the
new platforms where you all argued it was broken on. So either way
around you're going to need to fix those, and this isn't it.

Now, SMT siblings are all AP, by definition. So can't we simply send
them INIT instead of doing CLI;HLT, that way they drop into
Wait-for-SIPI and the ucode can sort it out?