Re: [PATCH v5 14/23] PM: EM: Support late CPUs booting and capacity adjustment

Lukasz Luba
2023-12-20T08:22:14Z

On 12/12/23 18:50, Dietmar Eggemann wrote:
> On 29/11/2023 12:08, Lukasz Luba wrote:
>> The patch adds needed infrastructure to handle the late CPUs boot, which
>> might change the previous CPUs capacity values. With this changes the new
>> CPUs which try to register EM will trigger the needed re-calculations for
>> other CPUs EMs. Thanks to that the em_per_state::performance values will
>> be aligned with the CPU capacity information after all CPUs finish the
>> boot and EM registrations.
>
> IMHO, it's worth mentioning here that this added functionality is the 1.
> use case of the modifiable EM.

Make sense. I will add that. It's quite important information, since
it also justifies the EM update feature.

>
> [...]
>
>> + * Adjustment of CPU performance values after boot, when all CPUs capacites
>> + * are correctly calculated.
>> + */
>> +static void em_adjust_new_capacity(struct device *dev,
>> + struct em_perf_domain *pd,
>> + u64 max_cap)
>> +{
>
> [...]
>
>> + /*
>> + * This is one-time-update, so give up the ownership in this updater.
>> + * The EM fwk will keep the reference and free the memory when needed.
>
> s/fwk/framework ?

OK

>
>> + */
>> + em_free_table(runtime_table);
>> +}
>> +
>> +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)) {
>> + pr_warn("no free memory\n");
>> + return;
>> + }
>> +
>> + /* Check if CPUs capacity has changed than update EM */
>
> s/than/then ?
>
> Maybe this comment is not needed since there is (1) further down?

Yes, I'll remove that.

>
>
>> + for_each_possible_cpu(cpu) {
>> + struct cpufreq_policy *policy;
>> + unsigned long em_max_perf;
>> + struct device *dev;
>> + int nr_states;
>> +
>> + if (cpumask_test_cpu(cpu, cpu_done_mask))
>> + continue;
>> +
>> + policy = cpufreq_cpu_get(cpu);
>> + if (!policy) {
>> + pr_debug("Accessing cpu%d policy failed\n", cpu);
>> + schedule_delayed_work(&em_update_work,
>> + msecs_to_jiffies(1000));
>> + break;
>> + }
>> + cpufreq_cpu_put(policy);
>> +
>> + pd = em_cpu_get(cpu);
>> + if (!pd || em_is_artificial(pd))
>> + continue;
>> +
>> + cpumask_or(cpu_done_mask, cpu_done_mask,
>> + em_span_cpus(pd));
>> +
>> + nr_states = pd->nr_perf_states;
>> + cpu_capacity = arch_scale_cpu_capacity(cpu);
>> +
>> + table = em_get_table(pd);
>> + em_max_perf = table[pd->nr_perf_states - 1].performance;
>> + em_put_table();
>> +
>> + /*
>> + * Check if the CPU capacity has been adjusted during boot
>> + * and trigger the update for new performance values.
>> + */
>
> (1)
>
> …