linux-kernel - Re: [PATCH 12/14] cpufreq/amd-pstate: Cache a pointer to policy in cpudata

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2877ad66-0148-483e-83b3-fab5da5dbe1f@kernel.org>
Date: Tue, 11 Feb 2025 13:17:23 -0600
From: Mario Limonciello <superm1@...nel.org>
To: Dhananjay Ugwekar <Dhananjay.Ugwekar@....com>,
 "Gautham R . Shenoy" <gautham.shenoy@....com>,
 Perry Yuan <perry.yuan@....com>
Cc: "open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)"
 <linux-kernel@...r.kernel.org>,
 "open list:CPU FREQUENCY SCALING FRAMEWORK" <linux-pm@...r.kernel.org>,
 Mario Limonciello <mario.limonciello@....com>
Subject: Re: [PATCH 12/14] cpufreq/amd-pstate: Cache a pointer to policy in
 cpudata

On 2/11/2025 07:13, Dhananjay Ugwekar wrote:
> On 2/7/2025 3:26 AM, Mario Limonciello wrote:
>> From: Mario Limonciello <mario.limonciello@....com>
>>
>> In order to access the policy from a notification block it will
>> need to be stored in cpudata.
> 
> This might break the cpufreq_policy ref counting right?, if we cache the pointer
> and use it independent of the ref counting framework.

Would it be reasonable to bump the ref count when we take the pointer?

I'm not sure if this will work properly.

> 
>>
>> Signed-off-by: Mario Limonciello <mario.limonciello@....com>
>> ---
>>   drivers/cpufreq/amd-pstate.c | 13 +++++++------
>>   drivers/cpufreq/amd-pstate.h |  3 ++-
>>   2 files changed, 9 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
>> index 689de385d06da..5945b6c7f7e56 100644
>> --- a/drivers/cpufreq/amd-pstate.c
>> +++ b/drivers/cpufreq/amd-pstate.c
>> @@ -388,7 +388,7 @@ static int amd_pstate_set_energy_pref_index(struct cpufreq_policy *policy,
>>   	else
>>   		epp = epp_values[pref_index];
>>   
>> -	if (epp > 0 && cpudata->policy == CPUFREQ_POLICY_PERFORMANCE) {
>> +	if (epp > 0 && policy->policy == CPUFREQ_POLICY_PERFORMANCE) {
>>   		pr_debug("EPP cannot be set under performance policy\n");
>>   		return -EBUSY;
>>   	}
>> @@ -689,7 +689,7 @@ static void amd_pstate_update_min_max_limit(struct cpufreq_policy *policy)
>>   	perf.max_limit_perf = freq_to_perf(perf, cpudata->nominal_freq, policy->max);
>>   	perf.min_limit_perf = freq_to_perf(perf, cpudata->nominal_freq, policy->min);
>>   
>> -	if (cpudata->policy == CPUFREQ_POLICY_PERFORMANCE)
>> +	if (policy->policy == CPUFREQ_POLICY_PERFORMANCE)
>>   		perf.min_limit_perf = min(perf.nominal_perf, perf.max_limit_perf);
>>   
>>   	WRITE_ONCE(cpudata->perf, perf);
>> @@ -1042,6 +1042,7 @@ static int amd_pstate_cpu_init(struct cpufreq_policy *policy)
>>   		return -ENOMEM;
>>   
>>   	cpudata->cpu = policy->cpu;
>> +	cpudata->policy = policy;
>>   
>>   	mutex_init(&cpudata->lock);
>>   	guard(mutex)(&cpudata->lock);
>> @@ -1224,9 +1225,8 @@ static ssize_t show_energy_performance_available_preferences(
>>   {
>>   	int i = 0;
>>   	int offset = 0;
>> -	struct amd_cpudata *cpudata = policy->driver_data;
>>   
>> -	if (cpudata->policy == CPUFREQ_POLICY_PERFORMANCE)
>> +	if (policy->policy == CPUFREQ_POLICY_PERFORMANCE)
>>   		return sysfs_emit_at(buf, offset, "%s\n",
>>   				energy_perf_strings[EPP_INDEX_PERFORMANCE]);
>>   
>> @@ -1543,6 +1543,7 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
>>   		return -ENOMEM;
>>   
>>   	cpudata->cpu = policy->cpu;
>> +	cpudata->policy = policy;
>>   
>>   	mutex_init(&cpudata->lock);
>>   	guard(mutex)(&cpudata->lock);
>> @@ -1632,7 +1633,7 @@ static int amd_pstate_epp_update_limit(struct cpufreq_policy *policy)
>>   
>>   	amd_pstate_update_min_max_limit(policy);
>>   
>> -	if (cpudata->policy == CPUFREQ_POLICY_PERFORMANCE)
>> +	if (policy->policy == CPUFREQ_POLICY_PERFORMANCE)
>>   		epp = 0;
>>   	else
>>   		epp = READ_ONCE(cpudata->epp_cached);
>> @@ -1651,7 +1652,7 @@ static int amd_pstate_epp_set_policy(struct cpufreq_policy *policy)
>>   	if (!policy->cpuinfo.max_freq)
>>   		return -ENODEV;
>>   
>> -	cpudata->policy = policy->policy;
>> +	cpudata->policy = policy;
>>   
>>   	ret = amd_pstate_epp_update_limit(policy);
>>   	if (ret)
>> diff --git a/drivers/cpufreq/amd-pstate.h b/drivers/cpufreq/amd-pstate.h
>> index 7501d30db9953..16ce631a6c3d5 100644
>> --- a/drivers/cpufreq/amd-pstate.h
>> +++ b/drivers/cpufreq/amd-pstate.h
>> @@ -97,9 +97,10 @@ struct amd_cpudata {
>>   
>>   	struct mutex	lock;
>>   
>> +	struct cpufreq_policy *policy;
>> +
>>   	/* EPP feature related attributes*/
>>   	u8	epp_cached;
>> -	u32	policy;
>>   	bool	suspended;
>>   	u8	epp_default;
>>   };
>