* [PATCH 1/2] perf, x86: Add INTEL_FLAGS_UEVENT_CONSTRAINT @ 2014-09-10 0:49 Andi Kleen 2014-09-10 0:49 ` [PATCH 2/2] perf, x86: Use INTEL_FLAGS_UEVENT_CONSTRAINT for PRECDIST Andi Kleen 0 siblings, 1 reply; 11+ messages in thread From: Andi Kleen @ 2014-09-10 0:49 UTC (permalink / raw) To: peterz; +Cc: linux-kernel, mingo, eranian, tglx, Andi Kleen From: Andi Kleen <ak@linux.intel.com> Add a FLAGS_UEVENT_CONSTRAINT macro that allows us to match on event+umask, and in additional all flags. This is needed to ensure the INV and CMASK fields are zero for specific events, as this can cause undefined behavior. Signed-off-by: Andi Kleen <ak@linux.intel.com> --- arch/x86/kernel/cpu/perf_event.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/arch/x86/kernel/cpu/perf_event.h b/arch/x86/kernel/cpu/perf_event.h index fc5eb39..4e6cdb0 100644 --- a/arch/x86/kernel/cpu/perf_event.h +++ b/arch/x86/kernel/cpu/perf_event.h @@ -253,6 +253,10 @@ struct cpu_hw_events { #define INTEL_UEVENT_CONSTRAINT(c, n) \ EVENT_CONSTRAINT(c, n, INTEL_ARCH_EVENT_MASK) +/* Like UEVENT_CONSTRAINT, but match flags too */ +#define INTEL_FLAGS_UEVENT_CONSTRAINT(c, n) \ + EVENT_CONSTRAINT(c, n, INTEL_ARCH_EVENT_MASK|X86_ALL_EVENT_FLAGS) + #define INTEL_PLD_CONSTRAINT(c, n) \ __EVENT_CONSTRAINT(c, n, INTEL_ARCH_EVENT_MASK|X86_ALL_EVENT_FLAGS, \ HWEIGHT(n), 0, PERF_X86_EVENT_PEBS_LDLAT) -- 1.9.3 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] perf, x86: Use INTEL_FLAGS_UEVENT_CONSTRAINT for PRECDIST 2014-09-10 0:49 [PATCH 1/2] perf, x86: Add INTEL_FLAGS_UEVENT_CONSTRAINT Andi Kleen @ 2014-09-10 0:49 ` Andi Kleen 2014-09-10 7:59 ` Peter Zijlstra 0 siblings, 1 reply; 11+ messages in thread From: Andi Kleen @ 2014-09-10 0:49 UTC (permalink / raw) To: peterz; +Cc: linux-kernel, mingo, eranian, tglx, Andi Kleen From: Andi Kleen <ak@linux.intel.com> The earlier commit 86a04461a made near all PEBS on Sandy/IvyBridge/Haswell to reject non zero flags. However this wasn't done for the INST_RETIRED.PREC_DIST event because no suitable macro existed. Now that we have INTEL_FLAGS_UEVENT_CONSTRAINT enforce zero flags for INST_RETIRED.PREC_DIST too. Signed-off-by: Andi Kleen <ak@linux.intel.com> --- arch/x86/kernel/cpu/perf_event_intel_ds.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/x86/kernel/cpu/perf_event_intel_ds.c b/arch/x86/kernel/cpu/perf_event_intel_ds.c index 9dc4199..e578c66 100644 --- a/arch/x86/kernel/cpu/perf_event_intel_ds.c +++ b/arch/x86/kernel/cpu/perf_event_intel_ds.c @@ -606,7 +606,7 @@ struct event_constraint intel_westmere_pebs_event_constraints[] = { }; struct event_constraint intel_snb_pebs_event_constraints[] = { - INTEL_UEVENT_CONSTRAINT(0x01c0, 0x2), /* INST_RETIRED.PRECDIST */ + INTEL_FLAGS_UEVENT_CONSTRAINT(0x01c0, 0x2), /* INST_RETIRED.PRECDIST */ INTEL_PLD_CONSTRAINT(0x01cd, 0x8), /* MEM_TRANS_RETIRED.LAT_ABOVE_THR */ INTEL_PST_CONSTRAINT(0x02cd, 0x8), /* MEM_TRANS_RETIRED.PRECISE_STORES */ /* UOPS_RETIRED.ALL, inv=1, cmask=16 (cycles:p). */ @@ -617,7 +617,7 @@ struct event_constraint intel_snb_pebs_event_constraints[] = { }; struct event_constraint intel_ivb_pebs_event_constraints[] = { - INTEL_UEVENT_CONSTRAINT(0x01c0, 0x2), /* INST_RETIRED.PRECDIST */ + INTEL_FLAGS_UEVENT_CONSTRAINT(0x01c0, 0x2), /* INST_RETIRED.PRECDIST */ INTEL_PLD_CONSTRAINT(0x01cd, 0x8), /* MEM_TRANS_RETIRED.LAT_ABOVE_THR */ INTEL_PST_CONSTRAINT(0x02cd, 0x8), /* MEM_TRANS_RETIRED.PRECISE_STORES */ /* UOPS_RETIRED.ALL, inv=1, cmask=16 (cycles:p). */ @@ -628,7 +628,7 @@ struct event_constraint intel_ivb_pebs_event_constraints[] = { }; struct event_constraint intel_hsw_pebs_event_constraints[] = { - INTEL_UEVENT_CONSTRAINT(0x01c0, 0x2), /* INST_RETIRED.PRECDIST */ + INTEL_FLAGS_UEVENT_CONSTRAINT(0x01c0, 0x2), /* INST_RETIRED.PRECDIST */ INTEL_PLD_CONSTRAINT(0x01cd, 0xf), /* MEM_TRANS_RETIRED.* */ /* UOPS_RETIRED.ALL, inv=1, cmask=16 (cycles:p). */ INTEL_FLAGS_EVENT_CONSTRAINT(0x108001c2, 0xf), -- 1.9.3 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] perf, x86: Use INTEL_FLAGS_UEVENT_CONSTRAINT for PRECDIST 2014-09-10 0:49 ` [PATCH 2/2] perf, x86: Use INTEL_FLAGS_UEVENT_CONSTRAINT for PRECDIST Andi Kleen @ 2014-09-10 7:59 ` Peter Zijlstra 2014-09-10 7:59 ` Peter Zijlstra 2014-09-10 14:02 ` Andi Kleen 0 siblings, 2 replies; 11+ messages in thread From: Peter Zijlstra @ 2014-09-10 7:59 UTC (permalink / raw) To: Andi Kleen; +Cc: linux-kernel, mingo, eranian, tglx, Andi Kleen [-- Attachment #1: Type: text/plain, Size: 257 bytes --] On Tue, Sep 09, 2014 at 05:49:08PM -0700, Andi Kleen wrote: > From: Andi Kleen <ak@linux.intel.com> > > The earlier commit 86a04461a made near all PEBS on > Sandy/IvyBridge/Haswell to reject non zero flags. What's magic about nehalem and westmere? [-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] perf, x86: Use INTEL_FLAGS_UEVENT_CONSTRAINT for PRECDIST 2014-09-10 7:59 ` Peter Zijlstra @ 2014-09-10 7:59 ` Peter Zijlstra 2014-09-10 8:37 ` Stephane Eranian 2014-09-10 14:02 ` Andi Kleen 1 sibling, 1 reply; 11+ messages in thread From: Peter Zijlstra @ 2014-09-10 7:59 UTC (permalink / raw) To: Andi Kleen; +Cc: linux-kernel, mingo, eranian, tglx, Andi Kleen [-- Attachment #1: Type: text/plain, Size: 405 bytes --] On Wed, Sep 10, 2014 at 09:59:26AM +0200, Peter Zijlstra wrote: > On Tue, Sep 09, 2014 at 05:49:08PM -0700, Andi Kleen wrote: > > From: Andi Kleen <ak@linux.intel.com> > > > > The earlier commit 86a04461a made near all PEBS on > > Sandy/IvyBridge/Haswell to reject non zero flags. > > What's magic about nehalem and westmere? And core2 and whatever else we support PEBS for, for that matter. [-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] perf, x86: Use INTEL_FLAGS_UEVENT_CONSTRAINT for PRECDIST 2014-09-10 7:59 ` Peter Zijlstra @ 2014-09-10 8:37 ` Stephane Eranian 2014-09-10 8:39 ` Peter Zijlstra 0 siblings, 1 reply; 11+ messages in thread From: Stephane Eranian @ 2014-09-10 8:37 UTC (permalink / raw) To: Peter Zijlstra; +Cc: Andi Kleen, LKML, Ingo Molnar, Thomas Gleixner, Andi Kleen On Wed, Sep 10, 2014 at 9:59 AM, Peter Zijlstra <peterz@infradead.org> wrote: > On Wed, Sep 10, 2014 at 09:59:26AM +0200, Peter Zijlstra wrote: >> On Tue, Sep 09, 2014 at 05:49:08PM -0700, Andi Kleen wrote: >> > From: Andi Kleen <ak@linux.intel.com> >> > >> > The earlier commit 86a04461a made near all PEBS on >> > Sandy/IvyBridge/Haswell to reject non zero flags. >> >> What's magic about nehalem and westmere? > > And core2 and whatever else we support PEBS for, for that matter. nhm, wsm, core2 do not have precise distribution (PREC_DIST) umask to inst_Retired. But as I said before, PEBS events do not support any filters. cycles -> uops_retired:any_inv=1:cmask=1 being an exception. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] perf, x86: Use INTEL_FLAGS_UEVENT_CONSTRAINT for PRECDIST 2014-09-10 8:37 ` Stephane Eranian @ 2014-09-10 8:39 ` Peter Zijlstra 0 siblings, 0 replies; 11+ messages in thread From: Peter Zijlstra @ 2014-09-10 8:39 UTC (permalink / raw) To: Stephane Eranian Cc: Andi Kleen, LKML, Ingo Molnar, Thomas Gleixner, Andi Kleen [-- Attachment #1: Type: text/plain, Size: 889 bytes --] On Wed, Sep 10, 2014 at 10:37:14AM +0200, Stephane Eranian wrote: > On Wed, Sep 10, 2014 at 9:59 AM, Peter Zijlstra <peterz@infradead.org> wrote: > > On Wed, Sep 10, 2014 at 09:59:26AM +0200, Peter Zijlstra wrote: > >> On Tue, Sep 09, 2014 at 05:49:08PM -0700, Andi Kleen wrote: > >> > From: Andi Kleen <ak@linux.intel.com> > >> > > >> > The earlier commit 86a04461a made near all PEBS on > >> > Sandy/IvyBridge/Haswell to reject non zero flags. > >> > >> What's magic about nehalem and westmere? > > > > And core2 and whatever else we support PEBS for, for that matter. > > nhm, wsm, core2 do not have precise distribution (PREC_DIST) umask to > inst_Retired. That was a comment to 86a04461a, I should have spotted it at the time. There is no sane reason to only change PEBS for a subset of chips. That needs to be fixed before I'll take anything else from Andi. [-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] perf, x86: Use INTEL_FLAGS_UEVENT_CONSTRAINT for PRECDIST 2014-09-10 7:59 ` Peter Zijlstra 2014-09-10 7:59 ` Peter Zijlstra @ 2014-09-10 14:02 ` Andi Kleen 2014-09-10 14:22 ` Peter Zijlstra 2014-09-10 20:38 ` Thomas Gleixner 1 sibling, 2 replies; 11+ messages in thread From: Andi Kleen @ 2014-09-10 14:02 UTC (permalink / raw) To: Peter Zijlstra; +Cc: Andi Kleen, linux-kernel, mingo, eranian, tglx On Wed, Sep 10, 2014 at 09:59:26AM +0200, Peter Zijlstra wrote: > On Tue, Sep 09, 2014 at 05:49:08PM -0700, Andi Kleen wrote: > > From: Andi Kleen <ak@linux.intel.com> > > > > The earlier commit 86a04461a made near all PEBS on > > Sandy/IvyBridge/Haswell to reject non zero flags. > > What's magic about nehalem and westmere? I wasn't able to confirm their behavior explicitly, so I felt it best to leave them alone. But in principle adding the _FLAGS changes there too would make sense too. -Andi -- ak@linux.intel.com -- Speaking for myself only ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] perf, x86: Use INTEL_FLAGS_UEVENT_CONSTRAINT for PRECDIST 2014-09-10 14:02 ` Andi Kleen @ 2014-09-10 14:22 ` Peter Zijlstra 2014-09-23 9:58 ` Peter Zijlstra 2014-09-10 20:38 ` Thomas Gleixner 1 sibling, 1 reply; 11+ messages in thread From: Peter Zijlstra @ 2014-09-10 14:22 UTC (permalink / raw) To: Andi Kleen; +Cc: Andi Kleen, linux-kernel, mingo, eranian, tglx On Wed, Sep 10, 2014 at 07:02:58AM -0700, Andi Kleen wrote: > On Wed, Sep 10, 2014 at 09:59:26AM +0200, Peter Zijlstra wrote: > > On Tue, Sep 09, 2014 at 05:49:08PM -0700, Andi Kleen wrote: > > > From: Andi Kleen <ak@linux.intel.com> > > > > > > The earlier commit 86a04461a made near all PEBS on > > > Sandy/IvyBridge/Haswell to reject non zero flags. > > > > What's magic about nehalem and westmere? > > I wasn't able to confirm their behavior explicitly, so I felt > it best to leave them alone. > > But in principle adding the _FLAGS changes there too would > make sense too. Yeah please do that patch ASAP, having PEBS behave differently across uarchs is wrong. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] perf, x86: Use INTEL_FLAGS_UEVENT_CONSTRAINT for PRECDIST 2014-09-10 14:22 ` Peter Zijlstra @ 2014-09-23 9:58 ` Peter Zijlstra 2014-10-03 4:02 ` Ingo Molnar 0 siblings, 1 reply; 11+ messages in thread From: Peter Zijlstra @ 2014-09-23 9:58 UTC (permalink / raw) To: Andi Kleen; +Cc: Andi Kleen, linux-kernel, mingo, eranian, tglx On Wed, Sep 10, 2014 at 04:22:32PM +0200, Peter Zijlstra wrote: > On Wed, Sep 10, 2014 at 07:02:58AM -0700, Andi Kleen wrote: > > On Wed, Sep 10, 2014 at 09:59:26AM +0200, Peter Zijlstra wrote: > > > On Tue, Sep 09, 2014 at 05:49:08PM -0700, Andi Kleen wrote: > > > > From: Andi Kleen <ak@linux.intel.com> > > > > > > > > The earlier commit 86a04461a made near all PEBS on > > > > Sandy/IvyBridge/Haswell to reject non zero flags. > > > > > > What's magic about nehalem and westmere? > > > > I wasn't able to confirm their behavior explicitly, so I felt > > it best to leave them alone. > > > > But in principle adding the _FLAGS changes there too would > > make sense too. > > Yeah please do that patch ASAP, having PEBS behave differently across > uarchs is wrong. Ping! ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] perf, x86: Use INTEL_FLAGS_UEVENT_CONSTRAINT for PRECDIST 2014-09-23 9:58 ` Peter Zijlstra @ 2014-10-03 4:02 ` Ingo Molnar 0 siblings, 0 replies; 11+ messages in thread From: Ingo Molnar @ 2014-10-03 4:02 UTC (permalink / raw) To: Peter Zijlstra Cc: Andi Kleen, Andi Kleen, linux-kernel, eranian, tglx, Arnaldo Carvalho de Melo, H. Peter Anvin * Peter Zijlstra <peterz@infradead.org> wrote: > On Wed, Sep 10, 2014 at 04:22:32PM +0200, Peter Zijlstra wrote: > > On Wed, Sep 10, 2014 at 07:02:58AM -0700, Andi Kleen wrote: > > > On Wed, Sep 10, 2014 at 09:59:26AM +0200, Peter Zijlstra wrote: > > > > On Tue, Sep 09, 2014 at 05:49:08PM -0700, Andi Kleen wrote: > > > > > From: Andi Kleen <ak@linux.intel.com> > > > > > > > > > > The earlier commit 86a04461a made near all PEBS on > > > > > Sandy/IvyBridge/Haswell to reject non zero flags. > > > > > > > > What's magic about nehalem and westmere? > > > > > > I wasn't able to confirm their behavior explicitly, so I felt > > > it best to leave them alone. > > > > > > But in principle adding the _FLAGS changes there too would > > > make sense too. > > > > Yeah please do that patch ASAP, having PEBS behave differently across > > uarchs is wrong. > > Ping! Also, I have to NAK this patch for trivial style reasons: the patch adds gobs of misaligned, misplaced spaces instead of using proper tabs - and apparently that's not the first time Andi did that to the constraints tables. This is annoying and ineffective and Andi Kleen continues to introduce trivial flaws into the kernel despite repeated complaints and requests for him to use available tools to check patch quality such as scripts/checkpatch.pl. As a result I'm dropping most patches from Andi in the perf/core pipeline and I will delay them to at least until v3.19 when I might have more time to waste on reviewing trivially flawed patches. I only took this single patch: perf/x86/intel/uncore: Fix minor race in box set up which fixes a bug, and I've dropped all other patches from Andi. Next time I see trivial flaws in Andi's patches I'll drop and delay them for two cycles, not one, to give them ample time to be improved. Thanks, Ingo ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] perf, x86: Use INTEL_FLAGS_UEVENT_CONSTRAINT for PRECDIST 2014-09-10 14:02 ` Andi Kleen 2014-09-10 14:22 ` Peter Zijlstra @ 2014-09-10 20:38 ` Thomas Gleixner 1 sibling, 0 replies; 11+ messages in thread From: Thomas Gleixner @ 2014-09-10 20:38 UTC (permalink / raw) To: Andi Kleen; +Cc: Peter Zijlstra, Andi Kleen, linux-kernel, mingo, eranian On Wed, 10 Sep 2014, Andi Kleen wrote: > On Wed, Sep 10, 2014 at 09:59:26AM +0200, Peter Zijlstra wrote: > > On Tue, Sep 09, 2014 at 05:49:08PM -0700, Andi Kleen wrote: > > > From: Andi Kleen <ak@linux.intel.com> > > > > > > The earlier commit 86a04461a made near all PEBS on > > > Sandy/IvyBridge/Haswell to reject non zero flags. > > > > What's magic about nehalem and westmere? > > I wasn't able to confirm their behavior explicitly, so I felt > it best to leave them alone. > > But in principle adding the _FLAGS changes there too would > make sense too. Can you please once in a while come up with a precise answer to a question instead of wrapping your internal knowledge into "in principle", "would" and other magic keywords? Thanks tglx ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2014-10-03 4:02 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-09-10 0:49 [PATCH 1/2] perf, x86: Add INTEL_FLAGS_UEVENT_CONSTRAINT Andi Kleen 2014-09-10 0:49 ` [PATCH 2/2] perf, x86: Use INTEL_FLAGS_UEVENT_CONSTRAINT for PRECDIST Andi Kleen 2014-09-10 7:59 ` Peter Zijlstra 2014-09-10 7:59 ` Peter Zijlstra 2014-09-10 8:37 ` Stephane Eranian 2014-09-10 8:39 ` Peter Zijlstra 2014-09-10 14:02 ` Andi Kleen 2014-09-10 14:22 ` Peter Zijlstra 2014-09-23 9:58 ` Peter Zijlstra 2014-10-03 4:02 ` Ingo Molnar 2014-09-10 20:38 ` Thomas Gleixner
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.