Re: [PATCH 1/2] perf: ignore LBR and offcore_rsp.

[Peter Zijlstra <peterz@infradead.org>]

On Wed, Jun 18, 2014 at 03:52:55PM +0000, Liang, Kan wrote:
> diff --git a/arch/x86/kernel/cpu/perf_event.h b/arch/x86/kernel/cpu/perf_event.h
> index 3b2f9bd..f828ddd 100644
> --- a/arch/x86/kernel/cpu/perf_event.h
> +++ b/arch/x86/kernel/cpu/perf_event.h
> @@ -555,8 +555,9 @@ static inline void __x86_pmu_enable_event(struct hw_perf_event *hwc,
> {
>     u64 disable_mask = __this_cpu_read(cpu_hw_events.perf_ctr_virt_mask);
> -    if (hwc->extra_reg.reg)
> -          wrmsrl(hwc->extra_reg.reg, hwc->extra_reg.config);
> +    if (hwc->extra_reg.reg &&
> +          (wrmsrl_safe(hwc->extra_reg.reg, hwc->extra_reg.config) < 0))
> +          return;
>     wrmsrl(hwc->config_base, (hwc->config | enable_mask) & ~disable_mask);
> }
> diff --git a/arch/x86/kernel/cpu/perf_event_intel_lbr.c b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
> index d82d155..6f2d1e9 100644
> --- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
> +++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
> @@ -157,7 +157,8 @@ static void intel_pmu_lbr_reset_32(void)
>     int i;
>      for (i = 0; i < x86_pmu.lbr_nr; i++)
> -          wrmsrl(x86_pmu.lbr_from + i, 0);
> +          if (wrmsrl_safe(x86_pmu.lbr_from + i, 0ULL) < 0)
> +               return;
> }
>  static void intel_pmu_lbr_reset_64(void)
> @@ -165,8 +166,9 @@ static void intel_pmu_lbr_reset_64(void)
>     int i;
>      for (i = 0; i < x86_pmu.lbr_nr; i++) {
> -          wrmsrl(x86_pmu.lbr_from + i, 0);
> -          wrmsrl(x86_pmu.lbr_to   + i, 0);
> +          if (wrmsrl_safe(x86_pmu.lbr_from + i, 0ULL) < 0 ||
> +              wrmsrl_safe(x86_pmu.lbr_to   + i, 0ULL) < 0)
> +               return;
>     }
> }
> @@ -241,7 +243,7 @@ static inline u64 intel_pmu_lbr_tos(void)
> {
>     u64 tos;
> -    rdmsrl(x86_pmu.lbr_tos, tos);
> +    rdmsrl_safe(x86_pmu.lbr_tos, &tos);
>      return tos;
> }
> @@ -262,7 +264,9 @@ static void intel_pmu_lbr_read_32(struct cpu_hw_events *cpuc)
>                u64     lbr;
>           } msr_lastbranch;
> -          rdmsrl(x86_pmu.lbr_from + lbr_idx, msr_lastbranch.lbr);
> +          if (rdmsrl_safe(x86_pmu.lbr_from + lbr_idx,
> +                    &msr_lastbranch.lbr) < 0)
> +               break;
>            cpuc->lbr_entries[i].from  = msr_lastbranch.from;
>           cpuc->lbr_entries[i].to         = msr_lastbranch.to;
> @@ -292,8 +296,9 @@ static void intel_pmu_lbr_read_64(struct cpu_hw_events *cpuc)
>           int skip = 0;
>           int lbr_flags = lbr_desc[lbr_format];
> -          rdmsrl(x86_pmu.lbr_from + lbr_idx, from);
> -          rdmsrl(x86_pmu.lbr_to   + lbr_idx, to);
> +          if (rdmsrl_safe(x86_pmu.lbr_from + lbr_idx, &from) < 0 ||
> +              rdmsrl_safe(x86_pmu.lbr_to   + lbr_idx, &to) < 0)
> +               break;
>            if (lbr_flags & LBR_EIP_FLAGS) {
>                mis = !!(from & LBR_FROM_FLAG_MISPRED);

So I really hate this patch, it makes the code hideous. Also, its a
death by a thousand cuts adding endless branches in this code.

Also, the offcore part is retarded, just make sure extra_reg isn't set.

As to the LBR, just make it so that we avoid calling the LBR code to
begin with; ideally without adding extra code to fast paths.