Skip to content

Commit

Permalink
cpufreq: Manage governor usage history with 'policy->last_governor'
Browse files Browse the repository at this point in the history
History of which governor was used last is common to all CPUs within a
policy and maintaining it per-cpu isn't the best approach for sure.

Apart from wasting memory, this also increases the complexity of
managing this data structure as it has to be updated for all CPUs.

To make that somewhat simpler, lets store this information in a new
field 'last_governor' in struct cpufreq_policy and update it on removal
of last cpu of a policy.

As a side-effect it also solves an old problem, consider a system with
two clusters 0 & 1. And there is one policy per cluster.

Cluster 0: CPU0 and 1.
Cluster 1: CPU2 and 3.

 - CPU2 is first brought online, and governor is set to performance
   (default as cpufreq_cpu_governor wasn't set).
 - Governor is changed to ondemand.
 - CPU2 is taken offline and cpufreq_cpu_governor is updated for CPU2.
 - CPU3 is brought online.
 - Because cpufreq_cpu_governor wasn't set for CPU3, the default governor
   performance is picked for CPU3.

This patch fixes the bug as we now have a single variable to update for
policy.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
  • Loading branch information
Viresh Kumar authored and Rafael J. Wysocki committed May 15, 2015
1 parent 9104bb2 commit 4573237
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 15 deletions.
30 changes: 15 additions & 15 deletions drivers/cpufreq/cpufreq.c
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,6 @@ static DEFINE_PER_CPU(struct cpufreq_policy *, cpufreq_cpu_data);
static DEFINE_RWLOCK(cpufreq_driver_lock);
DEFINE_MUTEX(cpufreq_governor_lock);

/* This one keeps track of the previously set governor of a removed CPU */
static DEFINE_PER_CPU(char[CPUFREQ_NAME_LEN], cpufreq_cpu_governor);

/* Flag to suspend/resume CPUFreq governors */
static bool cpufreq_suspended;

Expand Down Expand Up @@ -1017,7 +1014,7 @@ static void cpufreq_init_policy(struct cpufreq_policy *policy)
memcpy(&new_policy, policy, sizeof(*policy));

/* Update governor of new_policy to the governor used before hotplug */
gov = find_governor(per_cpu(cpufreq_cpu_governor, policy->cpu));
gov = find_governor(policy->last_governor);
if (gov)
pr_debug("Restoring governor %s for cpu %d\n",
policy->governor->name, policy->cpu);
Expand Down Expand Up @@ -1411,14 +1408,15 @@ static int __cpufreq_remove_dev_prepare(struct device *dev,
pr_err("%s: Failed to stop governor\n", __func__);
return ret;
}

strncpy(per_cpu(cpufreq_cpu_governor, cpu),
policy->governor->name, CPUFREQ_NAME_LEN);
}

down_read(&policy->rwsem);
down_write(&policy->rwsem);
cpus = cpumask_weight(policy->cpus);
up_read(&policy->rwsem);

if (has_target() && cpus == 1)
strncpy(policy->last_governor, policy->governor->name,
CPUFREQ_NAME_LEN);
up_write(&policy->rwsem);

if (cpu != policy->cpu) {
sysfs_remove_link(&dev->kobj, "cpufreq");
Expand Down Expand Up @@ -2135,20 +2133,22 @@ EXPORT_SYMBOL_GPL(cpufreq_register_governor);

void cpufreq_unregister_governor(struct cpufreq_governor *governor)
{
int cpu;
struct cpufreq_policy *policy;
unsigned long flags;

if (!governor)
return;

if (cpufreq_disabled())
return;

for_each_present_cpu(cpu) {
if (cpu_online(cpu))
continue;
if (!strcmp(per_cpu(cpufreq_cpu_governor, cpu), governor->name))
strcpy(per_cpu(cpufreq_cpu_governor, cpu), "\0");
/* clear last_governor for all inactive policies */
read_lock_irqsave(&cpufreq_driver_lock, flags);
for_each_inactive_policy(policy) {
if (!strcmp(policy->last_governor, governor->name))
strcpy(policy->last_governor, "\0");
}
read_unlock_irqrestore(&cpufreq_driver_lock, flags);

mutex_lock(&cpufreq_governor_mutex);
list_del(&governor->governor_list);
Expand Down
1 change: 1 addition & 0 deletions include/linux/cpufreq.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ struct cpufreq_policy {
struct cpufreq_governor *governor; /* see below */
void *governor_data;
bool governor_enabled; /* governor start/stop flag */
char last_governor[CPUFREQ_NAME_LEN]; /* last governor used */

struct work_struct update; /* if update_policy() needs to be
* called, but you're in IRQ context */
Expand Down

0 comments on commit 4573237

Please sign in to comment.