From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754660AbbLKP3A (ORCPT ); Fri, 11 Dec 2015 10:29:00 -0500 Received: from bombadil.infradead.org ([198.137.202.9]:37841 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751239AbbLKP27 (ORCPT ); Fri, 11 Dec 2015 10:28:59 -0500 Date: Fri, 11 Dec 2015 16:28:54 +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: <20151211152854.GX6356@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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1449840998-29902-4-git-send-email-alexander.shishkin@linux.intel.com> 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 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. > + } > + } > + > + /* > * Make the child state follow the state of the parent event, > * not its attr.disabled bit. We hold the parent's mutex, > * so we won't race with perf_event_{en, dis}able_family. > -- > 2.6.2 >