Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
Message-ID: <6955339f-9bc5-4a84-acdc-205b90d4127c@amd.com>
Date:   Fri, 13 Oct 2023 14:37:04 +0530
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] perf/x86/amd/uncore: fix error codes in amd_uncore_init()
Content-Language: en-US
To:     Ingo Molnar <mingo@kernel.org>
Cc:     Dan Carpenter <dan.carpenter@linaro.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Arnaldo Carvalho de Melo <acme@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Jiri Olsa <jolsa@kernel.org>,
        Namhyung Kim <namhyung@kernel.org>,
        Ian Rogers <irogers@google.com>,
        Adrian Hunter <adrian.hunter@intel.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Borislav Petkov <bp@alien8.de>,
        Dave Hansen <dave.hansen@linux.intel.com>, x86@kernel.org,
        "H. Peter Anvin" <hpa@zytor.com>, linux-perf-users@vger.kernel.org,
        linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org
References: <cec62eba-c4b8-4cb7-9671-58894dd4b974@moroto.mountain>
 <fbc90ef0-1418-4c7b-8f3e-7dd1a0d7499e@amd.com> <ZSkH64SpvOTOJSpE@gmail.com>
From:   Sandipan Das <sandipan.das@amd.com>
In-Reply-To: <ZSkH64SpvOTOJSpE@gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?blBTaWVoT01acGpNOVN5R056Mzg4VW5LUkYyK0dyb2pvZUcxaTcxMUptajlj?=
 =?utf-8?B?MVZ2TmxUKzdnbE5XZDN2L3FiZlJHNUxPU3lOcmVEWWVENUpKWmdUNE9PcHUw?=
 =?utf-8?B?S1hWLzMrKzVENjI0dFk0T1JHNGlUVTU2dDZqQmVVZDR4N2VOQzdhMERFczFZ?=
 =?utf-8?B?dTY0cXBFeXpEaHB4b3ZXREs5RlhtZmxoazREQnJ2YlU4bkJWZUEySnA1dU9q?=
 =?utf-8?B?eEJaS0p0bG13UFRjbUplWEVZbnV0SnVVWThLRFZma2htOVExQXdOaU1QZ1Rm?=
 =?utf-8?B?TmxLRWh5QUxVMDNpblRsVE5NcURhTnBON0VRZGs5eSt6MzBpMDRnWTMzb0lI?=
 =?utf-8?B?WjE0K1ViUkxWSkpsSW1OU0JWNndFRXBpUERaM1BXUGY3UmFwQVdYOCtMb2kw?=
 =?utf-8?B?eExxNjRyRGhWWjhuR3pTNkRTanl2YU9NOElwOTdHMU5nLzg5alNPU1JqcFM3?=
 =?utf-8?B?NHhRR3ZWVWlRcGlZb1JlQ1dnbzlaRGlSUGpMSXM4UTV6WC9RNFVkd0lUekRI?=
 =?utf-8?B?ZDIzVVRUUWtobXp1MjRMTEptWStjUG02SzZxbG1DczdDQ0hUUUdCUDZIRDhI?=
 =?utf-8?B?eXk0aTNaSjk1VzZmYjhiOEI0RHZxVFhDcnZ5YzZGREN5eStQQkFyUFFiZmpO?=
 =?utf-8?B?cU1FQ1hYVW5lZ2IrOCtNaGhkQUdEcmlZdm0wcWtWbDd0azNZcFhKZTRqOEZN?=
 =?utf-8?B?TEdnV01rUXQ1S3dhOSt2d1Zzc3NacEFzUU8vbjd5UjhEelJ4cy9sUWttYlg4?=
 =?utf-8?B?M3g4bjBpaEk2R0t3RUkxNXNzcVVvTEU0Ly9VSGZmZWdXVTVuRDY2Y2lGMGht?=
 =?utf-8?B?ZjVFeUp0RVdtemFsNjZ6QmpENHRHODNnMzlVRkxGSlJOU3l3TkJCcEhKM3N1?=
 =?utf-8?B?N1BUYVkxVzFpR1dDaWNvb0Ftbnpib25rS0ZxM0NSRjM3S2VXWlR3M2EvTVQ4?=
 =?utf-8?B?MHFlMGdkcmFIdEo0Z1F1Y3NSZEJRMkVrNnM5WFpjcWhMK28vUzZFdEtQMlNM?=
 =?utf-8?B?SExHTldKWWYvTGpGaHF0bVhtOGxHZ0FwUGFKWkQrUnNiQ3kvVk91cXA2eGs5?=
 =?utf-8?B?clhLYmFpOW5WYmtHcGZEWDdJM1oxN2k2VlpwZWRyQUYya01PSzFEelRwVzNu?=
 =?utf-8?B?WFhFMllaZ3lQRktiM1VZZXpWZDlRMFBQN3FNclBtclhvUW5IWHpiRVZBTDFR?=
 =?utf-8?B?dzV0bWR5V1dESUM2VW9CY043TjBnQlphWE5pc2xLUDZMcW1GbG96K2xuQ3Mv?=
 =?utf-8?B?ekpkZ2w1Q0RsVkx3c0dwVXE0bTZaZWo3aGNIQ3JGZEFzMVIwYnBCOUtPLzJK?=
 =?utf-8?B?VG1WTEY5SEx4eGZiWEJrRWhTSEVkTG82eGQvQ0Jac2luZGJEaHZnZU1JbWZW?=
 =?utf-8?B?ZFIxczZtblk5Mk93ZWU5WnRPREhFRlB1Z0NoSllHdURwUHpOSExaVWJiWU85?=
 =?utf-8?B?S0xqckxxMnJGSHQ0bzMrQ1J6MGJBWDQ4QVRsa2J6cUhwRC9CWEt2SENGZ1Js?=
 =?utf-8?B?VFgwVlF6bzRkR05kMHV5VEZCQks3blJON0tnMW01S2E4dmtLa2RpaDl6ZzZ0?=
 =?utf-8?B?cXpEcU91SWFtKzJtYld4amYxMjBndGhqcjRmb1czakJFb1kwUHRYSHJ6aXY5?=
 =?utf-8?B?TkhPekY4cExCb3dZWmJpL0p6SllwKytUbVh4TW9EMFJuOUdXeU5Od3JScGtJ?=
 =?utf-8?B?Z2lPblVub3B6OEk2SDNNam9OMkFHbFZLcnFYc3dXM2dwUHpPZWJ5N3poc1Mr?=
 =?utf-8?B?dWJkcDNyL2pjUi9rVXFPbThvOHNBcGMwcmFqK3Y4RWgrU0NvckpqSnhWbFJX?=
 =?utf-8?B?eDQzbVdTSEJIMS9IU3dNVFZqVUxVeDZNWU1HellXTmpwQXlZMEZpN0JTekJE?=
 =?utf-8?B?VWRPMFdZRmtaQTVTRzJ2VzNaeFlCVVpNSm9COXcydG1DajhhV0Y4RXBCdFk2?=
 =?utf-8?B?YUpIREhSV3I3QXRNaUtycGV2VCtKMmNLZ2pMZzk3SU9mUVdoeHZIOVVCVStS?=
 =?utf-8?B?aFVDdVNHV0VVTnhVWFRtdGtsMHgxTWo3b1BGbmNYeHR1QXprM21nN1lReFc3?=
 =?utf-8?B?Mnp3L2hLVmhOUmo0S24rTWZpRHhneG0vR2N0VHZPN0lIbkJFckV3cTN5MGVj?=
 =?utf-8?Q?IFFzY3TB2T1XbMveYTuGarTCC?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 8d6b3a05-0f50-4928-dfd8-08dbcbcbc205
X-MS-Exchange-CrossTenant-AuthSource: PH7PR12MB5712.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Oct 2023 09:07:00.0357
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: lJAh8oQ1ANpP88HS29TUJ/9EwlI3yD0uRqVLBcuFGmco6Qk6NzSKc0vz6J9L9m25TIt7JjWNHGwrHk5fLYxOjA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB4500
X-Spam-Status: No, score=-1.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,
        RCVD_IN_DNSWL_BLOCKED,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE
        autolearn=no autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Fri, 13 Oct 2023 02:07:14 -0700 (PDT)

On 10/13/2023 2:33 PM, Ingo Molnar wrote:
> 
> * Sandipan Das <sandipan.das@amd.com> wrote:
> 
>> 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@linaro.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.
> 
> Yeah, passing through the real error codes is usually better.
> 
> Here's it's probably a bit academic, as I don't think we are even using the 
> init return code in the init sequence iterator, see how the return code by 
> do_one_initcall() gets ignored by do_initcall_level() & do_pre_smp_initcalls() ...
> 
> Nevertheless, mind submitting this as a separate patch?
> 

Sure. Will do.