Re: [PATCH] cpufreq/amd-pstate: Remove unnecessary driver_lock in set_boost

["Gautham R. Shenoy" <gautham.shenoy@amd.com>]

On Thu, Jan 30, 2025 at 08:52:52AM +0000, Dhananjay Ugwekar wrote:
> set_boost is a per-policy function call, hence a driver wide lock is
> unnecessary. Also this mutex_acquire can collide with the mutex_acquire
> from the mode-switch path in status_store(), which can lead to a
> deadlock. So, remove it.

Looks good to me. The driver lock should only guard the state
changes. Everything else is a per-policy change and is better guarded
by the per-cpudata mutex.

Once Mario acks this patch, please respond to Viresh's series and let
him know that this patch needs to go in before his series. If he is ok
with it, he can include it in his series.


> 
> Signed-off-by: Dhananjay Ugwekar <dhananjay.ugwekar@amd.com>
> ---
> PS: This patch should ideally go before [1], as that patch uncovers this 
> bug and actually leads to a deadlock when switching the amd_pstate driver 
> mode.
> [1] https://lore.kernel.org/linux-pm/e16c06d4b8ffdb20e802ffe648f14dc515e60426.1737707712.git.viresh.kumar@linaro.org/
> ---
>  drivers/cpufreq/amd-pstate.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index d5be51bf8692..93788bce7e6a 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -740,7 +740,6 @@ static int amd_pstate_set_boost(struct cpufreq_policy *policy, int state)
>  		pr_err("Boost mode is not supported by this processor or SBIOS\n");
>  		return -EOPNOTSUPP;
>  	}
> -	guard(mutex)(&amd_pstate_driver_lock);
>  
>  	ret = amd_pstate_cpu_boost_update(policy, state);
>  	refresh_frequency_limits(policy);
> -- 
> 2.34.1
> 

-- 
Thanks and Regards
gautham.