Re: [PATCH v2 01/18] arm64: topology: Use __free(put_cpufreq_policy) for policy reference

[Ben Horgan <ben.horgan@arm.com>]

Hi Zihuan,

On 8/27/25 09:55, Zihuan Zhang wrote:
> Hi,
> 
> 在 2025/8/27 16:30, Ben Horgan 写道:
>> Hi Zihuan,
>>
>> On 8/27/25 03:31, Zihuan Zhang wrote:
>>> Replace the manual cpufreq_cpu_put() with __free(put_cpufreq_policy)
>>> annotation for policy references. This reduces the risk of reference
>>> counting mistakes and aligns the code with the latest kernel style.
>>>
>>> No functional change intended.
>>>
>>> Signed-off-by: Zihuan Zhang <zhangzihuan@kylinos.cn>
>>> ---
>>>   arch/arm64/kernel/topology.c | 9 +++------
>>>   1 file changed, 3 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
>>> index 5d07ee85bdae..e3cb6d54f35b 100644
>>> --- a/arch/arm64/kernel/topology.c
>>> +++ b/arch/arm64/kernel/topology.c
>>> @@ -307,17 +307,16 @@ int arch_freq_get_on_cpu(int cpu)
>>>            */
>>>           if (!housekeeping_cpu(cpu, HK_TYPE_TICK) ||
>>>               time_is_before_jiffies(last_update +
>>> msecs_to_jiffies(AMU_SAMPLE_EXP_MS))) {
>>> -            struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
>>> +            struct cpufreq_policy *policy __free(put_cpufreq_policy);
>> Based on the guidance, in include/linux/cleanup.h, I would expect the
>> assignment to be done on this line.
>>
>> "...the recommendation is to always define and assign variables in one
>>   * statement and not group variable definitions at the top of the
>>   * function when __free() is used."
> 
> 
> The reason I split the assignment into multiple lines is because
> scripts/checkpatch.pl gave a warning about the line being too long.
> 
> But if you think a single-line assignment is better, I will modify it
> accordingly.

My preference, for what it's worth, would be to keep it one statement
and split the line after the =.

> 
>>>               int ref_cpu;
>>>   +            policy = cpufreq_cpu_get(cpu);
>>>               if (!policy)
>>>                   return -EINVAL;
>>>                 if (!cpumask_intersects(policy->related_cpus,
>>> -                        housekeeping_cpumask(HK_TYPE_TICK))) {
>>> -                cpufreq_cpu_put(policy);
>>> +                        housekeeping_cpumask(HK_TYPE_TICK)))
>>>                   return -EOPNOTSUPP;
>>> -            }
>>>                 for_each_cpu_wrap(ref_cpu, policy->cpus, cpu + 1) {
>>>                   if (ref_cpu == start_cpu) {
>>> @@ -329,8 +328,6 @@ int arch_freq_get_on_cpu(int cpu)
>>>                       break;
>>>               }
>>>   -            cpufreq_cpu_put(policy);
>>> -
>>>               if (ref_cpu >= nr_cpu_ids)
>>>                   /* No alternative to pull info from */
>>>                   return -EAGAIN;
>> Thanks,
>>
>> Ben
>>

Thanks,

Ben