| Message ID | 8554829.NyiUUSuA9g@rjwysocki.net (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
On 4/16/25 19:01, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Some cpufreq drivers, like intel_pstate, have built-in governors that > are used instead of regular cpufreq governors, schedutil in particular, > but they can work with EAS just fine, so allow EAS to be used with > those drivers. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > > v0.3 -> v1 > * Rebase on top of the new [1-2/8]. > * Update the diagnostic message printed if the conditions are not met. > > This patch is regarded as a cleanup for 6.16. > > --- > drivers/cpufreq/cpufreq.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > --- a/drivers/cpufreq/cpufreq.c > +++ b/drivers/cpufreq/cpufreq.c > @@ -3054,7 +3054,16 @@ > > guard(cpufreq_policy_read)(policy); > > - return sugov_is_governor(policy); > + /* > + * For EAS compatibility, require that either schedutil is the policy > + * governor or the policy is governed directly by the cpufreq driver. > + * > + * In the latter case, it is assumed that EAS can only be enabled by the > + * cpufreq driver itself which will not enable EAS if it does not meet > + * the EAS' expectations regarding performance scaling response. > + */ > + return sugov_is_governor(policy) || (!policy->governor && > + policy->policy != CPUFREQ_POLICY_UNKNOWN); > } > > bool cpufreq_ready_for_eas(const struct cpumask *cpu_mask) > @@ -3064,7 +3073,7 @@ > /* Do not attempt EAS if schedutil is not being used. */ > for_each_cpu(cpu, cpu_mask) { > if (!cpufreq_policy_is_good_for_eas(cpu)) { > - pr_debug("rd %*pbl: schedutil is mandatory for EAS\n", > + pr_debug("rd %*pbl: EAS requirements not met\n", > cpumask_pr_args(cpu_mask)); I'd prefer to have at least "EAS cpufreq requirements" printed here. with that caveat Reviewed-by: Christian Loehle <christian.loehle@arm.com> Maybe we should amend the EAS documentation to reflect this? (And also emphasise that EAS will make cpufreq assumptions as if sugov was the governor regardless.) > return false; > } > > >
On Thu, Apr 17, 2025 at 2:19 PM Christian Loehle <christian.loehle@arm.com> wrote: > > On 4/16/25 19:01, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > Some cpufreq drivers, like intel_pstate, have built-in governors that > > are used instead of regular cpufreq governors, schedutil in particular, > > but they can work with EAS just fine, so allow EAS to be used with > > those drivers. > > > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > --- > > > > v0.3 -> v1 > > * Rebase on top of the new [1-2/8]. > > * Update the diagnostic message printed if the conditions are not met. > > > > This patch is regarded as a cleanup for 6.16. > > > > --- > > drivers/cpufreq/cpufreq.c | 13 +++++++++++-- > > 1 file changed, 11 insertions(+), 2 deletions(-) > > > > --- a/drivers/cpufreq/cpufreq.c > > +++ b/drivers/cpufreq/cpufreq.c > > @@ -3054,7 +3054,16 @@ > > > > guard(cpufreq_policy_read)(policy); > > > > - return sugov_is_governor(policy); > > + /* > > + * For EAS compatibility, require that either schedutil is the policy > > + * governor or the policy is governed directly by the cpufreq driver. > > + * > > + * In the latter case, it is assumed that EAS can only be enabled by the > > + * cpufreq driver itself which will not enable EAS if it does not meet > > + * the EAS' expectations regarding performance scaling response. > > + */ > > + return sugov_is_governor(policy) || (!policy->governor && > > + policy->policy != CPUFREQ_POLICY_UNKNOWN); > > } > > > > bool cpufreq_ready_for_eas(const struct cpumask *cpu_mask) > > @@ -3064,7 +3073,7 @@ > > /* Do not attempt EAS if schedutil is not being used. */ > > for_each_cpu(cpu, cpu_mask) { > > if (!cpufreq_policy_is_good_for_eas(cpu)) { > > - pr_debug("rd %*pbl: schedutil is mandatory for EAS\n", > > + pr_debug("rd %*pbl: EAS requirements not met\n", > > cpumask_pr_args(cpu_mask)); > > I'd prefer to have at least "EAS cpufreq requirements" printed here. Sure. > with that caveat > Reviewed-by: Christian Loehle <christian.loehle@arm.com> > > Maybe we should amend the EAS documentation to reflect this? Yes, the documentation should be updated. Which piece of it in particular I need to look at? > (And also emphasise that EAS will make cpufreq assumptions as if sugov > was the governor regardless.) Right.
On 4/17/25 14:01, Rafael J. Wysocki wrote: > On Thu, Apr 17, 2025 at 2:19 PM Christian Loehle > <christian.loehle@arm.com> wrote: >> >> On 4/16/25 19:01, Rafael J. Wysocki wrote: >>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >>> >>> Some cpufreq drivers, like intel_pstate, have built-in governors that >>> are used instead of regular cpufreq governors, schedutil in particular, >>> but they can work with EAS just fine, so allow EAS to be used with >>> those drivers. >>> >>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >>> --- >>> >>> v0.3 -> v1 >>> * Rebase on top of the new [1-2/8]. >>> * Update the diagnostic message printed if the conditions are not met. >>> >>> This patch is regarded as a cleanup for 6.16. >>> >>> --- >>> drivers/cpufreq/cpufreq.c | 13 +++++++++++-- >>> 1 file changed, 11 insertions(+), 2 deletions(-) >>> >>> --- a/drivers/cpufreq/cpufreq.c >>> +++ b/drivers/cpufreq/cpufreq.c >>> @@ -3054,7 +3054,16 @@ >>> >>> guard(cpufreq_policy_read)(policy); >>> >>> - return sugov_is_governor(policy); >>> + /* >>> + * For EAS compatibility, require that either schedutil is the policy >>> + * governor or the policy is governed directly by the cpufreq driver. >>> + * >>> + * In the latter case, it is assumed that EAS can only be enabled by the >>> + * cpufreq driver itself which will not enable EAS if it does not meet >>> + * the EAS' expectations regarding performance scaling response. >>> + */ >>> + return sugov_is_governor(policy) || (!policy->governor && >>> + policy->policy != CPUFREQ_POLICY_UNKNOWN); >>> } >>> >>> bool cpufreq_ready_for_eas(const struct cpumask *cpu_mask) >>> @@ -3064,7 +3073,7 @@ >>> /* Do not attempt EAS if schedutil is not being used. */ >>> for_each_cpu(cpu, cpu_mask) { >>> if (!cpufreq_policy_is_good_for_eas(cpu)) { >>> - pr_debug("rd %*pbl: schedutil is mandatory for EAS\n", >>> + pr_debug("rd %*pbl: EAS requirements not met\n", >>> cpumask_pr_args(cpu_mask)); >> >> I'd prefer to have at least "EAS cpufreq requirements" printed here. > > Sure. > >> with that caveat >> Reviewed-by: Christian Loehle <christian.loehle@arm.com> >> >> Maybe we should amend the EAS documentation to reflect this? > > Yes, the documentation should be updated. Which piece of it in > particular I need to look at? Documentation/scheduler/sched-energy.rst has: 6.4 - Schedutil governor so at least there.
--- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -3054,7 +3054,16 @@ guard(cpufreq_policy_read)(policy); - return sugov_is_governor(policy); + /* + * For EAS compatibility, require that either schedutil is the policy + * governor or the policy is governed directly by the cpufreq driver. + * + * In the latter case, it is assumed that EAS can only be enabled by the + * cpufreq driver itself which will not enable EAS if it does not meet + * the EAS' expectations regarding performance scaling response. + */ + return sugov_is_governor(policy) || (!policy->governor && + policy->policy != CPUFREQ_POLICY_UNKNOWN); } bool cpufreq_ready_for_eas(const struct cpumask *cpu_mask) @@ -3064,7 +3073,7 @@ /* Do not attempt EAS if schedutil is not being used. */ for_each_cpu(cpu, cpu_mask) { if (!cpufreq_policy_is_good_for_eas(cpu)) { - pr_debug("rd %*pbl: schedutil is mandatory for EAS\n", + pr_debug("rd %*pbl: EAS requirements not met\n", cpumask_pr_args(cpu_mask)); return false; }