On Fri, Jan 22, 2016 at 01:35:40PM +0200, Alexander Shishkin wrote: > Peter Zijlstra <pet...@infradead.org> writes: > > > So I think there's a number of problems still :-( > > Also, it does indeed race with > __perf_event_exit_task()/sync_child_event(), but that one I'd fix by > simply wrapping the sync_child_event()/free_event() in > > mutex_lock(&parent_event->child_mutex); > if (!is_orphan_event(parent_event)) { > sync_child_event(child_event); > free_event(child_event); > } > mutex_unlock(&parent_event->child_event);
So I've been staring at exactly that code for a while because Ingo managed to trigger that race (and I could reproduce with his 'workload'). But I'm not seeing how; both sites hold ctx->mutex and remove the event from the ctx->event_list. So the way I'm seeing it, either the orphan_work find and frees it, or the __perf_event_exit_task() one does, but I'm a bit stumped on how they can both do. Sure, the sync stuff is pointless if we're orphan, but I don't see how it can harm. > At some later point in time the code there could use a bit of > reshuffling, I guess. Yes.