Skip to content

Commit

Permalink
cppc_cpufreq: replace per-cpu data array with a list
Browse files Browse the repository at this point in the history
The cppc_cpudata per-cpu storage was inefficient (1) additional to causing
functional issues (2) when CPUs are hotplugged out, due to per-cpu data
being improperly initialised.

(1) The amount of information needed for CPPC performance control in its
    cpufreq driver depends on the domain (PSD) coordination type:

    ANY:    One set of CPPC control and capability data (e.g desired
            performance, highest/lowest performance, etc) applies to all
            CPUs in the domain.

    ALL:    Same as ANY. To be noted that this type is not currently
            supported. When supported, information about which CPUs
            belong to a domain is needed in order for frequency change
            requests to be sent to each of them.

    HW:     It's necessary to store CPPC control and capability
            information for all the CPUs. HW will then coordinate the
            performance state based on their limitations and requests.

    NONE:   Same as HW. No HW coordination is expected.

    Despite this, the previous initialisation code would indiscriminately
    allocate memory for all CPUs (all_cpu_data) and unnecessarily
    duplicate performance capabilities and the domain sharing mask and type
    for each possible CPU.

(2) With the current per-cpu structure, when having ANY coordination,
    the cppc_cpudata cpu information is not initialised (will remain 0)
    for all CPUs in a policy, other than policy->cpu. When policy->cpu is
    hotplugged out, the driver will incorrectly use the uninitialised (0)
    value of the other CPUs when making frequency changes. Additionally,
    the previous values stored in the perf_ctrls.desired_perf will be
    lost when policy->cpu changes.

Therefore replace the array of per cpu data with a list. The memory for
each structure is allocated at policy init, where a single structure
can be allocated per policy, not per cpu. In order to accommodate the
struct list_head node in the cppc_cpudata structure, the now unused cpu
and cur_policy variables are removed.

For example, on a arm64 Juno platform with 6 CPUs: (0, 1, 2, 3) in PSD1,
(4, 5) in PSD2 - ANY coordination, the memory allocation comparison shows:

Before patch:

 - ANY coordination:
   total    slack      req alloc/free  caller
       0        0        0     0/1     _kernel_size_le_hi32+0x0xffff800008ff7810
       0        0        0     0/6     _kernel_size_le_hi32+0x0xffff800008ff7808
     128       80       48     1/0     _kernel_size_le_hi32+0x0xffff800008ffc070
     768        0      768     6/0     _kernel_size_le_hi32+0x0xffff800008ffc0e4

After patch:

 - ANY coordination:
    total    slack      req alloc/free  caller
     256        0      256     2/0     _kernel_size_le_hi32+0x0xffff800008fed410
       0        0        0     0/2     _kernel_size_le_hi32+0x0xffff800008fed274

Additional notes:
 - A pointer to the policy's cppc_cpudata is stored in policy->driver_data
 - Driver registration is skipped if _CPC entries are not present.

Signed-off-by: Ionela Voinescu <ionela.voinescu@arm.com>
Tested-by: Mian Yousaf Kaukab <ykaukab@suse.de>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
  • Loading branch information
Ionela Voinescu authored and Rafael J. Wysocki committed Dec 15, 2020
1 parent cfdc589 commit a28b2bf
Show file tree
Hide file tree
Showing 3 changed files with 154 additions and 167 deletions.
141 changes: 60 additions & 81 deletions drivers/acpi/cppc_acpi.c
Original file line number Diff line number Diff line change
Expand Up @@ -413,109 +413,88 @@ static int acpi_get_psd(struct cpc_desc *cpc_ptr, acpi_handle handle)
return result;
}

bool acpi_cpc_valid(void)
{
struct cpc_desc *cpc_ptr;
int cpu;

for_each_possible_cpu(cpu) {
cpc_ptr = per_cpu(cpc_desc_ptr, cpu);
if (!cpc_ptr)
return false;
}

return true;
}
EXPORT_SYMBOL_GPL(acpi_cpc_valid);

/**
* acpi_get_psd_map - Map the CPUs in a common freq domain.
* @all_cpu_data: Ptrs to CPU specific CPPC data including PSD info.
* acpi_get_psd_map - Map the CPUs in the freq domain of a given cpu
* @cpu: Find all CPUs that share a domain with cpu.
* @cpu_data: Pointer to CPU specific CPPC data including PSD info.
*
* Return: 0 for success or negative value for err.
*/
int acpi_get_psd_map(struct cppc_cpudata **all_cpu_data)
int acpi_get_psd_map(unsigned int cpu, struct cppc_cpudata *cpu_data)
{
int count_target;
int retval = 0;
unsigned int i, j;
cpumask_var_t covered_cpus;
struct cppc_cpudata *pr, *match_pr;
struct acpi_psd_package *pdomain;
struct acpi_psd_package *match_pdomain;
struct cpc_desc *cpc_ptr, *match_cpc_ptr;

if (!zalloc_cpumask_var(&covered_cpus, GFP_KERNEL))
return -ENOMEM;
struct acpi_psd_package *match_pdomain;
struct acpi_psd_package *pdomain;
int count_target, i;

/*
* Now that we have _PSD data from all CPUs, let's setup P-state
* domain info.
*/
for_each_possible_cpu(i) {
if (cpumask_test_cpu(i, covered_cpus))
continue;

pr = all_cpu_data[i];
cpc_ptr = per_cpu(cpc_desc_ptr, i);
if (!cpc_ptr) {
retval = -EFAULT;
goto err_ret;
}
cpc_ptr = per_cpu(cpc_desc_ptr, cpu);
if (!cpc_ptr)
return -EFAULT;

pdomain = &(cpc_ptr->domain_info);
cpumask_set_cpu(i, pr->shared_cpu_map);
cpumask_set_cpu(i, covered_cpus);
if (pdomain->num_processors <= 1)
continue;
pdomain = &(cpc_ptr->domain_info);
cpumask_set_cpu(cpu, cpu_data->shared_cpu_map);
if (pdomain->num_processors <= 1)
return 0;

/* Validate the Domain info */
count_target = pdomain->num_processors;
if (pdomain->coord_type == DOMAIN_COORD_TYPE_SW_ALL)
pr->shared_type = CPUFREQ_SHARED_TYPE_ALL;
else if (pdomain->coord_type == DOMAIN_COORD_TYPE_HW_ALL)
pr->shared_type = CPUFREQ_SHARED_TYPE_HW;
else if (pdomain->coord_type == DOMAIN_COORD_TYPE_SW_ANY)
pr->shared_type = CPUFREQ_SHARED_TYPE_ANY;

for_each_possible_cpu(j) {
if (i == j)
continue;

match_cpc_ptr = per_cpu(cpc_desc_ptr, j);
if (!match_cpc_ptr) {
retval = -EFAULT;
goto err_ret;
}
/* Validate the Domain info */
count_target = pdomain->num_processors;
if (pdomain->coord_type == DOMAIN_COORD_TYPE_SW_ALL)
cpu_data->shared_type = CPUFREQ_SHARED_TYPE_ALL;
else if (pdomain->coord_type == DOMAIN_COORD_TYPE_HW_ALL)
cpu_data->shared_type = CPUFREQ_SHARED_TYPE_HW;
else if (pdomain->coord_type == DOMAIN_COORD_TYPE_SW_ANY)
cpu_data->shared_type = CPUFREQ_SHARED_TYPE_ANY;

match_pdomain = &(match_cpc_ptr->domain_info);
if (match_pdomain->domain != pdomain->domain)
continue;
for_each_possible_cpu(i) {
if (i == cpu)
continue;

/* Here i and j are in the same domain */
if (match_pdomain->num_processors != count_target) {
retval = -EFAULT;
goto err_ret;
}
match_cpc_ptr = per_cpu(cpc_desc_ptr, i);
if (!match_cpc_ptr)
goto err_fault;

if (pdomain->coord_type != match_pdomain->coord_type) {
retval = -EFAULT;
goto err_ret;
}
match_pdomain = &(match_cpc_ptr->domain_info);
if (match_pdomain->domain != pdomain->domain)
continue;

cpumask_set_cpu(j, covered_cpus);
cpumask_set_cpu(j, pr->shared_cpu_map);
}
/* Here i and cpu are in the same domain */
if (match_pdomain->num_processors != count_target)
goto err_fault;

for_each_cpu(j, pr->shared_cpu_map) {
if (i == j)
continue;
if (pdomain->coord_type != match_pdomain->coord_type)
goto err_fault;

match_pr = all_cpu_data[j];
match_pr->shared_type = pr->shared_type;
cpumask_copy(match_pr->shared_cpu_map,
pr->shared_cpu_map);
}
cpumask_set_cpu(i, cpu_data->shared_cpu_map);
}
goto out;

err_ret:
for_each_possible_cpu(i) {
pr = all_cpu_data[i];
return 0;

/* Assume no coordination on any error parsing domain info */
cpumask_clear(pr->shared_cpu_map);
cpumask_set_cpu(i, pr->shared_cpu_map);
pr->shared_type = CPUFREQ_SHARED_TYPE_NONE;
}
out:
free_cpumask_var(covered_cpus);
return retval;
err_fault:
/* Assume no coordination on any error parsing domain info */
cpumask_clear(cpu_data->shared_cpu_map);
cpumask_set_cpu(cpu, cpu_data->shared_cpu_map);
cpu_data->shared_type = CPUFREQ_SHARED_TYPE_NONE;

return -EFAULT;
}
EXPORT_SYMBOL_GPL(acpi_get_psd_map);

Expand Down
Loading

0 comments on commit a28b2bf

Please sign in to comment.