Message-ID: <20150916214337.GJ11551@kernel.org>
Date:	Wed, 16 Sep 2015 18:43:37 -0300
From:	Arnaldo Carvalho de Melo <acme@...hat.com>
To:	Wang Nan <wangnan0@...wei.com>
Cc:	ast@...mgrid.com, masami.hiramatsu.pt@...achi.com,
	namhyung@...nel.org, a.p.zijlstra@...llo.nl,
	brendan.d.gregg@...il.com, daniel@...earbox.net, dsahern@...il.com,
	hekuang@...wei.com, jolsa@...nel.org, lizefan@...wei.com,
	paulus@...ba.org, xiakaixu@...wei.com, pi3orama@....com,
	linux-kernel@...r.kernel.org, acme@...nel.org
Subject: Re: [PATCH 05/27] perf record, bpf: Parse and create probe points
 for BPF programs

Em Sun, Sep 06, 2015 at 07:13:21AM +0000, Wang Nan escreveu:
> This patch introduces bpf__{un,}probe() functions to enable callers to
> create kprobe points based on section names of BPF programs. It parses

Ok, so now I see that when we do:

	perf record --event bpf_prog.o usleep 1

We are potentially inserting multiple events, one for each eBPF section
found in bpf_prog.o, right?

I.e. multiple evsels, so, when parsing, we should create the kprobes and
from there then create evsels and insert in the normal list that would
eventually get spliced to the evlist being processed, I think.

So, and reading that comment on 4/27, we need to allow that to happen at
parse_events() time, i.e. avoid adding a dummy evsel that would then,
later, be "expanded" into potentially multiple non-dummy evsels.

I put the first 3 patches, with some adjustments, in my perf/ebpf
branch, will try to do the above, i.e. do away with that dummy, allow
parsing the bpf_prog.o sections at parse_events() time.

- Arnaldo

> the section names of each eBPF program and creates corresponding 'struct
> perf_probe_event' structures. The parse_perf_probe_command() function is
> used to do the main parsing work.
> 
> Parsing result is stored into an array to satisify
> {convert,apply}_perf_probe_events(). It accepts an array of
> 'struct perf_probe_event' and do all the work in one call.
> 
> Define PERF_BPF_PROBE_GROUP as "perf_bpf_probe", which will be used as
> the group name of all eBPF probing points.
> 
> probe_conf.max_probes is set to MAX_PROBES to support glob matching.
> 
> Before ending of bpf__probe(), data in each 'struct perf_probe_event' is
> cleaned. Things will be changed by following patches because they need
> 'struct probe_trace_event' in them,
> 
> Signed-off-by: Wang Nan <wangnan0@...wei.com>
> Cc: Alexei Starovoitov <ast@...mgrid.com>
> Cc: Brendan Gregg <brendan.d.gregg@...il.com>
> Cc: Daniel Borkmann <daniel@...earbox.net>
> Cc: David Ahern <dsahern@...il.com>
> Cc: He Kuang <hekuang@...wei.com>
> Cc: Jiri Olsa <jolsa@...nel.org>
> Cc: Kaixu Xia <xiakaixu@...wei.com>
> Cc: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
> Cc: Namhyung Kim <namhyung@...nel.org>
> Cc: Paul Mackerras <paulus@...ba.org>
> Cc: Peter Zijlstra <a.p.zijlstra@...llo.nl>
> Cc: Zefan Li <lizefan@...wei.com>
> Cc: pi3orama@....com
> Link: http://lkml.kernel.org/n/1436445342-1402-21-git-send-email-wangnan0@huawei.com
> Link: http://lkml.kernel.org/n/1436445342-1402-23-git-send-email-wangnan0@huawei.com
> [Merged by two patches
>  wangnan: Utilize new perf probe API {convert,apply,cleanup}_perf_probe_events()
> ]
> Signed-off-by: Arnaldo Carvalho de Melo <acme@...hat.com>
> ---
>  tools/perf/builtin-record.c  |  19 +++++-
>  tools/perf/util/bpf-loader.c | 149 +++++++++++++++++++++++++++++++++++++++++++
>  tools/perf/util/bpf-loader.h |  13 ++++
>  3 files changed, 180 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index f886706..b56109f 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -1141,7 +1141,23 @@ int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused)
>  	if (err)
>  		goto out_bpf_clear;
>  
> -	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.
> +	 *
> +	 * bpf__unprobe() is safe even if bpf__probe() failed, and it
> +	 * also calls symbol__init. Therefore, goto out_symbol_exit
> +	 * is safe when probe failed.
> +	 */
> +	err = bpf__probe();
> +	if (err) {
> +		bpf__strerror_probe(err, errbuf, sizeof(errbuf));
> +
> +		pr_err("Probing at events in BPF object failed.\n");
> +		pr_err("\t%s\n", errbuf);
> +		goto out_symbol_exit;
> +	}
>  
>  	symbol__init(NULL);
>  
> @@ -1202,6 +1218,7 @@ out_symbol_exit:
>  	perf_evlist__delete(rec->evlist);
>  	symbol__exit();
>  	auxtrace_record__free(rec->itr);
> +	bpf__unprobe();
>  out_bpf_clear:
>  	bpf__clear();
>  	return err;
> diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
> index 88531ea..10505cb 100644
> --- a/tools/perf/util/bpf-loader.c
> +++ b/tools/perf/util/bpf-loader.c
> @@ -9,6 +9,8 @@
>  #include "perf.h"
>  #include "debug.h"
>  #include "bpf-loader.h"
> +#include "probe-event.h"
> +#include "probe-finder.h"
>  
>  #define DEFINE_PRINT_FN(name, level) \
>  static int libbpf_##name(const char *fmt, ...)	\
> @@ -28,6 +30,58 @@ DEFINE_PRINT_FN(debug, 1)
>  
>  static bool libbpf_initialized;
>  
> +static int
> +config_bpf_program(struct bpf_program *prog, struct perf_probe_event *pev)
> +{
> +	const char *config_str;
> +	int err;
> +
> +	config_str = bpf_program__title(prog, false);
> +	if (!config_str) {
> +		pr_debug("bpf: unable to get title for program\n");
> +		return -EINVAL;
> +	}
> +
> +	pr_debug("bpf: config program '%s'\n", config_str);
> +	err = parse_perf_probe_command(config_str, pev);
> +	if (err < 0) {
> +		pr_debug("bpf: '%s' is not a valid config string\n",
> +			 config_str);
> +		/* parse failed, don't need clear pev. */
> +		return -EINVAL;
> +	}
> +
> +	if (pev->group && strcmp(pev->group, PERF_BPF_PROBE_GROUP)) {
> +		pr_debug("bpf: '%s': group for event is set and not '%s'.\n",
> +			 config_str, PERF_BPF_PROBE_GROUP);
> +		err = -EINVAL;
> +		goto errout;
> +	} else if (!pev->group)
> +		pev->group = strdup(PERF_BPF_PROBE_GROUP);
> +
> +	if (!pev->group) {
> +		pr_debug("bpf: strdup failed\n");
> +		err = -ENOMEM;
> +		goto errout;
> +	}
> +
> +	if (!pev->event) {
> +		pr_debug("bpf: '%s': event name is missing\n",
> +			 config_str);
> +		err = -EINVAL;
> +		goto errout;
> +	}
> +
> +	pr_debug("bpf: config '%s' is ok\n", config_str);
> +
> +	return 0;
> +
> +errout:
> +	if (pev)
> +		clear_perf_probe_event(pev);
> +	return err;
> +}
> +
>  int bpf__prepare_load(const char *filename)
>  {
>  	struct bpf_object *obj;
> @@ -59,6 +113,90 @@ void bpf__clear(void)
>  		bpf_object__close(obj);
>  }
>  
> +static bool is_probed;
> +
> +int bpf__unprobe(void)
> +{
> +	struct strfilter *delfilter;
> +	int ret;
> +
> +	if (!is_probed)
> +		return 0;
> +
> +	delfilter = strfilter__new(PERF_BPF_PROBE_GROUP ":*", NULL);
> +	if (!delfilter) {
> +		pr_debug("Failed to create delfilter when unprobing\n");
> +		return -ENOMEM;
> +	}
> +
> +	ret = del_perf_probe_events(delfilter);
> +	strfilter__delete(delfilter);
> +	if (ret < 0 && is_probed)
> +		pr_debug("Error: failed to delete events: %s\n",
> +			 strerror(-ret));
> +	else
> +		is_probed = false;
> +	return ret < 0 ? ret : 0;
> +}
> +
> +int bpf__probe(void)
> +{
> +	int err, nr_events = 0;
> +	struct bpf_object *obj, *tmp;
> +	struct bpf_program *prog;
> +	struct perf_probe_event *pevs;
> +
> +	pevs = calloc(MAX_PROBES, sizeof(pevs[0]));
> +	if (!pevs)
> +		return -ENOMEM;
> +
> +	bpf_object__for_each_safe(obj, tmp) {
> +		bpf_object__for_each_program(prog, obj) {
> +			err = config_bpf_program(prog, &pevs[nr_events++]);
> +			if (err < 0)
> +				goto out;
> +
> +			if (nr_events >= MAX_PROBES) {
> +				pr_debug("Too many (more than %d) events\n",
> +					 MAX_PROBES);
> +				err = -ERANGE;
> +				goto out;
> +			};
> +		}
> +	}
> +
> +	if (!nr_events) {
> +		/*
> +		 * Don't call following code to prevent perf report failure
> +		 * init_symbol_maps can fail when perf is started by non-root
> +		 * user, which prevent non-root user track normal events even
> +		 * without using BPF, because bpf__probe() is called by
> +		 * 'perf record' unconditionally.
> +		 */
> +		err = 0;
> +		goto out;
> +	}
> +
> +	probe_conf.max_probes = MAX_PROBES;
> +	/* Let convert_perf_probe_events generates probe_trace_event (tevs) */
> +	err = convert_perf_probe_events(pevs, nr_events);
> +	if (err < 0) {
> +		pr_debug("bpf_probe: failed to convert perf probe events");
> +		goto out;
> +	}
> +
> +	err = apply_perf_probe_events(pevs, nr_events);
> +	if (err < 0)
> +		pr_debug("bpf probe: failed to probe events\n");
> +	else
> +		is_probed = true;
> +out_cleanup:
> +	cleanup_perf_probe_events(pevs, nr_events);
> +out:
> +	free(pevs);
> +	return err < 0 ? err : 0;
> +}
> +
>  #define bpf__strerror_head(err, buf, size) \
>  	char sbuf[STRERR_BUFSIZE], *emsg;\
>  	if (!size)\
> @@ -90,3 +228,14 @@ int bpf__strerror_prepare_load(const char *filename, int err,
>  	bpf__strerror_end(buf, size);
>  	return 0;
>  }
> +
> +int bpf__strerror_probe(int err, char *buf, size_t size)
> +{
> +	bpf__strerror_head(err, buf, size);
> +	bpf__strerror_entry(ERANGE, "Too many (more than %d) events",
> +			    MAX_PROBES);
> +	bpf__strerror_entry(ENOENT, "Selected kprobe point doesn't exist.");
> +	bpf__strerror_entry(EEXIST, "Selected kprobe point already exist, try perf probe -d '*'.");
> +	bpf__strerror_end(buf, size);
> +	return 0;
> +}
> diff --git a/tools/perf/util/bpf-loader.h b/tools/perf/util/bpf-loader.h
> index 12be630..6b09a85 100644
> --- a/tools/perf/util/bpf-loader.h
> +++ b/tools/perf/util/bpf-loader.h
> @@ -9,10 +9,15 @@
>  #include <string.h>
>  #include "debug.h"
>  
> +#define PERF_BPF_PROBE_GROUP "perf_bpf_probe"
> +
>  #ifdef HAVE_LIBBPF_SUPPORT
>  int bpf__prepare_load(const char *filename);
>  int bpf__strerror_prepare_load(const char *filename, int err,
>  			       char *buf, size_t size);
> +int bpf__probe(void);
> +int bpf__unprobe(void);
> +int bpf__strerror_probe(int err, char *buf, size_t size);
>  
>  void bpf__clear(void);
>  #else
> @@ -22,6 +27,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) { }
>  
>  static inline int
> @@ -43,5 +50,11 @@ bpf__strerror_prepare_load(const char *filename __maybe_unused,
>  {
>  	return __bpf_strerror(buf, size);
>  }
> +
> +static inline int bpf__strerror_probe(int err __maybe_unused,
> +				      char *buf, size_t size)
> +{
> +	return __bpf_strerror(buf, size);
> +}
>  #endif
>  #endif
> -- 
> 2.1.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists