On 19-06-19, 00:23, Rafael J. Wysocki wrote:
> In patch [3/5] you could point notifiers for both min and max freq to the same
> notifier head. Both of your notifiers end up calling cpufreq_update_policy()
> anyway.
I tried it and the changes in qos.c file look fine. But I don't like at all how
cpufreq.c looks now. We only register for min-freq notifier now and that takes
care of max as well. What could have been better is if we could have registered
a freq-notifier instead of min/max, which isn't possible as well because of how
qos framework works.
Honestly, the cpufreq changes look hacky to me :(
What do you say.
--
viresh
---
drivers/base/power/qos.c | 15 ++++++++-------
drivers/cpufreq/cpufreq.c | 38 ++++++++------------------------------
include/linux/cpufreq.h | 3 +--
3 files changed, 17 insertions(+), 39 deletions(-)
diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c
index cde2692b97f9..9bbf2d2a3376 100644
--- a/drivers/base/power/qos.c
+++ b/drivers/base/power/qos.c
@@ -202,20 +202,20 @@ static int dev_pm_qos_constraints_allocate(struct device
*dev)
if (!qos)
return -ENOMEM;
- n = kzalloc(3 * sizeof(*n), GFP_KERNEL);
+ n = kzalloc(2 * sizeof(*n), GFP_KERNEL);
if (!n) {
kfree(qos);
return -ENOMEM;
}
+ BLOCKING_INIT_NOTIFIER_HEAD(n);
c = &qos->resume_latency;
plist_head_init(&c->list);
c->target_value = PM_QOS_RESUME_LATENCY_DEFAULT_VALUE;
c->default_value = PM_QOS_RESUME_LATENCY_DEFAULT_VALUE;
c->no_constraint_value = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
c->type = PM_QOS_MIN;
- c->notifiers = n;
- BLOCKING_INIT_NOTIFIER_HEAD(n);
+ c->notifiers = n++;
c = &qos->latency_tolerance;
plist_head_init(&c->list);
@@ -224,14 +224,16 @@ static int dev_pm_qos_constraints_allocate(struct device
*dev)
c->no_constraint_value = PM_QOS_LATENCY_TOLERANCE_NO_CONSTRAINT;
c->type = PM_QOS_MIN;
+ /* Same notifier head is used for both min/max frequency */
+ BLOCKING_INIT_NOTIFIER_HEAD(n);
+
c = &qos->min_frequency;
plist_head_init(&c->list);
c->target_value = PM_QOS_MIN_FREQUENCY_DEFAULT_VALUE;
c->default_value = PM_QOS_MIN_FREQUENCY_DEFAULT_VALUE;
c->no_constraint_value = PM_QOS_MIN_FREQUENCY_DEFAULT_VALUE;
c->type = PM_QOS_MAX;
- c->notifiers = ++n;
- BLOCKING_INIT_NOTIFIER_HEAD(n);
+ c->notifiers = n;
c = &qos->max_frequency;
plist_head_init(&c->list);
@@ -239,8 +241,7 @@ static int dev_pm_qos_constraints_allocate(struct device
*dev)
c->default_value = PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE;
c->no_constraint_value = PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE;
c->type = PM_QOS_MIN;
- c->notifiers = ++n;
- BLOCKING_INIT_NOTIFIER_HEAD(n);
+ c->notifiers = n;
INIT_LIST_HEAD(&qos->flags.list);
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 1344e1b1307f..1605dba1327e 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1139,19 +1139,10 @@ static int cpufreq_update_freq(struct cpufreq_policy
*policy)
return 0;
}
-static int cpufreq_notifier_min(struct notifier_block *nb, unsigned long freq,
+static int cpufreq_notifier_qos(struct notifier_block *nb, unsigned long freq,
void *data)
{
- struct cpufreq_policy *policy = container_of(nb, struct cpufreq_policy,
nb_min);
-
- return cpufreq_update_freq(policy);
-}
-
-static int cpufreq_notifier_max(struct notifier_block *nb, unsigned long freq,
- void *data)
-{
- struct cpufreq_policy *policy = container_of(nb, struct cpufreq_policy,
nb_max);
+ struct cpufreq_policy *policy = container_of(nb, struct cpufreq_policy,
nb_qos);
return cpufreq_update_freq(policy);
@@ -1214,10 +1205,10 @@ static struct cpufreq_policy
*cpufreq_policy_alloc(unsigned int cpu)
goto err_free_real_cpus;
}
- policy->nb_min.notifier_call = cpufreq_notifier_min;
- policy->nb_max.notifier_call = cpufreq_notifier_max;
+ policy->nb_qos.notifier_call = cpufreq_notifier_qos;
- ret = dev_pm_qos_add_notifier(dev, &policy->nb_min,
+ /* Notifier for min frequency also takes care of max frequency notifier
*/
+ ret = dev_pm_qos_add_notifier(dev, &policy->nb_qos,
DEV_PM_QOS_MIN_FREQUENCY);
if (ret) {
dev_err(dev, "Failed to register MIN QoS notifier: %d
(%*pbl)\n",
@@ -1225,18 +1216,10 @@ static struct cpufreq_policy
*cpufreq_policy_alloc(unsigned int cpu)
goto err_kobj_remove;
}
- ret = dev_pm_qos_add_notifier(dev, &policy->nb_max,
- DEV_PM_QOS_MAX_FREQUENCY);
- if (ret) {
- dev_err(dev, "Failed to register MAX QoS notifier: %d
(%*pbl)\n",
- ret, cpumask_pr_args(policy->cpus));
- goto err_min_qos_notifier;
- }
-
policy->min_freq_req = kzalloc(2 * sizeof(*policy->min_freq_req),
GFP_KERNEL);
if (!policy->min_freq_req)
- goto err_max_qos_notifier;
+ goto err_min_qos_notifier;
policy->max_freq_req = policy->min_freq_req + 1;
INIT_LIST_HEAD(&policy->policy_list);
@@ -1250,11 +1233,8 @@ static struct cpufreq_policy
*cpufreq_policy_alloc(unsigned int cpu)
policy->cpu = cpu;
return policy;
-err_max_qos_notifier:
- dev_pm_qos_remove_notifier(dev, &policy->nb_max,
- DEV_PM_QOS_MAX_FREQUENCY);
err_min_qos_notifier:
- dev_pm_qos_remove_notifier(dev, &policy->nb_min,
+ dev_pm_qos_remove_notifier(dev, &policy->nb_qos,
DEV_PM_QOS_MIN_FREQUENCY);
err_kobj_remove:
cpufreq_policy_put_kobj(policy);
@@ -1284,9 +1264,7 @@ 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_remove_notifier(dev, &policy->nb_qos,
DEV_PM_QOS_MIN_FREQUENCY);
cancel_work_sync(&policy->req_work);
dev_pm_qos_remove_request(policy->max_freq_req);
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 6bbed9af4fd2..2080d6490ed1 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -145,8 +145,7 @@ struct cpufreq_policy {
/* Pointer to the cooling device if used for thermal mitigation */
struct thermal_cooling_device *cdev;
- struct notifier_block nb_min;
- struct notifier_block nb_max;
+ struct notifier_block nb_qos;
};
struct cpufreq_freqs {
