Re: [RFC PATCH v6 28/32] perf record: Probe at kprobe points

[Namhyung Kim <namhyung@kernel.org>]

On Tue, Jun 09, 2015 at 05:50:32AM +0000, Wang Nan wrote:
> In this patch, kprobe points are created using add_perf_probe_events.
> Since all events are already grouped together in an array, calling
> add_perf_probe_events() once creates all of them.
> 
> probe_conf.max_probes is set to MAX_PROBES to support glob matching.
> 
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> ---
>  tools/perf/builtin-record.c  | 14 ++++++++++++-
>  tools/perf/util/bpf-loader.c | 47 ++++++++++++++++++++++++++++++++++++++++++++
>  tools/perf/util/bpf-loader.h |  4 ++++
>  3 files changed, 64 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index 91aa2a3..a7c178e 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -29,6 +29,7 @@
>  #include "util/data.h"
>  #include "util/auxtrace.h"
>  #include "util/parse-branch-options.h"
> +#include "util/bpf-loader.h"
>  
>  #include <unistd.h>
>  #include <sched.h>
> @@ -1108,7 +1109,17 @@ int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused)
>  	if (err)
>  		return err;
>  
> -	err = -ENOMEM;
> +	/*
> +	 * bpf__probe must be called before symbol__init() because we
> +	 * need init_symbol_maps. If called after symbol__init,
> +	 * symbol_conf.sort_by_name won't take effect.
> +	 */
> +	err = bpf__probe();
> +	if (err) {
> +		pr_err("Probing at events in BPF object failed.\n");
> +		pr_err("Try perf probe -d '*' to remove existing probe events.\n");

Wouldn't it be better to call bpf__unprobe() here or goto
out_symbol_exit?

Thanks,
Namhyung


> +		return err;
> +	}
>  
>  	symbol__init(NULL);
>  
> @@ -1169,6 +1180,7 @@ out_symbol_exit:
>  	perf_evlist__delete(rec->evlist);
>  	symbol__exit();
>  	auxtrace_record__free(rec->itr);
> +	bpf__unprobe();
>  	return err;
>  }
>  
> diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
> index d786efc..8c0bf46 100644
> --- a/tools/perf/util/bpf-loader.c
> +++ b/tools/perf/util/bpf-loader.c
> @@ -373,3 +373,50 @@ void bpf__clear(void)
>  	bpf_object__for_each(obj, tmp)
>  		bpf_object__close(obj);
>  }
> +
> +static bool is_probing = false;
> +
> +int bpf__unprobe(void)
> +{
> +	struct strfilter *delfilter;
> +	int ret;
> +
> +	if (!is_probing)
> +		return 0;
> +
> +	delfilter = strfilter__new(PERF_BPF_PROBE_GROUP ":*", NULL);
> +	if (!delfilter) {
> +		pr_err("Failed to create delfilter when unprobing\n");
> +		return -ENOMEM;
> +	}
> +
> +	ret = del_perf_probe_events(delfilter);
> +	strfilter__delete(delfilter);
> +	if (ret < 0 && is_probing)
> +		pr_err("Error: failed to delete events: %s\n",
> +			strerror(-ret));
> +	else
> +		is_probing = false;
> +	return ret < 0 ? ret : 0;
> +}
> +
> +int bpf__probe(void)
> +{
> +	int err;
> +
> +	if (nr_probe_events <= 0)
> +		return 0;
> +
> +	probe_conf.max_probes = MAX_PROBES;
> +	/* Let add_perf_probe_events keeps probe_trace_event */
> +	err = add_perf_probe_events(probe_event_array,
> +				    nr_probe_events,
> +				    false);
> +	/* add_perf_probe_events return negative when fail */
> +	if (err < 0)
> +		pr_err("bpf probe: failed to probe events\n");
> +	else
> +		is_probing = true;
> +
> +	return err < 0 ? err : 0;
> +}
> diff --git a/tools/perf/util/bpf-loader.h b/tools/perf/util/bpf-loader.h
> index 2ed7a16..7387fb6 100644
> --- a/tools/perf/util/bpf-loader.h
> +++ b/tools/perf/util/bpf-loader.h
> @@ -21,6 +21,8 @@ extern int perf_bpf_config(const char *var, const char *value);
>  
>  #ifdef HAVE_LIBBPF_SUPPORT
>  int bpf__prepare_load(const char *filename, bool source);
> +int bpf__probe(void);
> +int bpf__unprobe(void);
>  
>  void bpf__clear(void);
>  #else
> @@ -30,6 +32,8 @@ static inline int bpf__prepare_load(const char *filename __maybe_unused)
>  	return -1;
>  }
>  
> +static inline int bpf__probe(void) { return 0; }
> +static inline int bpf__unprobe(void) { return 0; }
>  static inline void bpf__clear(void) { }
>  #endif
>  #endif
> -- 
> 1.8.3.4
>