Re: [PATCH v4 01/10] arm64: topology: Use scope-based cleanup helper

[Jonathan Cameron]

On Wed,  3 Sep 2025 21:17:24 +0800
Zihuan Zhang <zhangzihuan@xxxxxxxxxx> 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@xxxxxxxxxx>
Diff is being slightly annoying in not quite showing enough context
to see that it is fine to release this at the end of scope.
What you have done looks correct to me but there is a further
potential cleanup

	for_each_cpu_wrap(ref_cpu, policy->cpus, cpu + 1) {
		if (ref_cpu == start_cpu) {
			/* Prevent verifying same CPU twice */
			ref_cpu = nr_cpu_ids;
			break;
//here, might as well return -EAGAIN as only thing in this block
//and now no advantage in sharing the exit path with the one that
//says we never matched the next condition.
		}
		if (!idle_cpu(ref_cpu))
			break;
		}

Either way I'm fine with this.
Reviewed-by: Jonathan Cameron <jonathan.cameron@xxxxxxxxxx>

> ---
>  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..0eebd3492669 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) =
> +				cpufreq_cpu_get(cpu);
>  			int ref_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;