On Tue, May 28, 2019 at 02:56:15PM +0200, Peter Zijlstra wrote: > On Tue, May 21, 2019 at 02:40:50PM -0700, kan.li...@linux.intel.com wrote: > > > diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c > > index e9075d57853d..07ecfe75f0e6 100644 > > --- a/arch/x86/events/core.c > > +++ b/arch/x86/events/core.c > > @@ -91,16 +91,20 @@ u64 x86_perf_event_update(struct perf_event *event) > > new_raw_count) != prev_raw_count) > > goto again;
AFAICT, you don't actually need that cmpxchg loop for the metric crud. > > > > - /* > > - * Now we have the new raw value and have updated the prev > > - * timestamp already. We can now calculate the elapsed delta > > - * (event-)time and add that to the generic event. > > - * > > - * Careful, not all hw sign-extends above the physical width > > - * of the count. > > - */ > > - delta = (new_raw_count << shift) - (prev_raw_count << shift); > > - delta >>= shift; > > + if (unlikely(hwc->flags & PERF_X86_EVENT_UPDATE)) > > + delta = x86_pmu.metric_update_event(event, new_raw_count); > > + else { > > + /* > > + * Now we have the new raw value and have updated the prev > > + * timestamp already. We can now calculate the elapsed delta > > + * (event-)time and add that to the generic event. > > + * > > + * Careful, not all hw sign-extends above the physical width > > + * of the count. > > + */ > > + delta = (new_raw_count << shift) - (prev_raw_count << shift); > > + delta >>= shift; > > + } > > > > local64_add(delta, &event->count); > > local64_sub(delta, &hwc->period_left); > > > @@ -1186,6 +1194,9 @@ int x86_perf_event_set_period(struct perf_event > > *event) > > if (idx == INTEL_PMC_IDX_FIXED_BTS) > > return 0; > > > > + if (x86_pmu.set_period && x86_pmu.set_period(event)) > > + goto update_userpage; > > + > > /* > > * If we are way outside a reasonable range then just skip forward: > > */ > > @@ -1234,6 +1245,7 @@ int x86_perf_event_set_period(struct perf_event > > *event) > > (u64)(-left) & x86_pmu.cntval_mask); > > } > > > > +update_userpage: > > perf_event_update_userpage(event); > > > > return ret; > > > *groan*.... that's pretty terrible.