Re: [PATCH v0 3/5] perf: Introduce instruction trace filtering

[Peter Zijlstra]

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
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/