From 387b51709db546033cf9a940860b6614a5cda5b6 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 28 Mar 2025 21:40:43 +0100 Subject: cpufreq: Consolidate some code in cpufreq_online() Notice that the policy->cpu update in cpufreq_policy_alloc() can be moved to cpufreq_online() and then it can be carried out under the policy rwsem, along with the clearing of policy->governor (unnecessary in the "new policy" code branch, but also not harmful). If this is done, the bottom parts of the "if (policy)" branches become identical and they can be collapsed and moved below the conditional. Modify the code accordingly which makes it somewhat easier to follow. No intentional functional impact. Signed-off-by: Rafael J. Wysocki Acked-by: Viresh Kumar Reviewed-by: Mario Limonciello Acked-by: Sudeep Holla Tested-by: Sudeep Holla Link: https://patch.msgid.link/13741234.uLZWGnKmhe@rjwysocki.net --- drivers/cpufreq/cpufreq.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 3841c9da6cac..72c31a99c5c3 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1334,7 +1334,6 @@ static struct cpufreq_policy *cpufreq_policy_alloc(unsigned int cpu) init_waitqueue_head(&policy->transition_wait); INIT_WORK(&policy->update, handle_update); - policy->cpu = cpu; return policy; err_min_qos_notifier: @@ -1422,17 +1421,18 @@ static int cpufreq_online(unsigned int cpu) /* This is the only online CPU for the policy. Start over. */ new_policy = false; - down_write(&policy->rwsem); - policy->cpu = cpu; - policy->governor = NULL; } else { new_policy = true; policy = cpufreq_policy_alloc(cpu); if (!policy) return -ENOMEM; - down_write(&policy->rwsem); } + down_write(&policy->rwsem); + + policy->cpu = cpu; + policy->governor = NULL; + if (!new_policy && cpufreq_driver->online) { /* Recover policy->cpus using related_cpus */ cpumask_copy(policy->cpus, policy->related_cpus); -- cgit v1.2.3 From 68974e3a15b92639396f39d1cd09d5288ba687bc Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 28 Mar 2025 21:41:49 +0100 Subject: cpufreq: Split cpufreq_online() In preparation for the introduction of cpufreq policy locking guards, move the part of cpufreq_online() that is carried out under the policy rwsem into a separate function called cpufreq_policy_online(). No intentional functional impact. Signed-off-by: Rafael J. Wysocki Acked-by: Viresh Kumar Reviewed-by: Mario Limonciello Acked-by: Sudeep Holla Tested-by: Sudeep Holla Link: https://patch.msgid.link/3354747.aeNJFYEL58@rjwysocki.net --- drivers/cpufreq/cpufreq.c | 95 +++++++++++++++++++++++++++-------------------- 1 file changed, 54 insertions(+), 41 deletions(-) diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 72c31a99c5c3..31ede4d998d2 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1402,32 +1402,13 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy) kfree(policy); } -static int cpufreq_online(unsigned int cpu) +static int cpufreq_policy_online(struct cpufreq_policy *policy, + unsigned int cpu, bool new_policy) { - struct cpufreq_policy *policy; - bool new_policy; unsigned long flags; unsigned int j; int ret; - pr_debug("%s: bringing CPU%u online\n", __func__, cpu); - - /* Check if this CPU already has a policy to manage it */ - policy = per_cpu(cpufreq_cpu_data, cpu); - if (policy) { - WARN_ON(!cpumask_test_cpu(cpu, policy->related_cpus)); - if (!policy_is_inactive(policy)) - return cpufreq_add_policy_cpu(policy, cpu); - - /* This is the only online CPU for the policy. Start over. */ - new_policy = false; - } else { - new_policy = true; - policy = cpufreq_policy_alloc(cpu); - if (!policy) - return -ENOMEM; - } - down_write(&policy->rwsem); policy->cpu = cpu; @@ -1454,7 +1435,7 @@ static int cpufreq_online(unsigned int cpu) if (ret) { pr_debug("%s: %d: initialization failed\n", __func__, __LINE__); - goto out_free_policy; + goto out_clear_policy; } /* @@ -1605,8 +1586,59 @@ static int cpufreq_online(unsigned int cpu) goto out_destroy_policy; } +out_unlock: up_write(&policy->rwsem); + return ret; + +out_destroy_policy: + for_each_cpu(j, policy->real_cpus) + remove_cpu_dev_symlink(policy, j, get_cpu_device(j)); + +out_offline_policy: + if (cpufreq_driver->offline) + cpufreq_driver->offline(policy); + +out_exit_policy: + if (cpufreq_driver->exit) + cpufreq_driver->exit(policy); + +out_clear_policy: + cpumask_clear(policy->cpus); + + goto out_unlock; +} + +static int cpufreq_online(unsigned int cpu) +{ + struct cpufreq_policy *policy; + bool new_policy; + int ret; + + pr_debug("%s: bringing CPU%u online\n", __func__, cpu); + + /* Check if this CPU already has a policy to manage it */ + policy = per_cpu(cpufreq_cpu_data, cpu); + if (policy) { + WARN_ON(!cpumask_test_cpu(cpu, policy->related_cpus)); + if (!policy_is_inactive(policy)) + return cpufreq_add_policy_cpu(policy, cpu); + + /* This is the only online CPU for the policy. Start over. */ + new_policy = false; + } else { + new_policy = true; + policy = cpufreq_policy_alloc(cpu); + if (!policy) + return -ENOMEM; + } + + ret = cpufreq_policy_online(policy, cpu, new_policy); + if (ret) { + cpufreq_policy_free(policy); + return ret; + } + kobject_uevent(&policy->kobj, KOBJ_ADD); /* Callback for handling stuff after policy is ready */ @@ -1633,25 +1665,6 @@ static int cpufreq_online(unsigned int cpu) pr_debug("initialization complete\n"); return 0; - -out_destroy_policy: - for_each_cpu(j, policy->real_cpus) - remove_cpu_dev_symlink(policy, j, get_cpu_device(j)); - -out_offline_policy: - if (cpufreq_driver->offline) - cpufreq_driver->offline(policy); - -out_exit_policy: - if (cpufreq_driver->exit) - cpufreq_driver->exit(policy); - -out_free_policy: - cpumask_clear(policy->cpus); - up_write(&policy->rwsem); - - cpufreq_policy_free(policy); - return ret; } /** -- cgit v1.2.3 From 6fec833b9d70c54ceacbf7d07665215fbd0cddef Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 28 Mar 2025 21:42:48 +0100 Subject: cpufreq: Add and use cpufreq policy locking guards Introduce "read" and "write" locking guards for cpufreq policies and use them where applicable in the cpufreq core. No intentional functional impact. Signed-off-by: Rafael J. Wysocki Acked-by: Viresh Kumar Reviewed-by: Mario Limonciello Acked-by: Sudeep Holla Tested-by: Sudeep Holla Link: https://patch.msgid.link/8518682.T7Z3S40VBb@rjwysocki.net --- drivers/cpufreq/cpufreq.c | 121 +++++++++++++++++++++------------------------- include/linux/cpufreq.h | 6 +++ 2 files changed, 60 insertions(+), 67 deletions(-) diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 31ede4d998d2..a3abd9c0440d 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1009,17 +1009,16 @@ static ssize_t show(struct kobject *kobj, struct attribute *attr, char *buf) { struct cpufreq_policy *policy = to_policy(kobj); struct freq_attr *fattr = to_attr(attr); - ssize_t ret = -EBUSY; if (!fattr->show) return -EIO; - down_read(&policy->rwsem); + guard(cpufreq_policy_read)(policy); + if (likely(!policy_is_inactive(policy))) - ret = fattr->show(policy, buf); - up_read(&policy->rwsem); + return fattr->show(policy, buf); - return ret; + return -EBUSY; } static ssize_t store(struct kobject *kobj, struct attribute *attr, @@ -1027,17 +1026,16 @@ static ssize_t store(struct kobject *kobj, struct attribute *attr, { struct cpufreq_policy *policy = to_policy(kobj); struct freq_attr *fattr = to_attr(attr); - ssize_t ret = -EBUSY; if (!fattr->store) return -EIO; - down_write(&policy->rwsem); + guard(cpufreq_policy_write)(policy); + if (likely(!policy_is_inactive(policy))) - ret = fattr->store(policy, buf, count); - up_write(&policy->rwsem); + return fattr->store(policy, buf, count); - return ret; + return -EBUSY; } static void cpufreq_sysfs_release(struct kobject *kobj) @@ -1195,7 +1193,8 @@ static int cpufreq_add_policy_cpu(struct cpufreq_policy *policy, unsigned int cp if (cpumask_test_cpu(cpu, policy->cpus)) return 0; - down_write(&policy->rwsem); + guard(cpufreq_policy_write)(policy); + if (has_target()) cpufreq_stop_governor(policy); @@ -1206,7 +1205,7 @@ static int cpufreq_add_policy_cpu(struct cpufreq_policy *policy, unsigned int cp if (ret) pr_err("%s: Failed to start governor\n", __func__); } - up_write(&policy->rwsem); + return ret; } @@ -1226,9 +1225,10 @@ static void handle_update(struct work_struct *work) container_of(work, struct cpufreq_policy, update); pr_debug("handle_update for cpu %u called\n", policy->cpu); - down_write(&policy->rwsem); + + guard(cpufreq_policy_write)(policy); + refresh_frequency_limits(policy); - up_write(&policy->rwsem); } static int cpufreq_notifier_min(struct notifier_block *nb, unsigned long freq, @@ -1254,11 +1254,11 @@ static void cpufreq_policy_put_kobj(struct cpufreq_policy *policy) struct kobject *kobj; struct completion *cmp; - down_write(&policy->rwsem); - cpufreq_stats_free_table(policy); - kobj = &policy->kobj; - cmp = &policy->kobj_unregister; - up_write(&policy->rwsem); + scoped_guard(cpufreq_policy_write, policy) { + cpufreq_stats_free_table(policy); + kobj = &policy->kobj; + cmp = &policy->kobj_unregister; + } kobject_put(kobj); /* @@ -1409,7 +1409,7 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy, unsigned int j; int ret; - down_write(&policy->rwsem); + guard(cpufreq_policy_write)(policy); policy->cpu = cpu; policy->governor = NULL; @@ -1586,10 +1586,7 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy, goto out_destroy_policy; } -out_unlock: - up_write(&policy->rwsem); - - return ret; + return 0; out_destroy_policy: for_each_cpu(j, policy->real_cpus) @@ -1606,7 +1603,7 @@ out_exit_policy: out_clear_policy: cpumask_clear(policy->cpus); - goto out_unlock; + return ret; } static int cpufreq_online(unsigned int cpu) @@ -1754,11 +1751,10 @@ static int cpufreq_offline(unsigned int cpu) return 0; } - down_write(&policy->rwsem); + guard(cpufreq_policy_write)(policy); __cpufreq_offline(cpu, policy); - up_write(&policy->rwsem); return 0; } @@ -1775,33 +1771,29 @@ static void cpufreq_remove_dev(struct device *dev, struct subsys_interface *sif) if (!policy) return; - down_write(&policy->rwsem); + scoped_guard(cpufreq_policy_write, policy) { + if (cpu_online(cpu)) + __cpufreq_offline(cpu, policy); - if (cpu_online(cpu)) - __cpufreq_offline(cpu, policy); + remove_cpu_dev_symlink(policy, cpu, dev); - remove_cpu_dev_symlink(policy, cpu, dev); + if (!cpumask_empty(policy->real_cpus)) + return; - if (!cpumask_empty(policy->real_cpus)) { - up_write(&policy->rwsem); - return; - } + /* + * Unregister cpufreq cooling once all the CPUs of the policy + * are removed. + */ + if (cpufreq_thermal_control_enabled(cpufreq_driver)) { + cpufreq_cooling_unregister(policy->cdev); + policy->cdev = NULL; + } - /* - * Unregister cpufreq cooling once all the CPUs of the policy are - * removed. - */ - if (cpufreq_thermal_control_enabled(cpufreq_driver)) { - cpufreq_cooling_unregister(policy->cdev); - policy->cdev = NULL; + /* We did light-weight exit earlier, do full tear down now */ + if (cpufreq_driver->offline && cpufreq_driver->exit) + cpufreq_driver->exit(policy); } - /* We did light-weight exit earlier, do full tear down now */ - if (cpufreq_driver->offline && cpufreq_driver->exit) - cpufreq_driver->exit(policy); - - up_write(&policy->rwsem); - cpufreq_policy_free(policy); } @@ -1954,15 +1946,16 @@ unsigned int cpufreq_get(unsigned int cpu) struct cpufreq_policy *policy = cpufreq_cpu_get(cpu); unsigned int ret_freq = 0; - if (policy) { - down_read(&policy->rwsem); + if (!policy) + return 0; + + scoped_guard(cpufreq_policy_read, policy) { if (cpufreq_driver->get) ret_freq = __cpufreq_get(policy); - up_read(&policy->rwsem); - - cpufreq_cpu_put(policy); } + cpufreq_cpu_put(policy); + return ret_freq; } EXPORT_SYMBOL(cpufreq_get); @@ -2022,9 +2015,9 @@ void cpufreq_suspend(void) for_each_active_policy(policy) { if (has_target()) { - down_write(&policy->rwsem); - cpufreq_stop_governor(policy); - up_write(&policy->rwsem); + scoped_guard(cpufreq_policy_write, policy) { + cpufreq_stop_governor(policy); + } } if (cpufreq_driver->suspend && cpufreq_driver->suspend(policy)) @@ -2065,9 +2058,9 @@ void cpufreq_resume(void) pr_err("%s: Failed to resume driver: %s\n", __func__, cpufreq_driver->name); } else if (has_target()) { - down_write(&policy->rwsem); - ret = cpufreq_start_governor(policy); - up_write(&policy->rwsem); + scoped_guard(cpufreq_policy_write, policy) { + ret = cpufreq_start_governor(policy); + } if (ret) pr_err("%s: Failed to start governor for CPU%u's policy\n", @@ -2434,15 +2427,9 @@ int cpufreq_driver_target(struct cpufreq_policy *policy, unsigned int target_freq, unsigned int relation) { - int ret; + guard(cpufreq_policy_write)(policy); - down_write(&policy->rwsem); - - ret = __cpufreq_driver_target(policy, target_freq, relation); - - up_write(&policy->rwsem); - - return ret; + return __cpufreq_driver_target(policy, target_freq, relation); } EXPORT_SYMBOL_GPL(cpufreq_driver_target); diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index 400fee6427a5..cb972d2aa8df 100644 --- a/include/linux/cpufreq.h +++ b/include/linux/cpufreq.h @@ -170,6 +170,12 @@ struct cpufreq_policy { struct notifier_block nb_max; }; +DEFINE_GUARD(cpufreq_policy_write, struct cpufreq_policy *, + down_write(&_T->rwsem), up_write(&_T->rwsem)) + +DEFINE_GUARD(cpufreq_policy_read, struct cpufreq_policy *, + down_read(&_T->rwsem), up_read(&_T->rwsem)) + /* * Used for passing new cpufreq policy data to the cpufreq driver's ->verify() * callback for sanitization. That callback is only expected to modify the min -- cgit v1.2.3 From 973207ae3d7c3c92df4a382df5d7bd695deaa904 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 28 Mar 2025 21:43:46 +0100 Subject: cpufreq: intel_pstate: Rearrange max frequency updates handling code Rename __intel_pstate_update_max_freq() to intel_pstate_update_max_freq() and move the cpufreq policy reference counting and locking into it (and implement the locking with the recently introduced cpufreq policy "write" locking guard). No intentional functional impact. Signed-off-by: Rafael J. Wysocki Reviewed-by: Mario Limonciello Acked-by: Srinivas Pandruvada Link: https://patch.msgid.link/2315023.iZASKD2KPV@rjwysocki.net --- drivers/cpufreq/intel_pstate.c | 52 ++++++++++++++---------------------------- 1 file changed, 17 insertions(+), 35 deletions(-) diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c index 4aad79d26c64..108e4c6a371e 100644 --- a/drivers/cpufreq/intel_pstate.c +++ b/drivers/cpufreq/intel_pstate.c @@ -1353,9 +1353,16 @@ static void intel_pstate_update_policies(void) cpufreq_update_policy(cpu); } -static void __intel_pstate_update_max_freq(struct cpudata *cpudata, - struct cpufreq_policy *policy) +static bool intel_pstate_update_max_freq(struct cpudata *cpudata) { + struct cpufreq_policy *policy __free(put_cpufreq_policy); + + policy = cpufreq_cpu_get(cpudata->cpu); + if (!policy) + return false; + + guard(cpufreq_policy_write)(policy); + if (hwp_active) intel_pstate_get_hwp_cap(cpudata); @@ -1363,44 +1370,24 @@ static void __intel_pstate_update_max_freq(struct cpudata *cpudata, cpudata->pstate.max_freq : cpudata->pstate.turbo_freq; refresh_frequency_limits(policy); + + return true; } static void intel_pstate_update_limits(unsigned int cpu) { - struct cpufreq_policy *policy = cpufreq_cpu_acquire(cpu); - struct cpudata *cpudata; - - if (!policy) - return; - - cpudata = all_cpu_data[cpu]; - - __intel_pstate_update_max_freq(cpudata, policy); - - /* Prevent the driver from being unregistered now. */ - mutex_lock(&intel_pstate_driver_lock); + struct cpudata *cpudata = all_cpu_data[cpu]; - cpufreq_cpu_release(policy); - - hybrid_update_capacity(cpudata); - - mutex_unlock(&intel_pstate_driver_lock); + if (intel_pstate_update_max_freq(cpudata)) + hybrid_update_capacity(cpudata); } static void intel_pstate_update_limits_for_all(void) { int cpu; - for_each_possible_cpu(cpu) { - struct cpufreq_policy *policy = cpufreq_cpu_acquire(cpu); - - if (!policy) - continue; - - __intel_pstate_update_max_freq(all_cpu_data[cpu], policy); - - cpufreq_cpu_release(policy); - } + for_each_possible_cpu(cpu) + intel_pstate_update_max_freq(all_cpu_data[cpu]); mutex_lock(&hybrid_capacity_lock); @@ -1840,13 +1827,8 @@ static void intel_pstate_notify_work(struct work_struct *work) { struct cpudata *cpudata = container_of(to_delayed_work(work), struct cpudata, hwp_notify_work); - struct cpufreq_policy *policy = cpufreq_cpu_acquire(cpudata->cpu); - - if (policy) { - __intel_pstate_update_max_freq(cpudata, policy); - - cpufreq_cpu_release(policy); + if (intel_pstate_update_max_freq(cpudata)) { /* * The driver will not be unregistered while this function is * running, so update the capacity without acquiring the driver -- cgit v1.2.3 From 9a74bfdfd07f99b09fa4aaccacd17248b8629a07 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 28 Mar 2025 21:44:50 +0100 Subject: cpufreq: Use locking guard and __free() in cpufreq_update_policy() Instead of using cpufreq_cpu_acquire() and cpufreq_cpu_release() in cpufreq_update_policy(), which is the last user of these functions, make it use __free() for policy reference counting cleanup and the "write" locking guard for policy locking. No intentional functional impact. Signed-off-by: Rafael J. Wysocki Acked-by: Viresh Kumar Reviewed-by: Mario Limonciello Acked-by: Sudeep Holla Tested-by: Sudeep Holla Link: https://patch.msgid.link/22654186.EfDdHjke4D@rjwysocki.net --- drivers/cpufreq/cpufreq.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index a3abd9c0440d..40244ff620b6 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -2780,23 +2780,23 @@ static int cpufreq_set_policy(struct cpufreq_policy *policy, */ void cpufreq_update_policy(unsigned int cpu) { - struct cpufreq_policy *policy = cpufreq_cpu_acquire(cpu); + struct cpufreq_policy *policy __free(put_cpufreq_policy); + policy = cpufreq_cpu_get(cpu); if (!policy) return; + guard(cpufreq_policy_write)(policy); + /* * BIOS might change freq behind our back * -> ask driver for current freq and notify governors about a change */ if (cpufreq_driver->get && has_target() && (cpufreq_suspended || WARN_ON(!cpufreq_verify_current_freq(policy, false)))) - goto unlock; + return; refresh_frequency_limits(policy); - -unlock: - cpufreq_cpu_release(policy); } EXPORT_SYMBOL(cpufreq_update_policy); -- cgit v1.2.3 From c7282dce257480f0f22ed3db69cfb400a18709f4 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 28 Mar 2025 21:45:38 +0100 Subject: cpufreq: Drop cpufreq_cpu_acquire() and cpufreq_cpu_release() Since cpufreq_cpu_acquire() and cpufreq_cpu_release() have no more users in the tree, remove them. No intentional functional impact. Signed-off-by: Rafael J. Wysocki Acked-by: Viresh Kumar Reviewed-by: Mario Limonciello Acked-by: Sudeep Holla Tested-by: Sudeep Holla Link: https://patch.msgid.link/3880470.kQq0lBPeGt@rjwysocki.net --- drivers/cpufreq/cpufreq.c | 45 --------------------------------------------- include/linux/cpufreq.h | 2 -- 2 files changed, 47 deletions(-) diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 40244ff620b6..2ed873777fb5 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -255,51 +255,6 @@ void cpufreq_cpu_put(struct cpufreq_policy *policy) } EXPORT_SYMBOL_GPL(cpufreq_cpu_put); -/** - * cpufreq_cpu_release - Unlock a policy and decrement its usage counter. - * @policy: cpufreq policy returned by cpufreq_cpu_acquire(). - */ -void cpufreq_cpu_release(struct cpufreq_policy *policy) -{ - if (WARN_ON(!policy)) - return; - - lockdep_assert_held(&policy->rwsem); - - up_write(&policy->rwsem); - - cpufreq_cpu_put(policy); -} - -/** - * cpufreq_cpu_acquire - Find policy for a CPU, mark it as busy and lock it. - * @cpu: CPU to find the policy for. - * - * Call cpufreq_cpu_get() to get a reference on the cpufreq policy for @cpu and - * if the policy returned by it is not NULL, acquire its rwsem for writing. - * Return the policy if it is active or release it and return NULL otherwise. - * - * The policy returned by this function has to be released with the help of - * cpufreq_cpu_release() in order to release its rwsem and balance its usage - * counter properly. - */ -struct cpufreq_policy *cpufreq_cpu_acquire(unsigned int cpu) -{ - struct cpufreq_policy *policy = cpufreq_cpu_get(cpu); - - if (!policy) - return NULL; - - down_write(&policy->rwsem); - - if (policy_is_inactive(policy)) { - cpufreq_cpu_release(policy); - return NULL; - } - - return policy; -} - /********************************************************************* * EXTERNALLY AFFECTING FREQUENCY CHANGES * *********************************************************************/ diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index cb972d2aa8df..a33a094ef755 100644 --- a/include/linux/cpufreq.h +++ b/include/linux/cpufreq.h @@ -241,8 +241,6 @@ void disable_cpufreq(void); u64 get_cpu_idle_time(unsigned int cpu, u64 *wall, int io_busy); -struct cpufreq_policy *cpufreq_cpu_acquire(unsigned int cpu); -void cpufreq_cpu_release(struct cpufreq_policy *policy); int cpufreq_get_policy(struct cpufreq_policy *policy, unsigned int cpu); void refresh_frequency_limits(struct cpufreq_policy *policy); void cpufreq_update_policy(unsigned int cpu); -- cgit v1.2.3 From ece898da386214cf9bf693fe21694b556b785428 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 28 Mar 2025 21:46:22 +0100 Subject: cpufreq: Use __free() for policy reference counting cleanup Use __free() for policy reference counting cleanup where applicable in the cpufreq core. No intentional functional impact. Signed-off-by: Rafael J. Wysocki Acked-by: Viresh Kumar Reviewed-by: Mario Limonciello Acked-by: Sudeep Holla Tested-by: Sudeep Holla Link: https://patch.msgid.link/9437968.CDJkKcVGEf@rjwysocki.net --- drivers/cpufreq/cpufreq.c | 57 +++++++++++++++++++++-------------------------- 1 file changed, 25 insertions(+), 32 deletions(-) diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 2ed873777fb5..29130aa1b09a 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1818,27 +1818,26 @@ static unsigned int cpufreq_verify_current_freq(struct cpufreq_policy *policy, b */ unsigned int cpufreq_quick_get(unsigned int cpu) { - struct cpufreq_policy *policy; - unsigned int ret_freq = 0; + struct cpufreq_policy *policy __free(put_cpufreq_policy) = NULL; unsigned long flags; read_lock_irqsave(&cpufreq_driver_lock, flags); if (cpufreq_driver && cpufreq_driver->setpolicy && cpufreq_driver->get) { - ret_freq = cpufreq_driver->get(cpu); + unsigned int ret_freq = cpufreq_driver->get(cpu); + read_unlock_irqrestore(&cpufreq_driver_lock, flags); + return ret_freq; } read_unlock_irqrestore(&cpufreq_driver_lock, flags); policy = cpufreq_cpu_get(cpu); - if (policy) { - ret_freq = policy->cur; - cpufreq_cpu_put(policy); - } + if (policy) + return policy->cur; - return ret_freq; + return 0; } EXPORT_SYMBOL(cpufreq_quick_get); @@ -1850,15 +1849,13 @@ EXPORT_SYMBOL(cpufreq_quick_get); */ unsigned int cpufreq_quick_get_max(unsigned int cpu) { - struct cpufreq_policy *policy = cpufreq_cpu_get(cpu); - unsigned int ret_freq = 0; + struct cpufreq_policy *policy __free(put_cpufreq_policy); - if (policy) { - ret_freq = policy->max; - cpufreq_cpu_put(policy); - } + policy = cpufreq_cpu_get(cpu); + if (policy) + return policy->max; - return ret_freq; + return 0; } EXPORT_SYMBOL(cpufreq_quick_get_max); @@ -1870,15 +1867,13 @@ EXPORT_SYMBOL(cpufreq_quick_get_max); */ __weak unsigned int cpufreq_get_hw_max_freq(unsigned int cpu) { - struct cpufreq_policy *policy = cpufreq_cpu_get(cpu); - unsigned int ret_freq = 0; + struct cpufreq_policy *policy __free(put_cpufreq_policy); - if (policy) { - ret_freq = policy->cpuinfo.max_freq; - cpufreq_cpu_put(policy); - } + policy = cpufreq_cpu_get(cpu); + if (policy) + return policy->cpuinfo.max_freq; - return ret_freq; + return 0; } EXPORT_SYMBOL(cpufreq_get_hw_max_freq); @@ -1898,20 +1893,18 @@ static unsigned int __cpufreq_get(struct cpufreq_policy *policy) */ unsigned int cpufreq_get(unsigned int cpu) { - struct cpufreq_policy *policy = cpufreq_cpu_get(cpu); - unsigned int ret_freq = 0; + struct cpufreq_policy *policy __free(put_cpufreq_policy); + policy = cpufreq_cpu_get(cpu); if (!policy) return 0; - scoped_guard(cpufreq_policy_read, policy) { - if (cpufreq_driver->get) - ret_freq = __cpufreq_get(policy); - } + guard(cpufreq_policy_read)(policy); - cpufreq_cpu_put(policy); + if (cpufreq_driver->get) + return __cpufreq_get(policy); - return ret_freq; + return 0; } EXPORT_SYMBOL(cpufreq_get); @@ -2566,7 +2559,8 @@ EXPORT_SYMBOL_GPL(cpufreq_unregister_governor); */ int cpufreq_get_policy(struct cpufreq_policy *policy, unsigned int cpu) { - struct cpufreq_policy *cpu_policy; + struct cpufreq_policy *cpu_policy __free(put_cpufreq_policy); + if (!policy) return -EINVAL; @@ -2576,7 +2570,6 @@ int cpufreq_get_policy(struct cpufreq_policy *policy, unsigned int cpu) memcpy(policy, cpu_policy, sizeof(*policy)); - cpufreq_cpu_put(cpu_policy); return 0; } EXPORT_SYMBOL(cpufreq_get_policy); -- cgit v1.2.3 From 684e1855211145b71f8d9aaa49d2cc67067cd42a Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 28 Mar 2025 21:47:31 +0100 Subject: cpufreq: Introduce cpufreq_policy_refresh() Since cpufreq_update_limits() obtains a cpufreq policy pointer for the given CPU and reference counts the object pointed to by it, calling cpufreq_update_policy() from cpufreq_update_limits() is somewhat wasteful because that function calls cpufreq_cpu_get() on the same CPU again. To avoid that unnecessary overhead, move the part of the code running under the policy rwsem from cpufreq_update_policy() to a new function called cpufreq_policy_refresh() and invoke that new function from both cpufreq_update_policy() and cpufreq_update_limits(). Signed-off-by: Rafael J. Wysocki Acked-by: Viresh Kumar Reviewed-by: Mario Limonciello Acked-by: Sudeep Holla Tested-by: Sudeep Holla Link: https://patch.msgid.link/6047110.MhkbZ0Pkbq@rjwysocki.net --- drivers/cpufreq/cpufreq.c | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 29130aa1b09a..c885e0ec174f 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -2717,6 +2717,21 @@ static int cpufreq_set_policy(struct cpufreq_policy *policy, return ret; } +static void cpufreq_policy_refresh(struct cpufreq_policy *policy) +{ + guard(cpufreq_policy_write)(policy); + + /* + * BIOS might change freq behind our back + * -> ask driver for current freq and notify governors about a change + */ + if (cpufreq_driver->get && has_target() && + (cpufreq_suspended || WARN_ON(!cpufreq_verify_current_freq(policy, false)))) + return; + + refresh_frequency_limits(policy); +} + /** * cpufreq_update_policy - Re-evaluate an existing cpufreq policy. * @cpu: CPU to re-evaluate the policy for. @@ -2734,17 +2749,7 @@ void cpufreq_update_policy(unsigned int cpu) if (!policy) return; - guard(cpufreq_policy_write)(policy); - - /* - * BIOS might change freq behind our back - * -> ask driver for current freq and notify governors about a change - */ - if (cpufreq_driver->get && has_target() && - (cpufreq_suspended || WARN_ON(!cpufreq_verify_current_freq(policy, false)))) - return; - - refresh_frequency_limits(policy); + cpufreq_policy_refresh(policy); } EXPORT_SYMBOL(cpufreq_update_policy); @@ -2753,7 +2758,7 @@ EXPORT_SYMBOL(cpufreq_update_policy); * @cpu: CPU to update the policy limits for. * * Invoke the driver's ->update_limits callback if present or call - * cpufreq_update_policy() for @cpu. + * cpufreq_policy_refresh() for @cpu. */ void cpufreq_update_limits(unsigned int cpu) { @@ -2766,7 +2771,7 @@ void cpufreq_update_limits(unsigned int cpu) if (cpufreq_driver->update_limits) cpufreq_driver->update_limits(cpu); else - cpufreq_update_policy(cpu); + cpufreq_policy_refresh(policy); } EXPORT_SYMBOL_GPL(cpufreq_update_limits); -- cgit v1.2.3 From eaff6b62d3439ca6ee00dba4f77673a8c37dac20 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 28 Mar 2025 21:48:54 +0100 Subject: cpufreq: Pass policy pointer to ->update_limits() Since cpufreq_update_limits() obtains a cpufreq policy pointer for the given CPU and reference counts the corresponding policy object, it may as well pass the policy pointer to the cpufreq driver's ->update_limits() callback which allows that callback to avoid invoking cpufreq_cpu_get() for the same CPU. Accordingly, redefine ->update_limits() to take a policy pointer instead of a CPU number and update both drivers implementing it, intel_pstate and amd-pstate, as needed. Signed-off-by: Rafael J. Wysocki Acked-by: Viresh Kumar Reviewed-by: Mario Limonciello Acked-by: Srinivas Pandruvada Acked-by: Sudeep Holla Tested-by: Sudeep Holla Link: https://patch.msgid.link/8560367.NyiUUSuA9g@rjwysocki.net --- drivers/cpufreq/amd-pstate.c | 7 ++----- drivers/cpufreq/cpufreq.c | 2 +- drivers/cpufreq/intel_pstate.c | 29 ++++++++++++++++++----------- include/linux/cpufreq.h | 2 +- 4 files changed, 22 insertions(+), 18 deletions(-) diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c index 6789eed1bb5b..b9d59c7425f5 100644 --- a/drivers/cpufreq/amd-pstate.c +++ b/drivers/cpufreq/amd-pstate.c @@ -821,19 +821,16 @@ static void amd_pstate_init_prefcore(struct amd_cpudata *cpudata) schedule_work(&sched_prefcore_work); } -static void amd_pstate_update_limits(unsigned int cpu) +static void amd_pstate_update_limits(struct cpufreq_policy *policy) { - struct cpufreq_policy *policy __free(put_cpufreq_policy) = cpufreq_cpu_get(cpu); struct amd_cpudata *cpudata; u32 prev_high = 0, cur_high = 0; bool highest_perf_changed = false; + unsigned int cpu = policy->cpu; if (!amd_pstate_prefcore) return; - if (!policy) - return; - if (amd_get_highest_perf(cpu, &cur_high)) return; diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index c885e0ec174f..2b91ba503b32 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -2769,7 +2769,7 @@ void cpufreq_update_limits(unsigned int cpu) return; if (cpufreq_driver->update_limits) - cpufreq_driver->update_limits(cpu); + cpufreq_driver->update_limits(policy); else cpufreq_policy_refresh(policy); } diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c index 108e4c6a371e..f5ca04b98b92 100644 --- a/drivers/cpufreq/intel_pstate.c +++ b/drivers/cpufreq/intel_pstate.c @@ -1353,14 +1353,9 @@ static void intel_pstate_update_policies(void) cpufreq_update_policy(cpu); } -static bool intel_pstate_update_max_freq(struct cpudata *cpudata) +static void __intel_pstate_update_max_freq(struct cpufreq_policy *policy, + struct cpudata *cpudata) { - struct cpufreq_policy *policy __free(put_cpufreq_policy); - - policy = cpufreq_cpu_get(cpudata->cpu); - if (!policy) - return false; - guard(cpufreq_policy_write)(policy); if (hwp_active) @@ -1370,16 +1365,28 @@ static bool intel_pstate_update_max_freq(struct cpudata *cpudata) cpudata->pstate.max_freq : cpudata->pstate.turbo_freq; refresh_frequency_limits(policy); +} + +static bool intel_pstate_update_max_freq(struct cpudata *cpudata) +{ + struct cpufreq_policy *policy __free(put_cpufreq_policy); + + policy = cpufreq_cpu_get(cpudata->cpu); + if (!policy) + return false; + + __intel_pstate_update_max_freq(policy, cpudata); return true; } -static void intel_pstate_update_limits(unsigned int cpu) +static void intel_pstate_update_limits(struct cpufreq_policy *policy) { - struct cpudata *cpudata = all_cpu_data[cpu]; + struct cpudata *cpudata = all_cpu_data[policy->cpu]; - if (intel_pstate_update_max_freq(cpudata)) - hybrid_update_capacity(cpudata); + __intel_pstate_update_max_freq(policy, cpudata); + + hybrid_update_capacity(cpudata); } static void intel_pstate_update_limits_for_all(void) diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index a33a094ef755..f3cf2adea18f 100644 --- a/include/linux/cpufreq.h +++ b/include/linux/cpufreq.h @@ -399,7 +399,7 @@ struct cpufreq_driver { unsigned int (*get)(unsigned int cpu); /* Called to update policy limits on firmware notifications. */ - void (*update_limits)(unsigned int cpu); + void (*update_limits)(struct cpufreq_policy *policy); /* optional */ int (*bios_limit)(int cpu, unsigned int *limit); -- cgit v1.2.3 From 589a7c406a721f5d3a818ad0003799180f027dfa Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 10 Apr 2025 12:20:43 +0200 Subject: cpufreq: Drop unused cpufreq_get_policy() A recent change has introduced a bug into cpufreq_get_policy(), but this function is not used, so it's better to drop it altogether. Reported-by: Dan Carpenter Signed-off-by: Rafael J. Wysocki Acked-by: Viresh Kumar Acked-by: Sudeep Holla Link: https://patch.msgid.link/2802770.mvXUDI8C0e@rjwysocki.net --- drivers/cpufreq/cpufreq.c | 25 ------------------------- include/linux/cpufreq.h | 1 - 2 files changed, 26 deletions(-) diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 2b91ba503b32..2e7c730f0f7a 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -2549,31 +2549,6 @@ EXPORT_SYMBOL_GPL(cpufreq_unregister_governor); * POLICY INTERFACE * *********************************************************************/ -/** - * cpufreq_get_policy - get the current cpufreq_policy - * @policy: struct cpufreq_policy into which the current cpufreq_policy - * is written - * @cpu: CPU to find the policy for - * - * Reads the current cpufreq policy. - */ -int cpufreq_get_policy(struct cpufreq_policy *policy, unsigned int cpu) -{ - struct cpufreq_policy *cpu_policy __free(put_cpufreq_policy); - - if (!policy) - return -EINVAL; - - cpu_policy = cpufreq_cpu_get(cpu); - if (!cpu_policy) - return -EINVAL; - - memcpy(policy, cpu_policy, sizeof(*policy)); - - return 0; -} -EXPORT_SYMBOL(cpufreq_get_policy); - DEFINE_PER_CPU(unsigned long, cpufreq_pressure); /** diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index f3cf2adea18f..850fe7371cb1 100644 --- a/include/linux/cpufreq.h +++ b/include/linux/cpufreq.h @@ -241,7 +241,6 @@ void disable_cpufreq(void); u64 get_cpu_idle_time(unsigned int cpu, u64 *wall, int io_busy); -int cpufreq_get_policy(struct cpufreq_policy *policy, unsigned int cpu); void refresh_frequency_limits(struct cpufreq_policy *policy); void cpufreq_update_policy(unsigned int cpu); void cpufreq_update_limits(unsigned int cpu); -- cgit v1.2.3