| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <d09a70c8-4d07-476a-87d3-671c24c97b32@gmail.com>
Date: Sun, 9 Nov 2025 23:36:05 +0500
From: Muhammad Usama Anjum <MUsamaAnjum@...il.com>
To: "Rafael J. Wysocki" <rafael@...nel.org>,
Linux PM <linux-pm@...r.kernel.org>
Cc: LKML <linux-kernel@...r.kernel.org>,
Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>
Subject: Re: [PATCH v1] cpufreq: intel_pstate: Use mutex guard for driver
locking
On 11/7/25 11:18 PM, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
>
> Use guard(mutex)(&intel_pstate_driver_lock), or the scoped variant of
> it, wherever intel_pstate_driver_lock needs to be held.
It really simplifies the code.
Reviewed-by: Muhammad Usama Anjum <usama.anjum@...labora.com>
>
> This allows some local variables and goto statements to be dropped as
> they are not necessary any more.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> ---
> drivers/cpufreq/intel_pstate.c | 99 +++++++++++++----------------------------
> 1 file changed, 33 insertions(+), 66 deletions(-)
>
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -1467,7 +1467,8 @@ static void set_power_ctl_ee_state(bool
> {
> u64 power_ctl;
>
> - mutex_lock(&intel_pstate_driver_lock);
> + guard(mutex)(&intel_pstate_driver_lock);
> +
> rdmsrq(MSR_IA32_POWER_CTL, power_ctl);
> if (input) {
> power_ctl &= ~BIT(MSR_IA32_POWER_CTL_BIT_EE);
> @@ -1477,7 +1478,6 @@ static void set_power_ctl_ee_state(bool
> power_ctl_ee_state = POWER_CTL_EE_DISABLE;
> }
> wrmsrq(MSR_IA32_POWER_CTL, power_ctl);
> - mutex_unlock(&intel_pstate_driver_lock);
> }
>
> static void intel_pstate_hwp_enable(struct cpudata *cpudata);
> @@ -1599,13 +1599,9 @@ static int intel_pstate_update_status(co
> static ssize_t show_status(struct kobject *kobj,
> struct kobj_attribute *attr, char *buf)
> {
> - ssize_t ret;
> -
> - mutex_lock(&intel_pstate_driver_lock);
> - ret = intel_pstate_show_status(buf);
> - mutex_unlock(&intel_pstate_driver_lock);
> + guard(mutex)(&intel_pstate_driver_lock);
>
> - return ret;
> + return intel_pstate_show_status(buf);
> }
>
> static ssize_t store_status(struct kobject *a, struct kobj_attribute *b,
> @@ -1614,11 +1610,13 @@ static ssize_t store_status(struct kobje
> char *p = memchr(buf, '\n', count);
> int ret;
>
> - mutex_lock(&intel_pstate_driver_lock);
> + guard(mutex)(&intel_pstate_driver_lock);
> +
> ret = intel_pstate_update_status(buf, p ? p - buf : count);
> - mutex_unlock(&intel_pstate_driver_lock);
> + if (ret < 0)
> + return ret;
>
> - return ret < 0 ? ret : count;
> + return count;
> }
>
> static ssize_t show_turbo_pct(struct kobject *kobj,
> @@ -1628,12 +1626,10 @@ static ssize_t show_turbo_pct(struct kob
> int total, no_turbo, turbo_pct;
> uint32_t turbo_fp;
>
> - mutex_lock(&intel_pstate_driver_lock);
> + guard(mutex)(&intel_pstate_driver_lock);
>
> - if (!intel_pstate_driver) {
> - mutex_unlock(&intel_pstate_driver_lock);
> + if (!intel_pstate_driver)
> return -EAGAIN;
> - }
>
> cpu = all_cpu_data[0];
>
> @@ -1642,8 +1638,6 @@ static ssize_t show_turbo_pct(struct kob
> turbo_fp = div_fp(no_turbo, total);
> turbo_pct = 100 - fp_toint(mul_fp(turbo_fp, int_tofp(100)));
>
> - mutex_unlock(&intel_pstate_driver_lock);
> -
> return sprintf(buf, "%u\n", turbo_pct);
> }
>
> @@ -1653,38 +1647,26 @@ static ssize_t show_num_pstates(struct k
> struct cpudata *cpu;
> int total;
>
> - mutex_lock(&intel_pstate_driver_lock);
> + guard(mutex)(&intel_pstate_driver_lock);
>
> - if (!intel_pstate_driver) {
> - mutex_unlock(&intel_pstate_driver_lock);
> + if (!intel_pstate_driver)
> return -EAGAIN;
> - }
>
> cpu = all_cpu_data[0];
> total = cpu->pstate.turbo_pstate - cpu->pstate.min_pstate + 1;
>
> - mutex_unlock(&intel_pstate_driver_lock);
> -
> return sprintf(buf, "%u\n", total);
> }
>
> static ssize_t show_no_turbo(struct kobject *kobj,
> struct kobj_attribute *attr, char *buf)
> {
> - ssize_t ret;
> -
> - mutex_lock(&intel_pstate_driver_lock);
> + guard(mutex)(&intel_pstate_driver_lock);
>
> - if (!intel_pstate_driver) {
> - mutex_unlock(&intel_pstate_driver_lock);
> + if (!intel_pstate_driver)
> return -EAGAIN;
> - }
> -
> - ret = sprintf(buf, "%u\n", global.no_turbo);
> -
> - mutex_unlock(&intel_pstate_driver_lock);
>
> - return ret;
> + return sprintf(buf, "%u\n", global.no_turbo);
> }
>
> static ssize_t store_no_turbo(struct kobject *a, struct kobj_attribute *b,
> @@ -1696,29 +1678,25 @@ static ssize_t store_no_turbo(struct kob
> if (sscanf(buf, "%u", &input) != 1)
> return -EINVAL;
>
> - mutex_lock(&intel_pstate_driver_lock);
> + guard(mutex)(&intel_pstate_driver_lock);
>
> - if (!intel_pstate_driver) {
> - count = -EAGAIN;
> - goto unlock_driver;
> - }
> + if (!intel_pstate_driver)
> + return -EAGAIN;
>
> no_turbo = !!clamp_t(int, input, 0, 1);
>
> WRITE_ONCE(global.turbo_disabled, turbo_is_disabled());
> if (global.turbo_disabled && !no_turbo) {
> pr_notice("Turbo disabled by BIOS or unavailable on processor\n");
> - count = -EPERM;
> if (global.no_turbo)
> - goto unlock_driver;
> - else
> - no_turbo = 1;
> - }
> + return -EPERM;
>
> - if (no_turbo == global.no_turbo) {
> - goto unlock_driver;
> + no_turbo = 1;
> }
>
> + if (no_turbo == global.no_turbo)
> + return count;
> +
> WRITE_ONCE(global.no_turbo, no_turbo);
>
> mutex_lock(&intel_pstate_limits_lock);
> @@ -1737,9 +1715,6 @@ static ssize_t store_no_turbo(struct kob
> intel_pstate_update_limits_for_all();
> arch_set_max_freq_ratio(no_turbo);
>
> -unlock_driver:
> - mutex_unlock(&intel_pstate_driver_lock);
> -
> return count;
> }
>
> @@ -1789,12 +1764,10 @@ static ssize_t store_max_perf_pct(struct
> if (ret != 1)
> return -EINVAL;
>
> - mutex_lock(&intel_pstate_driver_lock);
> + guard(mutex)(&intel_pstate_driver_lock);
>
> - if (!intel_pstate_driver) {
> - mutex_unlock(&intel_pstate_driver_lock);
> + if (!intel_pstate_driver)
> return -EAGAIN;
> - }
>
> mutex_lock(&intel_pstate_limits_lock);
>
> @@ -1807,8 +1780,6 @@ static ssize_t store_max_perf_pct(struct
> else
> update_qos_requests(FREQ_QOS_MAX);
>
> - mutex_unlock(&intel_pstate_driver_lock);
> -
> return count;
> }
>
> @@ -1822,12 +1793,10 @@ static ssize_t store_min_perf_pct(struct
> if (ret != 1)
> return -EINVAL;
>
> - mutex_lock(&intel_pstate_driver_lock);
> + guard(mutex)(&intel_pstate_driver_lock);
>
> - if (!intel_pstate_driver) {
> - mutex_unlock(&intel_pstate_driver_lock);
> + if (!intel_pstate_driver)
> return -EAGAIN;
> - }
>
> mutex_lock(&intel_pstate_limits_lock);
>
> @@ -1841,8 +1810,6 @@ static ssize_t store_min_perf_pct(struct
> else
> update_qos_requests(FREQ_QOS_MIN);
>
> - mutex_unlock(&intel_pstate_driver_lock);
> -
> return count;
> }
>
> @@ -1863,10 +1830,10 @@ static ssize_t store_hwp_dynamic_boost(s
> if (ret)
> return ret;
>
> - mutex_lock(&intel_pstate_driver_lock);
> + guard(mutex)(&intel_pstate_driver_lock);
> +
> hwp_boost = !!input;
> intel_pstate_update_policies();
> - mutex_unlock(&intel_pstate_driver_lock);
>
> return count;
> }
> @@ -3977,9 +3944,9 @@ hwp_cpu_matched:
>
> }
>
> - mutex_lock(&intel_pstate_driver_lock);
> - rc = intel_pstate_register_driver(default_driver);
> - mutex_unlock(&intel_pstate_driver_lock);
> + scoped_guard(mutex, &intel_pstate_driver_lock) {
> + rc = intel_pstate_register_driver(default_driver);
> + }
> if (rc) {
> intel_pstate_sysfs_remove();
> return rc;
>
>
>
Powered by blists - more mailing lists