On Tue, Jul 15, 2014 at 11:11:04AM +0200, Peter Zijlstra wrote: > On Mon, Jul 14, 2014 at 10:18:54PM +0200, Jiri Olsa wrote: > > On Mon, Jul 14, 2014 at 01:18:33PM +0200, Peter Zijlstra wrote: > > > On Fri, Jul 11, 2014 at 01:56:19PM +0200, Jiri Olsa wrote: > > > > From: Jiri Olsa <jo...@redhat.com>
SNIP > > > > I don't think this is correct, perf_event_init_context() can come in > > > concurrently and the first place it runs into ->child_mutex is after its > > > already allocated and created the (first) child event. > > > > just noticed this.. I'm working on the other version we decide, but FWIW > > there's also mutex_lock(&child_ctx->mutex); before removing the context, > > that should protect it against perf_event_init_context call > > Oh, more fail :-) > > You have: > > perf_event::child_mutex > perf_event_context::mutex > > The existing code has: > > perf_event_context::mutex > perf_event::child_context > > See for example: > > perf_event_init_context() > mutex_lock(&parent_ctx->mutex) > inherit_task_group() > inherit_group() > inherit_event() > mutex_lock(&parent_event->child_mutex) > > and > > perf_event_for_each() > mutex_lock(&ctx->mutex) > perf_event_for_each_child() > mutex_lock(&event->child_mutex) > > So the patch creates an AB-BA deadlock. ouch, right.. I'll try to come with other way thanks, jirka -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
