Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1621188
| From | "Jin, Yao" <yao.jin@linux.intel.com> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH v3 2/5] perf/x86/intel: Record branch type |
| Date | 2017-04-11 14:00 +0200 |
| Message-ID | <tv2CK-6tD-3@gated-at.bofh.it> (permalink) |
| References | <tuUc9-15U-7@gated-at.bofh.it> <tuUc9-15U-5@gated-at.bofh.it> <tuYSt-463-3@gated-at.bofh.it> <tuZbQ-4s3-11@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On 4/11/2017 4:18 PM, Peter Zijlstra wrote:
> On Tue, Apr 11, 2017 at 09:52:19AM +0200, Peter Zijlstra wrote:
>> On Tue, Apr 11, 2017 at 06:56:30PM +0800, Jin Yao wrote:
>>> @@ -960,6 +1006,11 @@ intel_pmu_lbr_filter(struct cpu_hw_events *cpuc)
>>> cpuc->lbr_entries[i].from = 0;
>>> compress = true;
>>> }
>>> +
>>> + if ((br_sel & X86_BR_TYPE_SAVE) == X86_BR_TYPE_SAVE)
>>> + cpuc->lbr_entries[i].type = common_branch_type(type);
>>> + else
>>> + cpuc->lbr_entries[i].type = PERF_BR_NONE;
>>> }
> I was wondering WTH you did that else; because it should already be 0
> (aka, BR_NONE).
Yes. I will remove the else code. Thanks!
> Then I found intel_pmu_lbr_read_32() is already broken,
> and you just broke intel_pmu_lbr_read_64().
>
> Arguably we should add a union on the last __u64 with a name for the
> entire thing, but the below is the minimal fix.
>
> ---
> Subject: perf,x86: Avoid exposing wrong/stale data in intel_pmu_lbr_read_32()
> From: Peter Zijlstra <peterz@infradead.org>
> Date: Tue Apr 11 10:10:28 CEST 2017
>
> When the perf_branch_entry::{in_tx,abort,cycles} fields were added,
> intel_pmu_lbr_read_32() wasn't updated to initialize them.
>
> Fixes: 135c5612c460 ("perf/x86/intel: Support Haswell/v4 LBR format")
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
> --- a/arch/x86/events/intel/lbr.c
> +++ b/arch/x86/events/intel/lbr.c
> @@ -507,6 +507,9 @@ static void intel_pmu_lbr_read_32(struct
> cpuc->lbr_entries[i].to = msr_lastbranch.to;
> cpuc->lbr_entries[i].mispred = 0;
> cpuc->lbr_entries[i].predicted = 0;
> + cpuc->lbr_entries[i].in_tx = 0;
> + cpuc->lbr_entries[i].abort = 0;
> + cpuc->lbr_entries[i].cycles = 0;
> cpuc->lbr_entries[i].reserved = 0;
> }
> cpuc->lbr_stack.nr = i;
I will add cpuc->lbr_entries[i].type = 0 in my patch.
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH v3 2/5] perf/x86/intel: Record branch type Jin Yao <yao.jin@linux.intel.com> - 2017-04-11 05:00 +0200
Re: [PATCH v3 2/5] perf/x86/intel: Record branch type Peter Zijlstra <peterz@infradead.org> - 2017-04-11 10:00 +0200
Re: [PATCH v3 2/5] perf/x86/intel: Record branch type Peter Zijlstra <peterz@infradead.org> - 2017-04-11 10:20 +0200
Re: [PATCH v3 2/5] perf/x86/intel: Record branch type "Jin, Yao" <yao.jin@linux.intel.com> - 2017-04-11 14:00 +0200
Re: [PATCH v3 2/5] perf/x86/intel: Record branch type "Jin, Yao" <yao.jin@linux.intel.com> - 2017-04-11 10:20 +0200
Re: [PATCH v3 2/5] perf/x86/intel: Record branch type Peter Zijlstra <peterz@infradead.org> - 2017-04-11 10:40 +0200
Re: [PATCH v3 2/5] perf/x86/intel: Record branch type "Jin, Yao" <yao.jin@linux.intel.com> - 2017-04-11 13:40 +0200
csiph-web