| Message ID | 20240109164655.626085-2-vincent.guittot@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Rework system pressure interface to the scheduler | expand |
On Tue, Jan 9, 2024 at 5:47 PM Vincent Guittot <vincent.guittot@linaro.org> wrote: > > Provide to the scheduler a feedback about the temporary max available > capacity. Unlike arch_update_thermal_pressure, this doesn't need to be > filtered as the pressure will happen for dozens ms or more. > > Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org> Acked-by: Rafael J. Wysocki <rafael@kernel.org> and I think I've given the tag on this patch already. > --- > drivers/cpufreq/cpufreq.c | 36 ++++++++++++++++++++++++++++++++++++ > include/linux/cpufreq.h | 10 ++++++++++ > 2 files changed, 46 insertions(+) > > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c > index 44db4f59c4cc..f4eee3d107f1 100644 > --- a/drivers/cpufreq/cpufreq.c > +++ b/drivers/cpufreq/cpufreq.c > @@ -2563,6 +2563,40 @@ int cpufreq_get_policy(struct cpufreq_policy *policy, unsigned int cpu) > } > EXPORT_SYMBOL(cpufreq_get_policy); > > +DEFINE_PER_CPU(unsigned long, cpufreq_pressure); > + > +/** > + * cpufreq_update_pressure() - Update cpufreq pressure for CPUs > + * @policy: cpufreq policy of the CPUs. > + * > + * Update the value of cpufreq pressure for all @cpus in the policy. > + */ > +static void cpufreq_update_pressure(struct cpufreq_policy *policy) > +{ > + unsigned long max_capacity, capped_freq, pressure; > + u32 max_freq; > + int cpu; > + > + cpu = cpumask_first(policy->related_cpus); > + max_freq = arch_scale_freq_ref(cpu); > + capped_freq = policy->max; > + > + /* > + * Handle properly the boost frequencies, which should simply clean > + * the cpufreq pressure value. > + */ > + if (max_freq <= capped_freq) { > + pressure = 0; > + } else { > + max_capacity = arch_scale_cpu_capacity(cpu); > + pressure = max_capacity - > + mult_frac(max_capacity, capped_freq, max_freq); > + } > + > + for_each_cpu(cpu, policy->related_cpus) > + WRITE_ONCE(per_cpu(cpufreq_pressure, cpu), pressure); > +} > + > /** > * cpufreq_set_policy - Modify cpufreq policy parameters. > * @policy: Policy object to modify. > @@ -2618,6 +2652,8 @@ static int cpufreq_set_policy(struct cpufreq_policy *policy, > policy->max = __resolve_freq(policy, policy->max, CPUFREQ_RELATION_H); > trace_cpu_frequency_limits(policy); > > + cpufreq_update_pressure(policy); > + > policy->cached_target_freq = UINT_MAX; > > pr_debug("new min and max freqs are %u - %u kHz\n", > diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h > index afda5f24d3dd..b1d97edd3253 100644 > --- a/include/linux/cpufreq.h > +++ b/include/linux/cpufreq.h > @@ -241,6 +241,12 @@ struct kobject *get_governor_parent_kobj(struct cpufreq_policy *policy); > void cpufreq_enable_fast_switch(struct cpufreq_policy *policy); > void cpufreq_disable_fast_switch(struct cpufreq_policy *policy); > bool has_target_index(void); > + > +DECLARE_PER_CPU(unsigned long, cpufreq_pressure); > +static inline unsigned long cpufreq_get_pressure(int cpu) > +{ > + return per_cpu(cpufreq_pressure, cpu); > +} > #else > static inline unsigned int cpufreq_get(unsigned int cpu) > { > @@ -263,6 +269,10 @@ static inline bool cpufreq_supports_freq_invariance(void) > return false; > } > static inline void disable_cpufreq(void) { } > +static inline unsigned long cpufreq_get_pressure(int cpu) > +{ > + return 0; > +} > #endif > > #ifdef CONFIG_CPU_FREQ_STAT > -- > 2.34.1 >
On Tue, 9 Jan 2024 at 17:49, Rafael J. Wysocki <rafael@kernel.org> wrote: > > On Tue, Jan 9, 2024 at 5:47 PM Vincent Guittot > <vincent.guittot@linaro.org> wrote: > > > > Provide to the scheduler a feedback about the temporary max available > > capacity. Unlike arch_update_thermal_pressure, this doesn't need to be > > filtered as the pressure will happen for dozens ms or more. > > > > Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org> > > Acked-by: Rafael J. Wysocki <rafael@kernel.org> > > and I think I've given the tag on this patch already. yes, I preferred to not add it after the crap that I did in the v3 with the cleanup of this [1/5] patch Thanks > > > --- > > drivers/cpufreq/cpufreq.c | 36 ++++++++++++++++++++++++++++++++++++ > > include/linux/cpufreq.h | 10 ++++++++++ > > 2 files changed, 46 insertions(+) > > > > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c > > index 44db4f59c4cc..f4eee3d107f1 100644 > > --- a/drivers/cpufreq/cpufreq.c > > +++ b/drivers/cpufreq/cpufreq.c > > @@ -2563,6 +2563,40 @@ int cpufreq_get_policy(struct cpufreq_policy *policy, unsigned int cpu) > > } > > EXPORT_SYMBOL(cpufreq_get_policy); > > > > +DEFINE_PER_CPU(unsigned long, cpufreq_pressure); > > + > > +/** > > + * cpufreq_update_pressure() - Update cpufreq pressure for CPUs > > + * @policy: cpufreq policy of the CPUs. > > + * > > + * Update the value of cpufreq pressure for all @cpus in the policy. > > + */ > > +static void cpufreq_update_pressure(struct cpufreq_policy *policy) > > +{ > > + unsigned long max_capacity, capped_freq, pressure; > > + u32 max_freq; > > + int cpu; > > + > > + cpu = cpumask_first(policy->related_cpus); > > + max_freq = arch_scale_freq_ref(cpu); > > + capped_freq = policy->max; > > + > > + /* > > + * Handle properly the boost frequencies, which should simply clean > > + * the cpufreq pressure value. > > + */ > > + if (max_freq <= capped_freq) { > > + pressure = 0; > > + } else { > > + max_capacity = arch_scale_cpu_capacity(cpu); > > + pressure = max_capacity - > > + mult_frac(max_capacity, capped_freq, max_freq); > > + } > > + > > + for_each_cpu(cpu, policy->related_cpus) > > + WRITE_ONCE(per_cpu(cpufreq_pressure, cpu), pressure); > > +} > > + > > /** > > * cpufreq_set_policy - Modify cpufreq policy parameters. > > * @policy: Policy object to modify. > > @@ -2618,6 +2652,8 @@ static int cpufreq_set_policy(struct cpufreq_policy *policy, > > policy->max = __resolve_freq(policy, policy->max, CPUFREQ_RELATION_H); > > trace_cpu_frequency_limits(policy); > > > > + cpufreq_update_pressure(policy); > > + > > policy->cached_target_freq = UINT_MAX; > > > > pr_debug("new min and max freqs are %u - %u kHz\n", > > diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h > > index afda5f24d3dd..b1d97edd3253 100644 > > --- a/include/linux/cpufreq.h > > +++ b/include/linux/cpufreq.h > > @@ -241,6 +241,12 @@ struct kobject *get_governor_parent_kobj(struct cpufreq_policy *policy); > > void cpufreq_enable_fast_switch(struct cpufreq_policy *policy); > > void cpufreq_disable_fast_switch(struct cpufreq_policy *policy); > > bool has_target_index(void); > > + > > +DECLARE_PER_CPU(unsigned long, cpufreq_pressure); > > +static inline unsigned long cpufreq_get_pressure(int cpu) > > +{ > > + return per_cpu(cpufreq_pressure, cpu); > > +} > > #else > > static inline unsigned int cpufreq_get(unsigned int cpu) > > { > > @@ -263,6 +269,10 @@ static inline bool cpufreq_supports_freq_invariance(void) > > return false; > > } > > static inline void disable_cpufreq(void) { } > > +static inline unsigned long cpufreq_get_pressure(int cpu) > > +{ > > + return 0; > > +} > > #endif > > > > #ifdef CONFIG_CPU_FREQ_STAT > > -- > > 2.34.1 > >
On 09-01-24, 17:46, Vincent Guittot wrote: > Provide to the scheduler a feedback about the temporary max available > capacity. Unlike arch_update_thermal_pressure, this doesn't need to be > filtered as the pressure will happen for dozens ms or more. > > Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org> > --- > drivers/cpufreq/cpufreq.c | 36 ++++++++++++++++++++++++++++++++++++ > include/linux/cpufreq.h | 10 ++++++++++ > 2 files changed, 46 insertions(+) Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
On 01/09/24 17:46, Vincent Guittot wrote: > Provide to the scheduler a feedback about the temporary max available > capacity. Unlike arch_update_thermal_pressure, this doesn't need to be > filtered as the pressure will happen for dozens ms or more. > > Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org> > --- > drivers/cpufreq/cpufreq.c | 36 ++++++++++++++++++++++++++++++++++++ > include/linux/cpufreq.h | 10 ++++++++++ > 2 files changed, 46 insertions(+) > > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c > index 44db4f59c4cc..f4eee3d107f1 100644 > --- a/drivers/cpufreq/cpufreq.c > +++ b/drivers/cpufreq/cpufreq.c > @@ -2563,6 +2563,40 @@ int cpufreq_get_policy(struct cpufreq_policy *policy, unsigned int cpu) > } > EXPORT_SYMBOL(cpufreq_get_policy); > > +DEFINE_PER_CPU(unsigned long, cpufreq_pressure); > + > +/** > + * cpufreq_update_pressure() - Update cpufreq pressure for CPUs > + * @policy: cpufreq policy of the CPUs. > + * > + * Update the value of cpufreq pressure for all @cpus in the policy. > + */ > +static void cpufreq_update_pressure(struct cpufreq_policy *policy) > +{ > + unsigned long max_capacity, capped_freq, pressure; > + u32 max_freq; > + int cpu; > + > + cpu = cpumask_first(policy->related_cpus); > + max_freq = arch_scale_freq_ref(cpu); > + capped_freq = policy->max; > + > + /* > + * Handle properly the boost frequencies, which should simply clean > + * the cpufreq pressure value. > + */ > + if (max_freq <= capped_freq) { > + pressure = 0; > + } else { > + max_capacity = arch_scale_cpu_capacity(cpu); > + pressure = max_capacity - > + mult_frac(max_capacity, capped_freq, max_freq); > + } > + > + for_each_cpu(cpu, policy->related_cpus) > + WRITE_ONCE(per_cpu(cpufreq_pressure, cpu), pressure); Is this WRITE_ONCE() required? I don't see why. But assuming I missed something, better pair it with READ_ONCE() in cpufreq_get_pressure()? Beside that, LGTM Reviewed-by: Qais Yousef <qyousef@layalina.io> > +} > + > /** > * cpufreq_set_policy - Modify cpufreq policy parameters. > * @policy: Policy object to modify. > @@ -2618,6 +2652,8 @@ static int cpufreq_set_policy(struct cpufreq_policy *policy, > policy->max = __resolve_freq(policy, policy->max, CPUFREQ_RELATION_H); > trace_cpu_frequency_limits(policy); > > + cpufreq_update_pressure(policy); > + > policy->cached_target_freq = UINT_MAX; > > pr_debug("new min and max freqs are %u - %u kHz\n", > diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h > index afda5f24d3dd..b1d97edd3253 100644 > --- a/include/linux/cpufreq.h > +++ b/include/linux/cpufreq.h > @@ -241,6 +241,12 @@ struct kobject *get_governor_parent_kobj(struct cpufreq_policy *policy); > void cpufreq_enable_fast_switch(struct cpufreq_policy *policy); > void cpufreq_disable_fast_switch(struct cpufreq_policy *policy); > bool has_target_index(void); > + > +DECLARE_PER_CPU(unsigned long, cpufreq_pressure); > +static inline unsigned long cpufreq_get_pressure(int cpu) > +{ > + return per_cpu(cpufreq_pressure, cpu); > +} > #else > static inline unsigned int cpufreq_get(unsigned int cpu) > { > @@ -263,6 +269,10 @@ static inline bool cpufreq_supports_freq_invariance(void) > return false; > } > static inline void disable_cpufreq(void) { } > +static inline unsigned long cpufreq_get_pressure(int cpu) > +{ > + return 0; > +} > #endif > > #ifdef CONFIG_CPU_FREQ_STAT > -- > 2.34.1 >
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 44db4f59c4cc..f4eee3d107f1 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -2563,6 +2563,40 @@ int cpufreq_get_policy(struct cpufreq_policy *policy, unsigned int cpu) } EXPORT_SYMBOL(cpufreq_get_policy); +DEFINE_PER_CPU(unsigned long, cpufreq_pressure); + +/** + * cpufreq_update_pressure() - Update cpufreq pressure for CPUs + * @policy: cpufreq policy of the CPUs. + * + * Update the value of cpufreq pressure for all @cpus in the policy. + */ +static void cpufreq_update_pressure(struct cpufreq_policy *policy) +{ + unsigned long max_capacity, capped_freq, pressure; + u32 max_freq; + int cpu; + + cpu = cpumask_first(policy->related_cpus); + max_freq = arch_scale_freq_ref(cpu); + capped_freq = policy->max; + + /* + * Handle properly the boost frequencies, which should simply clean + * the cpufreq pressure value. + */ + if (max_freq <= capped_freq) { + pressure = 0; + } else { + max_capacity = arch_scale_cpu_capacity(cpu); + pressure = max_capacity - + mult_frac(max_capacity, capped_freq, max_freq); + } + + for_each_cpu(cpu, policy->related_cpus) + WRITE_ONCE(per_cpu(cpufreq_pressure, cpu), pressure); +} + /** * cpufreq_set_policy - Modify cpufreq policy parameters. * @policy: Policy object to modify. @@ -2618,6 +2652,8 @@ static int cpufreq_set_policy(struct cpufreq_policy *policy, policy->max = __resolve_freq(policy, policy->max, CPUFREQ_RELATION_H); trace_cpu_frequency_limits(policy); + cpufreq_update_pressure(policy); + policy->cached_target_freq = UINT_MAX; pr_debug("new min and max freqs are %u - %u kHz\n", diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index afda5f24d3dd..b1d97edd3253 100644 --- a/include/linux/cpufreq.h +++ b/include/linux/cpufreq.h @@ -241,6 +241,12 @@ struct kobject *get_governor_parent_kobj(struct cpufreq_policy *policy); void cpufreq_enable_fast_switch(struct cpufreq_policy *policy); void cpufreq_disable_fast_switch(struct cpufreq_policy *policy); bool has_target_index(void); + +DECLARE_PER_CPU(unsigned long, cpufreq_pressure); +static inline unsigned long cpufreq_get_pressure(int cpu) +{ + return per_cpu(cpufreq_pressure, cpu); +} #else static inline unsigned int cpufreq_get(unsigned int cpu) { @@ -263,6 +269,10 @@ static inline bool cpufreq_supports_freq_invariance(void) return false; } static inline void disable_cpufreq(void) { } +static inline unsigned long cpufreq_get_pressure(int cpu) +{ + return 0; +} #endif #ifdef CONFIG_CPU_FREQ_STAT
Provide to the scheduler a feedback about the temporary max available capacity. Unlike arch_update_thermal_pressure, this doesn't need to be filtered as the pressure will happen for dozens ms or more. Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org> --- drivers/cpufreq/cpufreq.c | 36 ++++++++++++++++++++++++++++++++++++ include/linux/cpufreq.h | 10 ++++++++++ 2 files changed, 46 insertions(+)