public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jiri Olsa <jolsa@kernel.org>
To: linux-kernel@vger.kernel.org
Cc: Jiri Olsa <jolsa@redhat.com>,
	Alexander Yarygin <yarygin@linux.vnet.ibm.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Corey Ashford <cjashfor@linux.vnet.ibm.com>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Ingo Molnar <mingo@kernel.org>, Paul Mackerras <paulus@samba.org>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Jiri Olsa <jolsa@kernel.org>
Subject: [PATCH 2/5] perf: Destroy event's children on task exit
Date: Fri, 11 Jul 2014 13:56:19 +0200	[thread overview]
Message-ID: <1405079782-8139-3-git-send-email-jolsa@kernel.org> (raw)
In-Reply-To: <1405079782-8139-1-git-send-email-jolsa@kernel.org>

From: Jiri Olsa <jolsa@redhat.com>

When task exits we close:
  1) all events that are installed in task
  2) all events owned by task (via file descriptor)

But we don't close children events of 2) events. Those children
events stay until the child task exits and are useless with the
parent being gone, because we have no way to get to values any
more.

Plus if the event stays installed in task even with the owner task
gone, it runs the perf callback any time the task forks, for no
real reason.

Closing all children events events when the owner task of the
parent event is closed.

Cc: Alexander Yarygin <yarygin@linux.vnet.ibm.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 kernel/events/core.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 71a56ae..37797dd 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7535,6 +7535,32 @@ static void perf_event_exit_task_context(struct task_struct *child, int ctxn)
 	put_ctx(child_ctx);
 }
 
+static void perf_event_exit_children(struct perf_event *parent)
+{
+	struct perf_event *child, *tmp;
+
+	mutex_lock(&parent->child_mutex);
+	list_for_each_entry_safe(child, tmp, &parent->child_list,
+				 child_list) {
+		struct perf_event_context *child_ctx = child->ctx;
+
+		/*
+		 * Child events got removed from child_list under
+		 * child_mutex and then freed. So it's safe to access
+		 * childs context in here, because the child holds
+		 * context ref.
+		 */
+		mutex_lock(&child_ctx->mutex);
+		perf_remove_from_context(child, true);
+		mutex_unlock(&child_ctx->mutex);
+
+		list_del_init(&child->child_list);
+		put_event(parent);
+		free_event(child);
+	}
+	mutex_unlock(&parent->child_mutex);
+}
+
 /*
  * When a child task exits, feed back event values to parent events.
  */
@@ -7555,6 +7581,7 @@ void perf_event_exit_task(struct task_struct *child)
 		 */
 		smp_wmb();
 		event->owner = NULL;
+		perf_event_exit_children(event);
 	}
 	mutex_unlock(&child->perf_event_mutex);
 
-- 
1.8.3.1


  parent reply	other threads:[~2014-07-11 11:56 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-11 11:56 [PATCH 0/5] perf: Fix tracepoint events permissions check Jiri Olsa
2014-07-11 11:56 ` [PATCH 1/5] perf: Make perf_init_event function static Jiri Olsa
2014-07-11 11:56 ` Jiri Olsa [this message]
2014-07-11 13:23   ` [PATCH 2/5] perf: Destroy event's children on task exit Peter Zijlstra
2014-07-11 13:31     ` Jiri Olsa
2014-07-16 12:20     ` Ingo Molnar
2014-07-14 11:18   ` Peter Zijlstra
2014-07-14 11:43     ` Jiri Olsa
2014-07-14 13:02       ` Peter Zijlstra
2014-07-14 13:22         ` Jiri Olsa
2014-07-14 13:35           ` Peter Zijlstra
2014-07-14 14:21             ` Jiri Olsa
2014-07-16 12:14             ` Jiri Olsa
2014-07-14 20:18     ` Jiri Olsa
2014-07-15  9:11       ` Peter Zijlstra
2014-07-15  9:31         ` Jiri Olsa
2014-07-11 11:56 ` [PATCH 3/5] perf: Initialize owner before calling event_init callback Jiri Olsa
2014-07-11 11:56 ` [PATCH 4/5] perf: Move event owner retrieval into perf_event_get_owner Jiri Olsa
2014-07-11 11:56 ` [PATCH 5/5] perf: Check event's owner permission in tracepoint init callback Jiri Olsa
2014-07-11 12:02 ` [PATCH 0/5] perf: Fix tracepoint events permissions check Jiri Olsa
2014-07-28  8:28 ` [tip:perf/core] perf: Check permission only for parent tracepoint event tip-bot for Jiri Olsa

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1405079782-8139-3-git-send-email-jolsa@kernel.org \
    --to=jolsa@kernel.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=acme@kernel.org \
    --cc=cjashfor@linux.vnet.ibm.com \
    --cc=fweisbec@gmail.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=paulus@samba.org \
    --cc=yarygin@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox