On Fri, Jul 24, 2020 at 03:10:52PM -0400, Liang, Kan wrote: > > > > Per the SIBLING patch this then wants to be: > > > > > > > > if (!is_slots_event(leader)) > > > > return -EINVAL; > > > > > > > > event->event_caps |= PERF_EV_CAP_SIBLING. > > > > /* > > > > * Only once we have a METRICs sibling to we > > > > * need TopDown magic. > > > > */ > > > > leader->hw.flags |= PERF_X86_EVENT_TOPDOWN; > > Since we only set the flag for the SLOTS event now, the V7 patch will treat > the metric events as normal events, which trigger an error.
Damn, that was a silly oversight on my part. > I think we don't need the PERF_X86_EVENT_TOPDOWN flag anymore. > If it's a non-sampling slots event, apply the special function. > If it's a metric event, do nothing. > diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c > index 0f3d01562ded..02dfee0b6615 100644 > --- a/arch/x86/events/core.c > +++ b/arch/x86/events/core.c > @@ -73,10 +73,10 @@ u64 x86_perf_event_update(struct perf_event *event) > u64 prev_raw_count, new_raw_count; > u64 delta; > > - if (unlikely(!hwc->event_base)) > + if (unlikely(!hwc->event_base || is_metric_event(event))) > return 0; > > - if (unlikely(is_topdown_count(event)) && x86_pmu.update_topdown_event) > + if (unlikely(is_slots_count(event)) && x86_pmu.update_topdown_event) > return x86_pmu.update_topdown_event(event); > > /* > @@ -1280,11 +1280,10 @@ int x86_perf_event_set_period(struct perf_event > *event) > s64 period = hwc->sample_period; > int ret = 0, idx = hwc->idx; > > - if (unlikely(!hwc->event_base)) > + if (unlikely(!hwc->event_base || is_metric_event(event))) > return 0; > > - if (unlikely(is_topdown_count(event)) && > - x86_pmu.set_topdown_event_period) > + if (unlikely(is_slots_count(event)) && x86_pmu.set_topdown_event_period) > return x86_pmu.set_topdown_event_period(event); > > /* This; I don't like that much, it adds even more conditions to fairly hot code. I was even considering adding a static_branch for x86_pmu.intel_cap.perf_metrics. Anyway, let me fix this.
