Messages in this thread | | | From | Alexander Shishkin <> | Subject | Re: [PATCH v0 3/5] perf: Introduce instruction trace filtering | Date | Fri, 11 Dec 2015 19:13:10 +0200 |
| |
Peter Zijlstra <peterz@infradead.org> writes:
> 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.
But children will have different mappings, so the actual filter configurations will still differ between parents and children. I guess I could split the filter in two parts: one that's defined by the user and one that we calculated from vma addresses, that we later program into hardware.
Regards, -- Alex
| |