public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: kan.liang@linux.intel.com
Cc: acme@kernel.org, mingo@redhat.com, linux-kernel@vger.kernel.org,
	tglx@linutronix.de, jolsa@kernel.org, eranian@google.com,
	alexander.shishkin@linux.intel.com, ak@linux.intel.com
Subject: Re: [PATCH 05/22] perf/x86: Support constraint ranges
Date: Tue, 19 Mar 2019 16:27:12 +0100	[thread overview]
Message-ID: <20190319152712.GI6521@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <20190319145309.GI5996@hirez.programming.kicks-ass.net>

On Tue, Mar 19, 2019 at 03:53:09PM +0100, Peter Zijlstra wrote:
> On Mon, Mar 18, 2019 at 02:41:27PM -0700, kan.liang@linux.intel.com wrote:

> > The changes costs ~2k text size according to 0day report.
> 
> Where?! there isn't much code here.

> > @@ -71,6 +72,12 @@ struct event_constraint {
> >  #define PERF_X86_EVENT_AUTO_RELOAD	0x0400 /* use PEBS auto-reload */
> >  #define PERF_X86_EVENT_LARGE_PEBS	0x0800 /* use large PEBS */
> >  
> > +static inline bool constraint_match(struct event_constraint *c, u64 ecode)
> > +{
> > +	ecode &= c->cmask;
> > +	return ecode == c->code ||
> > +		(c->range_end && ecode >= c->code && ecode <= c->range_end);
> > +}
> >  
> >  struct amd_nb {
> >  	int nb_id;  /* NorthBridge id */
> 
> That's all the code, how does that add up to 2k ?

By my counting it adds all of 108 bytes of text. What it does do is add
5784 bytes of data. But that too appears not to be required.

The below patch does the same, it adds 37 bytes of text and no
additional data.

It mostly works because of how Intel event codes are 'small'. We can
easily compress the constraint to allow a u64 size, but I don't think
that is needed. If we need to cover AMD64_EVENTSEL_EVENT the range
compare needs to get fixed anyway.


---
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -2688,7 +2688,7 @@ x86_get_event_constraints(struct cpu_hw_
 
 	if (x86_pmu.event_constraints) {
 		for_each_event_constraint(c, x86_pmu.event_constraints) {
-			if ((event->hw.config & c->cmask) == c->code) {
+			if (constraint_match(c, event->hw.config)) {
 				event->hw.flags |= c->flags;
 				return c;
 			}
--- a/arch/x86/events/intel/ds.c
+++ b/arch/x86/events/intel/ds.c
@@ -858,7 +858,7 @@ struct event_constraint *intel_pebs_cons
 
 	if (x86_pmu.pebs_constraints) {
 		for_each_event_constraint(c, x86_pmu.pebs_constraints) {
-			if ((event->hw.config & c->cmask) == c->code) {
+			if (constraint_match(c, event->hw.config)) {
 				event->hw.flags |= c->flags;
 				return c;
 			}
--- a/arch/x86/events/perf_event.h
+++ b/arch/x86/events/perf_event.h
@@ -54,6 +54,7 @@ struct event_constraint {
 	int	weight;
 	int	overlap;
 	int	flags;
+	unsigned int	size;
 };
 /*
  * struct hw_perf_event.flags flags
@@ -71,6 +72,10 @@ struct event_constraint {
 #define PERF_X86_EVENT_AUTO_RELOAD	0x0400 /* use PEBS auto-reload */
 #define PERF_X86_EVENT_LARGE_PEBS	0x0800 /* use large PEBS */
 
+static inline bool constraint_match(struct event_constraint *c, u64 ecode)
+{
+	return ((ecode & c->cmask) - c->code) <= (u64)c->size;
+}
 
 struct amd_nb {
 	int nb_id;  /* NorthBridge id */
@@ -257,18 +262,25 @@ struct cpu_hw_events {
 	void				*kfree_on_online[X86_PERF_KFREE_MAX];
 };
 
-#define __EVENT_CONSTRAINT(c, n, m, w, o, f) {\
+#define __EVENT_CONSTRAINT_RANGE(c, e, n, m, w, o, f) {	\
 	{ .idxmsk64 = (n) },		\
 	.code = (c),			\
+	.size = (e) - (c),		\
 	.cmask = (m),			\
 	.weight = (w),			\
 	.overlap = (o),			\
 	.flags = f,			\
 }
 
+#define __EVENT_CONSTRAINT(c, n, m, w, o, f) \
+	__EVENT_CONSTRAINT_RANGE(c, c, n, m, w, o, f)
+
 #define EVENT_CONSTRAINT(c, n, m)	\
 	__EVENT_CONSTRAINT(c, n, m, HWEIGHT(n), 0, 0)
 
+#define EVENT_CONSTRAINT_RANGE(c, e, n, m) \
+	__EVENT_CONSTRAINT_RANGE(c, e, n, m, HWEIGHT(n), 0, 0)
+
 #define INTEL_EXCLEVT_CONSTRAINT(c, n)	\
 	__EVENT_CONSTRAINT(c, n, ARCH_PERFMON_EVENTSEL_EVENT, HWEIGHT(n),\
 			   0, PERF_X86_EVENT_EXCL)
@@ -304,6 +316,12 @@ struct cpu_hw_events {
 	EVENT_CONSTRAINT(c, n, ARCH_PERFMON_EVENTSEL_EVENT)
 
 /*
+ * Constraint on a range of Event codes
+ */
+#define INTEL_EVENT_CONSTRAINT_RANGE(c, e, n)			\
+	EVENT_CONSTRAINT_RANGE(c, e, n, ARCH_PERFMON_EVENTSEL_EVENT)
+
+/*
  * Constraint on the Event code + UMask + fixed-mask
  *
  * filter mask to validate fixed counter events.
@@ -350,6 +368,9 @@ struct cpu_hw_events {
 #define INTEL_FLAGS_EVENT_CONSTRAINT(c, n) \
 	EVENT_CONSTRAINT(c, n, INTEL_ARCH_EVENT_MASK|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)
+
 /* Check only flags, but allow all event/umask */
 #define INTEL_ALL_EVENT_CONSTRAINT(code, n)	\
 	EVENT_CONSTRAINT(code, n, X86_ALL_EVENT_FLAGS)
@@ -366,6 +387,11 @@ struct cpu_hw_events {
 			  ARCH_PERFMON_EVENTSEL_EVENT|X86_ALL_EVENT_FLAGS, \
 			  HWEIGHT(n), 0, PERF_X86_EVENT_PEBS_LD_HSW)
 
+#define INTEL_FLAGS_EVENT_CONSTRAINT_DATALA_LD_RANGE(code, end, n) \
+	__EVENT_CONSTRAINT_RANGE(code, end, n,				\
+			  ARCH_PERFMON_EVENTSEL_EVENT|X86_ALL_EVENT_FLAGS, \
+			  HWEIGHT(n), 0, PERF_X86_EVENT_PEBS_LD_HSW)
+
 #define INTEL_FLAGS_EVENT_CONSTRAINT_DATALA_XLD(code, n) \
 	__EVENT_CONSTRAINT(code, n,			\
 			  ARCH_PERFMON_EVENTSEL_EVENT|X86_ALL_EVENT_FLAGS, \

  reply	other threads:[~2019-03-19 15:27 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-18 21:41 [PATCH 00/22] perf: Add Icelake support kan.liang
2019-03-18 21:41 ` [PATCH 01/22] perf/core: Support outputting registers from a separate array kan.liang
2019-03-19 13:00   ` Peter Zijlstra
2019-03-19 14:13     ` Peter Zijlstra
2019-03-18 21:41 ` [PATCH 02/22] perf/x86/intel: Extract memory code PEBS parser for reuse kan.liang
2019-03-19 13:14   ` Peter Zijlstra
2019-03-18 21:41 ` [PATCH 03/22] perf/x86/intel: Support adaptive PEBSv4 kan.liang
2019-03-19 14:47   ` Peter Zijlstra
2019-03-19 16:03     ` Andi Kleen
2019-03-19 16:11       ` Peter Zijlstra
2019-03-19 21:20     ` Liang, Kan
2019-03-19 21:38     ` Andi Kleen
2019-03-20 15:58       ` Peter Zijlstra
2019-03-18 21:41 ` [PATCH 04/22] perf/x86/lbr: Avoid reading the LBRs when adaptive PEBS handles them kan.liang
2019-03-18 21:41 ` [PATCH 05/22] perf/x86: Support constraint ranges kan.liang
2019-03-19 14:53   ` Peter Zijlstra
2019-03-19 15:27     ` Peter Zijlstra [this message]
2019-03-19 15:57     ` Andi Kleen
2019-03-19 16:09       ` Peter Zijlstra
2019-03-18 21:41 ` [PATCH 06/22] perf/x86/intel: Add Icelake support kan.liang
2019-03-20  0:08   ` Stephane Eranian
2019-03-20 14:20     ` Liang, Kan
2019-03-18 21:41 ` [PATCH 07/22] perf/x86/intel/cstate: " kan.liang
2019-03-18 21:41 ` [PATCH 08/22] perf/x86/intel/rapl: " kan.liang
2019-03-18 21:41 ` [PATCH 09/22] perf/x86/msr: " kan.liang
2019-03-18 21:41 ` [PATCH 10/22] perf/x86/intel/uncore: Add Intel Icelake uncore support kan.liang
2019-03-18 21:41 ` [PATCH 11/22] perf/core: Support a REMOVE transaction kan.liang
2019-03-19 15:29   ` Peter Zijlstra
2019-03-18 21:41 ` [PATCH 12/22] perf/x86/intel: Basic support for metrics counters kan.liang
2019-03-18 21:41 ` [PATCH 13/22] perf/x86/intel: Support overflows on SLOTS kan.liang
2019-03-18 21:41 ` [PATCH 14/22] perf/x86/intel: Support hardware TopDown metrics kan.liang
2019-03-18 21:41 ` [PATCH 15/22] perf/x86/intel: Set correct weight for topdown subevent counters kan.liang
2019-03-18 21:41 ` [PATCH 16/22] perf/x86/intel: Export new top down events for Icelake kan.liang
2019-03-18 21:41 ` [PATCH 17/22] perf/x86/intel: Disable sampling read slots and topdown kan.liang
2019-03-18 21:41 ` [PATCH 18/22] perf/x86/intel: Support CPUID 10.ECX to disable fixed counters kan.liang
2019-03-18 21:41 ` [PATCH 19/22] perf, tools: Add support for recording and printing XMM registers kan.liang
2019-03-18 21:41 ` [PATCH 20/22] perf, tools, stat: Support new per thread TopDown metrics kan.liang
2019-03-18 21:41 ` [PATCH 21/22] perf, tools: Add documentation for topdown metrics kan.liang
2019-03-18 21:41 ` [PATCH 22/22] perf vendor events intel: Add JSON files for Icelake kan.liang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190319152712.GI6521@hirez.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=acme@kernel.org \
    --cc=ak@linux.intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=eranian@google.com \
    --cc=jolsa@kernel.org \
    --cc=kan.liang@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox