| From 3966c3feca3fd10b2935caa0b4a08c7dd59469e5 Mon Sep 17 00:00:00 2001 |
| From: "Lendacky, Thomas" <Thomas.Lendacky@amd.com> |
| Date: Tue, 2 Apr 2019 15:21:18 +0000 |
| Subject: x86/perf/amd: Remove need to check "running" bit in NMI handler |
| |
| From: Lendacky, Thomas <Thomas.Lendacky@amd.com> |
| |
| commit 3966c3feca3fd10b2935caa0b4a08c7dd59469e5 upstream. |
| |
| Spurious interrupt support was added to perf in the following commit, almost |
| a decade ago: |
| |
| 63e6be6d98e1 ("perf, x86: Catch spurious interrupts after disabling counters") |
| |
| The two previous patches (resolving the race condition when disabling a |
| PMC and NMI latency mitigation) allow for the removal of this older |
| spurious interrupt support. |
| |
| Currently in x86_pmu_stop(), the bit for the PMC in the active_mask bitmap |
| is cleared before disabling the PMC, which sets up a race condition. This |
| race condition was mitigated by introducing the running bitmap. That race |
| condition can be eliminated by first disabling the PMC, waiting for PMC |
| reset on overflow and then clearing the bit for the PMC in the active_mask |
| bitmap. The NMI handler will not re-enable a disabled counter. |
| |
| If x86_pmu_stop() is called from the perf NMI handler, the NMI latency |
| mitigation support will guard against any unhandled NMI messages. |
| |
| Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com> |
| Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> |
| Cc: <stable@vger.kernel.org> # 4.14.x- |
| Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> |
| Cc: Arnaldo Carvalho de Melo <acme@kernel.org> |
| Cc: Arnaldo Carvalho de Melo <acme@redhat.com> |
| Cc: Borislav Petkov <bp@alien8.de> |
| Cc: Jiri Olsa <jolsa@redhat.com> |
| Cc: Linus Torvalds <torvalds@linux-foundation.org> |
| Cc: Namhyung Kim <namhyung@kernel.org> |
| Cc: Peter Zijlstra <peterz@infradead.org> |
| Cc: Stephane Eranian <eranian@google.com> |
| Cc: Thomas Gleixner <tglx@linutronix.de> |
| Cc: Vince Weaver <vincent.weaver@maine.edu> |
| Link: https://lkml.kernel.org/r/Message-ID: |
| Signed-off-by: Ingo Molnar <mingo@kernel.org> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| arch/x86/events/amd/core.c | 21 +++++++++++++++++++-- |
| arch/x86/events/core.c | 13 +++---------- |
| 2 files changed, 22 insertions(+), 12 deletions(-) |
| |
| --- a/arch/x86/events/amd/core.c |
| +++ b/arch/x86/events/amd/core.c |
| @@ -4,8 +4,8 @@ |
| #include <linux/init.h> |
| #include <linux/slab.h> |
| #include <linux/delay.h> |
| -#include <linux/nmi.h> |
| #include <asm/apicdef.h> |
| +#include <asm/nmi.h> |
| |
| #include "../perf_event.h" |
| |
| @@ -491,6 +491,23 @@ static void amd_pmu_disable_all(void) |
| } |
| } |
| |
| +static void amd_pmu_disable_event(struct perf_event *event) |
| +{ |
| + x86_pmu_disable_event(event); |
| + |
| + /* |
| + * This can be called from NMI context (via x86_pmu_stop). The counter |
| + * may have overflowed, but either way, we'll never see it get reset |
| + * by the NMI if we're already in the NMI. And the NMI latency support |
| + * below will take care of any pending NMI that might have been |
| + * generated by the overflow. |
| + */ |
| + if (in_nmi()) |
| + return; |
| + |
| + amd_pmu_wait_on_overflow(event->hw.idx); |
| +} |
| + |
| /* |
| * Because of NMI latency, if multiple PMC counters are active or other sources |
| * of NMIs are received, the perf NMI handler can handle one or more overflowed |
| @@ -738,7 +755,7 @@ static __initconst const struct x86_pmu |
| .disable_all = amd_pmu_disable_all, |
| .enable_all = x86_pmu_enable_all, |
| .enable = x86_pmu_enable_event, |
| - .disable = x86_pmu_disable_event, |
| + .disable = amd_pmu_disable_event, |
| .hw_config = amd_pmu_hw_config, |
| .schedule_events = x86_schedule_events, |
| .eventsel = MSR_K7_EVNTSEL0, |
| --- a/arch/x86/events/core.c |
| +++ b/arch/x86/events/core.c |
| @@ -1349,8 +1349,9 @@ void x86_pmu_stop(struct perf_event *eve |
| struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events); |
| struct hw_perf_event *hwc = &event->hw; |
| |
| - if (__test_and_clear_bit(hwc->idx, cpuc->active_mask)) { |
| + if (test_bit(hwc->idx, cpuc->active_mask)) { |
| x86_pmu.disable(event); |
| + __clear_bit(hwc->idx, cpuc->active_mask); |
| cpuc->events[hwc->idx] = NULL; |
| WARN_ON_ONCE(hwc->state & PERF_HES_STOPPED); |
| hwc->state |= PERF_HES_STOPPED; |
| @@ -1447,16 +1448,8 @@ int x86_pmu_handle_irq(struct pt_regs *r |
| apic_write(APIC_LVTPC, APIC_DM_NMI); |
| |
| for (idx = 0; idx < x86_pmu.num_counters; idx++) { |
| - if (!test_bit(idx, cpuc->active_mask)) { |
| - /* |
| - * Though we deactivated the counter some cpus |
| - * might still deliver spurious interrupts still |
| - * in flight. Catch them: |
| - */ |
| - if (__test_and_clear_bit(idx, cpuc->running)) |
| - handled++; |
| + if (!test_bit(idx, cpuc->active_mask)) |
| continue; |
| - } |
| |
| event = cpuc->events[idx]; |
| |