Re: [RESEND Patch 1/2] perf/x86: Remove perf_events_lapic_init() calling from x86_pmu_enable()

[Peter Zijlstra <peterz@infradead.org>]

On Wed, Jul 03, 2024 at 06:57:02AM +0800, Dapeng Mi wrote:
> perf_events_lapic_init() helper is called to configure PMI to NMI vector
> and clear MASK bit simultaneously by writing APIC_LVTPC MSR. It's called
> firstly to initialize APIC_LVTPC MSR by init_hw_perf_events(), and the
> PMI handler would always to clear the MASK bit in APIC_LVTPC MSR by
> writing APIC_LVTPC MSR directly.
> 
> So it becomes unnecessary to call perf_events_lapic_init() again in
> x86_pmu_enable(), and worse x86_pmu_enable() could be called very
> frequently in some scenarios with very high context-switches. This would
> cause performance overhead which can't be ignored especially in KVM guest
> environment since frequent APIC_LVTPC writing would cause huge number
> of VM-Exits.
> 
> For example, in guest environment Geekbench score (running multiplxing
> perf-stat command in background) increases 1% and perf-sched benchmark
> increases 7% after removing perf_events_lapic_init() calling from
> x86_pmu_enable().
> 
> Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
> ---
>  arch/x86/events/core.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
> index 5b0dd07b1ef1..580923443813 100644
> --- a/arch/x86/events/core.c
> +++ b/arch/x86/events/core.c
> @@ -1347,7 +1347,6 @@ static void x86_pmu_enable(struct pmu *pmu)
>  			x86_pmu_start(event, PERF_EF_RELOAD);
>  		}
>  		cpuc->n_added = 0;
> -		perf_events_lapic_init();
>  	}

Stephane, I don't suppose you remember why you put that there? Afaict
the above reasoning is correct and this one is entirely superfluous, but
this hardware is 'funny' at the best of times and maybe I'm overlooking
something.