Re: [PATCH 6/8] perf/x86/intel: Validate return value of intel_pmu_init_hybrid()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Unnecessary error handling and dead cleanup code for early boot memory allocation failure.
--

commit e6fc94eb7f63fa466e8e1aeaed9a0d18231a88dc
Author: Dapeng Mi <dapeng1.mi@linux.intel.com>

perf/x86/intel: Validate return value of intel_pmu_init_hybrid()

This commit validates the return value of intel_pmu_init_hybrid() to prevent
attempts to access the uninitialized x86_pmu.hybrid_pmu[] array due to
allocation failure. It adds error paths that destroy the task_ctx_cache when
an error occurs.

> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> index ea3ab3050a3b1..efd9caa3502c5 100644
> --- a/arch/x86/events/intel/core.c
> +++ b/arch/x86/events/intel/core.c
> @@ -8539,7 +8540,9 @@ __init int intel_pmu_init(void)
>  		 *
>  		 * Initialize the common PerfMon capabilities here.
>  		 */
> -		intel_pmu_init_hybrid(hybrid_big_small);
> +		ret = intel_pmu_init_hybrid(hybrid_big_small);
> +		if (ret < 0)
> +			goto err;

[Severity: Low]
Is this error handling necessary for an early boot function?

Since intel_pmu_init() in arch/x86/events/intel/core.c is marked with __init,
it executes only during early boot. At this stage, if a physical memory
allocation fails, the system typically cannot boot anyway. Handling memory
allocation failures and returning -ENOMEM here is likely dead code.

[ ... ]

> @@ -8885,6 +8900,10 @@ __init int intel_pmu_init(void)
>  	intel_aux_output_init();
>  
>  	return 0;
> +
> +err:
> +	kmem_cache_destroy(x86_get_pmu(smp_processor_id())->task_ctx_cache);

[Severity: Low]
Will this cleanup code in intel_pmu_init() actually free anything?

If the allocation fails during intel_pmu_init_hybrid() and we jump to the err
label, task_ctx_cache has not been allocated yet. It is only allocated later
in the function via intel_pmu_lbr_init() or intel_pmu_arch_lbr_init().

This guarantees task_ctx_cache is NULL when this error path is taken, meaning
this call to kmem_cache_destroy() acts as a no-op.

> +	return ret;
>  }
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605011136.2043393-1-dapeng1.mi@linux.intel.com?part=6