Re: [PATCH v2 06/17] cpufreq/amd-pstate-ut: Use _free macro to free put policy

["Gautham R. Shenoy" <gautham.shenoy@amd.com>]

On Fri, Feb 14, 2025 at 06:52:33PM -0600, Mario Limonciello wrote:
> From: Mario Limonciello <mario.limonciello@amd.com>
> 
> Using a scoped cleanup macro simplifies cleanup code.
> 
> Reviewed-by: Dhananjay Ugwekar <dhananjay.ugwekar@amd.com>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

LGTM.

Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com>

-- 
Thanks and Regards
gautham.

> ---
>  drivers/cpufreq/amd-pstate-ut.c | 33 ++++++++++++++-------------------
>  1 file changed, 14 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate-ut.c
> index ba3e06f349c6d..9f790c7254d52 100644
> --- a/drivers/cpufreq/amd-pstate-ut.c
> +++ b/drivers/cpufreq/amd-pstate-ut.c
> @@ -26,6 +26,7 @@
>  #include <linux/module.h>
>  #include <linux/moduleparam.h>
>  #include <linux/fs.h>
> +#include <linux/cleanup.h>
>  
>  #include <acpi/cppc_acpi.h>
>  
> @@ -127,11 +128,12 @@ static void amd_pstate_ut_check_perf(u32 index)
>  	u32 highest_perf = 0, nominal_perf = 0, lowest_nonlinear_perf = 0, lowest_perf = 0;
>  	u64 cap1 = 0;
>  	struct cppc_perf_caps cppc_perf;
> -	struct cpufreq_policy *policy = NULL;
>  	struct amd_cpudata *cpudata = NULL;
>  	union perf_cached cur_perf;
>  
>  	for_each_possible_cpu(cpu) {
> +		struct cpufreq_policy *policy __free(put_cpufreq_policy) = NULL;
> +
>  		policy = cpufreq_cpu_get(cpu);
>  		if (!policy)
>  			break;
> @@ -142,7 +144,7 @@ static void amd_pstate_ut_check_perf(u32 index)
>  			if (ret) {
>  				amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_FAIL;
>  				pr_err("%s cppc_get_perf_caps ret=%d error!\n", __func__, ret);
> -				goto skip_test;
> +				return;
>  			}
>  
>  			highest_perf = cppc_perf.highest_perf;
> @@ -154,7 +156,7 @@ static void amd_pstate_ut_check_perf(u32 index)
>  			if (ret) {
>  				amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_FAIL;
>  				pr_err("%s read CPPC_CAP1 ret=%d error!\n", __func__, ret);
> -				goto skip_test;
> +				return;
>  			}
>  
>  			highest_perf = AMD_CPPC_HIGHEST_PERF(cap1);
> @@ -167,7 +169,7 @@ static void amd_pstate_ut_check_perf(u32 index)
>  		if (highest_perf != cur_perf.highest_perf && !cpudata->hw_prefcore) {
>  			pr_err("%s cpu%d highest=%d %d highest perf doesn't match\n",
>  				__func__, cpu, highest_perf, cpudata->perf.highest_perf);
> -			goto skip_test;
> +			return;
>  		}
>  		if (nominal_perf != cur_perf.nominal_perf ||
>  		   (lowest_nonlinear_perf != cur_perf.lowest_nonlinear_perf) ||
> @@ -177,7 +179,7 @@ static void amd_pstate_ut_check_perf(u32 index)
>  				__func__, cpu, nominal_perf, cpudata->perf.nominal_perf,
>  				lowest_nonlinear_perf, cpudata->perf.lowest_nonlinear_perf,
>  				lowest_perf, cpudata->perf.lowest_perf);
> -			goto skip_test;
> +			return;
>  		}
>  
>  		if (!((highest_perf >= nominal_perf) &&
> @@ -188,15 +190,11 @@ static void amd_pstate_ut_check_perf(u32 index)
>  			pr_err("%s cpu%d highest=%d >= nominal=%d > lowest_nonlinear=%d > lowest=%d > 0, the formula is incorrect!\n",
>  				__func__, cpu, highest_perf, nominal_perf,
>  				lowest_nonlinear_perf, lowest_perf);
> -			goto skip_test;
> +			return;
>  		}
> -		cpufreq_cpu_put(policy);
>  	}
>  
>  	amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_PASS;
> -	return;
> -skip_test:
> -	cpufreq_cpu_put(policy);
>  }
>  
>  /*
> @@ -207,10 +205,11 @@ static void amd_pstate_ut_check_perf(u32 index)
>  static void amd_pstate_ut_check_freq(u32 index)
>  {
>  	int cpu = 0;
> -	struct cpufreq_policy *policy = NULL;
>  	struct amd_cpudata *cpudata = NULL;
>  
>  	for_each_possible_cpu(cpu) {
> +		struct cpufreq_policy *policy __free(put_cpufreq_policy) = NULL;
> +
>  		policy = cpufreq_cpu_get(cpu);
>  		if (!policy)
>  			break;
> @@ -224,14 +223,14 @@ static void amd_pstate_ut_check_freq(u32 index)
>  			pr_err("%s cpu%d max=%d >= nominal=%d > lowest_nonlinear=%d > min=%d > 0, the formula is incorrect!\n",
>  				__func__, cpu, policy->cpuinfo.max_freq, cpudata->nominal_freq,
>  				cpudata->lowest_nonlinear_freq, policy->cpuinfo.min_freq);
> -			goto skip_test;
> +			return;
>  		}
>  
>  		if (cpudata->lowest_nonlinear_freq != policy->min) {
>  			amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_FAIL;
>  			pr_err("%s cpu%d cpudata_lowest_nonlinear_freq=%d policy_min=%d, they should be equal!\n",
>  				__func__, cpu, cpudata->lowest_nonlinear_freq, policy->min);
> -			goto skip_test;
> +			return;
>  		}
>  
>  		if (cpudata->boost_supported) {
> @@ -243,20 +242,16 @@ static void amd_pstate_ut_check_freq(u32 index)
>  				pr_err("%s cpu%d policy_max=%d should be equal cpu_max=%d or cpu_nominal=%d !\n",
>  					__func__, cpu, policy->max, policy->cpuinfo.max_freq,
>  					cpudata->nominal_freq);
> -				goto skip_test;
> +				return;
>  			}
>  		} else {
>  			amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_FAIL;
>  			pr_err("%s cpu%d must support boost!\n", __func__, cpu);
> -			goto skip_test;
> +			return;
>  		}
> -		cpufreq_cpu_put(policy);
>  	}
>  
>  	amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_PASS;
> -	return;
> -skip_test:
> -	cpufreq_cpu_put(policy);
>  }
>  
>  static int amd_pstate_set_mode(enum amd_pstate_mode mode)
> -- 
> 2.43.0
>