Skip to content

Commit

Permalink
cpufreq: Use per-policy frequency QoS
Browse files Browse the repository at this point in the history 
Replace the CPU device PM QoS used for the management of min and max
frequency constraints in cpufreq (and its users) with per-policy
frequency QoS to avoid problems with cpufreq policies covering
more then one CPU.

Namely, a cpufreq driver is registered with the subsys interface
which calls cpufreq_add_dev() for each CPU, starting from CPU0, so
currently the PM QoS notifiers are added to the first CPU in the
policy (i.e. CPU0 in the majority of cases).

In turn, when the cpufreq driver is unregistered, the subsys interface
doing that calls cpufreq_remove_dev() for each CPU, starting from CPU0,
and the PM QoS notifiers are only removed when cpufreq_remove_dev() is
called for the last CPU in the policy, say CPUx, which as a rule is
not CPU0 if the policy covers more than one CPU.  Then, the PM QoS
notifiers cannot be removed, because CPUx does not have them, and
they are still there in the device PM QoS notifiers list of CPU0,
which prevents new PM QoS notifiers from being registered for CPU0
on the next attempt to register the cpufreq driver.

The same issue occurs when the first CPU in the policy goes offline
before unregistering the driver.

After this change it does not matter which CPU is the policy CPU at
the driver registration time and whether or not it is online all the
time, because the frequency QoS is per policy and not per CPU.

Fixes: 67d874c ("cpufreq: Register notifiers with the PM QoS framework")
Reported-by: Dmitry Osipenko <[email protected]>
Tested-by: Dmitry Osipenko <[email protected]>
Reported-by: Sudeep Holla <[email protected]>
Tested-by: Sudeep Holla <[email protected]>
Diagnosed-by: Viresh Kumar <[email protected]>
Link: https://lore.kernel.org/linux-pm/5ad2624194baa2f53acc1f1e627eb7684c577a19.1562210705.git.viresh.kumar@linaro.org/T/#md2d89e95906b8c91c15f582146173dce2e86e99f
Link: https://lore.kernel.org/linux-pm/20191017094612.6tbkwoq4harsjcqv@vireshk-i7/T/#m30d48cc23b9a80467fbaa16e30f90b3828a5a29b
Signed-off-by: Rafael J. Wysocki <[email protected]>
Acked-by: Viresh Kumar <[email protected]>
  • Loading branch information
@rafaeljw
rafaeljw committed Oct 21, 2019
1 parent 77751a4 commit 3000ce3
Show file tree
Hide file tree
Showing 10 changed files with 114 additions and 114 deletions.
9 changes: 4 additions & 5 deletions drivers/acpi/processor_driver.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,14 +290,13 @@ static int acpi_processor_notifier(struct notifier_block *nb,
unsigned long event, void *data)
{
struct cpufreq_policy *policy = data;
int cpu = policy->cpu;

if (event == CPUFREQ_CREATE_POLICY) {
acpi_thermal_cpufreq_init(cpu);
acpi_processor_ppc_init(cpu);
acpi_thermal_cpufreq_init(policy);
acpi_processor_ppc_init(policy);
} else if (event == CPUFREQ_REMOVE_POLICY) {
acpi_processor_ppc_exit(cpu);
acpi_thermal_cpufreq_exit(cpu);
acpi_processor_ppc_exit(policy);
acpi_thermal_cpufreq_exit(policy);
}

return 0;
Expand Down
18 changes: 9 additions & 9 deletions drivers/acpi/processor_perflib.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,10 @@ static int acpi_processor_get_platform_limit(struct acpi_processor *pr)
pr->performance_platform_limit = (int)ppc;

if (ppc >= pr->performance->state_count ||
unlikely(!dev_pm_qos_request_active(&pr->perflib_req)))
unlikely(!freq_qos_request_active(&pr->perflib_req)))
return 0;

ret = dev_pm_qos_update_request(&pr->perflib_req,
ret = freq_qos_update_request(&pr->perflib_req,
pr->performance->states[ppc].core_frequency * 1000);
if (ret < 0) {
pr_warn("Failed to update perflib freq constraint: CPU%d (%d)\n",
Expand Down Expand Up @@ -157,28 +157,28 @@ void acpi_processor_ignore_ppc_init(void)
ignore_ppc = 0;
}

void acpi_processor_ppc_init(int cpu)
void acpi_processor_ppc_init(struct cpufreq_policy *policy)
{
int cpu = policy->cpu;
struct acpi_processor *pr = per_cpu(processors, cpu);
int ret;

if (!pr)
return;

ret = dev_pm_qos_add_request(get_cpu_device(cpu),
&pr->perflib_req, DEV_PM_QOS_MAX_FREQUENCY,
INT_MAX);
ret = freq_qos_add_request(&policy->constraints, &pr->perflib_req,
FREQ_QOS_MAX, INT_MAX);
if (ret < 0)
pr_err("Failed to add freq constraint for CPU%d (%d)\n", cpu,
ret);
}

void acpi_processor_ppc_exit(int cpu)
void acpi_processor_ppc_exit(struct cpufreq_policy *policy)
{
struct acpi_processor *pr = per_cpu(processors, cpu);
struct acpi_processor *pr = per_cpu(processors, policy->cpu);

if (pr)
dev_pm_qos_remove_request(&pr->perflib_req);
freq_qos_remove_request(&pr->perflib_req);
}

static int acpi_processor_get_performance_control(struct acpi_processor *pr)
Expand Down
18 changes: 9 additions & 9 deletions drivers/acpi/processor_thermal.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ static int cpufreq_set_cur_state(unsigned int cpu, int state)

pr = per_cpu(processors, i);

if (unlikely(!dev_pm_qos_request_active(&pr->thermal_req)))
if (unlikely(!freq_qos_request_active(&pr->thermal_req)))
continue;

policy = cpufreq_cpu_get(i);
Expand All @@ -116,7 +116,7 @@ static int cpufreq_set_cur_state(unsigned int cpu, int state)

cpufreq_cpu_put(policy);

ret = dev_pm_qos_update_request(&pr->thermal_req, max_freq);
ret = freq_qos_update_request(&pr->thermal_req, max_freq);
if (ret < 0) {
pr_warn("Failed to update thermal freq constraint: CPU%d (%d)\n",
pr->id, ret);
Expand All @@ -125,28 +125,28 @@ static int cpufreq_set_cur_state(unsigned int cpu, int state)
return 0;
}

void acpi_thermal_cpufreq_init(int cpu)
void acpi_thermal_cpufreq_init(struct cpufreq_policy *policy)
{
int cpu = policy->cpu;
struct acpi_processor *pr = per_cpu(processors, cpu);
int ret;

if (!pr)
return;

ret = dev_pm_qos_add_request(get_cpu_device(cpu),
&pr->thermal_req, DEV_PM_QOS_MAX_FREQUENCY,
INT_MAX);
ret = freq_qos_add_request(&policy->constraints, &pr->thermal_req,
FREQ_QOS_MAX, INT_MAX);
if (ret < 0)
pr_err("Failed to add freq constraint for CPU%d (%d)\n", cpu,
ret);
}

void acpi_thermal_cpufreq_exit(int cpu)
void acpi_thermal_cpufreq_exit(struct cpufreq_policy *policy)
{
struct acpi_processor *pr = per_cpu(processors, cpu);
struct acpi_processor *pr = per_cpu(processors, policy->cpu);

if (pr)
dev_pm_qos_remove_request(&pr->thermal_req);
freq_qos_remove_request(&pr->thermal_req);
}
#else /* ! CONFIG_CPU_FREQ */
static int cpufreq_get_max_state(unsigned int cpu)
Expand Down
59 changes: 26 additions & 33 deletions drivers/cpufreq/cpufreq.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -720,7 +720,7 @@ static ssize_t store_##file_name \
if (ret != 1) \
return -EINVAL; \
\
ret = dev_pm_qos_update_request(policy->object##_freq_req, val);\
ret = freq_qos_update_request(policy->object##_freq_req, val);\
return ret >= 0 ? count : ret; \
}

Expand Down Expand Up @@ -1202,19 +1202,21 @@ static struct cpufreq_policy *cpufreq_policy_alloc(unsigned int cpu)
goto err_free_real_cpus;
}

freq_constraints_init(&policy->constraints);

policy->nb_min.notifier_call = cpufreq_notifier_min;
policy->nb_max.notifier_call = cpufreq_notifier_max;

ret = dev_pm_qos_add_notifier(dev, &policy->nb_min,
DEV_PM_QOS_MIN_FREQUENCY);
ret = freq_qos_add_notifier(&policy->constraints, FREQ_QOS_MIN,
&policy->nb_min);
if (ret) {
dev_err(dev, "Failed to register MIN QoS notifier: %d (%*pbl)\n",
ret, cpumask_pr_args(policy->cpus));
goto err_kobj_remove;
}

ret = dev_pm_qos_add_notifier(dev, &policy->nb_max,
DEV_PM_QOS_MAX_FREQUENCY);
ret = freq_qos_add_notifier(&policy->constraints, FREQ_QOS_MAX,
&policy->nb_max);
if (ret) {
dev_err(dev, "Failed to register MAX QoS notifier: %d (%*pbl)\n",
ret, cpumask_pr_args(policy->cpus));
Expand All @@ -1232,8 +1234,8 @@ static struct cpufreq_policy *cpufreq_policy_alloc(unsigned int cpu)
return policy;

err_min_qos_notifier:
dev_pm_qos_remove_notifier(dev, &policy->nb_min,
DEV_PM_QOS_MIN_FREQUENCY);
freq_qos_remove_notifier(&policy->constraints, FREQ_QOS_MIN,
&policy->nb_min);
err_kobj_remove:
cpufreq_policy_put_kobj(policy);
err_free_real_cpus:
Expand All @@ -1250,7 +1252,6 @@ static struct cpufreq_policy *cpufreq_policy_alloc(unsigned int cpu)

static void cpufreq_policy_free(struct cpufreq_policy *policy)
{
struct device *dev = get_cpu_device(policy->cpu);
unsigned long flags;
int cpu;

Expand All @@ -1262,10 +1263,10 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy)
per_cpu(cpufreq_cpu_data, cpu) = NULL;
write_unlock_irqrestore(&cpufreq_driver_lock, flags);

dev_pm_qos_remove_notifier(dev, &policy->nb_max,
DEV_PM_QOS_MAX_FREQUENCY);
dev_pm_qos_remove_notifier(dev, &policy->nb_min,
DEV_PM_QOS_MIN_FREQUENCY);
freq_qos_remove_notifier(&policy->constraints, FREQ_QOS_MAX,
&policy->nb_max);
freq_qos_remove_notifier(&policy->constraints, FREQ_QOS_MIN,
&policy->nb_min);

if (policy->max_freq_req) {
/*
Expand All @@ -1274,10 +1275,10 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy)
*/
blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
CPUFREQ_REMOVE_POLICY, policy);
dev_pm_qos_remove_request(policy->max_freq_req);
freq_qos_remove_request(policy->max_freq_req);
}

dev_pm_qos_remove_request(policy->min_freq_req);
freq_qos_remove_request(policy->min_freq_req);
kfree(policy->min_freq_req);

cpufreq_policy_put_kobj(policy);
Expand Down Expand Up @@ -1357,8 +1358,6 @@ static int cpufreq_online(unsigned int cpu)
cpumask_and(policy->cpus, policy->cpus, cpu_online_mask);

if (new_policy) {
struct device *dev = get_cpu_device(cpu);

for_each_cpu(j, policy->related_cpus) {
per_cpu(cpufreq_cpu_data, j) = policy;
add_cpu_dev_symlink(policy, j);
Expand All @@ -1369,36 +1368,31 @@ static int cpufreq_online(unsigned int cpu)
if (!policy->min_freq_req)
goto out_destroy_policy;

ret = dev_pm_qos_add_request(dev, policy->min_freq_req,
DEV_PM_QOS_MIN_FREQUENCY,
policy->min);
ret = freq_qos_add_request(&policy->constraints,
policy->min_freq_req, FREQ_QOS_MIN,
policy->min);
if (ret < 0) {
/*
* So we don't call dev_pm_qos_remove_request() for an
* So we don't call freq_qos_remove_request() for an
* uninitialized request.
*/
kfree(policy->min_freq_req);
policy->min_freq_req = NULL;

dev_err(dev, "Failed to add min-freq constraint (%d)\n",
ret);
goto out_destroy_policy;
}

/*
* This must be initialized right here to avoid calling
* dev_pm_qos_remove_request() on uninitialized request in case
* freq_qos_remove_request() on uninitialized request in case
* of errors.
*/
policy->max_freq_req = policy->min_freq_req + 1;

ret = dev_pm_qos_add_request(dev, policy->max_freq_req,
DEV_PM_QOS_MAX_FREQUENCY,
policy->max);
ret = freq_qos_add_request(&policy->constraints,
policy->max_freq_req, FREQ_QOS_MAX,
policy->max);
if (ret < 0) {
policy->max_freq_req = NULL;
dev_err(dev, "Failed to add max-freq constraint (%d)\n",
ret);
goto out_destroy_policy;
}

Expand Down Expand Up @@ -2374,7 +2368,6 @@ int cpufreq_set_policy(struct cpufreq_policy *policy,
struct cpufreq_policy *new_policy)
{
struct cpufreq_governor *old_gov;
struct device *cpu_dev = get_cpu_device(policy->cpu);
int ret;

pr_debug("setting new policy for CPU %u: %u - %u kHz\n",
Expand All @@ -2386,8 +2379,8 @@ int cpufreq_set_policy(struct cpufreq_policy *policy,
* PM QoS framework collects all the requests from users and provide us
* the final aggregated value here.
*/
new_policy->min = dev_pm_qos_read_value(cpu_dev, DEV_PM_QOS_MIN_FREQUENCY);
new_policy->max = dev_pm_qos_read_value(cpu_dev, DEV_PM_QOS_MAX_FREQUENCY);
new_policy->min = freq_qos_read_value(&policy->constraints, FREQ_QOS_MIN);
new_policy->max = freq_qos_read_value(&policy->constraints, FREQ_QOS_MAX);

/* verify the cpu speed can be set within this limit */
ret = cpufreq_driver->verify(new_policy);
Expand Down Expand Up @@ -2518,7 +2511,7 @@ static int cpufreq_boost_set_sw(int state)
break;
}

ret = dev_pm_qos_update_request(policy->max_freq_req, policy->max);
ret = freq_qos_update_request(policy->max_freq_req, policy->max);
if (ret < 0)
break;
}
Expand Down
30 changes: 15 additions & 15 deletions drivers/cpufreq/intel_pstate.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1088,10 +1088,10 @@ static ssize_t store_no_turbo(struct kobject *a, struct kobj_attribute *b,

static struct cpufreq_driver intel_pstate;

static void update_qos_request(enum dev_pm_qos_req_type type)
static void update_qos_request(enum freq_qos_req_type type)
{
int max_state, turbo_max, freq, i, perf_pct;
struct dev_pm_qos_request *req;
struct freq_qos_request *req;
struct cpufreq_policy *policy;

for_each_possible_cpu(i) {
Expand All @@ -1112,7 +1112,7 @@ static void update_qos_request(enum dev_pm_qos_req_type type)
else
turbo_max = cpu->pstate.turbo_pstate;

if (type == DEV_PM_QOS_MIN_FREQUENCY) {
if (type == FREQ_QOS_MIN) {
perf_pct = global.min_perf_pct;
} else {
req++;
Expand All @@ -1122,7 +1122,7 @@ static void update_qos_request(enum dev_pm_qos_req_type type)
freq = DIV_ROUND_UP(turbo_max * perf_pct, 100);
freq *= cpu->pstate.scaling;

if (dev_pm_qos_update_request(req, freq) < 0)
if (freq_qos_update_request(req, freq) < 0)
pr_warn("Failed to update freq constraint: CPU%d\n", i);
}
}
Expand Down Expand Up @@ -1153,7 +1153,7 @@ static ssize_t store_max_perf_pct(struct kobject *a, struct kobj_attribute *b,
if (intel_pstate_driver == &intel_pstate)
intel_pstate_update_policies();
else
update_qos_request(DEV_PM_QOS_MAX_FREQUENCY);
update_qos_request(FREQ_QOS_MAX);

mutex_unlock(&intel_pstate_driver_lock);

Expand Down Expand Up @@ -1187,7 +1187,7 @@ static ssize_t store_min_perf_pct(struct kobject *a, struct kobj_attribute *b,
if (intel_pstate_driver == &intel_pstate)
intel_pstate_update_policies();
else
update_qos_request(DEV_PM_QOS_MIN_FREQUENCY);
update_qos_request(FREQ_QOS_MIN);

mutex_unlock(&intel_pstate_driver_lock);

Expand Down Expand Up @@ -2381,7 +2381,7 @@ static unsigned int intel_cpufreq_fast_switch(struct cpufreq_policy *policy,
static int intel_cpufreq_cpu_init(struct cpufreq_policy *policy)
{
int max_state, turbo_max, min_freq, max_freq, ret;
struct dev_pm_qos_request *req;
struct freq_qos_request *req;
struct cpudata *cpu;
struct device *dev;

Expand Down Expand Up @@ -2416,15 +2416,15 @@ static int intel_cpufreq_cpu_init(struct cpufreq_policy *policy)
max_freq = DIV_ROUND_UP(turbo_max * global.max_perf_pct, 100);
max_freq *= cpu->pstate.scaling;

ret = dev_pm_qos_add_request(dev, req, DEV_PM_QOS_MIN_FREQUENCY,
min_freq);
ret = freq_qos_add_request(&policy->constraints, req, FREQ_QOS_MIN,
min_freq);
if (ret < 0) {
dev_err(dev, "Failed to add min-freq constraint (%d)\n", ret);
goto free_req;
}

ret = dev_pm_qos_add_request(dev, req + 1, DEV_PM_QOS_MAX_FREQUENCY,
max_freq);
ret = freq_qos_add_request(&policy->constraints, req + 1, FREQ_QOS_MAX,
max_freq);
if (ret < 0) {
dev_err(dev, "Failed to add max-freq constraint (%d)\n", ret);
goto remove_min_req;
Expand All @@ -2435,7 +2435,7 @@ static int intel_cpufreq_cpu_init(struct cpufreq_policy *policy)
return 0;

remove_min_req:
dev_pm_qos_remove_request(req);
freq_qos_remove_request(req);
free_req:
kfree(req);
pstate_exit:
Expand All @@ -2446,12 +2446,12 @@ static int intel_cpufreq_cpu_init(struct cpufreq_policy *policy)

static int intel_cpufreq_cpu_exit(struct cpufreq_policy *policy)
{
struct dev_pm_qos_request *req;
struct freq_qos_request *req;

req = policy->driver_data;

dev_pm_qos_remove_request(req + 1);
dev_pm_qos_remove_request(req);
freq_qos_remove_request(req + 1);
freq_qos_remove_request(req);
kfree(req);

return intel_pstate_cpu_exit(policy);
Expand Down
Loading

0 comments on commit 3000ce3

Please sign in to comment.