From: Muhammad Usama Anjum <MUsamaAnjum@gmail.com>
To: "Rafael J. Wysocki" <rafael@kernel.org>,
Linux PM <linux-pm@vger.kernel.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Subject: Re: [PATCH v1] cpufreq: intel_pstate: Use mutex guard for driver locking
Date: Sun, 9 Nov 2025 23:36:05 +0500 [thread overview]
Message-ID: <d09a70c8-4d07-476a-87d3-671c24c97b32@gmail.com> (raw)
In-Reply-To: <2807232.mvXUDI8C0e@rafael.j.wysocki>
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;
>
>
>
prev parent reply other threads:[~2025-11-09 18:36 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
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 message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d09a70c8-4d07-476a-87d3-671c24c97b32@gmail.com \
--to=musamaanjum@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=rafael@kernel.org \
--cc=srinivas.pandruvada@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox