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

Namhyung Kim
2015-06-11T14:34:39Z

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
>