From: Rafael J. Wysocki <[email protected]>
Subject: [PATCH] cpufreq: Avoid configuring old governors as default with intel_pstate
Commit 33aa46f252c7 ("cpufreq: intel_pstate: Use passive mode by
default without HWP") was meant to cause intel_pstate without HWP
to be used in the passive mode with the schedutil governor on top of
it by default, but it missed the case in which either "ondemand" or
"conservative" was selected as the default governor in the existing
kernel config, in which case the previous old governor configuration
would be used, causing the default legacy governor to be used on top
of intel_pstate instead of schedutil.
Address this by preventing "ondemand" and "conservative" from being
configured as the default cpufreq governor in the case when schedutil
is the default choice for the default governor setting.
[Note that the default cpufreq governor can still be set via the
kernel command line if need be and that choice is not limited,
so if anyone really wants to use one of the legacy governors by
default, it can be achieved this way.]
Fixes: 33aa46f252c7 ("cpufreq: intel_pstate: Use passive mode by default without HWP")
Cc: 5.8+ <[email protected]> # 5.8+
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/cpufreq/Kconfig | 2 ++
1 file changed, 2 insertions(+)
Index: linux-pm/drivers/cpufreq/Kconfig
===================================================================
--- linux-pm.orig/drivers/cpufreq/Kconfig
+++ linux-pm/drivers/cpufreq/Kconfig
@@ -71,6 +71,7 @@ config CPU_FREQ_DEFAULT_GOV_USERSPACE
config CPU_FREQ_DEFAULT_GOV_ONDEMAND
bool "ondemand"
+ depends on !SMP || !X86_INTEL_PSTATE
select CPU_FREQ_GOV_ONDEMAND
select CPU_FREQ_GOV_PERFORMANCE
help
@@ -83,6 +84,7 @@ config CPU_FREQ_DEFAULT_GOV_ONDEMAND
config CPU_FREQ_DEFAULT_GOV_CONSERVATIVE
bool "conservative"
+ depends on !SMP || !X86_INTEL_PSTATE
select CPU_FREQ_GOV_CONSERVATIVE
select CPU_FREQ_GOV_PERFORMANCE
help
On 22-10-20, 18:23, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
> Subject: [PATCH] cpufreq: Avoid configuring old governors as default with intel_pstate
>
> Commit 33aa46f252c7 ("cpufreq: intel_pstate: Use passive mode by
> default without HWP") was meant to cause intel_pstate without HWP
> to be used in the passive mode with the schedutil governor on top of
> it by default, but it missed the case in which either "ondemand" or
> "conservative" was selected as the default governor in the existing
> kernel config, in which case the previous old governor configuration
> would be used, causing the default legacy governor to be used on top
> of intel_pstate instead of schedutil.
>
> Address this by preventing "ondemand" and "conservative" from being
> configured as the default cpufreq governor in the case when schedutil
> is the default choice for the default governor setting.
>
> [Note that the default cpufreq governor can still be set via the
> kernel command line if need be and that choice is not limited,
> so if anyone really wants to use one of the legacy governors by
> default, it can be achieved this way.]
>
> Fixes: 33aa46f252c7 ("cpufreq: intel_pstate: Use passive mode by default without HWP")
> Cc: 5.8+ <[email protected]> # 5.8+
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---
> drivers/cpufreq/Kconfig | 2 ++
> 1 file changed, 2 insertions(+)
>
> Index: linux-pm/drivers/cpufreq/Kconfig
> ===================================================================
> --- linux-pm.orig/drivers/cpufreq/Kconfig
> +++ linux-pm/drivers/cpufreq/Kconfig
> @@ -71,6 +71,7 @@ config CPU_FREQ_DEFAULT_GOV_USERSPACE
>
> config CPU_FREQ_DEFAULT_GOV_ONDEMAND
> bool "ondemand"
> + depends on !SMP || !X86_INTEL_PSTATE
> select CPU_FREQ_GOV_ONDEMAND
> select CPU_FREQ_GOV_PERFORMANCE
> help
> @@ -83,6 +84,7 @@ config CPU_FREQ_DEFAULT_GOV_ONDEMAND
>
> config CPU_FREQ_DEFAULT_GOV_CONSERVATIVE
> bool "conservative"
> + depends on !SMP || !X86_INTEL_PSTATE
While reading this first it felt like a SMP platforms related problem
(which I was surprised about), and then I understood what you are
doing.
I wonder if rewriting it this way makes it more readable with same
result eventually.
depends on !(X86_INTEL_PSTATE && SMP)
--
viresh
On Fri, Oct 23, 2020 at 8:17 AM Viresh Kumar <[email protected]> wrote:
>
> On 22-10-20, 18:23, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <[email protected]>
> > Subject: [PATCH] cpufreq: Avoid configuring old governors as default with intel_pstate
> >
> > Commit 33aa46f252c7 ("cpufreq: intel_pstate: Use passive mode by
> > default without HWP") was meant to cause intel_pstate without HWP
> > to be used in the passive mode with the schedutil governor on top of
> > it by default, but it missed the case in which either "ondemand" or
> > "conservative" was selected as the default governor in the existing
> > kernel config, in which case the previous old governor configuration
> > would be used, causing the default legacy governor to be used on top
> > of intel_pstate instead of schedutil.
> >
> > Address this by preventing "ondemand" and "conservative" from being
> > configured as the default cpufreq governor in the case when schedutil
> > is the default choice for the default governor setting.
> >
> > [Note that the default cpufreq governor can still be set via the
> > kernel command line if need be and that choice is not limited,
> > so if anyone really wants to use one of the legacy governors by
> > default, it can be achieved this way.]
> >
> > Fixes: 33aa46f252c7 ("cpufreq: intel_pstate: Use passive mode by default without HWP")
> > Cc: 5.8+ <[email protected]> # 5.8+
> > Signed-off-by: Rafael J. Wysocki <[email protected]>
> > ---
> > drivers/cpufreq/Kconfig | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > Index: linux-pm/drivers/cpufreq/Kconfig
> > ===================================================================
> > --- linux-pm.orig/drivers/cpufreq/Kconfig
> > +++ linux-pm/drivers/cpufreq/Kconfig
> > @@ -71,6 +71,7 @@ config CPU_FREQ_DEFAULT_GOV_USERSPACE
> >
> > config CPU_FREQ_DEFAULT_GOV_ONDEMAND
> > bool "ondemand"
> > + depends on !SMP || !X86_INTEL_PSTATE
> > select CPU_FREQ_GOV_ONDEMAND
> > select CPU_FREQ_GOV_PERFORMANCE
> > help
> > @@ -83,6 +84,7 @@ config CPU_FREQ_DEFAULT_GOV_ONDEMAND
> >
> > config CPU_FREQ_DEFAULT_GOV_CONSERVATIVE
> > bool "conservative"
> > + depends on !SMP || !X86_INTEL_PSTATE
>
> While reading this first it felt like a SMP platforms related problem
> (which I was surprised about), and then I understood what you are
> doing.
>
> I wonder if rewriting it this way makes it more readable with same
> result eventually.
>
> depends on !(X86_INTEL_PSTATE && SMP)
Agreed, will update.
Thanks!
From: Rafael J. Wysocki <[email protected]>
Commit 33aa46f252c7 ("cpufreq: intel_pstate: Use passive mode by
default without HWP") was meant to cause intel_pstate to be used
in the passive mode with the schedutil governor on top of it, but
it missed the case in which either "ondemand" or "conservative"
was selected as the default governor in the existing kernel config,
in which case the previous old governor configuration would be used,
causing the default legacy governor to be used on top of intel_pstate
instead of schedutil.
Address this by preventing "ondemand" and "conservative" from being
configured as the default cpufreq governor in the case when schedutil
is the default choice for the default governor setting.
[Note that the default cpufreq governor can still be set via the
kernel command line if need be and that choice is not limited,
so if anyone really wants to use one of the legacy governors by
default, it can be achieved this way.]
Fixes: 33aa46f252c7 ("cpufreq: intel_pstate: Use passive mode by default without HWP")
Reported-by: Julia Lawall <[email protected]>
Cc: 5.8+ <[email protected]> # 5.8+
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
The v2 addresses a review comment from Viresh regarding of the expression format
and adds a missing Reported-by for Julia.
---
drivers/cpufreq/Kconfig | 2 ++
1 file changed, 2 insertions(+)
Index: linux-pm/drivers/cpufreq/Kconfig
===================================================================
--- linux-pm.orig/drivers/cpufreq/Kconfig
+++ linux-pm/drivers/cpufreq/Kconfig
@@ -71,6 +71,7 @@ config CPU_FREQ_DEFAULT_GOV_USERSPACE
config CPU_FREQ_DEFAULT_GOV_ONDEMAND
bool "ondemand"
+ depends on !(X86_INTEL_PSTATE && SMP)
select CPU_FREQ_GOV_ONDEMAND
select CPU_FREQ_GOV_PERFORMANCE
help
@@ -83,6 +84,7 @@ config CPU_FREQ_DEFAULT_GOV_ONDEMAND
config CPU_FREQ_DEFAULT_GOV_CONSERVATIVE
bool "conservative"
+ depends on !(X86_INTEL_PSTATE && SMP)
select CPU_FREQ_GOV_CONSERVATIVE
select CPU_FREQ_GOV_PERFORMANCE
help
On 23-10-20, 17:15, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> Commit 33aa46f252c7 ("cpufreq: intel_pstate: Use passive mode by
> default without HWP") was meant to cause intel_pstate to be used
> in the passive mode with the schedutil governor on top of it, but
> it missed the case in which either "ondemand" or "conservative"
> was selected as the default governor in the existing kernel config,
> in which case the previous old governor configuration would be used,
> causing the default legacy governor to be used on top of intel_pstate
> instead of schedutil.
>
> Address this by preventing "ondemand" and "conservative" from being
> configured as the default cpufreq governor in the case when schedutil
> is the default choice for the default governor setting.
>
> [Note that the default cpufreq governor can still be set via the
> kernel command line if need be and that choice is not limited,
> so if anyone really wants to use one of the legacy governors by
> default, it can be achieved this way.]
>
> Fixes: 33aa46f252c7 ("cpufreq: intel_pstate: Use passive mode by default without HWP")
> Reported-by: Julia Lawall <[email protected]>
> Cc: 5.8+ <[email protected]> # 5.8+
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---
>
> The v2 addresses a review comment from Viresh regarding of the expression format
> and adds a missing Reported-by for Julia.
>
> ---
> drivers/cpufreq/Kconfig | 2 ++
> 1 file changed, 2 insertions(+)
Acked-by: Viresh Kumar <[email protected]>
--
viresh