From: Jiri Olsa <jolsa@redhat.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Jiri Olsa <jolsa@kernel.org>,
linux-kernel@vger.kernel.org,
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>
Subject: Re: [PATCH 2/5] perf: Destroy event's children on task exit
Date: Mon, 14 Jul 2014 15:22:23 +0200 [thread overview]
Message-ID: <20140714132223.GC17761@krava.redhat.com> (raw)
In-Reply-To: <20140714130237.GX9918@twins.programming.kicks-ass.net>
On Mon, Jul 14, 2014 at 03:02:37PM +0200, Peter Zijlstra wrote:
> On Mon, Jul 14, 2014 at 01:43:59PM +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 <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.
> > >
> > > Do we need this for the other patches, or is this an unrelated change?
> >
> > if we dont do it, the event stays installed without owner and
> > perf fork callback will be called and fail on permission checking
> > (because of owner == NULL) ... so yes, I think it's needed
>
> Oh, right. Alternatively, we don't need permission checking for inherits
> at all, if we're allowed to create the initial event, we should be good
> for inherits.
I could adress that in follow up patch.. or you want this instead
of this one? IMO we should close those events anyway..
jirka
next prev parent reply other threads:[~2014-07-14 13:23 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 ` [PATCH 2/5] perf: Destroy event's children on task exit Jiri Olsa
2014-07-11 13:23 ` 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 [this message]
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=20140714132223.GC17761@krava.redhat.com \
--to=jolsa@redhat.com \
--cc=acme@kernel.org \
--cc=cjashfor@linux.vnet.ibm.com \
--cc=fweisbec@gmail.com \
--cc=jolsa@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=paulus@samba.org \
--cc=peterz@infradead.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.