Re: [PATCH] [RESEND] Do not modify perf bias performance setting by default at boot

[Rafael J. Wysocki]

On Thu, Mar 14, 2019 at 3:42 PM Thomas Renninger <trenn@xxxxxxx> wrote:
>
> This is a revert of mainline git commits:
> commit b51ef52df71cb28e9d90cd1d48b79bf19f0bab06
> commit 17edf2d79f1ea6dfdb4c444801d928953b9f98d6
> commit abe48b108247e9b90b4c6739662a2e5c765ed114

I'm not quite convinced that reverting these is the right thing to do here.

> It is about this kernel message showing up on quite a lot servers:
> [    0.072652] ENERGY_PERF_BIAS: Set to 'normal', was 'performance'
> [    0.076003] ENERGY_PERF_BIAS: View and update with x86_energy_perf_policy(8)

What happens on boot is a matter of convention and the convention by
which the EPB is set to "neutral" at that point has been used for
quite a while.  Changing it now may confuse some users or even worse.

It would be fine to change the log level of these messages in my view, though.

> The background afaik were some BIOS which "forgot to set a default value".
> Certainly some (one?) broken laptop BIOS was meant, because
> the value zero (performance) is quite a reasonable default value on server systems
> and this is how a lot CPUs typically come up there.

That depends on the expected power/perf profile of the system and the
kernel has no information regarding that.  Arguably, user space can
set the EPB to whatever it wants at the init time according to the
information it has.  It is not quite realistic to expect that either
the BIOS or the kernel will do the right thing on all systems in that
respect anyway.

> Another sever bug with above patches:
> If you really set performance BIAS MSR value to 'performance'
> as mentioned in kernel boot log, it is reset on next CPU offline/online cycle
> (see below).

That is a bug.  There is no reason to change the EPB on CPU
offline/online and I agree that this needs to be fixed.  The reverts
would make this problem go away, but that's not the only possible way
to fix it.

Moreover, what is done during system-wide resume appears to be a bug
too.  Whatever EPB value is there during suspend should be saved and
it should be restored during resume. Reverting the commits above will
not fix that particular issue.

Thanks,
Rafael