perf: add memory access sampling support

PATCH Superseded Stephane Eranian 4 participants 37 messages Oct 29, 2012 – Oct 31, 2012

Subsystems (2)

x86 perf events

lore

Share & tools

lore

Stephane Eranian

author

2012-10-29 15:16 UTC #0 lore raw

cover perf: add memory access sampling support

This patch series had a new feature to the kernel perf_events interface and corresponding user level tool, perf.

With this patch, it is possible to sample (not trace) memory accesses (load, store). For loads, the instruction and data addresses are captured along with the latency and data source. For stores, the instruction and data addresses are capture along with limited cache and TLB information.

For load data source, the memory hierarchy level, the tlb, snoop and lock information is captured.

Although the perf_event interface is extended in a generic manner, sampling memory accesses requires HW support. The current patches implement the feature on Intel processors starting with Nehalem. The patches leverage the PEBS Load Latency and Precise Store mechanisms. Precise Store is present only on Sandy Bridge and Ivy Bridge based processors.

The perf tool is extended to make capturing and analyzing the data easier with a new command: perf mem.

$ perf mem -t load rec triad
$ perf mem -t load rep --stdio
# Samples: 19K of event 'cpu/mem-loads/pp'
# Total cost : 1013994
# Sort order : cost,mem,sym,dso,symbol_daddr,dso_daddr,snoop,tlb,locked
#
# Overhead      Samples     Cost  Memory access Symbol      Shared Obj  Data Symbol             Data Object Snoop  TLB access   Locked
# ........  ...........  .......  ............. ..........  ........... ......................  ........... ...... ............ ......
#
     0.10%            1      986  LFB hit       [.] triad   triad       [.] 0x00007f67dffe8038  [unknown]    None  L1 or L2 hit  No    
     0.09%            1      890  LFB hit       [.] triad   triad       [.] 0x00007f67df91a750  [unknown]    None  L1 or L2 hit  No    
     0.08%            1      826  LFB hit       [.] triad   triad       [.] 0x00007f67e288fba8  [unknown]    None  L1 or L2 hit  No    
     0.08%            1      825  LFB hit       [.] triad   triad       [.] 0x00007f67dea28c80  [unknown]    None  L1 or L2 hit  No    
     0.08%            1      787  LFB hit       [.] triad   triad       [.] 0x00007f67df055a60  [unknown]    None  L1 or L2 hit  No

The perf mem command is a wrapper around perf record/report. It passes the right options to the report and record commands. Note that the TUI mode is supported.

One powerful feature of perf is that users can toy with sort order to display the information in different format or from a different angle. This is particularly useful with memory sampling:

$ perf mem -t load rep --sort=mem
# Samples: 19K of event 'cpu/mem-loads/pp'
# Total cost : 1013994
# Sort order : mem
#
# Overhead      Samples             Memory access
# ........  ...........  ........................
#
    85.26%        10633  LFB hit                 
     7.35%         8151  L1 hit                  
     3.13%          383  L3 hit                  
     3.09%          195  Local RAM hit           
     1.16%          259  L2 hit                  
     0.00%            4  Uncached hit

Or if one is interested in the data view:

$ perf mem -t load rep --sort=symbol_daddr,cost
# Samples: 19K of event 'cpu/mem-loads/pp'
# Total cost : 1013994
# Sort order : symbol_daddr,cost
#
# Overhead      Samples             Data Symbol     Cost
# ........  ...........  ......................  .......
#
     0.10%            1  [.] 0x00007f67dffe8038      986
     0.09%            1  [.] 0x00007f67df91a750      890
     0.08%            1  [.] 0x00007f67e288fba8      826

One note on the cost displayed: On Intel processors with PEBS Load Latency, as described in the SDM, the cost encompasses the number of cycles from dispatch to Globally Observable (GO) state. That means, that it includes OOO execution. It is not usual to see L1D Hits with a cost of > 100 cycles. Always look at the memory level for an approximation of the access penalty, then interpret the cost value accordingly.

There is no cost associated with stores.

CAVEAT: Note that the data addresses are not resolved correctly currently due to a problem in perf data symbol resolution code which I have not been able to uncover so far.

Enjoy,

Signed-off-by Stephane Eranian <[email protected]>

Stephane Eranian (10):
perf/x86: improve sysfs event mapping with event string perf/x86: add flags to event constraints
perf: add generic memory sampling interface perf/x86: add memory profiling via PEBS Load Latency perf/x86: export PEBS load latency threshold register to sysfs perf/x86: add support for PEBS Precise Store
perf tools: add mem access sampling core support perf report: add support for mem access profiling perf record: add support for mem access profiling perf tools: add new mem command for memory access profiling

 arch/x86/include/asm/msr-index.h              |    1 +
 arch/x86/kernel/cpu/perf_event.c              |   31 +--
 arch/x86/kernel/cpu/perf_event.h              |   61 ++++-
 arch/x86/kernel/cpu/perf_event_intel.c        |   66 ++++-
 arch/x86/kernel/cpu/perf_event_intel_ds.c     |  182 +++++++++++++-
 arch/x86/kernel/cpu/perf_event_intel_uncore.c |    2 +-
 include/linux/perf_event.h                    |    5 +
 include/uapi/linux/perf_event.h               |   70 +++++-
 kernel/events/core.c                          |   12 +
 tools/perf/Documentation/perf-mem.txt         |   51 ++++
 tools/perf/Makefile                           |    1 +
 tools/perf/builtin-mem.c                      |  237 ++++++++++++++++++
 tools/perf/builtin-record.c                   |    2 +
 tools/perf/builtin-report.c                   |  131 +++++++++-
 tools/perf/builtin.h                          |    1 +
 tools/perf/command-list.txt                   |    1 +
 tools/perf/perf.c                             |    1 +
 tools/perf/perf.h                             |    1 +
 tools/perf/util/event.h                       |    2 +
 tools/perf/util/evsel.c                       |   15 ++
 tools/perf/util/hist.c                        |   66 ++++-
 tools/perf/util/hist.h                        |   13 +
 tools/perf/util/session.c                     |   44 ++++
 tools/perf/util/session.h                     |    4 +
 tools/perf/util/sort.c                        |  324 ++++++++++++++++++++++++-
 tools/perf/util/sort.h                        |   10 +-
 tools/perf/util/symbol.h                      |    7 +
 27 files changed, 1296 insertions(+), 45 deletions(-)
 create mode 100644 tools/perf/Documentation/perf-mem.txt
 create mode 100644 tools/perf/builtin-mem.c

Signature · 1 line

1.7.9.5

Stephane Eranian

author

2012-10-29 15:16 UTC #1 lore raw

PATCH 1/10 perf/x86: improve sysfs event mapping with event string

This patch extends Jiri's changes to make generic events mapping visible via sysfs. The patch extends the mechanism to non-generic events by allowing the mappings to be hardcoded in strings.

This mechanism will be used by the PEBS-LL patch later on.

Signed-off-by Stephane Eranian <[email protected]>

---
 arch/x86/kernel/cpu/perf_event.c |   26 ++++++++++++--------------
 arch/x86/kernel/cpu/perf_event.h |   26 ++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 14 deletions(-)

arch/x86/kernel/cpu/perf_event.c +12 ~~-14~~

index 0a55ab2..7c30b4c 100644
@@ -1316,20 +1316,22 @@ static struct attribute_group x86_pmu_format_group = {
 	.attrs = NULL,
 };
 
-struct perf_pmu_events_attr {
-	struct device_attribute attr;
-	u64 id;
-};
-
 /*
  * Remove all undefined events (x86_pmu.event_map(id) == 0)
  * out of events_attr attributes.
  */
 static void __init filter_events(struct attribute **attrs)
 {
+	struct device_attribute *d;
+	struct perf_pmu_events_attr *pmu_attr;
 	int i, j;
 
 	for (i = 0; attrs[i]; i++) {
+		d = (struct device_attribute *)attrs[i];
+		pmu_attr = container_of(d, struct perf_pmu_events_attr, attr);
+		/* str trumps id */
+		if (pmu_attr->event_str)
+			continue;
 		if (x86_pmu.event_map(i))
 			continue;

@@ -1348,17 +1350,13 @@ ssize_t events_sysfs_show(struct device *dev, struct device_attribute *attr,
 		container_of(attr, struct perf_pmu_events_attr, attr);
 
 	u64 config = x86_pmu.event_map(pmu_attr->id);
-	return x86_pmu.events_sysfs_show(page, config);
-}
 
-#define EVENT_VAR(_id)  event_attr_##_id
-#define EVENT_PTR(_id) &event_attr_##_id.attr.attr
+	/* string trumps id */
+	if (pmu_attr->event_str)
+		return sprintf(page, "%s", pmu_attr->event_str);
 
-#define EVENT_ATTR(_name, _id)					\
-static struct perf_pmu_events_attr EVENT_VAR(_id) = {		\
-	.attr = __ATTR(_name, 0444, events_sysfs_show, NULL),	\
-	.id   =  PERF_COUNT_HW_##_id,				\
-};
+	return x86_pmu.events_sysfs_show(page, config);
+}
 
 EVENT_ATTR(cpu-cycles,			CPU_CYCLES		);
 EVENT_ATTR(instructions,		INSTRUCTIONS		);

arch/x86/kernel/cpu/perf_event.h +26 -0

index 115c1ea..41479da 100644
@@ -419,6 +419,29 @@ do {									\
 #define ERF_NO_HT_SHARING	1
 #define ERF_HAS_RSP_1		2
 
+#define EVENT_VAR(_id)  event_attr_##_id
+#define EVENT_PTR(_id) &event_attr_##_id.attr.attr
+
+#define EVENT_ATTR(_name, _id)					\
+static struct perf_pmu_events_attr EVENT_VAR(_id) = {		\
+	.attr = __ATTR(_name, 0444, events_sysfs_show, NULL),	\
+	.id   =  PERF_COUNT_HW_##_id,				\
+	.event_str = NULL,					\
+};
+
+#define EVENT_ATTR_STR(_name, v, str)				  \
+static struct perf_pmu_events_attr event_attr_##v = {		  \
+	.attr      = __ATTR(_name, 0444, events_sysfs_show, NULL),\
+	.id        =  0,					  \
+	.event_str =  str,					  \
+};
+
+struct perf_pmu_events_attr {
+	struct device_attribute attr;
+	u64 id;
+	const char *event_str;
+};
+
 extern struct x86_pmu x86_pmu __read_mostly;
 
 DECLARE_PER_CPU(struct cpu_hw_events, cpu_hw_events);

@@ -633,6 +656,9 @@ int p6_pmu_init(void);
 
 int knc_pmu_init(void);
 
+ssize_t events_sysfs_show(struct device *dev, struct device_attribute *attr,
+			  char *page);
+
 #else /* CONFIG_CPU_SUP_INTEL */
 
 static inline void reserve_ds_buffers(void)

Signature · 1 line

1.7.9.5

Andi Kleen

2012-10-29 19:26 UTC #2 lore raw

Re: [Patch v1 01/10] perf/x86: improve sysfs event mapping with event string

Quoted 7 lines · Stephane Eranian

This patch extends Jiri's changes to make generic events mapping visible via sysfs. The patch extends the mechanism to non-generic events by allowing the mappings to be hardcoded in strings.
This mechanism will be used by the PEBS-LL patch later on.

I'm going to pull this patch into my Haswell patchkit to handle the same problem. I originally had a different solution, but this works as well with the latest perf/core.

-Andi

Stephane Eranian

author

2012-10-29 15:19 UTC #3 lore raw

PATCH 2/10 perf/x86: add flags to event constraints

In reply to Stephane Eranian

This patch adds a flags field to each event constraint. It can be used to store event specific features which can then later be used by scheduling code or low-level x86 code.

The flags are propagated into event->hw.flags during the get_event_constraint() call. They are cleared during the put_event_constraint() call.

This mechanism is going to be used by the PEBS-LL patches. It avoids defining yet another table to hold event specific information.

Signed-off-by Stephane Eranian <[email protected]>

---
 arch/x86/kernel/cpu/perf_event.c              |    2 +-
 arch/x86/kernel/cpu/perf_event.h              |    8 +++++---
 arch/x86/kernel/cpu/perf_event_intel.c        |    5 ++++-
 arch/x86/kernel/cpu/perf_event_intel_ds.c     |    4 +++-
 arch/x86/kernel/cpu/perf_event_intel_uncore.c |    2 +-
 include/linux/perf_event.h                    |    1 +
 6 files changed, 15 insertions(+), 7 deletions(-)

arch/x86/kernel/cpu/perf_event.c +1 -1

index 7c30b4c..9b56c20 100644
@@ -1470,7 +1470,7 @@ static int __init init_hw_perf_events(void)
 
 	unconstrained = (struct event_constraint)
 		__EVENT_CONSTRAINT(0, (1ULL << x86_pmu.num_counters) - 1,
-				   0, x86_pmu.num_counters, 0);
+				   0, x86_pmu.num_counters, 0, 0);
 
 	x86_pmu.attr_rdpmc = 1; /* enable userspace RDPMC usage by default */
 	x86_pmu_format_group.attrs = x86_pmu.format_attrs;

arch/x86/kernel/cpu/perf_event.h +5 -3

index 41479da..f78d6e8 100644
@@ -59,6 +59,7 @@ struct event_constraint {
 	u64	cmask;
 	int	weight;
 	int	overlap;
+	int	flags;
 };
 
 struct amd_nb {

@@ -170,16 +171,17 @@ struct cpu_hw_events {
 	void				*kfree_on_online;
 };
 
-#define __EVENT_CONSTRAINT(c, n, m, w, o) {\
+#define __EVENT_CONSTRAINT(c, n, m, w, o, f) {\
 	{ .idxmsk64 = (n) },		\
 	.code = (c),			\
 	.cmask = (m),			\
 	.weight = (w),			\
 	.overlap = (o),			\
+	.flags = f,			\
 }
 
 #define EVENT_CONSTRAINT(c, n, m)	\
-	__EVENT_CONSTRAINT(c, n, m, HWEIGHT(n), 0)
+	__EVENT_CONSTRAINT(c, n, m, HWEIGHT(n), 0, 0)
 
 /*
  * The overlap flag marks event constraints with overlapping counter

@@ -203,7 +205,7 @@ struct cpu_hw_events {
  * and its counter masks must be kept at a minimum.
  */
 #define EVENT_CONSTRAINT_OVERLAP(c, n, m)	\
-	__EVENT_CONSTRAINT(c, n, m, HWEIGHT(n), 1)
+	__EVENT_CONSTRAINT(c, n, m, HWEIGHT(n), 1, 0)
 
 /*
  * Constraint on the Event code.

arch/x86/kernel/cpu/perf_event_intel.c +4 -1

index 93b9e11..57d6527 100644
@@ -1367,8 +1367,10 @@ x86_get_event_constraints(struct cpu_hw_events *cpuc, struct perf_event *event)
 
 	if (x86_pmu.event_constraints) {
 		for_each_event_constraint(c, x86_pmu.event_constraints) {
-			if ((event->hw.config & c->cmask) == c->code)
+			if ((event->hw.config & c->cmask) == c->code) {
+				event->hw.flags |= c->flags;
 				return c;
+			}
 		}
 	}

@@ -1413,6 +1415,7 @@ intel_put_shared_regs_event_constraints(struct cpu_hw_events *cpuc,
 static void intel_put_event_constraints(struct cpu_hw_events *cpuc,
 					struct perf_event *event)
 {
+	event->hw.flags = 0;
 	intel_put_shared_regs_event_constraints(cpuc, event);
 }

arch/x86/kernel/cpu/perf_event_intel_ds.c +3 -1

index 826054a..f30d85b 100644
@@ -430,8 +430,10 @@ struct event_constraint *intel_pebs_constraints(struct perf_event *event)
 
 	if (x86_pmu.pebs_constraints) {
 		for_each_event_constraint(c, x86_pmu.pebs_constraints) {
-			if ((event->hw.config & c->cmask) == c->code)
+			if ((event->hw.config & c->cmask) == c->code) {
+				event->hw.flags |= c->flags;
 				return c;
+			}
 		}
 	}

arch/x86/kernel/cpu/perf_event_intel_uncore.c +1 -1

index 3cf3d97..cc39e64 100644
@@ -2438,7 +2438,7 @@ static int __init uncore_type_init(struct intel_uncore_type *type)
 
 	type->unconstrainted = (struct event_constraint)
 		__EVENT_CONSTRAINT(0, (1ULL << type->num_counters) - 1,
-				0, type->num_counters, 0);
+				0, type->num_counters, 0, 0);
 
 	for (i = 0; i < type->num_boxes; i++) {
 		pmus[i].func_id = -1;

include/linux/perf_event.h +1 -0

index 6bfb2faa..484cfbc 100644
@@ -128,6 +128,7 @@ struct hw_perf_event {
 			int		event_base_rdpmc;
 			int		idx;
 			int		last_cpu;
+			int		flags;
 
 			struct hw_perf_event_extra extra_reg;
 			struct hw_perf_event_extra branch_reg;

Signature · 1 line

1.7.9.5

Stephane Eranian

author

2012-10-29 15:16 UTC #4 lore raw

PATCH 3/10 perf: add generic memory sampling interface

In reply to Stephane Eranian

This patch adds PERF_SAMPLE_COST and PERF_SAMPLE_DSRC. The first collects a cost associated with the sampled event. In case of memory access, the cost would be the latency of the load, otherwise it defaults to the sampling period.

PERF_SAMPLE_DSRC collects the data source, i.e., where did the data associated with the sampled instruction come from. Information is stored in a perf_mem_dsrc structure. It contains opcode, mem level, tlb, snoop, lock information, subject to availability in hardware.

Signed-off-by Stephane Eranian <[email protected]>

---
 include/linux/perf_event.h      |    4 +++
 include/uapi/linux/perf_event.h |   70 ++++++++++++++++++++++++++++++++++++++-
 kernel/events/core.c            |   12 +++++++
 3 files changed, 85 insertions(+), 1 deletion(-)

include/linux/perf_event.h +4 -0

index 484cfbc..a323ee2 100644
@@ -579,6 +579,8 @@ struct perf_sample_data {
 		u32	reserved;
 	}				cpu_entry;
 	u64				period;
+	u64				cost;
+	union  perf_mem_dsrc		dsrc;
 	struct perf_callchain_entry	*callchain;
 	struct perf_raw_record		*raw;
 	struct perf_branch_stack	*br_stack;

@@ -597,6 +599,8 @@ static inline void perf_sample_data_init(struct perf_sample_data *data,
 	data->regs_user.abi = PERF_SAMPLE_REGS_ABI_NONE;
 	data->regs_user.regs = NULL;
 	data->stack_user_size = 0;
+	data->cost = period; /* by default */
+	data->dsrc.val = 0;
 }
 
 extern void perf_output_sample(struct perf_output_handle *handle,

include/uapi/linux/perf_event.h +69 -1

87 lines not on this page · raw message

kernel/events/core.c +12 -0

index dbccf83..a1cf8f2 100644
@@ -955,6 +955,12 @@ static void perf_event__header_size(struct perf_event *event)
 	if (sample_type & PERF_SAMPLE_READ)
 		size += event->read_size;
 
+	if (sample_type & PERF_SAMPLE_COST)
+		size += sizeof(data->cost);
+
+	if (sample_type & PERF_SAMPLE_DSRC)
+		size += sizeof(data->dsrc.val);
+
 	event->header_size = size;
 }

@@ -4169,6 +4175,12 @@ void perf_output_sample(struct perf_output_handle *handle,
 		perf_output_sample_ustack(handle,
 					  data->stack_user_size,
 					  data->regs_user.regs);
+
+	if (sample_type & PERF_SAMPLE_COST)
+		perf_output_put(handle, data->cost);
+
+	if (sample_type & PERF_SAMPLE_DSRC)
+		perf_output_put(handle, data->dsrc.val);
 }
 
 void perf_prepare_sample(struct perf_event_header *header,

Signature · 1 line

1.7.9.5

Stephane Eranian

author

2012-10-29 15:16 UTC #5 lore raw

PATCH 4/10 perf/x86: add memory profiling via PEBS Load Latency

In reply to Stephane Eranian

This patch adds support for memory profiling using the PEBS Load Latency facility.

Load accesses are sampled by HW and the instruction address, data address, load latency, data source, tlb, locked information can be saved in the sampling buffer if using the PERF_SAMPLE_COST (for latency), PERF_SAMPLE_ADDR, PERF_SAMPLE_DSRC types.

To enable PEBS Load Latency, users have to use the model specific event:
- on NHM/WSM: MEM_INST_RETIRED:LATENCY_ABOVE_THRESHOLD
- on SNB/IVB: MEM_TRANS_RETIRED:LATENCY_ABOVE_THRESHOLD

To make things easier, this patch also exports a generic alias via sysfs: mem-loads. It export the right event encoding based on the host CPU and can be used directly by the perf tool.

Loosely based on Intel's Lin Ming patch posted on LKML in July 2011.

Signed-off-by Stephane Eranian <[email protected]>

---
 arch/x86/include/asm/msr-index.h          |    1 +
 arch/x86/kernel/cpu/perf_event.c          |    3 +
 arch/x86/kernel/cpu/perf_event.h          |   22 ++++-
 arch/x86/kernel/cpu/perf_event_intel.c    |   56 ++++++++++++
 arch/x86/kernel/cpu/perf_event_intel_ds.c |  133 +++++++++++++++++++++++++++--
 5 files changed, 206 insertions(+), 9 deletions(-)

arch/x86/include/asm/msr-index.h +1 -0

index 7f0edce..5edc9c0 100644
@@ -66,6 +66,7 @@
 #define MSR_IA32_PEBS_ENABLE		0x000003f1
 #define MSR_IA32_DS_AREA		0x00000600
 #define MSR_IA32_PERF_CAPABILITIES	0x00000345
+#define MSR_PEBS_LD_LAT_THRESHOLD	0x000003f6
 
 #define MSR_MTRRfix64K_00000		0x00000250
 #define MSR_MTRRfix16K_80000		0x00000258

arch/x86/kernel/cpu/perf_event.c +3 -0

index 9b56c20..2fc34d6 100644
@@ -1475,6 +1475,9 @@ static int __init init_hw_perf_events(void)
 	x86_pmu.attr_rdpmc = 1; /* enable userspace RDPMC usage by default */
 	x86_pmu_format_group.attrs = x86_pmu.format_attrs;
 
+	if (x86_pmu.event_attrs)
+		x86_pmu_events_group.attrs = x86_pmu.event_attrs;
+
 	if (!x86_pmu.events_sysfs_show)
 		x86_pmu_events_group.attrs = &empty_attrs;
 	else

arch/x86/kernel/cpu/perf_event.h +21 -1

index f78d6e8..3c5aa72 100644
@@ -46,6 +46,7 @@ enum extra_reg_type {
 	EXTRA_REG_RSP_0 = 0,	/* offcore_response_0 */
 	EXTRA_REG_RSP_1 = 1,	/* offcore_response_1 */
 	EXTRA_REG_LBR   = 2,	/* lbr_select */
+	EXTRA_REG_LDLAT = 3,	/* ld_lat_threshold */
 
 	EXTRA_REG_MAX		/* number of entries needed */
 };

@@ -61,6 +62,10 @@ struct event_constraint {
 	int	overlap;
 	int	flags;
 };
+/*
+ * struct event_constraint flags
+ */
+#define PERF_X86_EVENT_PEBS_LDLAT	0x1 /* ld+ldlat data address sampling */
 
 struct amd_nb {
 	int nb_id;  /* NorthBridge id */

@@ -233,6 +238,10 @@ struct cpu_hw_events {
 #define INTEL_UEVENT_CONSTRAINT(c, n)	\
 	EVENT_CONSTRAINT(c, n, INTEL_ARCH_EVENT_MASK)
 
+#define INTEL_PLD_CONSTRAINT(c, n)	\
+	__EVENT_CONSTRAINT(c, n, INTEL_ARCH_EVENT_MASK, \
+			   HWEIGHT(n), 0, PERF_X86_EVENT_PEBS_LDLAT)
+
 #define EVENT_CONSTRAINT_END		\
 	EVENT_CONSTRAINT(0, 0, 0)

@@ -262,12 +271,22 @@ struct extra_reg {
 	.msr = (ms),		\
 	.config_mask = (m),	\
 	.valid_mask = (vm),	\
-	.idx = EXTRA_REG_##i	\
+	.idx = EXTRA_REG_##i,	\
 	}
 
 #define INTEL_EVENT_EXTRA_REG(event, msr, vm, idx)	\
 	EVENT_EXTRA_REG(event, msr, ARCH_PERFMON_EVENTSEL_EVENT, vm, idx)
 
+#define INTEL_UEVENT_EXTRA_REG(event, msr, vm, idx) \
+	EVENT_EXTRA_REG(event, msr, ARCH_PERFMON_EVENTSEL_EVENT | \
+			ARCH_PERFMON_EVENTSEL_UMASK, vm, idx)
+
+#define INTEL_UEVENT_PEBS_LDLAT_EXTRA_REG(c) \
+	INTEL_UEVENT_EXTRA_REG(c, \
+			       MSR_PEBS_LD_LAT_THRESHOLD, \
+			       0xffff, \
+			       LDLAT)
+
 #define EVENT_EXTRA_END EVENT_EXTRA_REG(0, 0, 0, 0, RSP_0)
 
 union perf_capabilities {

@@ -355,6 +374,7 @@ struct x86_pmu {
 	 */
 	int		attr_rdpmc;
 	struct attribute **format_attrs;
+	struct attribute **event_attrs;
 
 	ssize_t		(*events_sysfs_show)(char *page, u64 config);

arch/x86/kernel/cpu/perf_event_intel.c +56 -0

101 lines not on this page · raw message

arch/x86/kernel/cpu/perf_event_intel_ds.c +125 -8

185 lines not on this page · raw message

Signature · 1 line

1.7.9.5

Peter Zijlstra

maintainer

2012-10-29 15:23 UTC #6 lore raw

Re: [Patch v1 04/10] perf/x86: add memory profiling via PEBS Load Latency

arch/x86/kernel/cpu/perf_event_intel.c static struct event_constraint intel_gen_event_constraints[] __read_mostly = · line 152

Quoted 2 lines · Stephane Eranian

arch/x86/kernel/cpu/perf_event_intel.c +2 -0

+EVENT_ATTR_STR(mem-loads, mem_ld_nhm, "event=0x100b,umask=0x1,ldlat=3");
+EVENT_ATTR_STR(mem-loads, mem_ld_snb, "event=0xcd,umask=0x1,ldlat=3");

I haven't fully grokked the macro magic yet, but event=0x100b seems wrong, event only takes 8 bit on Intel.

Stephane Eranian

author

2012-10-29 15:24 UTC #7 lore raw

Re: [Patch v1 04/10] perf/x86: add memory profiling via PEBS Load Latency

Quoted 6 lines · Peter Zijlstra

arch/x86/kernel/cpu/perf_event_intel.c static struct event_constraint intel_gen_event_constraints[] __read_mostly = · line 152
Quoted 2 lines · Stephane Eranian
arch/x86/kernel/cpu/perf_event_intel.c +2 -0
+EVENT_ATTR_STR(mem-loads, mem_ld_nhm, "event=0x100b,umask=0x1,ldlat=3");
+EVENT_ATTR_STR(mem-loads, mem_ld_snb, "event=0xcd,umask=0x1,ldlat=3");
I haven't fully grokked the macro magic yet, but event=0x100b seems wrong, event only takes 8 bit on Intel.

Yeah, my bad. I messed up on this one. should be:
event=0x0b,umask=0x10,ldlat=3

Peter Zijlstra

maintainer

2012-10-29 15:35 UTC #8 lore raw

Re: [Patch v1 04/10] perf/x86: add memory profiling via PEBS Load Latency

In reply to Stephane Eranian

Quoted 12 lines · Stephane Eranian

arch/x86/kernel/cpu/perf_event_intel_ds.c +12 -0

+static u64 load_latency_data(u64 status)
+{
+       union intel_x86_pebs_dse dse;
+       u64 val;
+       int model = boot_cpu_data.x86_model;
+       int fam = boot_cpu_data.x86;
+
+       dse.val = status;
+
+       /*
+        * use the mapping table for bit 0-15
+        */

arch/x86/kernel/cpu/perf_event_intel_ds.c struct pebs_record_32 { · line 93

Quoted 10 lines

arch/x86/kernel/cpu/perf_event_intel_ds.c +10 -0

+       val = pebs_data_source[dse.ld_dse];
+
+       /*
+        * Nehalem models do not support TLB, Lock infos
+        */
+       if (fam == 0x6 && (model == 26 || model == 30
+           || model == 31 || model == 46)) {
+               val |= P(TLB, NA) | P(LOCK, NA);
+               return val;
+       }

I'm so 100% sure we'll forget to add a nhm model number if we ever find we missed one.

Could we either add a classification enum to x86_pmu that is set in the big model switch on init, or do this with your new constraints flags, where we have a different flag for NHM_LL vs SNB_LL or so?

Or if all else fails, add a quirk to the Intel Debugstore bits bitfield, something like pebs_ll_nhm.

Quoted 18 lines

arch/x86/kernel/cpu/perf_event_intel_ds.c +18 -0

+       /*
+        * bit 4: TLB access
+        * 0 = did not miss 2nd level TLB
+        * 1 = missed 2nd level TLB
+        */
+       if (dse.ld_stlb_miss)
+               val |= P(TLB, MISS) | P(TLB, L2);
+       else
+               val |= P(TLB, HIT) | P(TLB,L1) | P(TLB, L2);
+
+       /*
+        * bit 5: locked prefix
+        */
+       if (dse.ld_locked)
+               val |= P(LOCK, LOCKED);
+
+       return val;
+}

Stephane Eranian

author

2012-10-29 15:39 UTC #9 lore raw

Re: [Patch v1 04/10] perf/x86: add memory profiling via PEBS Load Latency

Quoted 27 lines · Peter Zijlstra

Quoted 12 lines · Stephane Eranian

arch/x86/kernel/cpu/perf_event_intel_ds.c +12 -0

+static u64 load_latency_data(u64 status)
+{
+       union intel_x86_pebs_dse dse;
+       u64 val;
+       int model = boot_cpu_data.x86_model;
+       int fam = boot_cpu_data.x86;
+
+       dse.val = status;
+
+       /*
+        * use the mapping table for bit 0-15
+        */

arch/x86/kernel/cpu/perf_event_intel_ds.c struct pebs_record_32 { · line 93

Quoted 10 lines

arch/x86/kernel/cpu/perf_event_intel_ds.c +10 -0

+       val = pebs_data_source[dse.ld_dse];
+
+       /*
+        * Nehalem models do not support TLB, Lock infos
+        */
+       if (fam == 0x6 && (model == 26 || model == 30
+           || model == 31 || model == 46)) {
+               val |= P(TLB, NA) | P(LOCK, NA);
+               return val;
+       }

I'm so 100% sure we'll forget to add a nhm model number if we ever find we missed one.

Quoted 4 lines

Could we either add a classification enum to x86_pmu that is set in the big model switch on init, or do this with your new constraints flags, where we have a different flag for NHM_LL vs SNB_LL or so?

Let's add something in x86_pmu. We could, for instance, generalize x86_pmu.er_flags into x86_pmu.flags. Instead of adding yet another flag field.

Quoted 21 lines

Or if all else fails, add a quirk to the Intel Debugstore bits bitfield, something like pebs_ll_nhm.

Quoted 18 lines

arch/x86/kernel/cpu/perf_event_intel_ds.c +18 -0

+       /*
+        * bit 4: TLB access
+        * 0 = did not miss 2nd level TLB
+        * 1 = missed 2nd level TLB
+        */
+       if (dse.ld_stlb_miss)
+               val |= P(TLB, MISS) | P(TLB, L2);
+       else
+               val |= P(TLB, HIT) | P(TLB,L1) | P(TLB, L2);
+
+       /*
+        * bit 5: locked prefix
+        */
+       if (dse.ld_locked)
+               val |= P(LOCK, LOCKED);
+
+       return val;
+}

Peter Zijlstra

maintainer

2012-10-29 15:38 UTC #10 lore raw

Re: [Patch v1 04/10] perf/x86: add memory profiling via PEBS Load Latency

In reply to Stephane Eranian

Quoted 18 lines · Stephane Eranian

arch/x86/kernel/cpu/perf_event_intel_ds.c +16 -0

+       fll = event->hw.flags & PERF_X86_EVENT_PEBS_LDLAT;
+
        perf_sample_data_init(&data, 0, event->hw.last_period);
 
+       data.period = event->hw.last_period;
+       sample_type = event->attr.sample_type;
+
+       /*
+        * if PEBS-LL or PreciseStore
+        */
+       if (fll) {
+               if (sample_type & PERF_SAMPLE_ADDR)
+                       data.addr = pebs->dla;
+
+               /*
+                * Use latency for cost (only avail with PEBS-LL)
+                */
+               if (fll && (sample_type & PERF_SAMPLE_COST))

arch/x86/kernel/cpu/perf_event_intel_ds.c static void __intel_pmu_pebs_event(struct perf_event *event, · line 692

Quoted 10 lines

arch/x86/kernel/cpu/perf_event_intel_ds.c +10 -0

+                       data.cost = pebs->lat;
+
+               /*
+                * data.dsrc encodes the data source
+                */
+               if (sample_type & PERF_SAMPLE_DSRC) {
+                       if (fll)
+                               data.dsrc.val = load_latency_data(pebs->dse);
+               }
+       }

Why does fl1 exist? the above looks really convoluted, all fl1 checks inside are pointless. Also 'fl1' is a bizarre name, 'pebs_ll' would be a better name I guess.

What's wrong with:

	if (event->hw.flags & PERF_X86_EVENT_PEBS_LDLAT) {
		if (sample_type & PERF_SAMPLE_ADDR)
			data.addr = pebs->dla;

		if (sample_type & PERF_SAMPLE_COST)
			data.cost = perf->lat;

		if (sample_type & PERF_SAMPLE_DSRC)
			data.dsrc.val = load_latency_data(pebs->dse);
	}

Stephane Eranian

author

2012-10-29 15:43 UTC #11 lore raw

Re: [Patch v1 04/10] perf/x86: add memory profiling via PEBS Load Latency

Quoted 26 lines · Peter Zijlstra

Quoted 25 lines · Stephane Eranian

arch/x86/kernel/cpu/perf_event_intel_ds.c +23 -0

+       fll = event->hw.flags & PERF_X86_EVENT_PEBS_LDLAT;
+
        perf_sample_data_init(&data, 0, event->hw.last_period);
 
+       data.period = event->hw.last_period;
+       sample_type = event->attr.sample_type;
+
+       /*
+        * if PEBS-LL or PreciseStore
+        */
+       if (fll) {
+               if (sample_type & PERF_SAMPLE_ADDR)
+                       data.addr = pebs->dla;
+
+               /*
+                * Use latency for cost (only avail with PEBS-LL)
+                */
+               if (fll && (sample_type & PERF_SAMPLE_COST))
+                       data.cost = pebs->lat;
+
+               /*
+                * data.dsrc encodes the data source
+                */
+               if (sample_type & PERF_SAMPLE_DSRC) {
+                       if (fll)

Quoted 9 lines

arch/x86/kernel/cpu/perf_event_intel_ds.c static void __intel_pmu_pebs_event(struct perf_event *event, · line 692
Quoted 3 lines
arch/x86/kernel/cpu/perf_event_intel_ds.c +3 -0
+                               data.dsrc.val = load_latency_data(pebs->dse);
+               }
+       }
Why does fl1 exist? the above looks really convoluted, all fl1 checks inside are pointless. Also 'fl1' is a bizarre name, 'pebs_ll' would be a better name I guess.

You meant fll, instead I think.

Quoted 13 lines

What's wrong with:

        if (event->hw.flags & PERF_X86_EVENT_PEBS_LDLAT) {
                if (sample_type & PERF_SAMPLE_ADDR)
                        data.addr = pebs->dla;

                if (sample_type & PERF_SAMPLE_COST)
                        data.cost = perf->lat;

                if (sample_type & PERF_SAMPLE_DSRC)
                        data.dsrc.val = load_latency_data(pebs->dse);
        }

Well, that would work too, but I am trying to factorize the code with Precise Store which is a later patch. But we could clone your proposal and do:

Quoted 7 lines

        if (event->hw.flags & PERF_X86_EVENT_PEBS_ST) {
                if (sample_type & PERF_SAMPLE_ADDR)
                        data.addr = pebs->dla;

                if (sample_type & PERF_SAMPLE_DSRC)
                        data.dsrc.val = precise_store_data(pebs->dse);
        }

I am fine with that too.

Peter Zijlstra

maintainer

2012-10-29 15:44 UTC #12 lore raw

Re: [Patch v1 04/10] perf/x86: add memory profiling via PEBS Load Latency

Quoted 1 line · Stephane Eranian

You meant fll, instead I think.

Oh, yes, too small font I guess.

Quoted 2 lines

Well, that would work too, but I am trying to factorize the code with Precise Store which is a later patch.

Yeah, just found that, its fine the way it is. Just looked funneh on first reading.

Andi Kleen

2012-10-29 19:42 UTC #13 lore raw

Re: [Patch v1 04/10] perf/x86: add memory profiling via PEBS Load Latency

In reply to Stephane Eranian

Quoted 5 lines

arch/x86/kernel/cpu/perf_event_intel.c +5 -0

+
+struct attribute *nhm_events_attrs[] = {
+	EVENT_PTR(CPU_CYCLES),
+	EVENT_PTR(INSTRUCTIONS),
+	EVENT_PTR(CACHE_REFERENCES),

arch/x86/kernel/cpu/perf_event_intel.c static struct event_constraint intel_gen_event_constraints[] __read_mostly = · line 167

Quoted 10 lines

arch/x86/kernel/cpu/perf_event_intel.c +10 -0

+	EVENT_PTR(CACHE_MISSES),
+	EVENT_PTR(BRANCH_INSTRUCTIONS),
+	EVENT_PTR(BRANCH_MISSES),
+	EVENT_PTR(BUS_CYCLES),
+	EVENT_PTR(STALLED_CYCLES_FRONTEND),
+	EVENT_PTR(STALLED_CYCLES_BACKEND),
+	EVENT_PTR(REF_CPU_CYCLES),
+	EVENT_PTR(mem_ld_nhm),
+	NULL,
+};

I thought Jiri's patch already exports all the generic ones?

Why do you need to replace the whole table?

BTW I still think my approach in the v4 Haswell patchkit is simpler and didn't rely on hardcoding these events.

-Andi

Stephane Eranian

author

2012-10-29 20:39 UTC #14 lore raw

Re: [Patch v1 04/10] perf/x86: add memory profiling via PEBS Load Latency

Quoted 18 lines · Andi Kleen

Quoted 5 lines
arch/x86/kernel/cpu/perf_event_intel.c +5 -0
+
+struct attribute *nhm_events_attrs[] = {
+     EVENT_PTR(CPU_CYCLES),
+     EVENT_PTR(INSTRUCTIONS),
+     EVENT_PTR(CACHE_REFERENCES),
arch/x86/kernel/cpu/perf_event_intel.c static struct event_constraint intel_gen_event_constraints[] __read_mostly = · line 167
Quoted 10 lines
arch/x86/kernel/cpu/perf_event_intel.c +10 -0
+     EVENT_PTR(CACHE_MISSES),
+     EVENT_PTR(BRANCH_INSTRUCTIONS),
+     EVENT_PTR(BRANCH_MISSES),
+     EVENT_PTR(BUS_CYCLES),
+     EVENT_PTR(STALLED_CYCLES_FRONTEND),
+     EVENT_PTR(STALLED_CYCLES_BACKEND),
+     EVENT_PTR(REF_CPU_CYCLES),
+     EVENT_PTR(mem_ld_nhm),
+     NULL,
+};
I thought Jiri's patch already exports all the generic ones?

Quoted 2 lines

Why do you need to replace the whole table?

Because I am extending them with one or two events based on cpu model. That was the easiest way of doing this instead of playing some kind of malloc+copy trick.

Quoted 3 lines

BTW I still think my approach in the v4 Haswell patchkit is simpler and didn't rely on hardcoding these events.

I don't care about those events. As I found out, they are not even used by perf because they are all hardcoded and that's what gets used. I assume they are exposed for reference only. I don't object to that. But I think the right mechanism would be one where you can add events at boot time based on CPU model. It could be used to add the common events as well in the common part of the init code.

Peter Zijlstra

maintainer

2012-10-29 20:44 UTC #15 lore raw

Re: [Patch v1 04/10] perf/x86: add memory profiling via PEBS Load Latency

Quoted 4 lines · Stephane Eranian

But I think the right mechanism would be one where you can add events at boot time based on CPU model. It could be used to add the common events as well in the common part of the init code.

mlin once posted something like that, it did some scary things with sysfs without using the driver model -- which might or might not be 'right' (TM).

If someone has the stomach to stare at that code and then convince gregkh that its not 'wrong' I'm all for it.

Andi Kleen

2012-10-29 21:16 UTC #16 lore raw

Re: [Patch v1 04/10] perf/x86: add memory profiling via PEBS Load Latency

In reply to Stephane Eranian

Quoted 5 lines

Quoted 2 lines
Why do you need to replace the whole table?
Because I am extending them with one or two events based on cpu model. That was the easiest way of doing this instead of playing some kind of malloc+copy trick.

I did malloc and copy.

Quoted 11 lines

Quoted 4 lines
BTW I still think my approach in the v4 Haswell patchkit is simpler and didn't rely on hardcoding these events.
I don't care about those events. As I found out, they are not even used by perf because they are all hardcoded and that's what gets used. I assume they are exposed for reference only. I don't object to that. But I think the right mechanism would be one where you can add events at boot time based on CPU model. It could be used to add the common events as well in the common part of the init code.

Yes that's what I did.

I don't think copying everything for everything new is a good approach.

-Andi

Signature · 1 line

[email protected] -- Speaking for myself only

Stephane Eranian

author

2012-10-29 21:32 UTC #17 lore raw

Re: [Patch v1 04/10] perf/x86: add memory profiling via PEBS Load Latency

Quoted 20 lines · Andi Kleen

Quoted 5 lines
Quoted 2 lines
Why do you need to replace the whole table?
Because I am extending them with one or two events based on cpu model. That was the easiest way of doing this instead of playing some kind of malloc+copy trick.
I did malloc and copy.
Quoted 11 lines
Quoted 4 lines
BTW I still think my approach in the v4 Haswell patchkit is simpler and didn't rely on hardcoding these events.
I don't care about those events. As I found out, they are not even used by perf because they are all hardcoded and that's what gets used. I assume they are exposed for reference only. I don't object to that. But I think the right mechanism would be one where you can add events at boot time based on CPU model. It could be used to add the common events as well in the common part of the init code.

Quoted 5 lines

Yes that's what I did.
I don't think copying everything for everything new is a good approach.

I agree. I did that because it was the easiest thing I could think of. Discovered I had to deal with all of this just two days ago when I rebased. Wasn't too happy to have to deal with this at the last minute.

In any case, if you have something, then looks like I need to wait until it's in before I can adjust this patch set.

Quoted 4 lines

-Andi
--
[email protected] -- Speaking for myself only

Andi Kleen

2012-10-29 21:56 UTC #18 lore raw

Re: [Patch v1 04/10] perf/x86: add memory profiling via PEBS Load Latency

Quoted 3 lines

I agree. I did that because it was the easiest thing I could think of. Discovered I had to deal with all of this just two days ago when I rebased. Wasn't too happy to have to deal with this at the last minute.

I'll repost, as soon as I debugged why Jiri's version of my 31/33 parser patch doesn't work.

-Andi

Signature · 1 line

[email protected] -- Speaking for myself only

Namhyung Kim

maintainer

2012-10-30 08:43 UTC #19 lore raw

Re: [Patch v1 04/10] perf/x86: add memory profiling via PEBS Load Latency

In reply to Stephane Eranian

Hi Stephane,

arch/x86/kernel/cpu/perf_event_intel_ds.c struct pebs_record_32 { · line 84

Quoted 4 lines · Stephane Eranian

arch/x86/kernel/cpu/perf_event_intel_ds.c +4 -0

+	/*
+	 * use the mapping table for bit 0-15
+	 */
+	val = pebs_data_source[dse.ld_dse];

I guess you meant bit 0-3, right?

Thanks,
Namhyung

Stephane Eranian

author

2012-10-29 15:16 UTC #20 lore raw

PATCH 5/10 perf/x86: export PEBS load latency threshold register to sysfs

In reply to Stephane Eranian

Make the PEBS Load Latency threshold register layout and encoding visible to user level tools.

Signed-off-by Stephane Eranian <[email protected]>

---
 arch/x86/kernel/cpu/perf_event_intel.c |    3 +++
 1 file changed, 3 insertions(+)

arch/x86/kernel/cpu/perf_event_intel.c +3 -0

index 059fc8c..c37b7f8 100644
@@ -1787,6 +1787,8 @@ static void intel_pmu_flush_branch_stack(void)
 
 PMU_FORMAT_ATTR(offcore_rsp, "config1:0-63");
 
+PMU_FORMAT_ATTR(ldlat, "config1:0-15");
+
 static struct attribute *intel_arch3_formats_attr[] = {
 	&format_attr_event.attr,
 	&format_attr_umask.attr,

@@ -1797,6 +1799,7 @@ static struct attribute *intel_arch3_formats_attr[] = {
 	&format_attr_cmask.attr,
 
 	&format_attr_offcore_rsp.attr, /* XXX do NHM/WSM + SNB breakout */
+	&format_attr_ldlat.attr, /* PEBS load latency */
 	NULL,
 };

Signature · 1 line

1.7.9.5

Stephane Eranian

author

2012-10-29 15:18 UTC #21 lore raw

PATCH 6/10 perf/x86: add support for PEBS Precise Store

In reply to Stephane Eranian

This patch adds support for PEBS Precise Store which is available on Intel Sandy Bridge and Ivy Bridge processors.

To use Precise store, the proper PEBS event must be used: mem_trans_retired:precise_stores. For the perf tool, the generic mem-stores event exported via sysfs can be used directly.

Signed-off-by Stephane Eranian <[email protected]>

---
 arch/x86/kernel/cpu/perf_event.h          |    5 +++
 arch/x86/kernel/cpu/perf_event_intel.c    |    2 ++
 arch/x86/kernel/cpu/perf_event_intel_ds.c |   49 +++++++++++++++++++++++++++--
 3 files changed, 54 insertions(+), 2 deletions(-)

arch/x86/kernel/cpu/perf_event.h +5 -0

index 3c5aa72..4e95c90 100644
@@ -66,6 +66,7 @@ struct event_constraint {
  * struct event_constraint flags
  */
 #define PERF_X86_EVENT_PEBS_LDLAT	0x1 /* ld+ldlat data address sampling */
+#define PERF_X86_EVENT_PEBS_ST		0x2 /* st data address sampling */
 
 struct amd_nb {
 	int nb_id;  /* NorthBridge id */

@@ -242,6 +243,10 @@ struct cpu_hw_events {
 	__EVENT_CONSTRAINT(c, n, INTEL_ARCH_EVENT_MASK, \
 			   HWEIGHT(n), 0, PERF_X86_EVENT_PEBS_LDLAT)
 
+#define INTEL_PST_CONSTRAINT(c, n)	\
+	__EVENT_CONSTRAINT(c, n, INTEL_ARCH_EVENT_MASK, \
+			  HWEIGHT(n), 0, PERF_X86_EVENT_PEBS_ST)
+
 #define EVENT_CONSTRAINT_END		\
 	EVENT_CONSTRAINT(0, 0, 0)

arch/x86/kernel/cpu/perf_event_intel.c +2 -0

index c37b7f8..bf25b7b 100644
@@ -150,6 +150,7 @@ EVENT_ATTR(ref-cycles,			REF_CPU_CYCLES		);
 
 EVENT_ATTR_STR(mem-loads, mem_ld_nhm, "event=0x100b,umask=0x1,ldlat=3");
 EVENT_ATTR_STR(mem-loads, mem_ld_snb, "event=0xcd,umask=0x1,ldlat=3");
+EVENT_ATTR_STR(mem-stores, mem_st_snb, "event=0xcd,umask=0x2");
 
 struct attribute *nhm_events_attrs[] = {
 	EVENT_PTR(CPU_CYCLES),

@@ -178,6 +179,7 @@ struct attribute *snb_events_attrs[] = {
 	EVENT_PTR(STALLED_CYCLES_BACKEND),
 	EVENT_PTR(REF_CPU_CYCLES),
 	EVENT_PTR(mem_ld_snb),
+	EVENT_PTR(mem_st_snb),
 	NULL,
 };

arch/x86/kernel/cpu/perf_event_intel_ds.c +47 -2

96 lines not on this page · raw message

Signature · 1 line

1.7.9.5

Peter Zijlstra

maintainer

2012-10-29 15:40 UTC #22 lore raw

Re: [Patch v1 06/10] perf/x86: add support for PEBS Precise Store

Quoted 6 lines · Stephane Eranian

arch/x86/kernel/cpu/perf_event_intel_ds.c +1 -1

-       if (fll) {
+       if (fll || fst) {
                if (sample_type & PERF_SAMPLE_ADDR)
                        data.addr = pebs->dla;

arch/x86/kernel/cpu/perf_event_intel_ds.c static void __intel_pmu_pebs_event(struct perf_event *event, · line 738

Quoted 8 lines

arch/x86/kernel/cpu/perf_event_intel_ds.c +2 -0

@@ -688,6 +731,8 @@ static void __intel_pmu_pebs_event(struct perf_event *event,
                if (sample_type & PERF_SAMPLE_DSRC) {
                        if (fll)
                                data.dsrc.val = load_latency_data(pebs->dse);
+                       else if (fst)
+                               data.dsrc.val = precise_store_data(pebs->dse);
                }
        }

Ah, this is why I guess.. hrm.. ok, n/m then.

Stephane Eranian

author

2012-10-29 15:44 UTC #23 lore raw

Re: [Patch v1 06/10] perf/x86: add support for PEBS Precise Store

Quoted 6 lines · Peter Zijlstra

Quoted 5 lines · Stephane Eranian

arch/x86/kernel/cpu/perf_event_intel_ds.c +1 -1

-       if (fll) {
+       if (fll || fst) {
                if (sample_type & PERF_SAMPLE_ADDR)
                        data.addr = pebs->dla;

Quoted 11 lines

arch/x86/kernel/cpu/perf_event_intel_ds.c static void __intel_pmu_pebs_event(struct perf_event *event, · line 738

Quoted 9 lines

arch/x86/kernel/cpu/perf_event_intel_ds.c +2 -0

@@ -688,6 +731,8 @@ static void __intel_pmu_pebs_event(struct perf_event *event,
                if (sample_type & PERF_SAMPLE_DSRC) {
                        if (fll)
                                data.dsrc.val = load_latency_data(pebs->dse);
+                       else if (fst)
+                               data.dsrc.val = precise_store_data(pebs->dse);
                }
        }

Ah, this is why I guess.. hrm.. ok, n/m then.

Yes, it is. But your proposal and my extension for Precise Store look good too.

Namhyung Kim

maintainer

2012-10-31 05:21 UTC #24 lore raw

Re: [Patch v1 06/10] perf/x86: add support for PEBS Precise Store

In reply to Stephane Eranian

Quoted 8 lines · Stephane Eranian

This patch adds support for PEBS Precise Store which is available on Intel Sandy Bridge and Ivy Bridge processors.
To use Precise store, the proper PEBS event must be used: mem_trans_retired:precise_stores. For the perf tool, the generic mem-stores event exported via sysfs can be used directly.

Just trivial nitpicks..

Quoted 3 lines

Signed-off-by Stephane Eranian <[email protected]>
---

[snip]

arch/x86/kernel/cpu/perf_event_intel_ds.c struct event_constraint intel_ivb_pebs_event_constraints[] = { · line 542

Quoted 13 lines

arch/x86/kernel/cpu/perf_event_intel_ds.c +2 -0

@@ -486,6 +524,7 @@ struct event_constraint intel_snb_pebs_event_constraints[] = {
 	INTEL_EVENT_CONSTRAINT(0xc4, 0xf),    /* BR_INST_RETIRED.* */
 	INTEL_EVENT_CONSTRAINT(0xc5, 0xf),    /* BR_MISP_RETIRED.* */
 	INTEL_PLD_CONSTRAINT(0x01cd, 0x8),    /* MEM_TRANS_RETIRED.LAT_ABOVE_THR */
+	INTEL_PST_CONSTRAINT(0x02cd, 0x8),    /* MEM_TRANS_RETIRED.PRECISE_STORES */
 	INTEL_EVENT_CONSTRAINT(0xd0, 0xf),    /* MEM_UOP_RETIRED.* */
 	INTEL_EVENT_CONSTRAINT(0xd1, 0xf),    /* MEM_LOAD_UOPS_RETIRED.* */
 	INTEL_EVENT_CONSTRAINT(0xd2, 0xf),    /* MEM_LOAD_UOPS_LLC_HIT_RETIRED.* */

@@ -500,6 +539,7 @@ struct event_constraint intel_ivb_pebs_event_constraints[] = {
         INTEL_EVENT_CONSTRAINT(0xc4, 0xf),    /* BR_INST_RETIRED.* */
         INTEL_EVENT_CONSTRAINT(0xc5, 0xf),    /* BR_MISP_RETIRED.* */
         INTEL_PLD_CONSTRAINT(0x01cd, 0x8),    /* MEM_TRANS_RETIRED.LAT_ABOVE_THR */
+	INTEL_PST_CONSTRAINT(0x02cd, 0x8),    /* MEM_TRANS_RETIRED.PRECISE_STORES */

White-space damaged? Oh, it seems already broken with spaces.

Quoted 3 lines

arch/x86/kernel/cpu/perf_event_intel_ds.c +0 -0

         INTEL_EVENT_CONSTRAINT(0xd0, 0xf),    /* MEM_UOP_RETIRED.* */
         INTEL_EVENT_CONSTRAINT(0xd1, 0xf),    /* MEM_LOAD_UOPS_RETIRED.* */
         INTEL_EVENT_CONSTRAINT(0xd2, 0xf),    /* MEM_LOAD_UOPS_LLC_HIT_RETIRED.* */

[snip]

Quoted 10 lines

arch/x86/kernel/cpu/perf_event_intel_ds.c +1 -1

@@ -672,7 +715,7 @@ static void __intel_pmu_pebs_event(struct perf_event *event,
 	/*
 	 * if PEBS-LL or PreciseStore
 	 */
-	if (fll) {
+	if (fll || fst) {
 		if (sample_type & PERF_SAMPLE_ADDR)
 			data.addr = pebs->dla;

arch/x86/kernel/cpu/perf_event_intel_ds.c static void __intel_pmu_pebs_event(struct perf_event *event, · line 734

Quoted 4 lines

arch/x86/kernel/cpu/perf_event_intel_ds.c +1 -0

@@ -688,6 +731,8 @@ static void __intel_pmu_pebs_event(struct perf_event *event,
 		if (sample_type & PERF_SAMPLE_DSRC) {
 			if (fll)
 				data.dsrc.val = load_latency_data(pebs->dse);
+			else if (fst)

Looks like it can be converted to a plain 'else'.

Thanks,
Namhyung

Stephane Eranian

author

2012-10-31 13:28 UTC #25 lore raw

Re: [Patch v1 06/10] perf/x86: add support for PEBS Precise Store

Quoted 16 lines · Namhyung Kim

Quoted 8 lines · Stephane Eranian
This patch adds support for PEBS Precise Store which is available on Intel Sandy Bridge and Ivy Bridge processors.
To use Precise store, the proper PEBS event must be used: mem_trans_retired:precise_stores. For the perf tool, the generic mem-stores event exported via sysfs can be used directly.
Just trivial nitpicks..
Quoted 3 lines
Signed-off-by Stephane Eranian <[email protected]>
---
[snip]

Quoted 16 lines

arch/x86/kernel/cpu/perf_event_intel_ds.c struct event_constraint intel_ivb_pebs_event_constraints[] = { · line 542

Quoted 13 lines

arch/x86/kernel/cpu/perf_event_intel_ds.c +2 -0

@@ -486,6 +524,7 @@ struct event_constraint intel_snb_pebs_event_constraints[] = {
      INTEL_EVENT_CONSTRAINT(0xc4, 0xf),    /* BR_INST_RETIRED.* */
      INTEL_EVENT_CONSTRAINT(0xc5, 0xf),    /* BR_MISP_RETIRED.* */
      INTEL_PLD_CONSTRAINT(0x01cd, 0x8),    /* MEM_TRANS_RETIRED.LAT_ABOVE_THR */
+     INTEL_PST_CONSTRAINT(0x02cd, 0x8),    /* MEM_TRANS_RETIRED.PRECISE_STORES */
      INTEL_EVENT_CONSTRAINT(0xd0, 0xf),    /* MEM_UOP_RETIRED.* */
      INTEL_EVENT_CONSTRAINT(0xd1, 0xf),    /* MEM_LOAD_UOPS_RETIRED.* */
      INTEL_EVENT_CONSTRAINT(0xd2, 0xf),    /* MEM_LOAD_UOPS_LLC_HIT_RETIRED.* */

@@ -500,6 +539,7 @@ struct event_constraint intel_ivb_pebs_event_constraints[] = {
         INTEL_EVENT_CONSTRAINT(0xc4, 0xf),    /* BR_INST_RETIRED.* */
         INTEL_EVENT_CONSTRAINT(0xc5, 0xf),    /* BR_MISP_RETIRED.* */
         INTEL_PLD_CONSTRAINT(0x01cd, 0x8),    /* MEM_TRANS_RETIRED.LAT_ABOVE_THR */
+     INTEL_PST_CONSTRAINT(0x02cd, 0x8),    /* MEM_TRANS_RETIRED.PRECISE_STORES */

White-space damaged? Oh, it seems already broken with spaces.

Yes, it was already damaged with white spaces.

Quoted 14 lines

Quoted 4 lines

arch/x86/kernel/cpu/perf_event_intel_ds.c +0 -0

         INTEL_EVENT_CONSTRAINT(0xd0, 0xf),    /* MEM_UOP_RETIRED.* */
         INTEL_EVENT_CONSTRAINT(0xd1, 0xf),    /* MEM_LOAD_UOPS_RETIRED.* */
         INTEL_EVENT_CONSTRAINT(0xd2, 0xf),    /* MEM_LOAD_UOPS_LLC_HIT_RETIRED.* */

[snip]

Quoted 9 lines

arch/x86/kernel/cpu/perf_event_intel_ds.c +1 -1

@@ -672,7 +715,7 @@ static void __intel_pmu_pebs_event(struct perf_event *event,
      /*
       * if PEBS-LL or PreciseStore
       */
-     if (fll) {
+     if (fll || fst) {
              if (sample_type & PERF_SAMPLE_ADDR)
                      data.addr = pebs->dla;

Quoted 8 lines

arch/x86/kernel/cpu/perf_event_intel_ds.c static void __intel_pmu_pebs_event(struct perf_event *event, · line 734
Quoted 5 lines
arch/x86/kernel/cpu/perf_event_intel_ds.c +1 -0
@@ -688,6 +731,8 @@ static void __intel_pmu_pebs_event(struct perf_event *event,
              if (sample_type & PERF_SAMPLE_DSRC) {
                      if (fll)
                              data.dsrc.val = load_latency_data(pebs->dse);
+                     else if (fst)
Looks like it can be converted to a plain 'else'.

Yeah. Cannot have fll && fst.

Stephane Eranian

author

2012-10-29 15:16 UTC #26 lore raw

PATCH 7/10 perf tools: add mem access sampling core support

In reply to Stephane Eranian

This patch adds the sorting and histogram support
functions to enable profiling of memory accesses.

The following sorting orders are added:
- symbol_daddr: data address symbol (or raw address)
- dso_daddr: data address shared object
- cost: access cost
- locked: access uses locked transaction
- tlb : TLB access
- mem : memory level of the access (L1, L2, L3, RAM, ...)
- snoop: access snoop mode

Signed-off-by Stephane Eranian <[email protected]>

---
 tools/perf/util/event.h   |    2 +
 tools/perf/util/evsel.c   |    9 ++
 tools/perf/util/hist.c    |   66 ++++++++-
 tools/perf/util/hist.h    |   13 ++
 tools/perf/util/session.c |   44 ++++++
 tools/perf/util/session.h |    4 +
 tools/perf/util/sort.c    |  324 ++++++++++++++++++++++++++++++++++++++++++++-
 tools/perf/util/sort.h    |   10 +-
 tools/perf/util/symbol.h  |    7 +
 9 files changed, 469 insertions(+), 10 deletions(-)

tools/perf/util/event.h +2 -0

index da97aff..b26b692 100644
@@ -89,6 +89,8 @@ struct perf_sample {
 	u64 period;
 	u32 cpu;
 	u32 raw_size;
+	u64 cost;
+	u64 dsrc;
 	void *raw_data;
 	struct ip_callchain *callchain;
 	struct branch_stack *branch_stack;

tools/perf/util/evsel.c +9 -0

index 618d411..c82a271 100644
@@ -1020,6 +1020,15 @@ int perf_evsel__parse_sample(struct perf_evsel *evsel, union perf_event *event,
 		}
 	}
 
+	if (type & PERF_SAMPLE_COST) {
+		data->cost = *array;
+		array++;
+	}
+
+	if (type & PERF_SAMPLE_DSRC) {
+		data->dsrc = *array;
+		array++;
+	}
 	return 0;
 }

tools/perf/util/hist.c +63 -3

114 lines not on this page · raw message

tools/perf/util/hist.h +13 -0

index c751624..3ae4f62 100644
@@ -48,6 +48,13 @@ enum hist_column {
 	HISTC_DSO_FROM,
 	HISTC_DSO_TO,
 	HISTC_SRCLINE,
+	HISTC_MEM_DADDR_SYMBOL,
+	HISTC_MEM_DADDR_DSO,
+	HISTC_MEM_COST,
+	HISTC_MEM_LOCKED,
+	HISTC_MEM_TLB,
+	HISTC_MEM_LVL,
+	HISTC_MEM_SNOOP,
 	HISTC_NR_COLS, /* Last entry */
 };

@@ -85,6 +92,12 @@ struct hist_entry *__hists__add_branch_entry(struct hists *self,
 					     struct branch_info *bi,
 					     u64 period);
 
+struct hist_entry *__hists__add_mem_entry(struct hists *self,
+					  struct addr_location *al,
+					  struct symbol *sym_parent,
+					  struct mem_info *mi,
+					  u64 period);
+
 void hists__output_resort(struct hists *self);
 void hists__output_resort_threaded(struct hists *hists);
 void hists__collapse_resort(struct hists *self);

tools/perf/util/session.c +44 -0

56 lines not on this page · raw message

tools/perf/util/session.h +4 -0

index dd64261..37ab693 100644
@@ -72,6 +72,10 @@ struct branch_info *machine__resolve_bstack(struct machine *self,
 					    struct thread *thread,
 					    struct branch_stack *bs);
 
+struct mem_info *machine__resolve_mem(struct machine *self,
+				      struct thread *thread,
+				      struct perf_sample *sample, u8 cpumode);
+
 bool perf_session__has_traces(struct perf_session *self, const char *msg);
 
 void mem_bswap_64(void *src, int byte_size);

tools/perf/util/sort.c +318 -6

360 lines not on this page · raw message

tools/perf/util/sort.h +9 -1

index 13761d8..175eb4c 100644
@@ -102,7 +102,8 @@ struct hist_entry {
 	};
 	struct branch_info	*branch_info;
 	struct hists		*hists;
-	struct callchain_root	callchain[0];
+	struct mem_info		*mem_info;
+	struct callchain_root	callchain[0]; /* must be last member */
 };
 
 enum sort_type {

@@ -118,6 +119,13 @@ enum sort_type {
 	SORT_SYM_TO,
 	SORT_MISPREDICT,
 	SORT_SRCLINE,
+	SORT_MEM_DADDR_SYMBOL,
+	SORT_MEM_DADDR_DSO,
+	SORT_MEM_COST,
+	SORT_MEM_LOCKED,
+	SORT_MEM_TLB,
+	SORT_MEM_LVL,
+	SORT_MEM_SNOOP,
 };
 
 /*

tools/perf/util/symbol.h +7 -0

index 8b6ef7f..4ccc729 100644
@@ -153,6 +153,13 @@ struct branch_info {
 	struct branch_flags flags;
 };
 
+struct mem_info {
+	struct addr_map_symbol iaddr;
+	struct addr_map_symbol daddr;
+	u64	cost;
+	union perf_mem_dsrc dsrc;
+};
+
 struct addr_location {
 	struct thread *thread;
 	struct map    *map;

Signature · 1 line

1.7.9.5

Andi Kleen

2012-10-29 16:55 UTC #27 lore raw

Re: [Patch v1 07/10] perf tools: add mem access sampling core support

Quoted 2 lines · Stephane Eranian

This patch adds the sorting and histogram support
functions to enable profiling of memory accesses.

I hope my weight patches get in soon.

Could you merge your cost with weight then?

Otherwise having both cost and weight would be a bit redundant.

-Andi

Stephane Eranian

author

2012-10-29 17:00 UTC #28 lore raw

Re: [Patch v1 07/10] perf tools: add mem access sampling core support

Quoted 8 lines · Andi Kleen

Quoted 2 lines · Stephane Eranian
This patch adds the sorting and histogram support
functions to enable profiling of memory accesses.
I hope my weight patches get in soon.
Could you merge your cost with weight then?

Those are very similar changes indeed. I need
to investigate whether I can use your directly.

Quoted 2 lines

Otherwise having both cost and weight would be a bit redundant.

Yes, we don't want that.

I realized the title of my patch was confusing. Should read: cost support.

Namhyung Kim

maintainer

2012-10-31 05:51 UTC #29 lore raw

Re: [Patch v1 07/10] perf tools: add mem access sampling core support

In reply to Stephane Eranian

Quoted 14 lines · Stephane Eranian

This patch adds the sorting and histogram support
functions to enable profiling of memory accesses.
The following sorting orders are added:
- symbol_daddr: data address symbol (or raw address)
- dso_daddr: data address shared object
- cost: access cost
- locked: access uses locked transaction
- tlb : TLB access
- mem : memory level of the access (L1, L2, L3, RAM, ...)
- snoop: access snoop mode
Signed-off-by Stephane Eranian <[email protected]>
---

[snip]

tools/perf/util/sort.c static int hist_entry__mispredict_snprintf(struct hist_entry *self, char *bf, · line 488

Quoted 9 lines

tools/perf/util/sort.c +9 -0

+/* --sort daddr_sym */
+static int64_t
+sort__daddr_cmp(struct hist_entry *left, struct hist_entry *right)
+{
+	struct addr_map_symbol *l = &left->mem_info->daddr;
+	struct addr_map_symbol *r = &right->mem_info->daddr;
+
+	return (int64_t)(r->addr - l->addr);
+}

Doesn't it need to compare symbol (start address) if any, before doing it with raw addresses?

Thanks,
Namhyung

Stephane Eranian

author

2012-10-31 13:30 UTC #30 lore raw

Re: [Patch v1 07/10] perf tools: add mem access sampling core support

Quoted 16 lines · Namhyung Kim

Quoted 14 lines · Stephane Eranian
This patch adds the sorting and histogram support
functions to enable profiling of memory accesses.
The following sorting orders are added:
- symbol_daddr: data address symbol (or raw address)
- dso_daddr: data address shared object
- cost: access cost
- locked: access uses locked transaction
- tlb : TLB access
- mem : memory level of the access (L1, L2, L3, RAM, ...)
- snoop: access snoop mode
Signed-off-by Stephane Eranian <[email protected]>
---
[snip]

Quoted 13 lines

tools/perf/util/sort.c static int hist_entry__mispredict_snprintf(struct hist_entry *self, char *bf, · line 488
Quoted 9 lines
tools/perf/util/sort.c +9 -0
+/* --sort daddr_sym */
+static int64_t
+sort__daddr_cmp(struct hist_entry *left, struct hist_entry *right)
+{
+     struct addr_map_symbol *l = &left->mem_info->daddr;
+     struct addr_map_symbol *r = &right->mem_info->daddr;
+
+     return (int64_t)(r->addr - l->addr);
+}
Doesn't it need to compare symbol (start address) if any, before doing it with raw addresses?

Possibly but we can't get to data symbols at the moment due to limitations in kernel + perf tool.

Stephane Eranian

author

2012-10-29 15:16 UTC #31 lore raw

PATCH 8/10 perf report: add support for mem access profiling

In reply to Stephane Eranian

This patch adds the --mem-mode option to perf report.

This mode requires a perf.data file created with memory access samples.

Signed-off-by Stephane Eranian <[email protected]>

---
 tools/perf/builtin-report.c |  131 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 127 insertions(+), 4 deletions(-)

tools/perf/builtin-report.c +127 -4

index 90d1162..4d2e4f5 100644
@@ -47,6 +47,7 @@ struct perf_report {
 	bool			show_full_info;
 	bool			show_threads;
 	bool			inverted_callchain;
+	bool			mem_mode;
 	struct perf_read_values	show_threads_values;
 	const char		*pretty_printing_style;
 	symbol_filter_t		annotate_init;

@@ -55,6 +56,96 @@ struct perf_report {
 	DECLARE_BITMAP(cpu_bitmap, MAX_NR_CPUS);
 };
 
+static int perf_report__add_mem_hist_entry(struct perf_tool *tool,
+					   struct addr_location *al,
+					   struct perf_sample *sample,
+					   struct perf_evsel *evsel,
+					   struct machine *machine,
+					   union perf_event *event)
+{
+	struct perf_report *rep = container_of(tool, struct perf_report, tool);
+	struct symbol *parent = NULL;
+	u8 cpumode = event->header.misc & PERF_RECORD_MISC_CPUMODE_MASK;
+	int err = 0;
+	struct hist_entry *he;
+	struct mem_info *mi, *mx;
+	uint64_t cost;
+
+	if ((sort__has_parent || symbol_conf.use_callchain)
+	    && sample->callchain) {
+		err = machine__resolve_callchain(machine, evsel, al->thread,
+						 sample, &parent);
+		if (err)
+			return err;
+	}
+
+	mi = machine__resolve_mem(machine, al->thread, sample, cpumode);
+	if (!mi)
+		return -ENOMEM;
+
+	if (rep->hide_unresolved && !al->sym)
+		return 0;
+
+	cost = mi->cost;
+	if (!cost)
+		cost = 1;
+
+	/*
+	 * The report shows the percentage of total branches captured
+	 * and not events sampled. Thus we use a pseudo period of 1.
+	 * Only in the newt browser we are doing integrated annotation,
+	 * so we don't allocated the extra space needed because the stdio
+	 * code will not use it.
+	 */
+	he = __hists__add_mem_entry(&evsel->hists, al, parent, mi,
+				    cost);
+	if (!he)
+		return -ENOMEM;
+
+	if (sort__has_sym && he->ms.sym && use_browser > 0) {
+		struct annotation *notes = symbol__annotation(he->ms.sym);
+
+		assert(evsel != NULL);
+
+		if (notes->src == NULL && symbol__alloc_hist(he->ms.sym) < 0)
+			goto out;
+
+		err = hist_entry__inc_addr_samples(he, evsel->idx, al->addr);
+		if (err)
+			goto out;
+	}
+
+	if (sort__has_sym && he->mem_info->daddr.sym && use_browser > 0) {
+		struct annotation *notes;
+
+		mx = he->mem_info;
+
+		notes = symbol__annotation(mx->daddr.sym);
+		if (!notes->src
+		    && symbol__alloc_hist(mx->daddr.sym) < 0)
+			goto out;
+
+		err = symbol__inc_addr_samples(mx->daddr.sym,
+					       mx->daddr.map,
+					       evsel->idx,
+					       mx->daddr.al_addr);
+		if (err)
+			goto out;
+	}
+
+	evsel->hists.stats.total_period += sample->period;
+	hists__inc_nr_events(&evsel->hists, PERF_RECORD_SAMPLE);
+	err = 0;
+
+	if (symbol_conf.use_callchain) {
+		err = callchain_append(he->callchain,
+				       &callchain_cursor,
+				       sample->period);
+	}
+out:
+	return err;
+}
+
 static int perf_report__add_branch_hist_entry(struct perf_tool *tool,
 					struct addr_location *al,
 					struct perf_sample *sample,

@@ -210,6 +301,12 @@ static int process_sample_event(struct perf_tool *tool,
 			pr_debug("problem adding lbr entry, skipping event\n");
 			return -1;
 		}
+	} else if (rep->mem_mode == 1) {
+		if (perf_report__add_mem_hist_entry(tool, &al, sample,
+						    evsel, machine, event)) {
+			pr_debug("problem adding mem entry, skipping event\n");
+			return -1;
+		}
 	} else {
 		if (al.map != NULL)
 			al.map->dso->hit = 1;

@@ -293,7 +390,8 @@ static void sig_handler(int sig __maybe_unused)
 	session_done = 1;
 }
 
-static size_t hists__fprintf_nr_sample_events(struct hists *self,
+static size_t hists__fprintf_nr_sample_events(struct perf_report *rep,
+					      struct hists *self,
 					      const char *evname, FILE *fp)
 {
 	size_t ret;

@@ -306,7 +404,11 @@ static size_t hists__fprintf_nr_sample_events(struct hists *self,
 	if (evname != NULL)
 		ret += fprintf(fp, " of event '%s'", evname);
 
-	ret += fprintf(fp, "\n# Event count (approx.): %" PRIu64, nr_events);
+	if (rep->mem_mode) {
+		ret += fprintf(fp, "\n# Total cost : %" PRIu64, nr_events);
+		ret += fprintf(fp, "\n# Sort order : %s", sort_order);
+	} else
+		ret += fprintf(fp, "\n# Event count (approx.): %" PRIu64, nr_events);
 	return ret + fprintf(fp, "\n#\n");
 }

@@ -320,7 +422,7 @@ static int perf_evlist__tty_browse_hists(struct perf_evlist *evlist,
 		struct hists *hists = &pos->hists;
 		const char *evname = perf_evsel__name(pos);
 
-		hists__fprintf_nr_sample_events(hists, evname, stdout);
+		hists__fprintf_nr_sample_events(rep, hists, evname, stdout);
 		hists__fprintf(hists, true, 0, 0, stdout);
 		fprintf(stdout, "\n\n");
 	}

@@ -596,7 +698,7 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
 		    "Use the stdio interface"),
 	OPT_STRING('s', "sort", &sort_order, "key[,key2...]",
 		   "sort by key(s): pid, comm, dso, symbol, parent, dso_to,"
-		   " dso_from, symbol_to, symbol_from, mispredict"),
+		   " dso_from, symbol_to, symbol_from, mispredict, mem, cost, symbol_daddr, dso_daddr, tlb, snoop, locked"),
 	OPT_BOOLEAN(0, "showcpuutilization", &symbol_conf.show_cpu_utilization,
 		    "Show sample percentage for different cpu modes"),
 	OPT_STRING('p', "parent", &parent_pattern, "regex",

@@ -642,6 +744,7 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
 		    "use branch records for histogram filling", parse_branch_mode),
 	OPT_STRING(0, "objdump", &objdump_path, "path",
 		   "objdump binary to use for disassembly and annotations"),
+	OPT_BOOLEAN(0, "mem-mode", &report.mem_mode, "mem access profile"),
 	OPT_END()
 	};

@@ -693,6 +796,18 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
 				     "dso_to,symbol_to";
 
 	}
+	if (report.mem_mode) {
+		if (sort__branch_mode == 1) {
+			fprintf(stderr, "branch and mem mode incompatible\n");
+			goto error;
+		}
+		/*
+		 * if no sort_order is provided, then specify
+		 * branch-mode specific order
+		 */
+		if (sort_order == default_sort_order)
+			sort_order = "cost,mem,sym,dso,symbol_daddr,dso_daddr,snoop,tlb,locked";
+	}
 
 	if (strcmp(report.input_name, "-") != 0)
 		setup_browser(true);

@@ -764,6 +879,14 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
 		sort_entry__setup_elide(&sort_sym_from, symbol_conf.sym_from_list, "sym_from", stdout);
 		sort_entry__setup_elide(&sort_sym_to, symbol_conf.sym_to_list, "sym_to", stdout);
 	} else {
+		if (report.mem_mode) {
+			sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "symbol_daddr", stdout);
+			sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "dso_daddr", stdout);
+			sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "mem", stdout);
+			sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "cost", stdout);
+			sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "tlb", stdout);
+			sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "snoop", stdout);
+		}
 		sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "dso", stdout);
 		sort_entry__setup_elide(&sort_sym, symbol_conf.sym_list, "symbol", stdout);
 	}

Signature · 1 line

1.7.9.5

Namhyung Kim

maintainer

2012-10-31 06:01 UTC #32 lore raw

Re: [Patch v1 08/10] perf report: add support for mem access profiling

Quoted 7 lines · Stephane Eranian

This patch adds the --mem-mode option to perf report.
This mode requires a perf.data file created with memory access samples.
Signed-off-by Stephane Eranian <[email protected]>
---

[snip]

tools/perf/builtin-report.c struct perf_report { · line 95

Quoted 7 lines

tools/perf/builtin-report.c +7 -0

+	cost = mi->cost;
+	if (!cost)
+		cost = 1;
+
+	/*
+	 * The report shows the percentage of total branches captured
+	 * and not events sampled. Thus we use a pseudo period of 1.

But cost won't be 1 anymore if PERF_SAMPLE_COST set, right?

tools/perf/builtin-report.c struct perf_report { · line 98

Quoted 3 lines

tools/perf/builtin-report.c +3 -0

+	 * Only in the newt browser we are doing integrated annotation,
+	 * so we don't allocated the extra space needed because the stdio
+	 * code will not use it.

Yes, and gtk too.

tools/perf/builtin-report.c struct perf_report { · line 105

Quoted 7 lines

tools/perf/builtin-report.c +7 -0

+	 */
+	he = __hists__add_mem_entry(&evsel->hists, al, parent, mi,
+				    cost);
+	if (!he)
+		return -ENOMEM;
+
+	if (sort__has_sym && he->ms.sym && use_browser > 0) {

So I'd rather write 'use_browser == 1' instead of '> 0'.

tools/perf/builtin-report.c struct perf_report { · line 118

Quoted 13 lines

tools/perf/builtin-report.c +13 -0

+		struct annotation *notes = symbol__annotation(he->ms.sym);
+
+		assert(evsel != NULL);
+
+		if (notes->src == NULL && symbol__alloc_hist(he->ms.sym) < 0)
+			goto out;
+
+		err = hist_entry__inc_addr_samples(he, evsel->idx, al->addr);
+		if (err)
+			goto out;
+	}
+
+	if (sort__has_sym && he->mem_info->daddr.sym && use_browser > 0) {

Ditto.

Thanks,
Namhyung

Stephane Eranian

author

2012-10-29 15:16 UTC #33 lore raw

PATCH 9/10 perf record: add support for mem access profiling

In reply to Stephane Eranian

Add the -l option to perf record to enable sampling access cost sampling.

Data address sampling is obtained via the -d option.

Signed-off-by Stephane Eranian <[email protected]>

---
 tools/perf/builtin-record.c |    2 ++
 tools/perf/perf.h           |    1 +
 tools/perf/util/evsel.c     |    6 ++++++
 3 files changed, 9 insertions(+)

tools/perf/builtin-record.c +2 -0

index 53c9892..cafe5d9 100644
@@ -1040,6 +1040,8 @@ const struct option record_options[] = {
 	OPT_CALLBACK('j', "branch-filter", &record.opts.branch_stack,
 		     "branch filter mask", "branch stack filter modes",
 		     parse_branch_stack),
+	OPT_BOOLEAN('l', "cost", &record.opts.cost,
+		    "event cost"),
 	OPT_END()
 };

tools/perf/perf.h +1 -0

index c50985e..ea2db82 100644
@@ -236,6 +236,7 @@ struct perf_record_opts {
 	bool	     sample_id_all_missing;
 	bool	     exclude_guest_missing;
 	bool	     period;
+	bool         cost;
 	unsigned int freq;
 	unsigned int mmap_pages;
 	unsigned int user_freq;

tools/perf/util/evsel.c +6 -0

index c82a271..6f84fbc 100644
@@ -474,6 +474,12 @@ void perf_evsel__config(struct perf_evsel *evsel, struct perf_record_opts *opts,
 		attr->sample_type	|= PERF_SAMPLE_CPU;
 	}
 
+	if (opts->cost)
+		attr->sample_type	|= PERF_SAMPLE_COST;
+
+	if (opts->sample_address)
+		attr->sample_type	|= PERF_SAMPLE_DSRC;
+
 	if (opts->no_delay) {
 		attr->watermark = 0;
 		attr->wakeup_events = 1;

Signature · 1 line

1.7.9.5

Stephane Eranian

author

2012-10-29 15:17 UTC #34 lore raw

PATCH 10/10 perf tools: add new mem command for memory access profiling

In reply to Stephane Eranian

This new command is a wrapper on top of perf record and perf report to make it easier to configure for memory access profiling.

To record loads:

$ perf mem -t load rec .....

To record stores:

$ perf mem -t store rec .....

To get the report:

$ perf mem -t load rep

Signed-off-by Stephane Eranian <[email protected]>

---
 tools/perf/Documentation/perf-mem.txt |   51 +++++++
 tools/perf/Makefile                   |    1 +
 tools/perf/builtin-mem.c              |  237 +++++++++++++++++++++++++++++++++
 tools/perf/builtin.h                  |    1 +
 tools/perf/command-list.txt           |    1 +
 tools/perf/perf.c                     |    1 +
 6 files changed, 292 insertions(+)
 create mode 100644 tools/perf/Documentation/perf-mem.txt
 create mode 100644 tools/perf/builtin-mem.c

tools/perf/Documentation/perf-mem.txt new file +51 -0

new file mode 100644
index 0000000..57ead4f
@@ -0,0 +1,51 @@
+perf-mem(1)
+===========
+
+NAME
+----
+perf-mem - Profile memory accesses
+
+SYNOPSIS
+--------
+[verse]
+'perf mem' -t load record <command>
+'perf mem' -t store record <command>
+'perf mem' -t load report
+'perf mem' -t store report
+
+DESCRIPTION
+-----------
+"perf mem -t <TYPE> record" runs a command and gathers memory operation data
+from it, into perf.data. Perf record options are accepted and are passed through.
+
+"perf mem -t <TYPE> report" displays the result. It invokes perf report with the
+right set of options to display a memory access profile.
+
+OPTIONS
+-------
+<command>...::
+	Any command you can specify in a shell.
+
+-t::
+--type=::
+	Select the memory operation type: load or store
+
+-R::
+--dump-raw-samples=::
+	Dump the raw decoded samples on the screen in a format that is easy to parse with
+	one sample per line.
+
+-x::
+--field-separator::
+	Specify the field separator used when dump raw samples (-R option). By default,
+	The separator is the space character.
+
+-C::
+--cpu-list::
+	Restrict dump of raw samples to those provided via this option. Note that the same
+	option can be passed in record mode. It will be interpreted the same way as perf
+	record.
+
+SEE ALSO
+--------
+linkperf:perf-record[1], linkperf:perf-report[1]

tools/perf/Makefile +1 -0

index 629fc6a..643b316 100644
@@ -459,6 +459,7 @@ BUILTIN_OBJS += $(OUTPUT)builtin-lock.o
 BUILTIN_OBJS += $(OUTPUT)builtin-kvm.o
 BUILTIN_OBJS += $(OUTPUT)builtin-test.o
 BUILTIN_OBJS += $(OUTPUT)builtin-inject.o
+BUILTIN_OBJS += $(OUTPUT)builtin-mem.o
 
 PERFLIBS = $(LIB_FILE) $(LIBTRACEEVENT)

tools/perf/builtin-mem.c new file +237 -0

237 lines not on this page · raw message

tools/perf/builtin.h +1 -0

index 08143bd..b210d62 100644
@@ -36,6 +36,7 @@ extern int cmd_kvm(int argc, const char **argv, const char *prefix);
 extern int cmd_test(int argc, const char **argv, const char *prefix);
 extern int cmd_trace(int argc, const char **argv, const char *prefix);
 extern int cmd_inject(int argc, const char **argv, const char *prefix);
+extern int cmd_mem(int argc, const char **argv, const char *prefix);
 
 extern int find_scripts(char **scripts_array, char **scripts_path_array);
 #endif

tools/perf/command-list.txt +1 -0

index 3e86bbd..2c5b621 100644
@@ -24,3 +24,4 @@ perf-kmem			mainporcelain common
 perf-lock			mainporcelain common
 perf-kvm			mainporcelain common
 perf-test			mainporcelain common
+perf-mem			mainporcelain common

tools/perf/perf.c +1 -0

index d480d8a..15995fa 100644
@@ -59,6 +59,7 @@ static struct cmd_struct commands[] = {
 	{ "trace",	cmd_trace,	0 },
 #endif
 	{ "inject",	cmd_inject,	0 },
+	{ "mem",	cmd_mem,	0 },
 };
 
 struct pager_config {

Signature · 1 line

1.7.9.5

Namhyung Kim

maintainer

2012-10-31 06:58 UTC #35 lore raw

Re: [Patch v1 10/10] perf tools: add new mem command for memory access profiling

Quoted 3 lines · Stephane Eranian

This new command is a wrapper on top of perf record and perf report to make it easier to configure for memory access profiling.

So this new command will be run only on speicific (PEBS > 2?) Intel machines, right? Is there anything we can do for others? Or at least it might emit a warning message..

Quoted 12 lines

To record loads:
$ perf mem -t load rec .....
To record stores:
$ perf mem -t store rec .....
To get the report:
$ perf mem -t load rep
Signed-off-by Stephane Eranian <[email protected]>
---

[snip]

Quoted 4 lines

tools/perf/Documentation/perf-mem.txt new file +4 -0
+perf-mem(1)
+===========
+
+NAME

tools/perf/Documentation/perf-mem.txt line 14

Quoted 10 lines

tools/perf/Documentation/perf-mem.txt new file +10 -0

+----
+perf-mem - Profile memory accesses
+
+SYNOPSIS
+--------
+[verse]
+'perf mem' -t load record <command>
+'perf mem' -t store record <command>
+'perf mem' -t load report
+'perf mem' -t store report

Is '-t' option mandatory? AFAISC it seems optional and defaults to load.

And is <command> for record also mandatory? Doesn't 'perf record' make it optional?

If so, the above can be written like you did in 'mem_usage':

'perf mem' [<options>] (record [<command>] | report)

Quoted 7 lines

tools/perf/Documentation/perf-mem.txt new file +7 -0

+
+DESCRIPTION
+-----------
+"perf mem -t <TYPE> record" runs a command and gathers memory operation data
+from it, into perf.data. Perf record options are accepted and are passed through.
+
+"perf mem -t <TYPE> report" displays the result. It invokes perf report with the

tools/perf/Documentation/perf-mem.txt line 31

Quoted 10 lines

tools/perf/Documentation/perf-mem.txt new file +10 -0

+right set of options to display a memory access profile.
+
+OPTIONS
+-------
+<command>...::
+	Any command you can specify in a shell.
+
+-t::
+--type=::
+	Select the memory operation type: load or store

It'd better saying it defaults to load.

tools/perf/Documentation/perf-mem.txt line 36

Quoted 5 lines

tools/perf/Documentation/perf-mem.txt new file +5 -0

+
+-R::
+--dump-raw-samples=::
+	Dump the raw decoded samples on the screen in a format that is easy to parse with
+	one sample per line.

Didn't we usually use -D switch for this?

tools/perf/Documentation/perf-mem.txt line 41

Quoted 5 lines

tools/perf/Documentation/perf-mem.txt new file +5 -0

+
+-x::
+--field-separator::
+	Specify the field separator used when dump raw samples (-R option). By default,
+	The separator is the space character.

And using -t for this will make it consistent with perf report IMHO.

Quoted 10 lines

tools/perf/Documentation/perf-mem.txt new file +10 -0

+
+-C::
+--cpu-list::
+	Restrict dump of raw samples to those provided via this option. Note that the same
+	option can be passed in record mode. It will be interpreted the same way as perf
+	record.
+
+SEE ALSO
+--------
+linkperf:perf-record[1], linkperf:perf-report[1]

[snip]

tools/perf/builtin-mem.c line 12

Quoted 4 lines

tools/perf/builtin-mem.c new file +4 -0

+#define MEM_OPERATION_LOAD	"load"
+#define MEM_OPERATION_STORE	"store"
+
+static char const	*input_name		= "perf.data";

We have a global 'input_name' as of commit 70cb4e963f77 ("perf tools:
Add a global variable 'const char *input_name'").

tools/perf/builtin-mem.c line 14

Quoted 2 lines

tools/perf/builtin-mem.c new file +2 -0

+static const char	*mem_operation		= MEM_OPERATION_LOAD;
+static const char	*csv_sep		= NULL;

Why not use symbol_conf.field_sep?

Quoted 15 lines

tools/perf/builtin-mem.c new file +15 -0

+
+struct perf_mem {
+	struct perf_tool	tool;
+	char const		*input_name;
+	symbol_filter_t		annotate_init;
+	bool			hide_unresolved;
+	bool			dump_raw;
+	const char		*cpu_list;
+	DECLARE_BITMAP(cpu_bitmap, MAX_NR_CPUS);
+};
+
+static const char * const mem_usage[] = {
+	"perf mem [<options>] {record <command> |report}",
+	NULL
+};

[snip]

Quoted 9 lines

tools/perf/builtin-mem.c new file +9 -0

+static int report_raw_events(struct perf_mem *mem)
+{
+	int err = -EINVAL;
+	int ret;
+	struct perf_session *session = perf_session__new(input_name, O_RDONLY,
+							 0, false, &mem->tool);
+
+	if (mem->cpu_list) {
+		ret = perf_session__cpu_bitmap(session, mem->cpu_list,

tools/perf/builtin-mem.c line 140

Quoted 10 lines

tools/perf/builtin-mem.c new file +10 -0

+					       mem->cpu_bitmap);
+		if (ret)
+			goto out_delete;
+	}
+
+	if (symbol__init() < 0)
+		return -1;
+
+	if (session == NULL)
+		return -ENOMEM;

This check should be moved before perf_session__cpu_bitmap() calls.

Thanks,
Namhyung

Quoted 13 lines

tools/perf/builtin-mem.c new file +13 -0

+
+	printf("# PID, TID, IP, ADDR, COST, DSRC, SYMBOL\n");
+
+	err = perf_session__process_events(session, &mem->tool);
+	if (err)
+		return err;
+
+	return 0;
+
+out_delete:
+	perf_session__delete(session);
+	return err;
+}

Stephane Eranian

author

2012-10-31 14:23 UTC #36 lore raw

Re: [Patch v1 10/10] perf tools: add new mem command for memory access profiling

Quoted 22 lines · Namhyung Kim

Quoted 3 lines · Stephane Eranian
This new command is a wrapper on top of perf record and perf report to make it easier to configure for memory access profiling.
So this new command will be run only on speicific (PEBS > 2?) Intel machines, right? Is there anything we can do for others? Or at least it might emit a warning message..
Quoted 12 lines
To record loads:
$ perf mem -t load rec .....
To record stores:
$ perf mem -t store rec .....
To get the report:
$ perf mem -t load rep
Signed-off-by Stephane Eranian <[email protected]>
---
[snip]

Quoted 17 lines

Quoted 4 lines
tools/perf/Documentation/perf-mem.txt new file +4 -0
+perf-mem(1)
+===========
+
+NAME
tools/perf/Documentation/perf-mem.txt line 14
Quoted 10 lines
tools/perf/Documentation/perf-mem.txt new file +10 -0
+----
+perf-mem - Profile memory accesses
+
+SYNOPSIS
+--------
+[verse]
+'perf mem' -t load record <command>
+'perf mem' -t store record <command>
+'perf mem' -t load report
+'perf mem' -t store report
Is '-t' option mandatory? AFAISC it seems optional and defaults to load.

Yes, defaults to load. Fixed.

Quoted 7 lines

And is <command> for record also mandatory? Doesn't 'perf record' make it optional?
If so, the above can be written like you did in 'mem_usage':
'perf mem' [<options>] (record [<command>] | report)

Fixed.

Quoted 12 lines

tools/perf/Documentation/perf-mem.txt new file +11 -0

+
+DESCRIPTION
+-----------
+"perf mem -t <TYPE> record" runs a command and gathers memory operation data
+from it, into perf.data. Perf record options are accepted and are passed through.
+
+"perf mem -t <TYPE> report" displays the result. It invokes perf report with the
+right set of options to display a memory access profile.
+
+OPTIONS
+-------

Quoted 9 lines

tools/perf/Documentation/perf-mem.txt line 31
Quoted 6 lines
+<command>...::
tools/perf/Documentation/perf-mem.txt new file +5 -0
+     Any command you can specify in a shell.
+
+-t::
+--type=::
+     Select the memory operation type: load or store
It'd better saying it defaults to load.

Done.

Quoted 8 lines

tools/perf/Documentation/perf-mem.txt line 36
Quoted 5 lines
tools/perf/Documentation/perf-mem.txt new file +5 -0
+
+-R::
+--dump-raw-samples=::
+     Dump the raw decoded samples on the screen in a format that is easy to parse with
+     one sample per line.
Didn't we usually use -D switch for this?

Using -D to be consistent with report.

Quoted 9 lines

tools/perf/Documentation/perf-mem.txt line 41
Quoted 5 lines
tools/perf/Documentation/perf-mem.txt new file +5 -0
+
+-x::
+--field-separator::
+     Specify the field separator used when dump raw samples (-R option). By default,
+     The separator is the space character.
And using -t for this will make it consistent with perf report IMHO.
Quoted 1 line
+

No it's -x there too.

Quoted 10 lines

Quoted 9 lines

+-C::
+--cpu-list::

tools/perf/Documentation/perf-mem.txt new file +7 -0

+     Restrict dump of raw samples to those provided via this option. Note that the same
+     option can be passed in record mode. It will be interpreted the same way as perf
+     record.
+
+SEE ALSO
+--------
+linkperf:perf-record[1], linkperf:perf-report[1]

[snip]

Quoted 9 lines

tools/perf/builtin-mem.c line 12
Quoted 4 lines
tools/perf/builtin-mem.c new file +4 -0
+#define MEM_OPERATION_LOAD   "load"
+#define MEM_OPERATION_STORE  "store"
+
+static char const    *input_name             = "perf.data";
We have a global 'input_name' as of commit 70cb4e963f77 ("perf tools:
Add a global variable 'const char *input_name'").

Yes. Fixed.

Quoted 5 lines

tools/perf/builtin-mem.c line 14
Quoted 2 lines
tools/perf/builtin-mem.c new file +2 -0
+static const char    *mem_operation          = MEM_OPERATION_LOAD;
+static const char    *csv_sep                = NULL;
Why not use symbol_conf.field_sep?

Done.

Quoted 29 lines

Quoted 15 lines

tools/perf/builtin-mem.c new file +15 -0

+
+struct perf_mem {
+     struct perf_tool        tool;
+     char const              *input_name;
+     symbol_filter_t         annotate_init;
+     bool                    hide_unresolved;
+     bool                    dump_raw;
+     const char              *cpu_list;
+     DECLARE_BITMAP(cpu_bitmap, MAX_NR_CPUS);
+};
+
+static const char * const mem_usage[] = {
+     "perf mem [<options>] {record <command> |report}",
+     NULL
+};

[snip]

Quoted 13 lines

tools/perf/builtin-mem.c new file +13 -0

+static int report_raw_events(struct perf_mem *mem)
+{
+     int err = -EINVAL;
+     int ret;
+     struct perf_session *session = perf_session__new(input_name, O_RDONLY,
+                                                      0, false, &mem->tool);
+
+     if (mem->cpu_list) {
+             ret = perf_session__cpu_bitmap(session, mem->cpu_list,
+                                            mem->cpu_bitmap);
+             if (ret)
+                     goto out_delete;
+     }

Quoted 9 lines

tools/perf/builtin-mem.c line 140
Quoted 6 lines
tools/perf/builtin-mem.c new file +6 -0
+
+     if (symbol__init() < 0)
+             return -1;
+
+     if (session == NULL)
+             return -ENOMEM;
This check should be moved before perf_session__cpu_bitmap() calls.

Yes.

Thanks.