This is the latest version of the perf offcore / extra regs
patchkit. I addressed all earlier comments I believe and
this should be ready for merge now.
- Allocating multiple registers is supported now
- I kept the percore allocation because the manual
kmalloc method turned out to be more complicated and
alloc_percpu() really already did what I wanted.
However I switched to using the CPU notifier to set up the
topology for hotplug, together with a bugfix to make sure
x86 sets up the topology before calling it.
- A few other improvements.
-Andi
From: Andi Kleen <[email protected]>
Signed-off-by: Andi Kleen <[email protected]>
---
tools/perf/Documentation/perf-list.txt | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/tools/perf/Documentation/perf-list.txt b/tools/perf/Documentation/perf-list.txt
index 399751b..700afd7 100644
--- a/tools/perf/Documentation/perf-list.txt
+++ b/tools/perf/Documentation/perf-list.txt
@@ -58,6 +58,13 @@ raw encoding of 0x1A8 can be used:
perf stat -e r1a8 -a sleep 1
perf record -e r1a8 ...
+Some special events on x86 encode additional data in the normally unused
+[32;63] bits of the raw value. This is particularly used for
+the OFFCORE_RESPONSE events on Intel Core i7. The 16bit
+mask in the OFFCORE_RESPONSE register is put into bits [32;48].
+For example the OFFCORE_RESPONSE_0.ANY_DATA.ANY_CACHE_DRAM event
+is encoded as r00007f11004301b7.
+
You should refer to the processor specific documentation for getting these
details. Some of them are referenced in the SEE ALSO section below.
--
1.7.1
From: Andi Kleen <[email protected]>
Intel Nehalem/Westmere have a special OFFCORE_RESPONSE event
that can be used to monitor any offcore accesses from a core.
This is a very useful event for various tunings, and it's
also needed to implement the generic LLC-* events correctly.
Unfortunately this event requires programming a mask in a separate
register. And worse this separate register is per core, not per
CPU thread.
This patch adds:
- Teaches perf_events that OFFCORE_RESPONSE needs extra parameters.
The extra parameters are passed by user space in the unused upper
32bits of the config word.
- Add support to the Intel perf_event core to schedule per
core resources. This adds fairly generic infrastructure that
can be also used for other per core resources.
The basic code has is patterned after the similar AMD northbridge
constraints code.
Thanks to Stephane Eranian who pointed out some problems
in the original version and suggested improvements.
Full git tree:
git://git.kernel.org/pub/scm/linux/kernel/git/ak/linux-misc-2.6.git perf-offcore2
Cc: [email protected]
v2: Lots of updates based on review feedback. Also fixes some issues
v3: Fix hotplug. Handle multiple extra registers. Fix init order.
Various improvements.
Signed-off-by: Andi Kleen <[email protected]>
---
arch/x86/kernel/cpu/perf_event.c | 70 ++++++++++++
arch/x86/kernel/cpu/perf_event_intel.c | 192 ++++++++++++++++++++++++++++++++
include/linux/perf_event.h | 2 +
3 files changed, 264 insertions(+), 0 deletions(-)
diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
index ed63101..346006a 100644
--- a/arch/x86/kernel/cpu/perf_event.c
+++ b/arch/x86/kernel/cpu/perf_event.c
@@ -93,6 +93,8 @@ struct amd_nb {
struct event_constraint event_constraints[X86_PMC_IDX_MAX];
};
+struct intel_percore;
+
#define MAX_LBR_ENTRIES 16
struct cpu_hw_events {
@@ -127,6 +129,13 @@ struct cpu_hw_events {
struct perf_branch_stack lbr_stack;
struct perf_branch_entry lbr_entries[MAX_LBR_ENTRIES];
+ /*
+ * Intel percore register state.
+ * Coordinate shared resources between HT threads.
+ */
+ int percore_used; /* Used by this CPU? */
+ struct intel_percore *per_core;
+
/*
* AMD specific bits
*/
@@ -175,6 +184,32 @@ struct cpu_hw_events {
#define for_each_event_constraint(e, c) \
for ((e) = (c); (e)->weight; (e)++)
+/*
+ * Extra registers for specific events.
+ * Some events need large masks and require external MSRs.
+ * Define a mapping to these extra registers.
+ * The actual contents are still encoded in unused parts of the
+ * original config u64.
+ */
+struct extra_reg {
+ unsigned int event;
+ unsigned int msr;
+ unsigned int extra_shift;
+ u64 config_mask;
+ u64 valid_mask;
+};
+
+#define EVENT_EXTRA_REG(e, ms, m, vm, es) { \
+ .event = (e), \
+ .msr = (ms), \
+ .config_mask = (m), \
+ .valid_mask = (vm), \
+ .extra_shift = (es), \
+ }
+#define INTEL_EVENT_EXTRA_REG(event, msr, vm, es) \
+ EVENT_EXTRA_REG(event, msr, ARCH_PERFMON_EVENTSEL_EVENT, vm, es)
+#define EVENT_EXTRA_END EVENT_EXTRA_REG(0, 0, 0, 0, 0)
+
union perf_capabilities {
struct {
u64 lbr_format : 6;
@@ -219,6 +254,7 @@ struct x86_pmu {
void (*put_event_constraints)(struct cpu_hw_events *cpuc,
struct perf_event *event);
struct event_constraint *event_constraints;
+ struct event_constraint *percore_constraints;
void (*quirks)(void);
int perfctr_second_write;
@@ -247,6 +283,11 @@ struct x86_pmu {
*/
unsigned long lbr_tos, lbr_from, lbr_to; /* MSR base regs */
int lbr_nr; /* hardware stack size */
+
+ /*
+ * Extra registers for events
+ */
+ struct extra_reg *extra_regs;
};
static struct x86_pmu x86_pmu __read_mostly;
@@ -321,6 +362,33 @@ again:
return new_raw_count;
}
+/*
+ * Find and validate any extra registers to set up.
+ */
+static int x86_pmu_extra_regs(u64 config, struct perf_event *event)
+{
+ struct extra_reg *er;
+ u64 extra;
+
+ event->hw.extra_reg = 0;
+ event->hw.extra_config = 0;
+
+ if (!x86_pmu.extra_regs)
+ return 0;
+
+ for (er = x86_pmu.extra_regs; er->msr; er++) {
+ if (er->event != (config & er->config_mask))
+ continue;
+ event->hw.extra_reg = er->msr;
+ extra = config >> er->extra_shift;
+ if (extra & ~er->valid_mask)
+ return -EINVAL;
+ event->hw.extra_config = extra;
+ break;
+ }
+ return 0;
+}
+
static atomic_t active_events;
static DEFINE_MUTEX(pmc_reserve_mutex);
@@ -876,6 +944,8 @@ static inline void __x86_pmu_enable_event(struct hw_perf_event *hwc,
u64 enable_mask)
{
wrmsrl(hwc->config_base + hwc->idx, hwc->config | enable_mask);
+ if (hwc->extra_reg)
+ wrmsrl(hwc->extra_reg, hwc->extra_config);
}
static inline void x86_pmu_disable_event(struct perf_event *event)
diff --git a/arch/x86/kernel/cpu/perf_event_intel.c b/arch/x86/kernel/cpu/perf_event_intel.c
index c8f5c08..675a666 100644
--- a/arch/x86/kernel/cpu/perf_event_intel.c
+++ b/arch/x86/kernel/cpu/perf_event_intel.c
@@ -1,5 +1,25 @@
#ifdef CONFIG_CPU_SUP_INTEL
+/*
+ * Per core state
+ * This used to coordinate shared resources for HT threads.
+ * Exists per CPU, but only the entry for the first CPU
+ * in the core is used.
+ */
+#define MAX_EXTRA_REGS 2
+
+struct er_account {
+ int ref; /* reference count */
+ unsigned int extra_reg; /* extra MSR number */
+ u64 extra_config; /* extra MSR config */
+};
+
+struct intel_percore {
+ raw_spinlock_t lock; /* protect structure */
+ struct er_account regs[MAX_EXTRA_REGS];
+};
+static struct intel_percore __percpu *intel_percore;
+
/*
* Intel PerfMon, used on Core and later.
*/
@@ -64,6 +84,18 @@ static struct event_constraint intel_nehalem_event_constraints[] =
EVENT_CONSTRAINT_END
};
+static struct extra_reg intel_nehalem_extra_regs[] =
+{
+ INTEL_EVENT_EXTRA_REG(0xb7, 0x1a6, 0xffff, 32), /* OFFCORE_RESPONSE_0 */
+ EVENT_EXTRA_END
+};
+
+static struct event_constraint intel_nehalem_percore_constraints[] =
+{
+ INTEL_EVENT_CONSTRAINT(0xb7, 0),
+ EVENT_CONSTRAINT_END
+};
+
static struct event_constraint intel_westmere_event_constraints[] =
{
FIXED_EVENT_CONSTRAINT(0x00c0, 0), /* INST_RETIRED.ANY */
@@ -76,6 +108,20 @@ static struct event_constraint intel_westmere_event_constraints[] =
EVENT_CONSTRAINT_END
};
+static struct extra_reg intel_westmere_extra_regs[] =
+{
+ INTEL_EVENT_EXTRA_REG(0xb7, 0x1a6, 0xffff, 32), /* OFFCORE_RESPONSE_0 */
+ INTEL_EVENT_EXTRA_REG(0xbb, 0x1a7, 0xffff, 32), /* OFFCORE_RESPONSE_1 */
+ EVENT_EXTRA_END
+};
+
+static struct event_constraint intel_westmere_percore_constraints[] =
+{
+ INTEL_EVENT_CONSTRAINT(0xb7, 0),
+ INTEL_EVENT_CONSTRAINT(0xbb, 0),
+ EVENT_CONSTRAINT_END
+};
+
static struct event_constraint intel_gen_event_constraints[] =
{
FIXED_EVENT_CONSTRAINT(0x00c0, 0), /* INST_RETIRED.ANY */
@@ -794,6 +840,66 @@ intel_bts_constraints(struct perf_event *event)
}
static struct event_constraint *
+intel_percore_constraints(struct cpu_hw_events *cpuc, struct perf_event *event)
+{
+ struct hw_perf_event *hwc = &event->hw;
+ unsigned int e = hwc->config & ARCH_PERFMON_EVENTSEL_EVENT;
+ struct event_constraint *c;
+ struct intel_percore *pc;
+ struct er_account *era;
+ int i;
+ int free_slot;
+ int found;
+
+ if (!x86_pmu.percore_constraints)
+ return NULL;
+
+ for (c = x86_pmu.percore_constraints; c->cmask; c++) {
+ if (e != c->code)
+ continue;
+
+ /*
+ * Allocate resource per core.
+ */
+ c = NULL;
+ pc = cpuc->per_core;
+ if (!pc)
+ break;
+ c = &emptyconstraint;
+ raw_spin_lock(&pc->lock);
+ free_slot = -1;
+ found = 0;
+ for (i = 0; i < MAX_EXTRA_REGS; i++) {
+ era = &pc->regs[i];
+ if (era->ref > 0 && hwc->extra_reg == era->extra_reg) {
+ /* Allow sharing same config */
+ if (hwc->extra_config == era->extra_config) {
+ era->ref++;
+ cpuc->percore_used = 1;
+ c = NULL;
+ }
+ /* else conflict */
+ found = 1;
+ break;
+ } else if (era->ref == 0 && free_slot == -1)
+ free_slot = i;
+ }
+ if (!found && free_slot != -1) {
+ era = &pc->regs[free_slot];
+ era->ref = 1;
+ era->extra_reg = hwc->extra_reg;
+ era->extra_config = hwc->extra_config;
+ cpuc->percore_used = 1;
+ c = NULL;
+ }
+ raw_spin_unlock(&pc->lock);
+ return c;
+ }
+
+ return NULL;
+}
+
+static struct event_constraint *
intel_get_event_constraints(struct cpu_hw_events *cpuc, struct perf_event *event)
{
struct event_constraint *c;
@@ -806,9 +912,50 @@ intel_get_event_constraints(struct cpu_hw_events *cpuc, struct perf_event *event
if (c)
return c;
+ c = intel_percore_constraints(cpuc, event);
+ if (c)
+ return c;
+
return x86_get_event_constraints(cpuc, event);
}
+static void intel_put_event_constraints(struct cpu_hw_events *cpuc,
+ struct perf_event *event)
+{
+ struct extra_reg *er;
+ struct intel_percore *pc;
+ struct er_account *era;
+ struct hw_perf_event *hwc = &event->hw;
+ int i, allref;
+
+ if (!cpuc->percore_used)
+ return;
+
+ for (er = x86_pmu.extra_regs; er->msr; er++) {
+ if (er->event != (hwc->config & er->config_mask))
+ continue;
+
+ pc = cpuc->per_core;
+ raw_spin_lock(&pc->lock);
+ for (i = 0; i < MAX_EXTRA_REGS; i++) {
+ era = &pc->regs[i];
+ if (era->ref > 0 &&
+ era->extra_config == hwc->extra_config &&
+ era->extra_reg == er->msr) {
+ era->ref--;
+ break;
+ }
+ }
+ allref = 0;
+ for (i = 0; i < MAX_EXTRA_REGS; i++)
+ allref += pc->regs[i].ref;
+ if (allref == 0)
+ cpuc->percore_used = 0;
+ raw_spin_unlock(&pc->lock);
+ break;
+ }
+}
+
static int intel_pmu_hw_config(struct perf_event *event)
{
int ret = x86_pmu_hw_config(event);
@@ -854,11 +1001,25 @@ static __initconst const struct x86_pmu core_pmu = {
*/
.max_period = (1ULL << 31) - 1,
.get_event_constraints = intel_get_event_constraints,
+ .put_event_constraints = intel_put_event_constraints,
.event_constraints = intel_core_event_constraints,
};
static void intel_pmu_cpu_starting(int cpu)
{
+ int leader;
+
+ /*
+ * When this called for the boot CPU the sibling information is not
+ * set up yet.
+ */
+ if (cpu == 0)
+ leader = 0;
+ else
+ leader = cpumask_first(topology_thread_cpumask(cpu));
+ per_cpu(cpu_hw_events, cpu).per_core =
+ per_cpu_ptr(intel_percore, leader);
+
init_debug_store_on_cpu(cpu);
/*
* Deal with CPUs that don't clear their LBRs on power-up.
@@ -892,6 +1053,7 @@ static __initconst const struct x86_pmu intel_pmu = {
*/
.max_period = (1ULL << 31) - 1,
.get_event_constraints = intel_get_event_constraints,
+ .put_event_constraints = intel_put_event_constraints,
.cpu_starting = intel_pmu_cpu_starting,
.cpu_dying = intel_pmu_cpu_dying,
@@ -923,6 +1085,28 @@ static void intel_clovertown_quirks(void)
x86_pmu.pebs_constraints = NULL;
}
+static int __initdata needs_percore;
+
+static __init int init_intel_percore(void)
+{
+ int cpu;
+
+ if (!needs_percore)
+ return 0;
+
+ intel_percore = alloc_percpu(struct intel_percore);
+ if (!intel_percore)
+ return -ENOMEM;
+ for_each_possible_cpu(cpu)
+ raw_spin_lock_init(&per_cpu_ptr(intel_percore, cpu)->lock);
+
+ return 0;
+}
+/*
+ * Runs later because per cpu allocations don't work early on.
+ */
+__initcall(init_intel_percore);
+
static __init int intel_pmu_init(void)
{
union cpuid10_edx edx;
@@ -1010,7 +1194,11 @@ static __init int intel_pmu_init(void)
intel_pmu_lbr_init_nhm();
x86_pmu.event_constraints = intel_nehalem_event_constraints;
+ x86_pmu.percore_constraints =
+ intel_nehalem_percore_constraints;
x86_pmu.enable_all = intel_pmu_nhm_enable_all;
+ x86_pmu.extra_regs = intel_nehalem_extra_regs;
+ needs_percore = 1;
pr_cont("Nehalem events, ");
break;
@@ -1032,7 +1220,11 @@ static __init int intel_pmu_init(void)
intel_pmu_lbr_init_nhm();
x86_pmu.event_constraints = intel_westmere_event_constraints;
+ x86_pmu.percore_constraints =
+ intel_westmere_percore_constraints;
x86_pmu.enable_all = intel_pmu_nhm_enable_all;
+ x86_pmu.extra_regs = intel_westmere_extra_regs;
+ needs_percore = 1;
pr_cont("Westmere events, ");
break;
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 057bf22..6124e93 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -529,6 +529,8 @@ struct hw_perf_event {
unsigned long event_base;
int idx;
int last_cpu;
+ unsigned int extra_reg;
+ u64 extra_config;
};
struct { /* software */
struct hrtimer hrtimer;
--
1.7.1
From: Andi Kleen <[email protected]>
When booting up a CPU set the various topology masks before
calling the CPU_STARTING notifier. This way the notifier
can actually use the masks.
This is needed for a perf change.
Signed-off-by: Andi Kleen <[email protected]>
---
arch/x86/kernel/smpboot.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 083e99d..9d2980e 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -281,6 +281,10 @@ static void __cpuinit smp_callin(void)
*/
smp_store_cpu_info(cpuid);
+ /* This must be done before setting cpu_online_mask */
+ set_cpu_sibling_map(raw_smp_processor_id());
+ wmb();
+
notify_cpu_starting(cpuid);
/*
@@ -322,10 +326,6 @@ notrace static void __cpuinit start_secondary(void *unused)
legacy_pic->unmask(0);
}
- /* This must be done before setting cpu_online_mask */
- set_cpu_sibling_map(raw_smp_processor_id());
- wmb();
-
/*
* We need to hold call_lock, so there is no inconsistency
* between the time smp_call_function() determines number of
--
1.7.1
From: Andi Kleen <[email protected]>
The generic perf LLC-* events do count the L2 caches, not the real
L3 LLC on Intel Nehalem and Westmere. This lead to quite some confusion.
Fixing this properly requires use of the special OFFCORE_RESPONSE
events which need a separate mask register. This has been implemented
in a earlier patch.
Now use this infrastructure to set correct events for the LLC-*
on Nehalem and Westmere
Signed-off-by: Andi Kleen <[email protected]>
---
arch/x86/kernel/cpu/perf_event.c | 12 ++++----
arch/x86/kernel/cpu/perf_event_intel.c | 46 +++++++++++++++++++++++---------
2 files changed, 39 insertions(+), 19 deletions(-)
diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
index 346006a..b1abe89 100644
--- a/arch/x86/kernel/cpu/perf_event.c
+++ b/arch/x86/kernel/cpu/perf_event.c
@@ -467,8 +467,9 @@ static inline int x86_pmu_initialized(void)
}
static inline int
-set_ext_hw_attr(struct hw_perf_event *hwc, struct perf_event_attr *attr)
+set_ext_hw_attr(struct hw_perf_event *hwc, struct perf_event *event)
{
+ struct perf_event_attr *attr = &event->attr;
unsigned int cache_type, cache_op, cache_result;
u64 config, val;
@@ -494,9 +495,8 @@ set_ext_hw_attr(struct hw_perf_event *hwc, struct perf_event_attr *attr)
if (val == -1)
return -EINVAL;
- hwc->config |= val;
-
- return 0;
+ hwc->config |= val & X86_RAW_EVENT_MASK;
+ return x86_pmu_extra_regs(val, event);
}
static int x86_setup_perfctr(struct perf_event *event)
@@ -521,10 +521,10 @@ static int x86_setup_perfctr(struct perf_event *event)
}
if (attr->type == PERF_TYPE_RAW)
- return 0;
+ return x86_pmu_extra_regs(event->attr.config, event);
if (attr->type == PERF_TYPE_HW_CACHE)
- return set_ext_hw_attr(hwc, attr);
+ return set_ext_hw_attr(hwc, event);
if (attr->config >= x86_pmu.max_events)
return -EINVAL;
diff --git a/arch/x86/kernel/cpu/perf_event_intel.c b/arch/x86/kernel/cpu/perf_event_intel.c
index 675a666..7017ef7 100644
--- a/arch/x86/kernel/cpu/perf_event_intel.c
+++ b/arch/x86/kernel/cpu/perf_event_intel.c
@@ -86,7 +86,7 @@ static struct event_constraint intel_nehalem_event_constraints[] =
static struct extra_reg intel_nehalem_extra_regs[] =
{
- INTEL_EVENT_EXTRA_REG(0xb7, 0x1a6, 0xffff, 32), /* OFFCORE_RESPONSE_0 */
+ INTEL_EVENT_EXTRA_REG(0xb7, 0x1a6, 0xffff, 32), /* OFFCORE_RESPONSE */
EVENT_EXTRA_END
};
@@ -170,16 +170,26 @@ static __initconst const u64 westmere_hw_cache_event_ids
},
[ C(LL ) ] = {
[ C(OP_READ) ] = {
- [ C(RESULT_ACCESS) ] = 0x0324, /* L2_RQSTS.LOADS */
- [ C(RESULT_MISS) ] = 0x0224, /* L2_RQSTS.LD_MISS */
+ /* OFFCORE_RESPONSE_0.ANY_DATA.LOCAL_CACHE */
+ [ C(RESULT_ACCESS) ] = 0x00000711004301b7,
+ /* OFFCORE_RESPONSE_1.ANY_DATA.ANY_LLC_MISS */
+ [ C(RESULT_MISS) ] = 0x0000f811004301bb,
},
+ /*
+ * Use RFO, not WRITEBACK, because a write miss would typically occur
+ * on RFO.
+ */
[ C(OP_WRITE) ] = {
- [ C(RESULT_ACCESS) ] = 0x0c24, /* L2_RQSTS.RFOS */
- [ C(RESULT_MISS) ] = 0x0824, /* L2_RQSTS.RFO_MISS */
+ /* OFFCORE_RESPONSE_1.ANY_RFO.LOCAL_CACHE */
+ [ C(RESULT_ACCESS) ] = 0x00000722004301bb,
+ /* OFFCORE_RESPONSE_0.ANY_RFO.ANY_LLC_MISS */
+ [ C(RESULT_MISS) ] = 0x0000f822004301b7,
},
[ C(OP_PREFETCH) ] = {
- [ C(RESULT_ACCESS) ] = 0x4f2e, /* LLC Reference */
- [ C(RESULT_MISS) ] = 0x412e, /* LLC Misses */
+ /* OFFCORE_RESPONSE_0.PREFETCH.LOCAL_CACHE */
+ [ C(RESULT_ACCESS) ] = 0x00000770004301b7,
+ /* OFFCORE_RESPONSE_1.PREFETCH.ANY_LLC_MISS */
+ [ C(RESULT_MISS) ] = 0x0000f870004301bb,
},
},
[ C(DTLB) ] = {
@@ -261,16 +271,26 @@ static __initconst const u64 nehalem_hw_cache_event_ids
},
[ C(LL ) ] = {
[ C(OP_READ) ] = {
- [ C(RESULT_ACCESS) ] = 0x0324, /* L2_RQSTS.LOADS */
- [ C(RESULT_MISS) ] = 0x0224, /* L2_RQSTS.LD_MISS */
+ /* OFFCORE_RESPONSE.ANY_DATA.LOCAL_CACHE */
+ [ C(RESULT_ACCESS) ] = 0x00000711004301b7,
+ /* OFFCORE_RESPONSE.ANY_DATA.ANY_LLC_MISS */
+ [ C(RESULT_MISS) ] = 0x0000f811004301b7,
},
+ /*
+ * Use RFO, not WRITEBACK, because a write miss would typically occur
+ * on RFO.
+ */
[ C(OP_WRITE) ] = {
- [ C(RESULT_ACCESS) ] = 0x0c24, /* L2_RQSTS.RFOS */
- [ C(RESULT_MISS) ] = 0x0824, /* L2_RQSTS.RFO_MISS */
+ /* OFFCORE_RESPONSE.ANY_RFO.LOCAL_CACHE */
+ [ C(RESULT_ACCESS) ] = 0x00000722004301b7,
+ /* OFFCORE_RESPONSE.ANY_RFO.ANY_LLC_MISS */
+ [ C(RESULT_MISS) ] = 0x0000f822004301b7,
},
[ C(OP_PREFETCH) ] = {
- [ C(RESULT_ACCESS) ] = 0x4f2e, /* LLC Reference */
- [ C(RESULT_MISS) ] = 0x412e, /* LLC Misses */
+ /* OFFCORE_RESPONSE.PREFETCH.LOCAL_CACHE */
+ [ C(RESULT_ACCESS) ] = 0x00000770004301b7,
+ /* OFFCORE_RESPONSE.PREFETCH.ANY_LLC_MISS */
+ [ C(RESULT_MISS) ] = 0x0000f870004301b7,
},
},
[ C(DTLB) ] = {
--
1.7.1
On Thu, 2010-11-18 at 11:47 +0100, Andi Kleen wrote:
> From: Andi Kleen <[email protected]>
>
> Intel Nehalem/Westmere have a special OFFCORE_RESPONSE event
> that can be used to monitor any offcore accesses from a core.
> This is a very useful event for various tunings, and it's
> also needed to implement the generic LLC-* events correctly.
>
> Unfortunately this event requires programming a mask in a separate
> register. And worse this separate register is per core, not per
> CPU thread.
>
> This patch adds:
> - Teaches perf_events that OFFCORE_RESPONSE needs extra parameters.
> The extra parameters are passed by user space in the unused upper
> 32bits of the config word.
> - Add support to the Intel perf_event core to schedule per
> core resources. This adds fairly generic infrastructure that
> can be also used for other per core resources.
> The basic code has is patterned after the similar AMD northbridge
> constraints code.
>
> Thanks to Stephane Eranian who pointed out some problems
> in the original version and suggested improvements.
>
> Full git tree:
> git://git.kernel.org/pub/scm/linux/kernel/git/ak/linux-misc-2.6.git perf-offcore2
> Cc: [email protected]
> v2: Lots of updates based on review feedback. Also fixes some issues
> v3: Fix hotplug. Handle multiple extra registers. Fix init order.
> Various improvements.
Stuff like that shouldn't be mixed in with the tags and usually goes
below the --- separator since its not supposed to end up in the
committed changelog.
> Signed-off-by: Andi Kleen <[email protected]>
> ---
> arch/x86/kernel/cpu/perf_event.c | 70 ++++++++++++
> arch/x86/kernel/cpu/perf_event_intel.c | 192 ++++++++++++++++++++++++++++++++
> include/linux/perf_event.h | 2 +
> 3 files changed, 264 insertions(+), 0 deletions(-)
> @@ -876,6 +944,8 @@ static inline void __x86_pmu_enable_event(struct hw_perf_event *hwc,
> u64 enable_mask)
> {
> wrmsrl(hwc->config_base + hwc->idx, hwc->config | enable_mask);
> + if (hwc->extra_reg)
> + wrmsrl(hwc->extra_reg, hwc->extra_config);
> }
Just wondering, shouldn't we program the extra msr _before_ we flip the
enable bit?
> +static __init int init_intel_percore(void)
> +{
> + int cpu;
> +
> + if (!needs_percore)
> + return 0;
> +
> + intel_percore = alloc_percpu(struct intel_percore);
> + if (!intel_percore)
> + return -ENOMEM;
> + for_each_possible_cpu(cpu)
> + raw_spin_lock_init(&per_cpu_ptr(intel_percore, cpu)->lock);
> +
> + return 0;
> +}
> +/*
> + * Runs later because per cpu allocations don't work early on.
> + */
> +__initcall(init_intel_percore);
I've got a patch moving the whole pmu init to early_initcall(), which is
after mm_init() so it would actually work.
On Thu, Nov 18, 2010 at 12:12:59PM +0100, Peter Zijlstra wrote:
> > u64 enable_mask)
> > {
> > wrmsrl(hwc->config_base + hwc->idx, hwc->config | enable_mask);
> > + if (hwc->extra_reg)
> > + wrmsrl(hwc->extra_reg, hwc->extra_config);
> > }
>
> Just wondering, shouldn't we program the extra msr _before_ we flip the
> enable bit?
Yes that makes sense.
> > + * Runs later because per cpu allocations don't work early on.
> > + */
> > +__initcall(init_intel_percore);
>
> I've got a patch moving the whole pmu init to early_initcall(), which is
> after mm_init() so it would actually work.
So do you want to make this patchkit depend on that patch?
Or just integrate it and then change later?
-Andi
--
[email protected] -- Speaking for myself only.
On Thu, 2010-11-18 at 12:16 +0100, Andi Kleen wrote:
>
> > > + * Runs later because per cpu allocations don't work early on.
> > > + */
> > > +__initcall(init_intel_percore);
> >
> > I've got a patch moving the whole pmu init to early_initcall(), which is
> > after mm_init() so it would actually work.
>
> So do you want to make this patchkit depend on that patch?
> Or just integrate it and then change later?
Doesn't really matter, I can fix it up afterwards, just wanted to let
you know.. I can also flip that enable thing.
I'll take these 4 patches and fixup these things and then see what falls
out ;-)
On Thu, 18 Nov 2010, Andi Kleen wrote:
> From: Andi Kleen <[email protected]>
>
> When booting up a CPU set the various topology masks before
> calling the CPU_STARTING notifier. This way the notifier
> can actually use the masks.
>
> This is needed for a perf change.
>
> Signed-off-by: Andi Kleen <[email protected]>
> ---
> arch/x86/kernel/smpboot.c | 8 ++++----
> 1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
> index 083e99d..9d2980e 100644
> --- a/arch/x86/kernel/smpboot.c
> +++ b/arch/x86/kernel/smpboot.c
> @@ -281,6 +281,10 @@ static void __cpuinit smp_callin(void)
> */
> smp_store_cpu_info(cpuid);
>
> + /* This must be done before setting cpu_online_mask */
Can we please fix up that comment to match that change ?
> + set_cpu_sibling_map(raw_smp_processor_id());
> + wmb();
> +
Thanks,
tglx
On Thu, 2010-11-18 at 11:47 +0100, Andi Kleen wrote:
> From: Andi Kleen <[email protected]>
>
> Intel Nehalem/Westmere have a special OFFCORE_RESPONSE event
> that can be used to monitor any offcore accesses from a core.
> This is a very useful event for various tunings, and it's
> also needed to implement the generic LLC-* events correctly.
>
> Unfortunately this event requires programming a mask in a separate
> register. And worse this separate register is per core, not per
> CPU thread.
>
> This patch adds:
> - Teaches perf_events that OFFCORE_RESPONSE needs extra parameters.
> The extra parameters are passed by user space in the unused upper
> 32bits of the config word.
> - Add support to the Intel perf_event core to schedule per
> core resources. This adds fairly generic infrastructure that
> can be also used for other per core resources.
> The basic code has is patterned after the similar AMD northbridge
> constraints code.
>
> Thanks to Stephane Eranian who pointed out some problems
> in the original version and suggested improvements.
WARNING: please, no space before tabs
#59: FILE: arch/x86/kernel/cpu/perf_event.c:136:
+^Iint ^I^I^I^Ipercore_used; /* Used by this CPU? */$
WARNING: please, no space before tabs
#80: FILE: arch/x86/kernel/cpu/perf_event.c:198:
+^Iu64 ^I^I^Iconfig_mask;$
WARNING: please, no space before tabs
#81: FILE: arch/x86/kernel/cpu/perf_event.c:199:
+^Iu64 ^I^I^Ivalid_mask;$
WARNING: please, no space before tabs
#85: FILE: arch/x86/kernel/cpu/perf_event.c:203:
+^I.event = (e), ^I\$
WARNING: please, no space before tabs
#86: FILE: arch/x86/kernel/cpu/perf_event.c:204:
+^I.msr = (ms),^I ^I\$
WARNING: please, no space before tabs
#87: FILE: arch/x86/kernel/cpu/perf_event.c:205:
+^I.config_mask = (m), ^I\$
WARNING: please, no space before tabs
#177: FILE: arch/x86/kernel/cpu/perf_event_intel.c:12:
+^Iint ^I^Iref;^I^I/* reference count */$
WARNING: please, no space before tabs
#179: FILE: arch/x86/kernel/cpu/perf_event_intel.c:14:
+^Iu64 ^I^Iextra_config;^I/* extra MSR config */$
WARNING: please, no space before tabs
#183: FILE: arch/x86/kernel/cpu/perf_event_intel.c:18:
+^Iraw_spinlock_t ^I^Ilock;^I^I/* protect structure */$
WARNING: please, no space before tabs
#184: FILE: arch/x86/kernel/cpu/perf_event_intel.c:19:
+^Istruct er_account ^Iregs[MAX_EXTRA_REGS];$
WARNING: please use device_initcall() instead of __initcall()
#408: FILE: arch/x86/kernel/cpu/perf_event_intel.c:1108:
+__initcall(init_intel_percore);
Fixed those up, please be more careful next time.
On Thu, Nov 18, 2010 at 12:52:33PM +0100, Thomas Gleixner wrote:
> On Thu, 18 Nov 2010, Andi Kleen wrote:
>
> > From: Andi Kleen <[email protected]>
> >
> > When booting up a CPU set the various topology masks before
> > calling the CPU_STARTING notifier. This way the notifier
> > can actually use the masks.
> >
> > This is needed for a perf change.
> >
> > Signed-off-by: Andi Kleen <[email protected]>
> > ---
> > arch/x86/kernel/smpboot.c | 8 ++++----
> > 1 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
> > index 083e99d..9d2980e 100644
> > --- a/arch/x86/kernel/smpboot.c
> > +++ b/arch/x86/kernel/smpboot.c
> > @@ -281,6 +281,10 @@ static void __cpuinit smp_callin(void)
> > */
> > smp_store_cpu_info(cpuid);
> >
> > + /* This must be done before setting cpu_online_mask */
>
> Can we please fix up that comment to match that change ?
Here's an updated patch.
x86: set cpu masks before calling CPU_STARTING notifiers
When booting up a CPU set the various topology masks before
calling the CPU_STARTING notifier. This way the notifier
can actually use the masks.
This is needed for a perf change.
Signed-off-by: Andi Kleen <[email protected]>
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 083e99d..c019c03 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -281,6 +281,13 @@ static void __cpuinit smp_callin(void)
*/
smp_store_cpu_info(cpuid);
+ /*
+ * This must be done before setting cpu_online_mask
+ * or calling notify_cpu_starting.
+ */
+ set_cpu_sibling_map(raw_smp_processor_id());
+ wmb();
+
notify_cpu_starting(cpuid);
/*
@@ -322,10 +329,6 @@ notrace static void __cpuinit start_secondary(void *unused)
legacy_pic->unmask(0);
}
- /* This must be done before setting cpu_online_mask */
- set_cpu_sibling_map(raw_smp_processor_id());
- wmb();
-
/*
* We need to hold call_lock, so there is no inconsistency
* between the time smp_call_function() determines number of
On Thu, Nov 18, 2010 at 6:47 PM, Andi Kleen <[email protected]> wrote:
> From: Andi Kleen <[email protected]>
>
> Intel Nehalem/Westmere have a special OFFCORE_RESPONSE event
> that can be used to monitor any offcore accesses from a core.
> This is a very useful event for various tunings, and it's
> also needed to implement the generic LLC-* events correctly.
>
> Unfortunately this event requires programming a mask in a separate
> register. And worse this separate register is per core, not per
> CPU thread.
This "separate register" is MSR_OFFCORE_RSP_0, right?
But from the SDM, MSR_OFFCORE_RSP_0 is "thread" scope,
see SDM 3b, Appendix B.4 MSRS IN THE INTEL? MICROARCHITECTURE CODENAME NEHALEM
Or am I missing some obvious thing?
Thanks,
Lin Ming
On Mon, Nov 22, 2010 at 1:23 PM, Lin Ming <[email protected]> wrote:
> On Thu, Nov 18, 2010 at 6:47 PM, Andi Kleen <[email protected]> wrote:
>> From: Andi Kleen <[email protected]>
>>
>> Intel Nehalem/Westmere have a special OFFCORE_RESPONSE event
>> that can be used to monitor any offcore accesses from a core.
>> This is a very useful event for various tunings, and it's
>> also needed to implement the generic LLC-* events correctly.
>>
>> Unfortunately this event requires programming a mask in a separate
>> register. And worse this separate register is per core, not per
>> CPU thread.
>
> This "separate register" is MSR_OFFCORE_RSP_0, right?
> But from the SDM, MSR_OFFCORE_RSP_0 is "thread" scope,
> see SDM 3b, Appendix B.4 MSRS IN THE INTEL® MICROARCHITECTURE CODENAME NEHALEM
>
> Or am I missing some obvious thing?
>
The manual is wrong on this.
> Thanks,
> Lin Ming
>
On Mon, Nov 22, 2010 at 8:47 PM, Stephane Eranian <[email protected]> wrote:
> On Mon, Nov 22, 2010 at 1:23 PM, Lin Ming <[email protected]> wrote:
>> On Thu, Nov 18, 2010 at 6:47 PM, Andi Kleen <[email protected]> wrote:
>>> From: Andi Kleen <[email protected]>
>>>
>>> Intel Nehalem/Westmere have a special OFFCORE_RESPONSE event
>>> that can be used to monitor any offcore accesses from a core.
>>> This is a very useful event for various tunings, and it's
>>> also needed to implement the generic LLC-* events correctly.
>>>
>>> Unfortunately this event requires programming a mask in a separate
>>> register. And worse this separate register is per core, not per
>>> CPU thread.
>>
>> This "separate register" is MSR_OFFCORE_RSP_0, right?
>> But from the SDM, ?MSR_OFFCORE_RSP_0 is "thread" scope,
>> see SDM 3b, Appendix B.4 MSRS IN THE INTEL? MICROARCHITECTURE CODENAME NEHALEM
>>
>> Or am I missing some obvious thing?
>>
> The manual is wrong on this.
Well, thanks for the info.
Commit-ID: 5ef428c4b5950dddce7311e84321abb3aff7ebb0
Gitweb: http://git.kernel.org/tip/5ef428c4b5950dddce7311e84321abb3aff7ebb0
Author: Andi Kleen <[email protected]>
AuthorDate: Thu, 18 Nov 2010 11:47:31 +0100
Committer: Ingo Molnar <[email protected]>
CommitDate: Fri, 26 Nov 2010 15:14:56 +0100
x86: Set cpu masks before calling CPU_STARTING notifiers
When booting up a CPU set the various topology masks before
calling the CPU_STARTING notifier. This way the notifier
can actually use the masks.
This is needed for a perf change.
Signed-off-by: Andi Kleen <[email protected]>
Signed-off-by: Peter Zijlstra <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/kernel/smpboot.c | 11 +++++++----
1 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index f0a0624..68f61ac 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -281,6 +281,13 @@ static void __cpuinit smp_callin(void)
*/
smp_store_cpu_info(cpuid);
+ /*
+ * This must be done before setting cpu_online_mask
+ * or calling notify_cpu_starting.
+ */
+ set_cpu_sibling_map(raw_smp_processor_id());
+ wmb();
+
notify_cpu_starting(cpuid);
/*
@@ -316,10 +323,6 @@ notrace static void __cpuinit start_secondary(void *unused)
*/
check_tsc_sync_target();
- /* This must be done before setting cpu_online_mask */
- set_cpu_sibling_map(raw_smp_processor_id());
- wmb();
-
/*
* We need to hold call_lock, so there is no inconsistency
* between the time smp_call_function() determines number of
On Thu, 2010-11-18 at 12:46 +0100, Peter Zijlstra wrote:
> On Thu, 2010-11-18 at 12:16 +0100, Andi Kleen wrote:
> >
> > > > + * Runs later because per cpu allocations don't work early on.
> > > > + */
> > > > +__initcall(init_intel_percore);
> > >
> > > I've got a patch moving the whole pmu init to early_initcall(), which is
> > > after mm_init() so it would actually work.
> >
> > So do you want to make this patchkit depend on that patch?
> > Or just integrate it and then change later?
>
> Doesn't really matter, I can fix it up afterwards, just wanted to let
> you know.. I can also flip that enable thing.
>
> I'll take these 4 patches and fixup these things and then see what falls
> out ;-)
OK, that initcall patch hit -tip and Ingo wants that allocation stuff
fixed up before committing those patches.
Could you run another version of these patches (you can ignore the
checkpatch output that results from the non-std struct initialization
layout)?
Just make the allocation path like the AMD NB stuff, use the cpu_prepare
and cpu_dead hooks to alloc/free and add a refcount in the percpu
object.
On Fri, 2010-11-26 at 16:28 +0100, Peter Zijlstra wrote:
> On Thu, 2010-11-18 at 12:46 +0100, Peter Zijlstra wrote:
> > On Thu, 2010-11-18 at 12:16 +0100, Andi Kleen wrote:
> > >
> > > > > + * Runs later because per cpu allocations don't work early on.
> > > > > + */
> > > > > +__initcall(init_intel_percore);
> > > >
> > > > I've got a patch moving the whole pmu init to early_initcall(), which is
> > > > after mm_init() so it would actually work.
> > >
> > > So do you want to make this patchkit depend on that patch?
> > > Or just integrate it and then change later?
> >
> > Doesn't really matter, I can fix it up afterwards, just wanted to let
> > you know.. I can also flip that enable thing.
> >
> > I'll take these 4 patches and fixup these things and then see what falls
> > out ;-)
>
> OK, that initcall patch hit -tip and Ingo wants that allocation stuff
> fixed up before committing those patches.
>
> Could you run another version of these patches (you can ignore the
> checkpatch output that results from the non-std struct initialization
> layout)?
>
> Just make the allocation path like the AMD NB stuff, use the cpu_prepare
> and cpu_dead hooks to alloc/free and add a refcount in the percpu
> object.
While looking at the AMD code, I think we can do away with amd_nb_lock,
all the hotplug stuff is serialized on the hotplug mutex anyway.
On Thu, 2010-11-18 at 11:47 +0100, Andi Kleen wrote:
> @@ -876,6 +944,8 @@ static inline void __x86_pmu_enable_event(struct hw_perf_event *hwc,
> u64 enable_mask)
> {
> wrmsrl(hwc->config_base + hwc->idx, hwc->config | enable_mask);
> + if (hwc->extra_reg)
> + wrmsrl(hwc->extra_reg, hwc->extra_config);
> }
I thought we agreed it made more sense to program the extra msr before
enabling the counter.
On Wed, 2010-12-01 at 15:27 +0100, Peter Zijlstra wrote:
> On Thu, 2010-11-18 at 11:47 +0100, Andi Kleen wrote:
> > @@ -876,6 +944,8 @@ static inline void __x86_pmu_enable_event(struct hw_perf_event *hwc,
> > u64 enable_mask)
> > {
> > wrmsrl(hwc->config_base + hwc->idx, hwc->config | enable_mask);
> > + if (hwc->extra_reg)
> > + wrmsrl(hwc->extra_reg, hwc->extra_config);
> > }
>
> I thought we agreed it made more sense to program the extra msr before
> enabling the counter.
Ah sorry, I seem to have mixed up the various versions.
The latest does indeed do as we agreed. Sorry for the noise.