From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752983AbbLKRNP (ORCPT ); Fri, 11 Dec 2015 12:13:15 -0500 Received: from mga01.intel.com ([192.55.52.88]:26650 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752107AbbLKRNO (ORCPT ); Fri, 11 Dec 2015 12:13:14 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.20,414,1444719600"; d="scan'208";a="871754094" From: Alexander Shishkin To: Peter Zijlstra Cc: Ingo Molnar , linux-kernel@vger.kernel.org, vince@deater.net, eranian@google.com, Arnaldo Carvalho de Melo , Mathieu Poirier Subject: Re: [PATCH v0 3/5] perf: Introduce instruction trace filtering In-Reply-To: <20151211170029.GI6373@twins.programming.kicks-ass.net> References: <1449840998-29902-1-git-send-email-alexander.shishkin@linux.intel.com> <1449840998-29902-4-git-send-email-alexander.shishkin@linux.intel.com> <20151211152854.GX6356@twins.programming.kicks-ass.net> <20151211170029.GI6373@twins.programming.kicks-ass.net> User-Agent: Notmuch/0.21 (http://notmuchmail.org) Emacs/24.5.1 (x86_64-pc-linux-gnu) Date: Fri, 11 Dec 2015 19:13:10 +0200 Message-ID: <87egesykm1.fsf@ashishki-desk.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Peter Zijlstra 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