linux-kernel - Re: [RESEND Patch 1/2] perf/x86: Remove perf_events_lapic_init() calling from x86_pmu

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20240702091523.GD11386@noisy.programming.kicks-ass.net>
Date: Tue, 2 Jul 2024 11:15:23 +0200
From: Peter Zijlstra <peterz@...radead.org>
To: Dapeng Mi <dapeng1.mi@...ux.intel.com>,
	Stephane Eranian <eranian@...gle.com>
Cc: Ingo Molnar <mingo@...hat.com>,
	Arnaldo Carvalho de Melo <acme@...nel.org>,
	Namhyung Kim <namhyung@...nel.org>, Ian Rogers <irogers@...gle.com>,
	Adrian Hunter <adrian.hunter@...el.com>,
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
	Kan Liang <kan.liang@...ux.intel.com>, linux-kernel@...r.kernel.org,
	Dapeng Mi <dapeng1.mi@...el.com>
Subject: Re: [RESEND Patch 1/2] perf/x86: Remove perf_events_lapic_init()
 calling from x86_pmu_enable()

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@...ux.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.