linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] perf/x86: fix INTEL_FLAGS_EVENT_CONSTRAINT* masking
@ 2019-05-09 21:45 Stephane Eranian
  2019-05-10  6:07 ` [tip:perf/urgent] perf/x86/intel: Fix " tip-bot for Stephane Eranian
  0 siblings, 1 reply; 4+ messages in thread
From: Stephane Eranian @ 2019-05-09 21:45 UTC (permalink / raw)
  To: linux-kernel; +Cc: peterz, ak, kan.liang, mingo, jolsa, vincent.weaver

On Intel Westmere, a cmdline as follows:
$ perf record -e cpu/event=0xc4,umask=0x2,name=br_inst_retired.near_call/p ....

Was failing. Yet the event+ umask support PEBS.

It turns out this is due to a bug in the the PEBS event constraint table for
westmere. All forms of BR_INST_RETIRED.* support PEBS. Therefore the constraint
mask should ignore the umask. The name of the macro INTEL_FLAGS_EVENT_CONSTRAINT()
hint that this is the case but it was not. That macros was checking both the
event code and event umask. Therefore, it was only matching on 0x00c4.
There are code+umask macros, they all have *UEVENT*.

This bug fixes the issue by checking only the event code in the mask.
Both single and range version are modified.

Signed-off-by: Stephane Eranian <eranian@google.com>
---
 arch/x86/events/perf_event.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
index 07fc84bb85c1..a6ac2f4f76fc 100644
--- a/arch/x86/events/perf_event.h
+++ b/arch/x86/events/perf_event.h
@@ -394,10 +394,10 @@ struct cpu_hw_events {
 
 /* Event constraint, but match on all event flags too. */
 #define INTEL_FLAGS_EVENT_CONSTRAINT(c, n) \
-	EVENT_CONSTRAINT(c, n, INTEL_ARCH_EVENT_MASK|X86_ALL_EVENT_FLAGS)
+	EVENT_CONSTRAINT(c, n, ARCH_PERFMON_EVENTSEL_EVENT|X86_ALL_EVENT_FLAGS)
 
 #define INTEL_FLAGS_EVENT_CONSTRAINT_RANGE(c, e, n)			\
-	EVENT_CONSTRAINT_RANGE(c, e, n, INTEL_ARCH_EVENT_MASK|X86_ALL_EVENT_FLAGS)
+	EVENT_CONSTRAINT_RANGE(c, e, n, ARCH_PERFMON_EVENTSEL_EVENT|X86_ALL_EVENT_FLAGS)
 
 /* Check only flags, but allow all event/umask */
 #define INTEL_ALL_EVENT_CONSTRAINT(code, n)	\
-- 
2.21.0.1020.gf2820cf01a-goog


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* [tip:perf/urgent] perf/x86/intel: Fix INTEL_FLAGS_EVENT_CONSTRAINT* masking
  2019-05-09 21:45 [PATCH] perf/x86: fix INTEL_FLAGS_EVENT_CONSTRAINT* masking Stephane Eranian
@ 2019-05-10  6:07 ` tip-bot for Stephane Eranian
  2019-05-18 21:16   ` Ingo Molnar
  0 siblings, 1 reply; 4+ messages in thread
From: tip-bot for Stephane Eranian @ 2019-05-10  6:07 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: vincent.weaver, hpa, acme, alexander.shishkin, mingo, jolsa,
	linux-kernel, torvalds, eranian, tglx, peterz

Commit-ID:  6b89d4c1ae8596a8c9240f169ef108704de373f2
Gitweb:     https://git.kernel.org/tip/6b89d4c1ae8596a8c9240f169ef108704de373f2
Author:     Stephane Eranian <eranian@google.com>
AuthorDate: Thu, 9 May 2019 14:45:56 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 10 May 2019 08:04:17 +0200

perf/x86/intel: Fix INTEL_FLAGS_EVENT_CONSTRAINT* masking

On Intel Westmere, a cmdline as follows:

  $ perf record -e cpu/event=0xc4,umask=0x2,name=br_inst_retired.near_call/p ....

was failing. Yet the event+ umask support PEBS.

It turns out this is due to a bug in the the PEBS event constraint table for
westmere. All forms of BR_INST_RETIRED.* support PEBS. Therefore the constraint
mask should ignore the umask. The name of the macro INTEL_FLAGS_EVENT_CONSTRAINT()
hint that this is the case but it was not. That macros was checking both the
event code and event umask. Therefore, it was only matching on 0x00c4.
There are code+umask macros, they all have *UEVENT*.

This bug fixes the issue by checking only the event code in the mask.
Both single and range version are modified.

Signed-off-by: Stephane Eranian <eranian@google.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
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: Thomas Gleixner <tglx@linutronix.de>
Cc: Vince Weaver <vincent.weaver@maine.edu>
Cc: kan.liang@intel.com
Link: http://lkml.kernel.org/r/20190509214556.123493-1-eranian@google.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/events/perf_event.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
index 07fc84bb85c1..a6ac2f4f76fc 100644
--- a/arch/x86/events/perf_event.h
+++ b/arch/x86/events/perf_event.h
@@ -394,10 +394,10 @@ struct cpu_hw_events {
 
 /* Event constraint, but match on all event flags too. */
 #define INTEL_FLAGS_EVENT_CONSTRAINT(c, n) \
-	EVENT_CONSTRAINT(c, n, INTEL_ARCH_EVENT_MASK|X86_ALL_EVENT_FLAGS)
+	EVENT_CONSTRAINT(c, n, ARCH_PERFMON_EVENTSEL_EVENT|X86_ALL_EVENT_FLAGS)
 
 #define INTEL_FLAGS_EVENT_CONSTRAINT_RANGE(c, e, n)			\
-	EVENT_CONSTRAINT_RANGE(c, e, n, INTEL_ARCH_EVENT_MASK|X86_ALL_EVENT_FLAGS)
+	EVENT_CONSTRAINT_RANGE(c, e, n, ARCH_PERFMON_EVENTSEL_EVENT|X86_ALL_EVENT_FLAGS)
 
 /* Check only flags, but allow all event/umask */
 #define INTEL_ALL_EVENT_CONSTRAINT(code, n)	\

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [tip:perf/urgent] perf/x86/intel: Fix INTEL_FLAGS_EVENT_CONSTRAINT* masking
  2019-05-10  6:07 ` [tip:perf/urgent] perf/x86/intel: Fix " tip-bot for Stephane Eranian
@ 2019-05-18 21:16   ` Ingo Molnar
  2019-05-19  1:41     ` Stephane Eranian
  0 siblings, 1 reply; 4+ messages in thread
From: Ingo Molnar @ 2019-05-18 21:16 UTC (permalink / raw)
  To: hpa, vincent.weaver, acme, alexander.shishkin, jolsa,
	linux-kernel, torvalds, eranian, tglx, peterz
  Cc: linux-tip-commits


* tip-bot for Stephane Eranian <tipbot@zytor.com> wrote:

> Commit-ID:  6b89d4c1ae8596a8c9240f169ef108704de373f2
> Gitweb:     https://git.kernel.org/tip/6b89d4c1ae8596a8c9240f169ef108704de373f2
> Author:     Stephane Eranian <eranian@google.com>
> AuthorDate: Thu, 9 May 2019 14:45:56 -0700
> Committer:  Ingo Molnar <mingo@kernel.org>
> CommitDate: Fri, 10 May 2019 08:04:17 +0200
> 
> perf/x86/intel: Fix INTEL_FLAGS_EVENT_CONSTRAINT* masking
> 
> On Intel Westmere, a cmdline as follows:
> 
>   $ perf record -e cpu/event=0xc4,umask=0x2,name=br_inst_retired.near_call/p ....
> 
> was failing. Yet the event+ umask support PEBS.
> 
> It turns out this is due to a bug in the the PEBS event constraint table for
> westmere. All forms of BR_INST_RETIRED.* support PEBS. Therefore the constraint
> mask should ignore the umask. The name of the macro INTEL_FLAGS_EVENT_CONSTRAINT()
> hint that this is the case but it was not. That macros was checking both the
> event code and event umask. Therefore, it was only matching on 0x00c4.
> There are code+umask macros, they all have *UEVENT*.
> 
> This bug fixes the issue by checking only the event code in the mask.
> Both single and range version are modified.
> 
> Signed-off-by: Stephane Eranian <eranian@google.com>
> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> 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: Thomas Gleixner <tglx@linutronix.de>
> Cc: Vince Weaver <vincent.weaver@maine.edu>
> Cc: kan.liang@intel.com
> Link: http://lkml.kernel.org/r/20190509214556.123493-1-eranian@google.com
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> ---
>  arch/x86/events/perf_event.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
> index 07fc84bb85c1..a6ac2f4f76fc 100644
> --- a/arch/x86/events/perf_event.h
> +++ b/arch/x86/events/perf_event.h
> @@ -394,10 +394,10 @@ struct cpu_hw_events {
>  
>  /* Event constraint, but match on all event flags too. */
>  #define INTEL_FLAGS_EVENT_CONSTRAINT(c, n) \
> -	EVENT_CONSTRAINT(c, n, INTEL_ARCH_EVENT_MASK|X86_ALL_EVENT_FLAGS)
> +	EVENT_CONSTRAINT(c, n, ARCH_PERFMON_EVENTSEL_EVENT|X86_ALL_EVENT_FLAGS)
>  
>  #define INTEL_FLAGS_EVENT_CONSTRAINT_RANGE(c, e, n)			\
> -	EVENT_CONSTRAINT_RANGE(c, e, n, INTEL_ARCH_EVENT_MASK|X86_ALL_EVENT_FLAGS)
> +	EVENT_CONSTRAINT_RANGE(c, e, n, ARCH_PERFMON_EVENTSEL_EVENT|X86_ALL_EVENT_FLAGS)

This commit broke one of my testboxes - and unfortunately I noticed this 
too late and the commit is now upstream.

The breakage is that 'perf top' stops working altogether, it errors out 
in the event creation:

 $ perf top --stdio
 Error:
 The sys_perf_event_open() syscall returned with 22 (Invalid argument) for event (cycles).

I bisected it back to this commit:

 6b89d4c1ae8596a8c9240f169ef108704de373f2 is the first bad commit
 commit 6b89d4c1ae8596a8c9240f169ef108704de373f2
 Author: Stephane Eranian <eranian@google.com>
 Date:   Thu May 9 14:45:56 2019 -0700

    perf/x86/intel: Fix INTEL_FLAGS_EVENT_CONSTRAINT* masking

The system is IvyBridge model 62, running a defconfig-ish kernel, and 
with perf_event_paranoid set to -1:

 [    3.756600] Performance Events: PEBS fmt1+, IvyBridge events, 16-deep LBR, full-width counters, Intel PMU driver.

 processor	: 39
 vendor_id	: GenuineIntel
 cpu family	: 6
 model		: 62
 model name	: Intel(R) Xeon(R) CPU E5-2680 v2 @ 2.80GHz
 stepping	: 4
 microcode	: 0x428

If I revert the commit 'perf top' starts working again.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [tip:perf/urgent] perf/x86/intel: Fix INTEL_FLAGS_EVENT_CONSTRAINT* masking
  2019-05-18 21:16   ` Ingo Molnar
@ 2019-05-19  1:41     ` Stephane Eranian
  0 siblings, 0 replies; 4+ messages in thread
From: Stephane Eranian @ 2019-05-19  1:41 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: H. Peter Anvin, Vince Weaver, Arnaldo Carvalho de Melo,
	Alexander Shishkin, Jiri Olsa, LKML, Linus Torvalds,
	Thomas Gleixner, Peter Zijlstra, linux-tip-commits

On Sat, May 18, 2019 at 2:16 PM Ingo Molnar <mingo@kernel.org> wrote:
>
>
> * tip-bot for Stephane Eranian <tipbot@zytor.com> wrote:
>
> > Commit-ID:  6b89d4c1ae8596a8c9240f169ef108704de373f2
> > Gitweb:     https://git.kernel.org/tip/6b89d4c1ae8596a8c9240f169ef108704de373f2
> > Author:     Stephane Eranian <eranian@google.com>
> > AuthorDate: Thu, 9 May 2019 14:45:56 -0700
> > Committer:  Ingo Molnar <mingo@kernel.org>
> > CommitDate: Fri, 10 May 2019 08:04:17 +0200
> >
> > perf/x86/intel: Fix INTEL_FLAGS_EVENT_CONSTRAINT* masking
> >
> > On Intel Westmere, a cmdline as follows:
> >
> >   $ perf record -e cpu/event=0xc4,umask=0x2,name=br_inst_retired.near_call/p ....
> >
> > was failing. Yet the event+ umask support PEBS.
> >
> > It turns out this is due to a bug in the the PEBS event constraint table for
> > westmere. All forms of BR_INST_RETIRED.* support PEBS. Therefore the constraint
> > mask should ignore the umask. The name of the macro INTEL_FLAGS_EVENT_CONSTRAINT()
> > hint that this is the case but it was not. That macros was checking both the
> > event code and event umask. Therefore, it was only matching on 0x00c4.
> > There are code+umask macros, they all have *UEVENT*.
> >
> > This bug fixes the issue by checking only the event code in the mask.
> > Both single and range version are modified.
> >
> > Signed-off-by: Stephane Eranian <eranian@google.com>
> > Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> > 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: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Vince Weaver <vincent.weaver@maine.edu>
> > Cc: kan.liang@intel.com
> > Link: http://lkml.kernel.org/r/20190509214556.123493-1-eranian@google.com
> > Signed-off-by: Ingo Molnar <mingo@kernel.org>
> > ---
> >  arch/x86/events/perf_event.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
> > index 07fc84bb85c1..a6ac2f4f76fc 100644
> > --- a/arch/x86/events/perf_event.h
> > +++ b/arch/x86/events/perf_event.h
> > @@ -394,10 +394,10 @@ struct cpu_hw_events {
> >
> >  /* Event constraint, but match on all event flags too. */
> >  #define INTEL_FLAGS_EVENT_CONSTRAINT(c, n) \
> > -     EVENT_CONSTRAINT(c, n, INTEL_ARCH_EVENT_MASK|X86_ALL_EVENT_FLAGS)
> > +     EVENT_CONSTRAINT(c, n, ARCH_PERFMON_EVENTSEL_EVENT|X86_ALL_EVENT_FLAGS)
> >
> >  #define INTEL_FLAGS_EVENT_CONSTRAINT_RANGE(c, e, n)                  \
> > -     EVENT_CONSTRAINT_RANGE(c, e, n, INTEL_ARCH_EVENT_MASK|X86_ALL_EVENT_FLAGS)
> > +     EVENT_CONSTRAINT_RANGE(c, e, n, ARCH_PERFMON_EVENTSEL_EVENT|X86_ALL_EVENT_FLAGS)
>
> This commit broke one of my testboxes - and unfortunately I noticed this
> too late and the commit is now upstream.
>
> The breakage is that 'perf top' stops working altogether, it errors out
> in the event creation:
>
>  $ perf top --stdio
>  Error:
>  The sys_perf_event_open() syscall returned with 22 (Invalid argument) for event (cycles).
>
> I bisected it back to this commit:
>
>  6b89d4c1ae8596a8c9240f169ef108704de373f2 is the first bad commit
>  commit 6b89d4c1ae8596a8c9240f169ef108704de373f2
>  Author: Stephane Eranian <eranian@google.com>
>  Date:   Thu May 9 14:45:56 2019 -0700
>
>     perf/x86/intel: Fix INTEL_FLAGS_EVENT_CONSTRAINT* masking
>
> The system is IvyBridge model 62, running a defconfig-ish kernel, and
> with perf_event_paranoid set to -1:
>
>  [    3.756600] Performance Events: PEBS fmt1+, IvyBridge events, 16-deep LBR, full-width counters, Intel PMU driver.
>
>  processor      : 39
>  vendor_id      : GenuineIntel
>  cpu family     : 6
>  model          : 62
>  model name     : Intel(R) Xeon(R) CPU E5-2680 v2 @ 2.80GHz
>  stepping       : 4
>  microcode      : 0x428
>
> If I revert the commit 'perf top' starts working again.
>
I have some ivybridge systems, let me debug this. This is likely
related to cycles:ppp stuff given what perf top does.
I think my patch is right, but there may be assumptions or bugs
elsewhere exposed by the fix.

>
> Thanks,
>
>         Ingo

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-05-19  1:41 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-09 21:45 [PATCH] perf/x86: fix INTEL_FLAGS_EVENT_CONSTRAINT* masking Stephane Eranian
2019-05-10  6:07 ` [tip:perf/urgent] perf/x86/intel: Fix " tip-bot for Stephane Eranian
2019-05-18 21:16   ` Ingo Molnar
2019-05-19  1:41     ` Stephane Eranian

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).