Skip to contentSkip to conversation

FAILED: patch "[PATCH] perf/x86/intel: Fix PEBS data source interpretation on" failed to apply to 4.5-stable tree

PATCH [email protected] 4 participants 8 messages Apr 9, 2016 – Apr 13, 2016

Subsystems (3)
lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

The patch below does not apply to the 4.5-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

Quoted 1 line

From e17dc65328057c00db7e1bfea249c8771a78b30b Mon Sep 17 00:00:00 2001

From: Andi Kleen <[email protected]>
Date: Tue, 1 Mar 2016 14:25:24 -0800
Subject: [PATCH] perf/x86/intel: Fix PEBS data source interpretation on
Nehalem/Westmere

Jiri reported some time ago that some entries in the PEBS data source table in perf do not agree with the SDM. We investigated and the bits changed for Sandy Bridge, but the SDM was not updated.

perf already implements the bits correctly for Sandy Bridge and later. This patch patches it up for Nehalem and Westmere.

arch/x86/events/intel/core.c +2 -0
index edac81c4295a..68fa55b4d42e 100644
@@ -3417,6 +3417,7 @@ __init int intel_pmu_init(void)
 		intel_perfmon_event_map[PERF_COUNT_HW_STALLED_CYCLES_BACKEND] =
 			X86_CONFIG(.event=0xb1, .umask=0x3f, .inv=1, .cmask=1);
 
+		intel_pmu_pebs_data_source_nhm();
 		x86_add_quirk(intel_nehalem_quirk);
 
 		pr_cont("Nehalem events, ");
@@ -3480,6 +3481,7 @@ __init int intel_pmu_init(void)
 		intel_perfmon_event_map[PERF_COUNT_HW_STALLED_CYCLES_BACKEND] =
 			X86_CONFIG(.event=0xb1, .umask=0x3f, .inv=1, .cmask=1);
 
+		intel_pmu_pebs_data_source_nhm();
 		pr_cont("Westmere events, ");
 		break;
 
arch/x86/events/intel/ds.c +10 -1
index a99a8cbac729..ce7211a07c0b 100644
@@ -51,7 +51,8 @@ union intel_x86_pebs_dse {
 #define OP_LH (P(OP, LOAD) | P(LVL, HIT))
 #define SNOOP_NONE_MISS (P(SNOOP, NONE) | P(SNOOP, MISS))
 
-static const u64 pebs_data_source[] = {
+/* Version for Sandy Bridge and later */
+static u64 pebs_data_source[] = {
 	P(OP, LOAD) | P(LVL, MISS) | P(LVL, L3) | P(SNOOP, NA),/* 0x00:ukn L3 */
 	OP_LH | P(LVL, L1)  | P(SNOOP, NONE),	/* 0x01: L1 local */
 	OP_LH | P(LVL, LFB) | P(SNOOP, NONE),	/* 0x02: LFB hit */
@@ -70,6 +71,14 @@ static const u64 pebs_data_source[] = {
 	OP_LH | P(LVL, UNC) | P(SNOOP, NONE), /* 0x0f: uncached */
 };
 
+/* Patch up minor differences in the bits */
+void __init intel_pmu_pebs_data_source_nhm(void)
+{
+	pebs_data_source[0x05] = OP_LH | P(LVL, L3)  | P(SNOOP, HIT);
+	pebs_data_source[0x06] = OP_LH | P(LVL, L3)  | P(SNOOP, HITM);
+	pebs_data_source[0x07] = OP_LH | P(LVL, L3)  | P(SNOOP, HITM);
+}
+
 static u64 precise_store_data(u64 status)
 {
 	union intel_x86_pebs_dse dse;
arch/x86/events/perf_event.h +2 -0
index 24e259eb0b3a..68155cafa8a1 100644
@@ -907,6 +907,8 @@ void intel_pmu_lbr_init_skl(void);
 
 void intel_pmu_lbr_init_knl(void);
 
+void intel_pmu_pebs_data_source_nhm(void);
+
 int intel_pmu_setup_lbr_filter(struct perf_event *event);
 
 void intel_pt_interrupt(void);
#1 lore raw
Quoted 5 lines · On Sat, Apr 09, 2016 at 10:47:29AM -0700, [email protected] wrote:

The patch below does not apply to the 4.5-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

Unfortunately perf decided to rename all x86 files, completely disregarding the needs for backports.

For perf backports you have to run

s#arch/x86/kernel/cpu/perf_event_intel.c#arch/x86/events/intel/core.c#g s#arch/x86/kernel/cpu/perf_event_intel_#arch/x86/events/intel/#g s#arch/x86/kernel/cpu/perf_event.c#arch/x86/events/core.c#g s#arch/x86/kernel/cpu/perf_event_amd.c#arch/x86/events/amd/core.c#g s#arch/x86/kernel/cpu/perf_event_amd_#arch/x86/events/amd/#g

over the patch headers. Then things should apply again.

-Andi

#2 lore raw
Quoted 3 lines · Andi Kleen

Unfortunately perf decided to rename all x86 files, completely disregarding the needs for backports.

So I agree that renaming tends to make backports nastier. That said, there *might* be a more convenient way:

Quoted 1 line

For perf backports you have to run

So a plain "git cherry-pick" will get it right too, simply because it does a three-way merge rather than just applying a patch - so you get all the usual git rename detection etc.

The advantage is that then you don't need to know about the exact renames you have to do on the patch.

Of course, I think Greg uses just quilt to apply those patches, so your "apply the rename to the patch" may be easier in this case.

Linus

#3 lore raw
Quoted 5 lines · Linus Torvalds
Quoted 1 line

For perf backports you have to run

So a plain "git cherry-pick" will get it right too, simply because it does a three-way merge rather than just applying a patch - so you get all the usual git rename detection etc.

It doesn't work for git rebase at least. I had to fix up a lot of my WIP perf feature branches manually :-/

-Andi

#4 lore raw
Quoted 3 lines · Andi Kleen

It doesn't work for git rebase at least. I had to fix up a lot of my WIP perf feature branches manually :-/

Actually, it does work for rebase, but rebase defaults to applying patches because it's faster, and you might be rebasing lots and lots of commits.

You should get the merging behavior with "-m" or "--merge". It's a lot slower, but it can be worth it.

(And "a lot slower" is all relative. if you're rebasing just a couple of patches it's not going to be all that noticeable. But if you're rebasing hundreds or thousands of commits, you will definitely notice the difference)

Linus

#5 lore raw
In reply to Linus Torvalds
Quoted 6 lines · Linus Torvalds
Quoted 3 lines · Andi Kleen

Unfortunately perf decided to rename all x86 files, completely disregarding the needs for backports.

So I agree that renaming tends to make backports nastier.

Yeah, and unfortunately there's no other way to do such changes, and the directory structure of the x86 drivers of perf grew really badly structured so we had to fix it.

In perf we go out on a limb to help backports, I regularly add Cc: stable tags.

In this case we made the renames in a single go to not make backporting a constant drip-drip pain, and we made it all as plain as possible, the pattern is very visible in the shortlogs as well:

27f6d22b037b perf/x86: Move perf_event.h to its new home
65a27a3510c8 perf/x86: Move perf_event_msr.c .............. => x86/events/msr.c 5e865ed44b39 perf/x86: Move perf_event_p6.c ............... => x86/events/intel/p6.c f03e97dbd2bb perf/x86: Move perf_event_p4.c ............... => x86/events/intel/p4.c edbb591870dc perf/x86: Move perf_event_knc.c .............. => x86/events/intel/knc.c ed367e6ca427 perf/x86: Move perf_event_intel_uncore_snbep.c => x86/events/intel/uncore_snbep.c
92553e40c629 perf/x86: Move perf_event_intel_uncore_snb.c => x86/events/intel/uncore_snb.c
35bf705c25e8 perf/x86: Move perf_event_intel_uncore_nhmex.c => x86/events/intel/uncore_nmhex.c 6bcb2db547be perf/x86: Move perf_event_intel_uncore.[ch] .. => x86/events/intel/uncore.[ch] 609d809f832d perf/x86: Move perf_event_intel_rapl.c ....... => x86/events/intel/rapl.c fd1c601c2578 perf/x86: Move perf_event_intel_pt.[ch] ...... => x86/events/intel/pt.[ch] c85cc4497f82 perf/x86: Move perf_event_intel_lbr.c ........ => x86/events/intel/lbr.c 7010d129137a perf/x86: Move perf_event_intel_ds.c ......... => x86/events/intel/ds.c 6aec1ad73656 perf/x86: Move perf_event_intel_cstate.c ..... => x86/events/intel/cstate.c 5c781a3daa30 perf/x86: Move perf_event_intel_cqm.c ........ => x86/events/intel/cqm.c e1069839dd68 perf/x86: Move perf_event_intel.c ............ => x86/events/intel/core.c af5d3aabc04a perf/x86: Move perf_event_intel_bts.c ........ => x86/events/intel/bts.c d0af1c0525d5 perf/x86: Move perf_event_amd_uncore.c .... => x86/events/amd/uncore.c 5b26547dd7fa perf/x86: Move perf_event_amd_iommu.[ch] .. => x86/events/amd/iommu.[ch] 218cfe4ed888 perf/x86: Move perf_event_amd_ibs.c ....... => x86/events/amd/ibs.c 39b0332a2158 perf/x86: Move perf_event_amd.c ........... => x86/events/amd/core.c fa9cbf320e99 perf/x86: Move perf_event.c ............... => x86/events/core.c

So the 'disregarding backports' accusation by Andi is unjust and plain nasty, and he should know it. Andi will remain in my /dev/null filter for a while longer.

Thanks,

Ingo

author maintainer
#6 lore raw
Quoted 34 lines · Ingo Molnar
Quoted 6 lines · Linus Torvalds
Quoted 3 lines · Andi Kleen

Unfortunately perf decided to rename all x86 files, completely disregarding the needs for backports.

So I agree that renaming tends to make backports nastier.

Yeah, and unfortunately there's no other way to do such changes, and the directory structure of the x86 drivers of perf grew really badly structured so we had to fix it.

In perf we go out on a limb to help backports, I regularly add Cc: stable tags.

In this case we made the renames in a single go to not make backporting a constant drip-drip pain, and we made it all as plain as possible, the pattern is very visible in the shortlogs as well:

27f6d22b037b perf/x86: Move perf_event.h to its new home
65a27a3510c8 perf/x86: Move perf_event_msr.c .............. => x86/events/msr.c 5e865ed44b39 perf/x86: Move perf_event_p6.c ............... => x86/events/intel/p6.c f03e97dbd2bb perf/x86: Move perf_event_p4.c ............... => x86/events/intel/p4.c edbb591870dc perf/x86: Move perf_event_knc.c .............. => x86/events/intel/knc.c ed367e6ca427 perf/x86: Move perf_event_intel_uncore_snbep.c => x86/events/intel/uncore_snbep.c
92553e40c629 perf/x86: Move perf_event_intel_uncore_snb.c => x86/events/intel/uncore_snb.c
35bf705c25e8 perf/x86: Move perf_event_intel_uncore_nhmex.c => x86/events/intel/uncore_nmhex.c 6bcb2db547be perf/x86: Move perf_event_intel_uncore.[ch] .. => x86/events/intel/uncore.[ch] 609d809f832d perf/x86: Move perf_event_intel_rapl.c ....... => x86/events/intel/rapl.c fd1c601c2578 perf/x86: Move perf_event_intel_pt.[ch] ...... => x86/events/intel/pt.[ch] c85cc4497f82 perf/x86: Move perf_event_intel_lbr.c ........ => x86/events/intel/lbr.c 7010d129137a perf/x86: Move perf_event_intel_ds.c ......... => x86/events/intel/ds.c 6aec1ad73656 perf/x86: Move perf_event_intel_cstate.c ..... => x86/events/intel/cstate.c

Quoted 8 lines

5c781a3daa30 perf/x86: Move perf_event_intel_cqm.c ........ => x86/events/intel/cqm.c e1069839dd68 perf/x86: Move perf_event_intel.c ............ => x86/events/intel/core.c af5d3aabc04a perf/x86: Move perf_event_intel_bts.c ........ => x86/events/intel/bts.c d0af1c0525d5 perf/x86: Move perf_event_amd_uncore.c .... => x86/events/amd/uncore.c 5b26547dd7fa perf/x86: Move perf_event_amd_iommu.[ch] .. => x86/events/amd/iommu.[ch] 218cfe4ed888 perf/x86: Move perf_event_amd_ibs.c ....... => x86/events/amd/ibs.c 39b0332a2158 perf/x86: Move perf_event_amd.c ........... => x86/events/amd/core.c fa9cbf320e99 perf/x86: Move perf_event.c ............... => x86/events/core.c

To be honest, using 'git cherry-pick' solved all of my problems here, so I'll not complain about renames anymore, or at least not for these files :)

thanks,

greg k-h

author maintainer
#7 lore raw
In reply to Andi Kleen
Quoted 12 lines · Andi Kleen
Quoted 5 lines · On Sat, Apr 09, 2016 at 10:47:29AM -0700, [email protected] wrote:

The patch below does not apply to the 4.5-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

Unfortunately perf decided to rename all x86 files, completely disregarding the needs for backports.

For perf backports you have to run

Quoted 7 lines

s#arch/x86/kernel/cpu/perf_event_intel.c#arch/x86/events/intel/core.c#g s#arch/x86/kernel/cpu/perf_event_intel_#arch/x86/events/intel/#g s#arch/x86/kernel/cpu/perf_event.c#arch/x86/events/core.c#g s#arch/x86/kernel/cpu/perf_event_amd.c#arch/x86/events/amd/core.c#g s#arch/x86/kernel/cpu/perf_event_amd_#arch/x86/events/amd/#g

over the patch headers. Then things should apply again.

Thanks, that worked.

Well, I used cherry-pick and then exported the series as patches, but the end-result was the same :)

greg k-h