Messages in this thread Patch in this message | | | Date | Wed, 19 Jun 2019 12:09:47 +0530 | From | Viresh Kumar <> | Subject | Re: [PATCH V3 4/5] cpufreq: Register notifiers with the PM QoS framework |
| |
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 {
| |