Messages in this thread | | | Date | Fri, 11 Dec 2015 16:33:54 +0100 | From | Peter Zijlstra <> | Subject | Re: [PATCH v0 3/5] perf: Introduce instruction trace filtering |
| |
On Fri, Dec 11, 2015 at 05:27:22PM +0200, Alexander Shishkin wrote: > 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(struct perf_event *event) > >> +{ > >> + int ret; > >> + > >> + /* > >> + * We can't use event_function_call() here, because that would > >> + * require ctx::mutex, but one of our callers is called with > >> + * mm::mmap_sem down, which would cause an inversion, see bullet > >> + * (2) in put_event(). > >> + */ > >> + do { > >> + if (READ_ONCE(event->state) != PERF_EVENT_STATE_ACTIVE) { > >> + ret = event->pmu->itrace_filter_setup(event); > >> + break; > > > > So this is tricky, if its not active it can be any moment, there is > > nothing serializing against that. > > Indeed. But we should be able to call pmu::itrace_filter_setup() > multiple times, so if after this we re-check that the event is still > inactive, we can return, otherwise proceed with the cross-call. Does > this make sense?
Dunno, I worry :-)
What if:
if (READ_ONCE(event->state) != PERF_EVENT_STATE_ACTIVE) { // we were INACTIVE, but now the event gets scheduled in // on _another_ CPU event->pmu->itrace_filter_setup() := { if (event->state == PERF_EVENT_STATE_ACTIVE) { /* muck with hardware */ } } }
Here too I feel a strict validation vs programming split would make sense.
We can always call the validation thing, we must not call the program thing !ACTIVE is a clear and simple rule.
| |