[PATCH 1/2] x86, perf: Remove warning for zero PEBS status

[PATCH 1/2] x86, perf: Remove warning for zero PEBS status

[Andi Kleen]

From: Andi Kleen <ak@linux.intel.com>

The recent change 75f80859b1 (... Robustify PEBS buffer drain)
causes lots of warnings on different CPUs before Skylake
when running PEBS intensive workloads.

They can have a zero status field in the PEBS record when
PEBS is racing with clearing of GLOBAl_STATUS.

This also can cause hangs (it seems there are still
problems with printk in NMI)

Disable the warning, but still ignore the record.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 arch/x86/kernel/cpu/perf_event_intel_ds.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/arch/x86/kernel/cpu/perf_event_intel_ds.c b/arch/x86/kernel/cpu/perf_event_intel_ds.c
index 7f11784..da294de 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_ds.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_ds.c
@@ -1237,10 +1237,7 @@ static void intel_pmu_drain_pebs_nhm(struct pt_regs *iregs)
 
 		bit = find_first_bit((unsigned long *)&pebs_status,
 					x86_pmu.max_pebs_events);
-		if (WARN(bit >= x86_pmu.max_pebs_events,
-			 "PEBS record without PEBS event! status=%Lx pebs_enabled=%Lx active_mask=%Lx",
-			 (unsigned long long)p->status, (unsigned long long)cpuc->pebs_enabled,
-			 *(unsigned long long *)cpuc->active_mask))
+		if (bit >= x86_pmu.max_pebs_events)
 			continue;
 
 		/*
-- 
2.4.3

[PATCH 2/2] x86, perf: Allow zero PEBS status with only single active event

[Andi Kleen]

From: Andi Kleen <ak@linux.intel.com>

Normally we drop PEBS events with a zero status field. But when
there is only a single PEBS event active we can assume the
PEBS record is for that event. The PEBS buffer is always flushed
when PEBS events are disabled, so there is no risk of mishandling
state PEBS records this way.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 arch/x86/kernel/cpu/perf_event_intel_ds.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/x86/kernel/cpu/perf_event_intel_ds.c b/arch/x86/kernel/cpu/perf_event_intel_ds.c
index da294de..1259065 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_ds.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_ds.c
@@ -1235,6 +1235,18 @@ static void intel_pmu_drain_pebs_nhm(struct pt_regs *iregs)
 		pebs_status = p->status & cpuc->pebs_enabled;
 		pebs_status &= (1ULL << x86_pmu.max_pebs_events) - 1;
 
+		/*
+		 * On some CPUs the PEBS status can be zero when PEBS is
+		 * racing with clearing of GLOBAL_STATUS.
+		 *
+		 * Normally we would drop that record, but in the
+		 * case when there is only a single active PEBS event
+		 * we can assume it's for that event.
+		 */
+		if (!pebs_status && cpuc->pebs_enabled &&
+			!(cpuc->pebs_enabled & (cpuc->pebs_enabled-1)))
+			pebs_status = cpuc->pebs_enabled;
+
 		bit = find_first_bit((unsigned long *)&pebs_status,
 					x86_pmu.max_pebs_events);
 		if (bit >= x86_pmu.max_pebs_events)
-- 
2.4.3

Re: [PATCH 1/2] x86, perf: Remove warning for zero PEBS status

[Peter Zijlstra]

On Thu, Dec 03, 2015 at 01:22:19PM -0800, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> The recent change 75f80859b1 (... Robustify PEBS buffer drain)
> causes lots of warnings on different CPUs before Skylake
> when running PEBS intensive workloads.
> 
> They can have a zero status field in the PEBS record when
> PEBS is racing with clearing of GLOBAl_STATUS.

Ah, so _that_ is the explanation! Please put that in a comment near that
code. This has long puzzled me.

Thanks for digging into that.

Re: [PATCH 2/2] x86, perf: Allow zero PEBS status with only single active event

[Peter Zijlstra]

On Thu, Dec 03, 2015 at 01:22:20PM -0800, Andi Kleen wrote:
> +		/*
> +		 * On some CPUs the PEBS status can be zero when PEBS is
> +		 * racing with clearing of GLOBAL_STATUS.
> +		 *
> +		 * Normally we would drop that record, but in the
> +		 * case when there is only a single active PEBS event
> +		 * we can assume it's for that event.
> +		 */
> +		if (!pebs_status && cpuc->pebs_enabled &&
> +			!(cpuc->pebs_enabled & (cpuc->pebs_enabled-1)))
> +			pebs_status = cpuc->pebs_enabled;
> +
>  		bit = find_first_bit((unsigned long *)&pebs_status,
>  					x86_pmu.max_pebs_events);
>  		if (bit >= x86_pmu.max_pebs_events)


Ah! I think this is the comment I just asked for.

[tip:perf/core] perf/x86: Remove warning for zero PEBS status

[tip-bot for Andi Kleen]

Commit-ID:  957ea1fdbcdb909e1540f06f06f1a9ce6e696efa
Gitweb:     http://git.kernel.org/tip/957ea1fdbcdb909e1540f06f06f1a9ce6e696efa
Author:     Andi Kleen <ak@linux.intel.com>
AuthorDate: Thu, 3 Dec 2015 13:22:19 -0800
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 6 Jan 2016 11:15:30 +0100

perf/x86: Remove warning for zero PEBS status

The recent commit:

  75f80859b130 ("perf/x86/intel/pebs: Robustify PEBS buffer drain")

causes lots of warnings on different CPUs before Skylake
when running PEBS intensive workloads.

They can have a zero status field in the PEBS record when
PEBS is racing with clearing of GLOBAl_STATUS.

This also can cause hangs (it seems there are still
problems with printk in NMI).

Disable the warning, but still ignore the record.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.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: http://lkml.kernel.org/r/1449177740-5422-1-git-send-email-andi@firstfloor.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/cpu/perf_event_intel_ds.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/arch/x86/kernel/cpu/perf_event_intel_ds.c b/arch/x86/kernel/cpu/perf_event_intel_ds.c
index 5db1c77..0e3a9c7 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_ds.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_ds.c
@@ -1232,10 +1232,7 @@ static void intel_pmu_drain_pebs_nhm(struct pt_regs *iregs)
 
 		bit = find_first_bit((unsigned long *)&pebs_status,
 					x86_pmu.max_pebs_events);
-		if (WARN(bit >= x86_pmu.max_pebs_events,
-			 "PEBS record without PEBS event! status=%Lx pebs_enabled=%Lx active_mask=%Lx",
-			 (unsigned long long)p->status, (unsigned long long)cpuc->pebs_enabled,
-			 *(unsigned long long *)cpuc->active_mask))
+		if (bit >= x86_pmu.max_pebs_events)
 			continue;
 
 		/*

[tip:perf/core] perf/x86: Allow zero PEBS status with only single active event

[tip-bot for Andi Kleen]

Commit-ID:  01330d7288e0050c5aaabc558059ff91589e67cd
Gitweb:     http://git.kernel.org/tip/01330d7288e0050c5aaabc558059ff91589e67cd
Author:     Andi Kleen <ak@linux.intel.com>
AuthorDate: Thu, 3 Dec 2015 13:22:20 -0800
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 6 Jan 2016 11:15:31 +0100

perf/x86: Allow zero PEBS status with only single active event

Normally we drop PEBS events with a zero status field. But when
there is only a single PEBS event active we can assume the
PEBS record is for that event. The PEBS buffer is always flushed
when PEBS events are disabled, so there is no risk of mishandling
state PEBS records this way.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.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: http://lkml.kernel.org/r/1449177740-5422-2-git-send-email-andi@firstfloor.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/cpu/perf_event_intel_ds.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/x86/kernel/cpu/perf_event_intel_ds.c b/arch/x86/kernel/cpu/perf_event_intel_ds.c
index 0e3a9c7..cd1993e 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_ds.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_ds.c
@@ -1230,6 +1230,18 @@ static void intel_pmu_drain_pebs_nhm(struct pt_regs *iregs)
 		pebs_status = p->status & cpuc->pebs_enabled;
 		pebs_status &= (1ULL << x86_pmu.max_pebs_events) - 1;
 
+		/*
+		 * On some CPUs the PEBS status can be zero when PEBS is
+		 * racing with clearing of GLOBAL_STATUS.
+		 *
+		 * Normally we would drop that record, but in the
+		 * case when there is only a single active PEBS event
+		 * we can assume it's for that event.
+		 */
+		if (!pebs_status && cpuc->pebs_enabled &&
+			!(cpuc->pebs_enabled & (cpuc->pebs_enabled-1)))
+			pebs_status = cpuc->pebs_enabled;
+
 		bit = find_first_bit((unsigned long *)&pebs_status,
 					x86_pmu.max_pebs_events);
 		if (bit >= x86_pmu.max_pebs_events)