[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <09c02c68-1bd0-4627-8bed-b1276a223e3a@amd.com>
Date: Tue, 11 Feb 2025 11:28:13 +0530
From: Dhananjay Ugwekar <Dhananjay.Ugwekar@....com>
To: Mario Limonciello <superm1@...nel.org>,
"Gautham R . Shenoy" <gautham.shenoy@....com>,
Perry Yuan <perry.yuan@....com>
Cc: "open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)"
<linux-kernel@...r.kernel.org>,
"open list:CPU FREQUENCY SCALING FRAMEWORK" <linux-pm@...r.kernel.org>,
Mario Limonciello <mario.limonciello@....com>
Subject: Re: [PATCH 06/14] cpufreq/amd-pstate-ut: Use _free macro to free put
policy
On 2/7/2025 3:26 AM, Mario Limonciello wrote:
> From: Mario Limonciello <mario.limonciello@....com>
>
> Using a scoped cleanup macro simplifies cleanup code.
Looks good to me,
Reviewed-by: Dhananjay Ugwekar <dhananjay.ugwekar@....com>
Thanks,
Dhananjay
>
> Signed-off-by: Mario Limonciello <mario.limonciello@....com>
> ---
> 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 d9ab98c6f56b1..adaa62fb2b04e 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,10 +128,11 @@ 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;
>
> for_each_possible_cpu(cpu) {
> + struct cpufreq_policy *policy __free(put_cpufreq_policy) = NULL;
> +
> policy = cpufreq_cpu_get(cpu);
> if (!policy)
> break;
> @@ -141,7 +143,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;
> @@ -153,7 +155,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);
> @@ -166,7 +168,7 @@ static void amd_pstate_ut_check_perf(u32 index)
> !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 != READ_ONCE(cpudata->perf.nominal_perf)) ||
> (lowest_nonlinear_perf != READ_ONCE(cpudata->perf.lowest_nonlinear_perf)) ||
> @@ -176,7 +178,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) &&
> @@ -187,15 +189,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);
> }
>
> /*
> @@ -206,10 +204,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;
> @@ -223,14 +222,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) {
> @@ -242,20 +241,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)
Powered by blists - more mailing lists