* [PATCH v1] cpufreq: intel_pstate: Use mutex guard for driver locking
@ 2025-11-07 18:18 Rafael J. Wysocki
2025-11-09 18:36 ` Muhammad Usama Anjum
0 siblings, 1 reply; 2+ messages in thread
From: Rafael J. Wysocki @ 2025-11-07 18:18 UTC (permalink / raw)
To: Linux PM; +Cc: LKML, Srinivas Pandruvada
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Use guard(mutex)(&intel_pstate_driver_lock), or the scoped variant of
it, wherever intel_pstate_driver_lock needs to be held.
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@intel.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;
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH v1] cpufreq: intel_pstate: Use mutex guard for driver locking
2025-11-07 18:18 [PATCH v1] cpufreq: intel_pstate: Use mutex guard for driver locking Rafael J. Wysocki
@ 2025-11-09 18:36 ` Muhammad Usama Anjum
0 siblings, 0 replies; 2+ messages in thread
From: Muhammad Usama Anjum @ 2025-11-09 18:36 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux PM; +Cc: LKML, Srinivas Pandruvada
On 11/7/25 11:18 PM, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.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@collabora.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@intel.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;
>
>
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2025-11-09 18:36 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-07 18:18 [PATCH v1] cpufreq: intel_pstate: Use mutex guard for driver locking Rafael J. Wysocki
2025-11-09 18:36 ` Muhammad Usama Anjum
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).