Re: [PATCH v4 10/10] PM: EM: Use scope-based cleanup helper

[Krzysztof Kozlowski <krzk@kernel.org>]

On 03/09/2025 15:17, 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>
> ---
>  kernel/power/energy_model.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c
> index ea7995a25780..99401678e809 100644
> --- a/kernel/power/energy_model.c
> +++ b/kernel/power/energy_model.c
> @@ -451,7 +451,7 @@ static void
>  em_cpufreq_update_efficiencies(struct device *dev, struct em_perf_state *table)
>  {
>  	struct em_perf_domain *pd = dev->em_pd;
> -	struct cpufreq_policy *policy;
> +	struct cpufreq_policy *policy __free(put_cpufreq_policy) = NULL;

This is not really correct coding style. Please read how to use
cleanup.h expressed in that header. You should have here proper
constructor or this should be moved. Or this should not be __free()...


>  	int found = 0;
>  	int i, cpu;
>  
> @@ -479,8 +479,6 @@ em_cpufreq_update_efficiencies(struct device *dev, struct em_perf_state *table)
>  			found++;
>  	}
>  
> -	cpufreq_cpu_put(policy);
> -
>  	if (!found)
>  		return;
>  
> @@ -787,7 +785,7 @@ static void em_check_capacity_update(void)
>  
>  	/* Check if CPUs capacity has changed than update EM */
>  	for_each_possible_cpu(cpu) {
> -		struct cpufreq_policy *policy;
> +		struct cpufreq_policy *policy __free(put_cpufreq_policy) = NULL;

Same problem here.

I don't think you really paid attention to my feedback last time.

Cleanup.h requires knowing what you do, not just blindly adding __free()
here and there.

Best regards,
Krzysztof