Messages in this thread | | | Date | Fri, 11 Dec 2015 18:00:29 +0100 | From | Peter Zijlstra <> | Subject | Re: [PATCH v0 3/5] perf: Introduce instruction trace filtering |
| |
On Fri, Dec 11, 2015 at 04:28:54PM +0100, Peter Zijlstra wrote: > On Fri, Dec 11, 2015 at 03:36:36PM +0200, Alexander Shishkin wrote: > > > @@ -9063,6 +9621,18 @@ inherit_event(struct perf_event *parent_event, > > get_ctx(child_ctx); > > > > /* > > + * Clone itrace filters from the parent, if any > > + */ > > + if (has_itrace_filter(child_event)) { > > + if (perf_itrace_filters_clone(child_event, parent_event, > > + child)) { > > + put_ctx(child_ctx); > > + free_event(child_event); > > + return NULL; > > So inherit_event()'s return policy is somewhat opaque, there's 3 > possible returns: > > 1) a valid struct perf_event pointer; the clone was successful > 2) ERR_PTR(err), the clone failed, abort inherit_group, fail fork() > 3) NULL, the clone failed, ignore, continue > > We return NULL under two special cases: > > - the original event doesn't exist anymore, we're an orphan, do not make > more orphans. > > - the parent event is dying > > > I'm fairly sure this return should be in the 2) category. If we cannot > fully clone the event something bad happened, we should not ignore it.
On second thought; we should not inherit the filters at all.
We should always use event->parent (if exists) for filters. Otherwise inherited events will get different filters if you change the filter after clone.
| |