Re: [RFC][PATCH v0.3 4/6] PM: EM: Move CPU capacity check to em_adjust_new_capacity()

[Lukasz Luba]

Hi Rafael,

On 3/7/25 19:17, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
>
> Move the check of the CPU capacity currently stored in the energy model
> against the arch_scale_cpu_capacity() value to em_adjust_new_capacity()
> so it will be done regardless of where the latter is called from.
>
> This will be useful when a new em_adjust_new_capacity() caller is added
> subsequently.
>
> While at it, move the pd local variable declaration in
> em_check_capacity_update() into the loop in which it is used.
>
> No intentional functional impact.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> ---
>   kernel/power/energy_model.c |   40 +++++++++++++++++-----------------------
>   1 file changed, 17 insertions(+), 23 deletions(-)
>
> --- a/kernel/power/energy_model.c
> +++ b/kernel/power/energy_model.c
> @@ -721,10 +721,24 @@
>    * Adjustment of CPU performance values after boot, when all CPUs capacites
>    * are correctly calculated.
>    */
> -static void em_adjust_new_capacity(struct device *dev,
> +static void em_adjust_new_capacity(unsigned int cpu, struct device *dev,
>   				   struct em_perf_domain *pd)
>   {
> +	unsigned long cpu_capacity = arch_scale_cpu_capacity(cpu);
>   	struct em_perf_table *em_table;
> +	struct em_perf_state *table;
> +	unsigned long em_max_perf;
> +
> +	rcu_read_lock();
> +	table = em_perf_state_from_pd(pd);
> +	em_max_perf = table[pd->nr_perf_states - 1].performance;
> +	rcu_read_unlock();
> +
> +	if (em_max_perf == cpu_capacity)
> +		return;
> +
> +	pr_debug("updating cpu%d cpu_cap=%lu old capacity=%lu\n", cpu,
> +		 cpu_capacity, em_max_perf);
>   
>   	em_table = em_table_dup(pd);
>   	if (!em_table) {
> @@ -740,9 +754,6 @@
>   static void em_check_capacity_update(void)
>   {
>   	cpumask_var_t cpu_done_mask;
> -	struct em_perf_state *table;
> -	struct em_perf_domain *pd;
> -	unsigned long cpu_capacity;
>   	int cpu;
>   
>   	if (!zalloc_cpumask_var(&cpu_done_mask, GFP_KERNEL)) {
> @@ -753,7 +764,7 @@
>   	/* Check if CPUs capacity has changed than update EM */
>   	for_each_possible_cpu(cpu) {
>   		struct cpufreq_policy *policy;
> -		unsigned long em_max_perf;
> +		struct em_perf_domain *pd;
>   		struct device *dev;
>   
>   		if (cpumask_test_cpu(cpu, cpu_done_mask))
> @@ -776,24 +787,7 @@
>   		cpumask_or(cpu_done_mask, cpu_done_mask,
>   			   em_span_cpus(pd));
>   
> -		cpu_capacity = arch_scale_cpu_capacity(cpu);
> -
> -		rcu_read_lock();
> -		table = em_perf_state_from_pd(pd);
> -		em_max_perf = table[pd->nr_perf_states - 1].performance;
> -		rcu_read_unlock();
> -
> -		/*
> -		 * Check if the CPU capacity has been adjusted during boot
> -		 * and trigger the update for new performance values.
> -		 */
> -		if (em_max_perf == cpu_capacity)
> -			continue;
> -
> -		pr_debug("updating cpu%d cpu_cap=%lu old capacity=%lu\n",
> -			 cpu, cpu_capacity, em_max_perf);
> -
> -		em_adjust_new_capacity(dev, pd);
> +		em_adjust_new_capacity(cpu, dev, pd);
>   	}
>   
>   	free_cpumask_var(cpu_done_mask);


LGTM,

Reviewed-by: Lukasz Luba <lukasz.luba@xxxxxxx>

Regards,
Lukasz