Re: [PATCH 0/9] perf: Adding better precise_ip field handling

From: Peter Zijlstra
Date: Mon May 13 2013 - 05:38:16 EST


On Sat, May 11, 2013 at 09:50:08AM +0200, Ingo Molnar wrote:
> That's really a red herring: there's absolutely no reason why the
> kernel could not pass back the level of precision it provided.

All I've been saying is that doing random precision without feedback is
confusing.

We also don't really have a good feedback channel for this kind of thing. The
best I can come up with is tagging each and every sample with the quality it
represents. I think we can do with only one extra PERF_RECORD_MISC bit, but it
looks like we're quickly running out of those things.

But I think the biggest problem is PEBS's inability do deal with REP prefixes;
see this email from Stephane: https://lkml.org/lkml/2011/2/1/177

It is really unfortunate for PEBS to have such a side-effect; but it makes all
memset/memcpy/memmove things appear like they have no cost. I'm very sure that
will surprise a number of people.

---
arch/x86/include/asm/perf_event.h | 4 +++-
arch/x86/kernel/cpu/perf_event.c | 2 ++
arch/x86/kernel/cpu/perf_event_intel_ds.c | 4 +++-
include/uapi/linux/perf_event.h | 1 +
4 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/perf_event.h b/arch/x86/include/asm/perf_event.h
index 57cb634..6908838 100644
--- a/arch/x86/include/asm/perf_event.h
+++ b/arch/x86/include/asm/perf_event.h
@@ -205,14 +205,16 @@ static inline u32 get_ibs_caps(void) { return 0; }
extern void perf_events_lapic_init(void);

/*
- * Abuse bits {3,5} of the cpu eflags register. These flags are otherwise
+ * Abuse bits {3,5,15} of the cpu eflags register. These flags are otherwise
* unused and ABI specified to be 0, so nobody should care what we do with
* them.
*
+ * CONSTANT - the IP has constant skid.
* EXACT - the IP points to the exact instruction that triggered the
* event (HW bugs exempt).
* VM - original X86_VM_MASK; see set_linear_ip().
*/
+#define PERF_EFLAGS_CONSTANT (1UL << 15)
#define PERF_EFLAGS_EXACT (1UL << 3)
#define PERF_EFLAGS_VM (1UL << 5)

diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
index 1025f3c..ca1f7dc 100644
--- a/arch/x86/kernel/cpu/perf_event.c
+++ b/arch/x86/kernel/cpu/perf_event.c
@@ -2078,6 +2078,8 @@ unsigned long perf_misc_flags(struct pt_regs *regs)

if (regs->flags & PERF_EFLAGS_EXACT)
misc |= PERF_RECORD_MISC_EXACT_IP;
+ else if (regs->flags & PERF_EFLAGS_CONSTANT)
+ misc |= PERF_RECORD_MISC_CONSTANT_SKID;

return misc;
}
diff --git a/arch/x86/kernel/cpu/perf_event_intel_ds.c b/arch/x86/kernel/cpu/perf_event_intel_ds.c
index 60250f6..757ecd4 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_ds.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_ds.c
@@ -753,10 +753,12 @@ static void __intel_pmu_pebs_event(struct perf_event *event,
regs.bp = pebs->bp;
regs.sp = pebs->sp;

+ regs.flags &= ~(PERF_EFLAGS_CONSTANT | PERF_EFLAGS_EXACT);
+
if (event->attr.precise_ip > 1 && intel_pmu_pebs_fixup_ip(&regs))
regs.flags |= PERF_EFLAGS_EXACT;
else
- regs.flags &= ~PERF_EFLAGS_EXACT;
+ regs.flags |= PERF_EFLAGS_CONSTANT;

if (has_branch_stack(event))
data.br_stack = &cpuc->lbr_stack;
diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index fb104e5..cb1f70f 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -451,6 +451,7 @@ struct perf_event_mmap_page {
* the actual instruction that triggered the event. See also
* perf_event_attr::precise_ip.
*/
+#define PERF_RECORD_MISC_CONSTANT_SKID (1 << 12)
#define PERF_RECORD_MISC_EXACT_IP (1 << 14)
/*
* Reserve the last bit to indicate some extended misc field

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/