[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <fbc90ef0-1418-4c7b-8f3e-7dd1a0d7499e@amd.com>
Date: Fri, 13 Oct 2023 13:14:04 +0530
From: Sandipan Das <sandipan.das@....com>
To: Dan Carpenter <dan.carpenter@...aro.org>
Cc: Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>,
Namhyung Kim <namhyung@...nel.org>,
Ian Rogers <irogers@...gle.com>,
Adrian Hunter <adrian.hunter@...el.com>,
Thomas Gleixner <tglx@...utronix.de>,
Borislav Petkov <bp@...en8.de>,
Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
"H. Peter Anvin" <hpa@...or.com>, linux-perf-users@...r.kernel.org,
linux-kernel@...r.kernel.org, kernel-janitors@...r.kernel.org
Subject: Re: [PATCH] perf/x86/amd/uncore: fix error codes in amd_uncore_init()
On 10/13/2023 12:48 PM, Dan Carpenter wrote:
> Some of the error paths in this function return don't initialize the
> error code. Return -ENODEV.
>
> Fixes: d6389d3ccc13 ("perf/x86/amd/uncore: Refactor uncore management")
> Signed-off-by: Dan Carpenter <dan.carpenter@...aro.org>
> ---
> arch/x86/events/amd/uncore.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/events/amd/uncore.c b/arch/x86/events/amd/uncore.c
> index 9b444ce24108..a389828f378c 100644
> --- a/arch/x86/events/amd/uncore.c
> +++ b/arch/x86/events/amd/uncore.c
> @@ -1009,7 +1009,8 @@ static struct amd_uncore uncores[UNCORE_TYPE_MAX] = {
> static int __init amd_uncore_init(void)
> {
> struct amd_uncore *uncore;
> - int ret, i;
> + int ret = -ENODEV;
> + int i;
>
> if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD &&
> boot_cpu_data.x86_vendor != X86_VENDOR_HYGON)
Thanks for catching this. I see that 'ret' remains uninitialized for cases
where the hotplug callback registration fails and was thinking if the
following is a better fix for this as the reason might not be ENODEV.
diff --git a/arch/x86/events/amd/uncore.c b/arch/x86/events/amd/uncore.c
index 91f01d6c8f7d..7d768dd01522 100644
--- a/arch/x86/events/amd/uncore.c
+++ b/arch/x86/events/amd/uncore.c
@@ -1039,20 +1039,25 @@ static int __init amd_uncore_init(void)
/*
* Install callbacks. Core will call them for each online cpu.
*/
- if (cpuhp_setup_state(CPUHP_PERF_X86_AMD_UNCORE_PREP,
- "perf/x86/amd/uncore:prepare",
- NULL, amd_uncore_cpu_dead))
+ ret = cpuhp_setup_state(CPUHP_PERF_X86_AMD_UNCORE_PREP,
+ "perf/x86/amd/uncore:prepare",
+ NULL, amd_uncore_cpu_dead);
+ if (ret)
goto fail;
- if (cpuhp_setup_state(CPUHP_AP_PERF_X86_AMD_UNCORE_STARTING,
- "perf/x86/amd/uncore:starting",
- amd_uncore_cpu_starting, NULL))
+ ret = cpuhp_setup_state(CPUHP_AP_PERF_X86_AMD_UNCORE_STARTING,
+ "perf/x86/amd/uncore:starting",
+ amd_uncore_cpu_starting, NULL);
+ if (ret)
goto fail_prep;
- if (cpuhp_setup_state(CPUHP_AP_PERF_X86_AMD_UNCORE_ONLINE,
- "perf/x86/amd/uncore:online",
- amd_uncore_cpu_online,
- amd_uncore_cpu_down_prepare))
+
+ ret = cpuhp_setup_state(CPUHP_AP_PERF_X86_AMD_UNCORE_ONLINE,
+ "perf/x86/amd/uncore:online",
+ amd_uncore_cpu_online,
+ amd_uncore_cpu_down_prepare);
+ if (ret)
goto fail_start;
+
return 0;
fail_start:
Powered by blists - more mailing lists