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

[Mario Limonciello <superm1@kernel.org>]

On 2/11/2025 07:13, Dhananjay Ugwekar wrote:
> On 2/7/2025 3:26 AM, Mario Limonciello wrote:
>> From: Mario Limonciello <mario.limonciello@amd.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@amd.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;
>>   };
>