Peter Zijlstra <[email protected]> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
