From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755401AbbLKRAd (ORCPT ); Fri, 11 Dec 2015 12:00:33 -0500 Received: from casper.infradead.org ([85.118.1.10]:51335 "EHLO casper.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753159AbbLKRAc (ORCPT ); Fri, 11 Dec 2015 12:00:32 -0500 Date: Fri, 11 Dec 2015 18:00:29 +0100 From: Peter Zijlstra To: Alexander Shishkin 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 Message-ID: <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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20151211152854.GX6356@twins.programming.kicks-ass.net> User-Agent: Mutt/1.5.21 (2012-12-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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.