Re: [PATCH 3/3] perf: Allow kernel filters on cpu events

[Mathieu Poirier]

On Thu, Jan 26, 2017 at 11:40:57AM +0200, Alexander Shishkin wrote:
> While supporting file-based address filters for cpu events requires some
> extra context switch handling, kernel address filters are easy, since the
> kernel mapping is preserved across address spaces. It is also useful as
> it permits tracing scheduling paths of the kernel.
> 
> This patch allows setting up kernel filters for cpu events.
> 
> Signed-off-by: Alexander Shishkin <alexander.shishkin@xxxxxxxxxxxxxxx>
> Cc: Mathieu Poirier <mathieu.poirier@xxxxxxxxxx>
> ---
>  include/linux/perf_event.h |  2 ++
>  kernel/events/core.c       | 42 ++++++++++++++++++++++++++++--------------
>  2 files changed, 30 insertions(+), 14 deletions(-)
> 
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index fcb37c81ca..f4ea0600b2 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -486,6 +486,7 @@ struct perf_addr_filter {
>   * @list:	list of filters for this event
>   * @lock:	spinlock that serializes accesses to the @list and event's
>   *		(and its children's) filter generations.
> + * @nr_file_filters:	number of file-based filters
>   *
>   * A child event will use parent's @list (and therefore @lock), so they are
>   * bundled together; see perf_event_addr_filters().
> @@ -493,6 +494,7 @@ struct perf_addr_filter {
>  struct perf_addr_filters_head {
>  	struct list_head	list;
>  	raw_spinlock_t		lock;
> +	unsigned int		nr_file_filters;
>  };
>  
>  /**
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 36770a13ef..2eeb8fec2f 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -8093,6 +8093,9 @@ static void perf_event_addr_filters_apply(struct perf_event *event)
>  	if (task == TASK_TOMBSTONE)
>  		return;
>  
> +	if (!ifh->nr_file_filters)
> +		return;

Is this mandatory or an optimisation to avoid circling through a list of filters
that don't included user space files?

> +
>  	mm = get_task_mm(event->ctx->task);
>  	if (!mm)
>  		goto restart;
> @@ -8269,6 +8272,18 @@ perf_event_parse_addr_filter(struct perf_event *event, char *fstr,
>  				if (!filename)
>  					goto fail;
>  
> +				/*
> +				 * For now, we only support file-based filters
> +				 * in per-task events; doing so for CPU-wide
> +				 * events requires additional context switching
> +				 * trickery, since same object code will be
> +				 * mapped at different virtual addresses in
> +				 * different processes.
> +				 */
> +				ret = -EOPNOTSUPP;
> +				if (!event->ctx->task)
> +					goto fail_free_name;
> +
>  				/* look up the path and grab its inode */
>  				ret = kern_path(filename, LOOKUP_FOLLOW, &path);
>  				if (ret)
> @@ -8284,6 +8299,8 @@ perf_event_parse_addr_filter(struct perf_event *event, char *fstr,
>  				    !S_ISREG(filter->inode->i_mode))
>  					/* free_filters_list() will iput() */
>  					goto fail;
> +
> +				event->addr_filters.nr_file_filters++;
>  			}
>  
>  			/* ready to consume more filters */
> @@ -8323,24 +8340,13 @@ perf_event_set_addr_filter(struct perf_event *event, char *filter_str)
>  	if (WARN_ON_ONCE(event->parent))
>  		return -EINVAL;
>  
> -	/*
> -	 * For now, we only support filtering in per-task events; doing so
> -	 * for CPU-wide events requires additional context switching trickery,
> -	 * since same object code will be mapped at different virtual
> -	 * addresses in different processes.
> -	 */
> -	if (!event->ctx->task)
> -		return -EOPNOTSUPP;
> -
>  	ret = perf_event_parse_addr_filter(event, filter_str, &filters);
>  	if (ret)
> -		return ret;
> +		goto fail_clear_files;
>  
>  	ret = event->pmu->addr_filters_validate(&filters);
> -	if (ret) {
> -		free_filters_list(&filters);
> -		return ret;
> -	}
> +	if (ret)
> +		goto fail_free_filters;
>  
>  	/* remove existing filters, if any */
>  	perf_addr_filters_splice(event, &filters);
> @@ -8349,6 +8355,14 @@ perf_event_set_addr_filter(struct perf_event *event, char *filter_str)
>  	perf_event_for_each_child(event, perf_event_addr_filters_apply);
>  
>  	return ret;
> +
> +fail_free_filters:
> +	free_filters_list(&filters);
> +
> +fail_clear_files:
> +	event->addr_filters.nr_file_filters = 0;
> +
> +	return ret;
>  }
>  
>  static int perf_event_set_filter(struct perf_event *event, void __user *arg)
> -- 
> 2.11.0
>