Messages in this thread | | | From | Alexander Shishkin <> | Subject | Re: [PATCH v0 3/5] perf: Introduce instruction trace filtering | Date | Fri, 11 Dec 2015 17:17:25 +0200 |
| |
Peter Zijlstra <peterz@infradead.org> writes:
> On Fri, Dec 11, 2015 at 03:36:36PM +0200, Alexander Shishkin wrote: >> +static int __perf_event_itrace_filters_setup(void *info) >> +{ >> + struct perf_event *event = info; >> + int ret; >> + >> + if (READ_ONCE(event->state) != PERF_EVENT_STATE_ACTIVE) >> + return -EAGAIN; >> + >> + /* matches smp_wmb() in event_sched_in() */ >> + smp_rmb(); >> + >> + /* >> + * There is a window with interrupts enabled before we get here, >> + * so we need to check again lest we try to stop another cpu's event. >> + */ >> + if (READ_ONCE(event->oncpu) != smp_processor_id()) >> + return -EAGAIN; >> + >> + event->pmu->stop(event, PERF_EF_UPDATE); >> + rcu_read_lock(); >> + ret = event->pmu->itrace_filter_setup(event); >> + rcu_read_unlock(); >> + event->pmu->start(event, PERF_EF_RELOAD); > > Would it not be more sensible to let the ::itrace_filter_setup() method > do the stop/start-ing if and when needed?
I don't have a strong opinion on this, the only question is, are we comfortable with pmu driver callback doing the rcu_read_lock/unlock, because it still needs to iterate the filter list. Other than that it's probably a good idea.
Thanks, -- Alex
| |