On Thu, Jan 11, 2018 at 10:21:25AM -0500, Liang, Kan wrote: SNIP
> > > > hum, but the PEBS drain is specific just for > > PERF_X86_EVENT_AUTO_RELOAD events, right? > > Accurately, PEBS drain is specific for PERF_X86_EVENT_FREERUNNING here. > PERF_X86_EVENT_FREERUNNING event must be _AUTO_RELOAD event. > But in some cases, _AUTO_RELOAD event cannot be _FREERUNNING event. > > Only the event which is both _FREERUNNING and _AUTO_RELOAD need to do PEBS > drain in _read(). > > So it does the check in intel_pmu_pebs_read() > + if (pebs_needs_sched_cb(cpuc)) > + return intel_pmu_drain_pebs_buffer(); > > > > > wrt readability maybe you could add function like: > > The existing function pebs_needs_sched_cb() can do the check. > We just need to expose it, and also the intel_pmu_drain_pebs_buffer(). > > But to be honest, I still cannot see a reason for that. > It could save a call to intel_pmu_pebs_read(), but _read() is not critical > path. It doesn't save much. hum, pmu->read is also called for PERF_SAMPLE_READ for sample, check perf_output_read for non sampling event you shouldn't be able to create PEBS event, there's check in x86_pmu_hw_config I agree it does not save much, it just confused me while I was reading the code, like why is this needed for all events with precise_ip jirka
